[PATCH 2/2] patman: Quieten down the alias checking

Sean Anderson seanga2 at gmail.com
Wed Dec 23 05:40:21 CET 2020


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 <sjg at chromium.org>
> ---
> 
>   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 <stefan.bruens at rwth-aachen.de>'.decode('utf-8')
>           rick = 'Richard III <richard at palace.gov>'
>           mel = b'Lord M\xc3\xablchett <clergy at palace.gov>'.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:
> 



More information about the U-Boot mailing list