All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Anderson <seanga2@gmail.com>
To: u-boot@lists.denx.de
Subject: [PATCH 2/2] patman: Quieten down the alias checking
Date: Tue, 22 Dec 2020 23:40:21 -0500	[thread overview]
Message-ID: <746cef9b-3666-c410-ead0-ac48ce501b50@gmail.com> (raw)
In-Reply-To: <20201223031448.2038902-2-sjg@chromium.org>

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

  reply	other threads:[~2020-12-23  4:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-23  3:14 [PATCH 1/2] this: patman: Update documentation to match new usage Simon Glass
2020-12-23  3:14 ` [PATCH 2/2] patman: Quieten down the alias checking Simon Glass
2020-12-23  4:40   ` Sean Anderson [this message]
2021-01-23 15:56     ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=746cef9b-3666-c410-ead0-ac48ce501b50@gmail.com \
    --to=seanga2@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.