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 <s...@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.bru...@rwth-aachen.de>'.decode('utf-8')
          rick = 'Richard III <rich...@palace.gov>'
          mel = b'Lord M\xc3\xablchett <cle...@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@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.blo...@napier.co.nz', 'j.blo...@napier.co.nz', 'm.popp...@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 via email to