Hi Sean, On Tue, 22 Dec 2020 at 21:40, Sean Anderson <[email protected]> wrote: > > On 12/22/20 10:14 PM, Simon Glass wrote: > > When a tag is used in a patch subject (e.g. "tag: rest of message") and > > it cannot be found as an alias, patman currently reports a fatal error, > > unless -t is provided, in which case it reports a warning. > > > > Experience suggest that the fatal error is not very useful. Instead, > > default to reporting a warning, with -t tell patman to ignore it > > altogether. > > > > Signed-off-by: Simon Glass <[email protected]> > > --- > > > > tools/patman/func_test.py | 2 +- > > tools/patman/gitutil.py | 45 +++++++++++++++++---------------------- > > tools/patman/main.py | 3 ++- > > tools/patman/series.py | 10 ++++----- > > 4 files changed, 28 insertions(+), 32 deletions(-) > > > > diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py > > index e7db36a85c3..cb35a92be23 100644 > > --- a/tools/patman/func_test.py > > +++ b/tools/patman/func_test.py > > @@ -186,7 +186,7 @@ class TestFunctional(unittest.TestCase): > > - Commit-notes > > """ > > process_tags = True > > - ignore_bad_tags = True > > + ignore_bad_tags = False > > stefan = b'Stefan Br\xc3\xbcns > > <[email protected]>'.decode('utf-8') > > rick = 'Richard III <[email protected]>' > > mel = b'Lord M\xc3\xablchett <[email protected]>'.decode('utf-8') > > diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py > > index 6c4d2417a04..0b5affec953 100644 > > --- a/tools/patman/gitutil.py > > +++ b/tools/patman/gitutil.py > > @@ -343,7 +343,7 @@ def CreatePatches(branch, start, count, ignore_binary, > > series): > > else: > > return None, files > > > > -def BuildEmailList(in_list, tag=None, alias=None, raise_on_error=True): > > +def BuildEmailList(in_list, tag=None, alias=None, warn_on_error=True): > > """Build a list of email addresses based on an input list. > > > > Takes a list of email addresses and aliases, and turns this into a > > list > > @@ -357,7 +357,7 @@ def BuildEmailList(in_list, tag=None, alias=None, > > raise_on_error=True): > > in_list: List of aliases/email addresses > > tag: Text to put before each address > > alias: Alias dictionary > > - raise_on_error: True to raise an error when an alias fails to > > match, > > + warn_on_error: True to raise an error when an alias fails to match, > > False to just print a message. > > > > Returns: > > @@ -380,7 +380,7 @@ def BuildEmailList(in_list, tag=None, alias=None, > > raise_on_error=True): > > quote = '"' if tag and tag[0] == '-' else '' > > raw = [] > > for item in in_list: > > - raw += LookupEmail(item, alias, raise_on_error=raise_on_error) > > + raw += LookupEmail(item, alias, warn_on_error=warn_on_error) > > result = [] > > for item in raw: > > if not item in result: > > @@ -414,7 +414,7 @@ def CheckSuppressCCConfig(): > > > > return True > > > > -def EmailPatches(series, cover_fname, args, dry_run, raise_on_error, > > cc_fname, > > +def EmailPatches(series, cover_fname, args, dry_run, warn_on_error, > > cc_fname, > > self_only=False, alias=None, in_reply_to=None, thread=False, > > smtp_server=None): > > """Email a patch series. > > @@ -424,8 +424,8 @@ def EmailPatches(series, cover_fname, args, dry_run, > > raise_on_error, cc_fname, > > cover_fname: filename of cover letter > > args: list of filenames of patch files > > dry_run: Just return the command that would be run > > - raise_on_error: True to raise an error when an alias fails to > > match, > > - False to just print a message. > > + warn_on_error: True to print a warning when an alias fails to > > match, > > + False to ignore it. > > cc_fname: Filename of Cc file for per-commit Cc > > self_only: True to just email to yourself as a test > > in_reply_to: If set we'll pass this to git as --in-reply-to. > > @@ -473,7 +473,7 @@ send --cc-cmd cc-fname" cover p1 p2' > > # Restore argv[0] since we clobbered it. > > >>> sys.argv[0] = _old_argv0 > > """ > > - to = BuildEmailList(series.get('to'), '--to', alias, raise_on_error) > > + to = BuildEmailList(series.get('to'), '--to', alias, warn_on_error) > > if not to: > > git_config_to = command.Output('git', 'config', 'sendemail.to', > > raise_on_error=False) > > @@ -485,9 +485,9 @@ send --cc-cmd cc-fname" cover p1 p2' > > "git config sendemail.to [email protected]") > > return > > cc = BuildEmailList(list(set(series.get('cc')) - > > set(series.get('to'))), > > - '--cc', alias, raise_on_error) > > + '--cc', alias, warn_on_error) > > if self_only: > > - to = BuildEmailList([os.getenv('USER')], '--to', alias, > > raise_on_error) > > + to = BuildEmailList([os.getenv('USER')], '--to', alias, > > warn_on_error) > > cc = [] > > cmd = ['git', 'send-email', '--annotate'] > > if smtp_server: > > @@ -509,7 +509,7 @@ send --cc-cmd cc-fname" cover p1 p2' > > return cmdstr > > > > > > -def LookupEmail(lookup_name, alias=None, raise_on_error=True, level=0): > > +def LookupEmail(lookup_name, alias=None, warn_on_error=True, level=0): > > """If an email address is an alias, look it up and return the full > > name > > > > TODO: Why not just use git's own alias feature? > > @@ -517,8 +517,8 @@ def LookupEmail(lookup_name, alias=None, > > raise_on_error=True, level=0): > > Args: > > lookup_name: Alias or email address to look up > > alias: Dictionary containing aliases (None to use settings > > default) > > - raise_on_error: True to raise an error when an alias fails to > > match, > > - False to just print a message. > > + warn_on_error: True to print a warning when an alias fails to > > match, > > + False to ignore it. > > > > Returns: > > tuple: > > @@ -545,18 +545,16 @@ def LookupEmail(lookup_name, alias=None, > > raise_on_error=True, level=0): > > >>> LookupEmail('all', alias) > > ['[email protected]', '[email protected]', > > '[email protected]'] > > >>> LookupEmail('odd', alias) > > - Traceback (most recent call last): > > - ... > > - ValueError: Alias 'odd' not found > > + Alias 'odd' not found > > + [] > > >>> LookupEmail('loop', alias) > > Traceback (most recent call last): > > ... > > OSError: Recursive email alias at 'other' > > - >>> LookupEmail('odd', alias, raise_on_error=False) > > - Alias 'odd' not found > > + >>> LookupEmail('odd', alias, warn_on_error=False) > > [] > > >>> # In this case the loop part will effectively be ignored. > > - >>> LookupEmail('loop', alias, raise_on_error=False) > > + >>> LookupEmail('loop', alias, warn_on_error=False) > > Recursive email alias at 'other' > > Recursive email alias at 'john' > > Recursive email alias at 'mary' > > @@ -574,7 +572,7 @@ def LookupEmail(lookup_name, alias=None, > > raise_on_error=True, level=0): > > out_list = [] > > if level > 10: > > msg = "Recursive email alias at '%s'" % lookup_name > > - if raise_on_error: > > + if warn_on_error: > > raise OSError(msg) > > else: > > print(col.Color(col.RED, msg)) > > @@ -583,18 +581,15 @@ def LookupEmail(lookup_name, alias=None, > > raise_on_error=True, level=0): > > if lookup_name: > > if not lookup_name in alias: > > msg = "Alias '%s' not found" % lookup_name > > - if raise_on_error: > > - raise ValueError(msg) > > - else: > > + if warn_on_error: > > print(col.Color(col.RED, msg)) > > - return out_list > > + return out_list > > for item in alias[lookup_name]: > > - todo = LookupEmail(item, alias, raise_on_error, level + 1) > > + todo = LookupEmail(item, alias, warn_on_error, level + 1) > > for new_item in todo: > > if not new_item in out_list: > > out_list.append(new_item) > > > > - #print("No match for alias '%s'" % lookup_name) > > return out_list > > > > def GetTopLevel(): > > diff --git a/tools/patman/main.py b/tools/patman/main.py > > index 342fd446a12..10dc816369f 100755 > > --- a/tools/patman/main.py > > +++ b/tools/patman/main.py > > @@ -68,7 +68,8 @@ send.add_argument('-n', '--dry-run', action='store_true', > > dest='dry_run', > > send.add_argument('-r', '--in-reply-to', type=str, action='store', > > help="Message ID that this series is in reply to") > > send.add_argument('-t', '--ignore-bad-tags', action='store_true', > > - default=False, help='Ignore bad tags / aliases') > > + default=False, > > + help='Ignore bad tags / aliases (default=warn)') > > Could this default True when process_tags = False? If I'm not using tags > as aliases, then it doesn't matter if they are missing.
OK I updated this in v2. Regards, Simon
