All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] this: patman: Update documentation to match new usage
@ 2020-12-23  3:14 Simon Glass
  2020-12-23  3:14 ` [PATCH 2/2] patman: Quieten down the alias checking Simon Glass
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Glass @ 2020-12-23  3:14 UTC (permalink / raw
  To: u-boot

With the subcommands some of the documentation examples are no-longer
correct. Fix all of them, so it is consistent.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/patman/README | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/patman/README b/tools/patman/README
index 6b806632f8c..53f55ce95d4 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -136,17 +136,17 @@ How to run it
 
 First do a dry run:
 
-$ ./tools/patman/patman -n
+$ ./tools/patman/patman send -n
 
 If it can't detect the upstream branch, try telling it how many patches
 there are in your series:
 
-$ ./tools/patman/patman -n -c5
+$ ./tools/patman/patman -c5 send -n
 
 This will create patch files in your current directory and tell you who
 it is thinking of sending them to. Take a look at the patch files.
 
-$ ./tools/patman/patman -n -c5 -s1
+$ ./tools/patman/patman -c5 -s1 send -n
 
 Similar to the above, but skip the first commit and take the next 5. This
 is useful if your top commit is for setting up testing.
@@ -433,12 +433,12 @@ but that you don't want to submit because there is an existing patch for it
 on the list. So you can tell patman to create and check some patches
 (skipping the first patch) with:
 
-    patman -s1 -n
+    patman -s1 send -n
 
 If you want to do all of them including the work-in-progress one, then
 (if you are tracking an upstream branch):
 
-    patman -n
+    patman send -n
 
 Let's say that patman reports an error in the second patch. Then:
 
@@ -450,7 +450,7 @@ Let's say that patman reports an error in the second patch. Then:
 
 Now you have an updated patch series. To check it:
 
-    patman -s1 -n
+    patman -s1 send -n
 
 Let's say it is now clean and you want to send it. Now you need to set up
 the destination. So amend the top commit with:
@@ -485,7 +485,7 @@ mmc and sparc, and the last one to sandbox.
 
 Now to send the patches, take off the -n flag:
 
-   patman -s1
+   patman -s1 send
 
 The patches will be created, shown in your editor, and then sent along with
 the cover letter. Note that patman's tags are automatically removed so that
-- 
2.29.2.729.g45daf8777d-goog

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] patman: Quieten down the alias checking
  2020-12-23  3:14 [PATCH 1/2] this: patman: Update documentation to match new usage Simon Glass
@ 2020-12-23  3:14 ` Simon Glass
  2020-12-23  4:40   ` Sean Anderson
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Glass @ 2020-12-23  3:14 UTC (permalink / raw
  To: u-boot

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)')
 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:
-- 
2.29.2.729.g45daf8777d-goog

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] patman: Quieten down the alias checking
  2020-12-23  3:14 ` [PATCH 2/2] patman: Quieten down the alias checking Simon Glass
@ 2020-12-23  4:40   ` Sean Anderson
  2021-01-23 15:56     ` Simon Glass
  0 siblings, 1 reply; 4+ messages in thread
From: Sean Anderson @ 2020-12-23  4:40 UTC (permalink / raw
  To: u-boot

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:
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] patman: Quieten down the alias checking
  2020-12-23  4:40   ` Sean Anderson
@ 2021-01-23 15:56     ` Simon Glass
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Glass @ 2021-01-23 15:56 UTC (permalink / raw
  To: u-boot

Hi Sean,

On Tue, 22 Dec 2020 at 21:40, Sean Anderson <seanga2@gmail.com> 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 <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.

OK I updated this in v2.

Regards,
Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-01-23 15:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2021-01-23 15:56     ` Simon Glass

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.