From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Tue, 22 Dec 2020 23:40:21 -0500 Subject: [PATCH 2/2] patman: Quieten down the alias checking In-Reply-To: <20201223031448.2038902-2-sjg@chromium.org> References: <20201223031448.2038902-1-sjg@chromium.org> <20201223031448.2038902-2-sjg@chromium.org> Message-ID: <746cef9b-3666-c410-ead0-ac48ce501b50@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- > > 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 '.decode('utf-8') > rick = 'Richard III ' > mel = b'Lord M\xc3\xablchett '.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 u-boot at lists.denx.de") > 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) > ['f.bloggs at napier.co.nz', 'j.bloggs at napier.co.nz', 'm.poppins at cloud.net'] > >>> 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. --Sean > send.add_argument('-T', '--thread', action='store_true', dest='thread', > default=False, help='Create patches as a single thread') > send.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store', > diff --git a/tools/patman/series.py b/tools/patman/series.py > index a6746e87c43..85a42a8b3fb 100644 > --- a/tools/patman/series.py > +++ b/tools/patman/series.py > @@ -234,7 +234,7 @@ class Series(dict): > str = 'Change log exists, but no version is set' > print(col.Color(col.RED, str)) > > - def MakeCcFile(self, process_tags, cover_fname, raise_on_error, > + def MakeCcFile(self, process_tags, cover_fname, warn_on_error, > add_maintainers, limit): > """Make a cc file for us to use for per-commit Cc automation > > @@ -243,8 +243,8 @@ class Series(dict): > Args: > process_tags: Process tags as if they were aliases > cover_fname: If non-None the name of the cover letter. > - 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. > add_maintainers: Either: > True/False to call the get_maintainers to CC maintainers > List of maintainers to include (for testing) > @@ -261,9 +261,9 @@ class Series(dict): > cc = [] > if process_tags: > cc += gitutil.BuildEmailList(commit.tags, > - raise_on_error=raise_on_error) > + warn_on_error=warn_on_error) > cc += gitutil.BuildEmailList(commit.cc_list, > - raise_on_error=raise_on_error) > + warn_on_error=warn_on_error) > if type(add_maintainers) == type(cc): > cc += add_maintainers > elif add_maintainers: >