[PATCH v5 0/6] add support for repository-local .patman config file

[PATCH v5 0/6] add support for repository-local .patman config file

[Maxim Cournoyer]

This series is based on top of series #333345.  It adds support for a
.patman config file local to the git repository.  A new test
'test_settings.py' test module is added; it is automatically
discovered and run via pytest, e.g.: 'cd tools/patman && pytest'.

Changes in v5:
- Use try/finally in cleared_command_line_args context manager

Changes in v4:
- Use tools.run to invoke git in test suite

Changes in v3:
- Clear command line arguments in test_git_local_config
- Also test overriding a 'send' command line argument

Changes in v2:
- Explicitly provide an empty args list to parse_known_args in test

Maxim Cournoyer (6):
  patman: fix pep8 warnings in settings module
  patman: replace deprecated SafeConfigParser with ConfigParser
  patman: import gitutil module where it is needed
  patman: set the default config_fname argument value to None
  patman: fail early in Setup when provided config file does not exist
  patman: additionally honor a local .patman config file

 tools/patman/__main__.py      |   3 +-
 tools/patman/patman.rst       |   8 ++-
 tools/patman/settings.py      | 101 +++++++++++++++++++++-------------
 tools/patman/test_settings.py |  67 ++++++++++++++++++++++
 4 files changed, 139 insertions(+), 40 deletions(-)
 create mode 100644 tools/patman/test_settings.py


base-commit: 609e301d41284dbca3e6d03fe038ea0807b3df93
-- 
2.38.1

[PATCH v5 1/6] patman: fix pep8 warnings in settings module

[Maxim Cournoyer]

Remove extraneous imports, variables and comply to PEP 8 maximum line
width, among other PEP 8 changes suggested by Pyflake.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/settings.py | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 903d6fcb0b..b6884a073e 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -4,16 +4,13 @@
 
 try:
     import configparser as ConfigParser
-except:
+except Exception:
     import ConfigParser
 
 import argparse
 import os
 import re
 
-from patman import command
-from patman import tools
-
 """Default settings per-project.
 
 These are used by _ProjectConfigParser.  Settings names should match
@@ -32,6 +29,7 @@ _default_settings = {
     },
 }
 
+
 class _ProjectConfigParser(ConfigParser.SafeConfigParser):
     """ConfigParser that handles projects.
 
@@ -83,8 +81,8 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser):
     def __init__(self, project_name):
         """Construct _ProjectConfigParser.
 
-        In addition to standard SafeConfigParser initialization, this also loads
-        project defaults.
+        In addition to standard SafeConfigParser initialization, this
+        also loads project defaults.
 
         Args:
             project_name: The name of the project.
@@ -155,6 +153,7 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser):
         item_dict.update(project_items)
         return {(item, val) for item, val in item_dict.items()}
 
+
 def ReadGitAliases(fname):
     """Read a git alias file. This is in the form used by git:
 
@@ -170,7 +169,7 @@ def ReadGitAliases(fname):
         print("Warning: Cannot find alias file '%s'" % fname)
         return
 
-    re_line = re.compile('alias\s+(\S+)\s+(.*)')
+    re_line = re.compile(r'alias\s+(\S+)\s+(.*)')
     for line in fd.readlines():
         line = line.strip()
         if not line or line[0] == '#':
@@ -190,6 +189,7 @@ def ReadGitAliases(fname):
 
     fd.close()
 
+
 def CreatePatmanConfigFile(gitutil, config_fname):
     """Creates a config file under $(HOME)/.patman if it can't find one.
 
@@ -200,12 +200,12 @@ def CreatePatmanConfigFile(gitutil, config_fname):
         None
     """
     name = gitutil.get_default_user_name()
-    if name == None:
+    if name is None:
         name = input("Enter name: ")
 
     email = gitutil.get_default_user_email()
 
-    if email == None:
+    if email is None:
         email = input("Enter email: ")
 
     try:
@@ -220,7 +220,8 @@ me: %s <%s>
 [bounces]
 nxp = Zhikang Zhang <zhikang.zhang@nxp.com>
 ''' % (name, email), file=f)
-    f.close();
+    f.close()
+
 
 def _UpdateDefaults(main_parser, config):
     """Update the given OptionParser defaults based on config.
@@ -242,8 +243,8 @@ def _UpdateDefaults(main_parser, config):
     # Find all the parsers and subparsers
     parsers = [main_parser]
     parsers += [subparser for action in main_parser._actions
-                  if isinstance(action, argparse._SubParsersAction)
-                  for _, subparser in action.choices.items()]
+                if isinstance(action, argparse._SubParsersAction)
+                for _, subparser in action.choices.items()]
 
     # Collect the defaults from each parser
     defaults = {}
@@ -270,8 +271,9 @@ def _UpdateDefaults(main_parser, config):
     # Set all the defaults and manually propagate them to subparsers
     main_parser.set_defaults(**defaults)
     for parser, pdefs in zip(parsers, parser_defaults):
-        parser.set_defaults(**{ k: v for k, v in defaults.items()
-                                    if k in pdefs })
+        parser.set_defaults(**{k: v for k, v in defaults.items()
+                               if k in pdefs})
+
 
 def _ReadAliasFile(fname):
     """Read in the U-Boot git alias file if it exists.
@@ -298,6 +300,7 @@ def _ReadAliasFile(fname):
         if bad_line:
             print(bad_line)
 
+
 def _ReadBouncesFile(fname):
     """Read in the bounces file if it exists
 
@@ -311,6 +314,7 @@ def _ReadBouncesFile(fname):
                     continue
                 bounces.add(line.strip())
 
+
 def GetItems(config, section):
     """Get the items from a section of the config.
 
@@ -323,10 +327,9 @@ def GetItems(config, section):
     """
     try:
         return config.items(section)
-    except ConfigParser.NoSectionError as e:
+    except ConfigParser.NoSectionError:
         return []
-    except:
-        raise
+
 
 def Setup(gitutil, parser, project_name, config_fname=''):
     """Set up the settings module by reading config files.
@@ -358,6 +361,7 @@ def Setup(gitutil, parser, project_name, config_fname=''):
 
     _UpdateDefaults(parser, config)
 
+
 # These are the aliases we understand, indexed by alias. Each member is a list.
 alias = {}
 bounces = set()
-- 
2.38.1

[PATCH v5 2/6] patman: replace deprecated SafeConfigParser with ConfigParser

[Maxim Cournoyer]

The SafeConfigParser class has been renamed in Python 3.2 to
ConfigParser, and the old alias has been deprecated since.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/settings.py | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index b6884a073e..7fb9d6d5a0 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -30,7 +30,7 @@ _default_settings = {
 }
 
 
-class _ProjectConfigParser(ConfigParser.SafeConfigParser):
+class _ProjectConfigParser(ConfigParser.ConfigParser):
     """ConfigParser that handles projects.
 
     There are two main goals of this class:
@@ -81,14 +81,14 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser):
     def __init__(self, project_name):
         """Construct _ProjectConfigParser.
 
-        In addition to standard SafeConfigParser initialization, this
-        also loads project defaults.
+        In addition to standard ConfigParser initialization, this also
+        loads project defaults.
 
         Args:
             project_name: The name of the project.
         """
         self._project_name = project_name
-        ConfigParser.SafeConfigParser.__init__(self)
+        ConfigParser.ConfigParser.__init__(self)
 
         # Update the project settings in the config based on
         # the _default_settings global.
@@ -100,31 +100,31 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser):
             self.set(project_settings, setting_name, setting_value)
 
     def get(self, section, option, *args, **kwargs):
-        """Extend SafeConfigParser to try project_section before section.
+        """Extend ConfigParser to try project_section before section.
 
         Args:
-            See SafeConfigParser.
+            See ConfigParser.
         Returns:
-            See SafeConfigParser.
+            See ConfigParser.
         """
         try:
-            val = ConfigParser.SafeConfigParser.get(
+            val = ConfigParser.ConfigParser.get(
                 self, "%s_%s" % (self._project_name, section), option,
                 *args, **kwargs
             )
         except (ConfigParser.NoSectionError, ConfigParser.NoOptionError):
-            val = ConfigParser.SafeConfigParser.get(
+            val = ConfigParser.ConfigParser.get(
                 self, section, option, *args, **kwargs
             )
         return val
 
     def items(self, section, *args, **kwargs):
-        """Extend SafeConfigParser to add project_section to section.
+        """Extend ConfigParser to add project_section to section.
 
         Args:
-            See SafeConfigParser.
+            See ConfigParser.
         Returns:
-            See SafeConfigParser.
+            See ConfigParser.
         """
         project_items = []
         has_project_section = False
@@ -132,7 +132,7 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser):
 
         # Get items from the project section
         try:
-            project_items = ConfigParser.SafeConfigParser.items(
+            project_items = ConfigParser.ConfigParser.items(
                 self, "%s_%s" % (self._project_name, section), *args, **kwargs
             )
             has_project_section = True
@@ -141,7 +141,7 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser):
 
         # Get top-level items
         try:
-            top_items = ConfigParser.SafeConfigParser.items(
+            top_items = ConfigParser.ConfigParser.items(
                 self, section, *args, **kwargs
             )
         except ConfigParser.NoSectionError:
-- 
2.38.1

[PATCH v5 3/6] patman: import gitutil module where it is needed

[Maxim Cournoyer]

Instead of propagating it from the module entry point (main script).

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/__main__.py | 3 +--
 tools/patman/settings.py | 8 +++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/patman/__main__.py b/tools/patman/__main__.py
index 11f19281fb..3f092367ec 100755
--- a/tools/patman/__main__.py
+++ b/tools/patman/__main__.py
@@ -22,7 +22,6 @@ if __name__ == "__main__":
 from patman import command
 from patman import control
 from patman import func_test
-from patman import gitutil
 from patman import project
 from patman import settings
 from patman import terminal
@@ -119,7 +118,7 @@ status.add_argument('-f', '--force', action='store_true',
 argv = sys.argv[1:]
 args, rest = parser.parse_known_args(argv)
 if hasattr(args, 'project'):
-    settings.Setup(gitutil, parser, args.project, '')
+    settings.Setup(parser, args.project, '')
     args, rest = parser.parse_known_args(argv)
 
 # If we have a command, it is safe to parse all arguments
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 7fb9d6d5a0..5efad5ed78 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -11,6 +11,8 @@ import argparse
 import os
 import re
 
+from patman import gitutil
+
 """Default settings per-project.
 
 These are used by _ProjectConfigParser.  Settings names should match
@@ -190,7 +192,7 @@ def ReadGitAliases(fname):
     fd.close()
 
 
-def CreatePatmanConfigFile(gitutil, config_fname):
+def CreatePatmanConfigFile(config_fname):
     """Creates a config file under $(HOME)/.patman if it can't find one.
 
     Args:
@@ -331,7 +333,7 @@ def GetItems(config, section):
         return []
 
 
-def Setup(gitutil, parser, project_name, config_fname=''):
+def Setup(parser, project_name, config_fname=''):
     """Set up the settings module by reading config files.
 
     Args:
@@ -348,7 +350,7 @@ def Setup(gitutil, parser, project_name, config_fname=''):
 
     if not os.path.exists(config_fname):
         print("No config file found ~/.patman\nCreating one...\n")
-        CreatePatmanConfigFile(gitutil, config_fname)
+        CreatePatmanConfigFile(config_fname)
 
     config.read(config_fname)
 
-- 
2.38.1

[PATCH v5 4/6] patman: set the default config_fname argument value to None

[Maxim Cournoyer]

This better matches Python conventions, allowing to easily test
whether the optional argument is provided.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/__main__.py | 2 +-
 tools/patman/settings.py | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/patman/__main__.py b/tools/patman/__main__.py
index 3f092367ec..15fd7603d7 100755
--- a/tools/patman/__main__.py
+++ b/tools/patman/__main__.py
@@ -118,7 +118,7 @@ status.add_argument('-f', '--force', action='store_true',
 argv = sys.argv[1:]
 args, rest = parser.parse_known_args(argv)
 if hasattr(args, 'project'):
-    settings.Setup(parser, args.project, '')
+    settings.Setup(parser, args.project)
     args, rest = parser.parse_known_args(argv)
 
 # If we have a command, it is safe to parse all arguments
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 5efad5ed78..8b846799df 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -333,19 +333,20 @@ def GetItems(config, section):
         return []
 
 
-def Setup(parser, project_name, config_fname=''):
+def Setup(parser, project_name, config_fname=None):
     """Set up the settings module by reading config files.
 
     Args:
-        parser:         The parser to update
+        parser:         The parser to update.
         project_name:   Name of project that we're working on; we'll look
             for sections named "project_section" as well.
-        config_fname:   Config filename to read ('' for default)
+        config_fname:   Config filename to read.
     """
     # First read the git alias file if available
     _ReadAliasFile('doc/git-mailrc')
     config = _ProjectConfigParser(project_name)
-    if config_fname == '':
+
+    if not config_fname:
         config_fname = '%s/.patman' % os.getenv('HOME')
 
     if not os.path.exists(config_fname):
-- 
2.38.1

[PATCH v5 5/6] patman: fail early in Setup when provided config file does not exist

[Maxim Cournoyer]

Rationale: if the user explicitly provide this argument, they probably
intend for it to be used.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/settings.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index 8b846799df..c05efd2475 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -340,12 +340,16 @@ def Setup(parser, project_name, config_fname=None):
         parser:         The parser to update.
         project_name:   Name of project that we're working on; we'll look
             for sections named "project_section" as well.
-        config_fname:   Config filename to read.
+        config_fname:   Config filename to read.  An error is raised if it
+            does not exist.
     """
     # First read the git alias file if available
     _ReadAliasFile('doc/git-mailrc')
     config = _ProjectConfigParser(project_name)
 
+    if config_fname and not os.path.exists(config_fname):
+        raise Exception(f'provided {config_fname} does not exist')
+
     if not config_fname:
         config_fname = '%s/.patman' % os.getenv('HOME')
 
-- 
2.38.1

[PATCH v5 6/6] patman: additionally honor a local .patman config file

[Maxim Cournoyer]

This enables versioning a project specific patman configuration file.
It also makes it possible to declare the project name, which is not a
useful thing to do in $HOME/.patman.  A new test is added, along
updated documentation.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

Changes in v5:
- Use try/finally in cleared_command_line_args context manager

Changes in v4:
- Use tools.run to invoke git in test suite

Changes in v3:
- Clear command line arguments in test_git_local_config
- Also test overriding a 'send' command line argument

Changes in v2:
- Explicitly provide an empty args list to parse_known_args in test

 tools/patman/patman.rst       |  8 ++++-
 tools/patman/settings.py      | 24 ++++++++++---
 tools/patman/test_settings.py | 67 +++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 5 deletions(-)
 create mode 100644 tools/patman/test_settings.py

diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
index 395fc0cd75..d7994c888b 100644
--- a/tools/patman/patman.rst
+++ b/tools/patman/patman.rst
@@ -74,7 +74,7 @@ out where to send patches pretty well.
 During the first run patman creates a config file for you by taking the default
 user name and email address from the global .gitconfig file.
 
-To add your own, create a file ~/.patman like this::
+To add your own, create a file `~/.patman` like this::
 
     # patman alias file
 
@@ -85,6 +85,12 @@ To add your own, create a file ~/.patman like this::
     wolfgang: Wolfgang Denk <wd@denx.de>
     others: Mike Frysinger <vapier@gentoo.org>, Fred Bloggs <f.bloggs@napier.net>
 
+Patman will also look for a `.patman` configuration file at the root
+of the current project git repository, which makes it possible to
+override the `project` settings variable or anything else in a
+project-specific way. The values of this "local" configuration file
+take precedence over those of the "global" one.
+
 Aliases are recursive.
 
 The checkpatch.pl in the U-Boot tools/ subdirectory will be located and
diff --git a/tools/patman/settings.py b/tools/patman/settings.py
index c05efd2475..636983e32d 100644
--- a/tools/patman/settings.py
+++ b/tools/patman/settings.py
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0+
 # Copyright (c) 2011 The Chromium OS Authors.
+# Copyright (c) 2022 Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
 #
 
 try:
@@ -336,6 +337,12 @@ def GetItems(config, section):
 def Setup(parser, project_name, config_fname=None):
     """Set up the settings module by reading config files.
 
+    Unless `config_fname` is specified, a `.patman` config file local
+    to the git repository is consulted, followed by the global
+    `$HOME/.patman`. If none exists, the later is created. Values
+    defined in the local config file take precedence over those
+    defined in the global one.
+
     Args:
         parser:         The parser to update.
         project_name:   Name of project that we're working on; we'll look
@@ -352,12 +359,21 @@ def Setup(parser, project_name, config_fname=None):
 
     if not config_fname:
         config_fname = '%s/.patman' % os.getenv('HOME')
+    has_config = os.path.exists(config_fname)
 
-    if not os.path.exists(config_fname):
-        print("No config file found ~/.patman\nCreating one...\n")
-        CreatePatmanConfigFile(config_fname)
+    git_local_config_fname = os.path.join(gitutil.get_top_level(), '.patman')
+    has_git_local_config = os.path.exists(git_local_config_fname)
 
-    config.read(config_fname)
+    # Read the git local config last, so that its values override
+    # those of the global config, if any.
+    if has_config:
+        config.read(config_fname)
+    if has_git_local_config:
+        config.read(git_local_config_fname)
+
+    if not (has_config or has_git_local_config):
+        print("No config file found.\nCreating ~/.patman...\n")
+        CreatePatmanConfigFile(config_fname)
 
     for name, value in GetItems(config, 'alias'):
         alias[name] = value.split(',')
diff --git a/tools/patman/test_settings.py b/tools/patman/test_settings.py
new file mode 100644
index 0000000000..c768a2fc64
--- /dev/null
+++ b/tools/patman/test_settings.py
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) 2022 Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
+#
+
+import argparse
+import contextlib
+import os
+import sys
+import tempfile
+
+from patman import settings
+from patman import tools
+
+
+@contextlib.contextmanager
+def empty_git_repository():
+    with tempfile.TemporaryDirectory() as tmpdir:
+        os.chdir(tmpdir)
+        tools.run('git', 'init', raise_on_error=True)
+        yield tmpdir
+
+
+@contextlib.contextmanager
+def cleared_command_line_args():
+    old_value = sys.argv[:]
+    sys.argv = [sys.argv[0]]
+    try:
+        yield
+    finally:
+        sys.argv = old_value
+
+
+def test_git_local_config():
+    # Clearing the command line arguments is required, otherwise
+    # arguments passed to the test running such as in 'pytest -k
+    # filter' would be processed by _UpdateDefaults and fail.
+    with cleared_command_line_args():
+        with empty_git_repository():
+            with tempfile.NamedTemporaryFile() as global_config:
+                global_config.write(b'[settings]\n'
+                                    b'project=u-boot\n')
+                global_config.flush()
+                parser = argparse.ArgumentParser()
+                parser.add_argument('-p', '--project', default='unknown')
+                subparsers = parser.add_subparsers(dest='cmd')
+                send = subparsers.add_parser('send')
+                send.add_argument('--no-check', action='store_false',
+                                  dest='check_patch', default=True)
+
+                # Test "global" config is used.
+                settings.Setup(parser, 'unknown', global_config.name)
+                args, _ = parser.parse_known_args([])
+                assert args.project == 'u-boot'
+                send_args, _ = send.parse_known_args([])
+                assert send_args.check_patch
+
+                # Test local config can shadow it.
+                with open('.patman', 'w', buffering=1) as f:
+                    f.write('[settings]\n'
+                            'project: guix-patches\n'
+                            'check_patch: False\n')
+                settings.Setup(parser, 'unknown', global_config.name)
+                args, _ = parser.parse_known_args([])
+                assert args.project == 'guix-patches'
+                send_args, _ = send.parse_known_args([])
+                assert not send_args.check_patch
-- 
2.38.1

Re: [PATCH v5 1/6] patman: fix pep8 warnings in settings module

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Remove extraneous imports, variables and comply to PEP 8 maximum line
> width, among other PEP 8 changes suggested by Pyflake.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/settings.py | 38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)

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

Re: [PATCH v5 2/6] patman: replace deprecated SafeConfigParser with ConfigParser

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> The SafeConfigParser class has been renamed in Python 3.2 to
> ConfigParser, and the old alias has been deprecated since.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/settings.py | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>

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

Re: [PATCH v5 3/6] patman: import gitutil module where it is needed

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Instead of propagating it from the module entry point (main script).
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/__main__.py | 3 +--
>  tools/patman/settings.py | 8 +++++---
>  2 files changed, 6 insertions(+), 5 deletions(-)

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

Re: [PATCH v5 4/6] patman: set the default config_fname argument value to None

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> This better matches Python conventions, allowing to easily test
> whether the optional argument is provided.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/__main__.py | 2 +-
>  tools/patman/settings.py | 9 +++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)

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

Re: [PATCH v5 5/6] patman: fail early in Setup when provided config file does not exist

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Rationale: if the user explicitly provide this argument, they probably
> intend for it to be used.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/settings.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

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

Re: [PATCH v5 6/6] patman: additionally honor a local .patman config file

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> This enables versioning a project specific patman configuration file.
> It also makes it possible to declare the project name, which is not a
> useful thing to do in $HOME/.patman.  A new test is added, along
> updated documentation.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> Changes in v5:
> - Use try/finally in cleared_command_line_args context manager
>
> Changes in v4:
> - Use tools.run to invoke git in test suite
>
> Changes in v3:
> - Clear command line arguments in test_git_local_config
> - Also test overriding a 'send' command line argument
>
> Changes in v2:
> - Explicitly provide an empty args list to parse_known_args in test
>
>  tools/patman/patman.rst       |  8 ++++-
>  tools/patman/settings.py      | 24 ++++++++++---
>  tools/patman/test_settings.py | 67 +++++++++++++++++++++++++++++++++++
>  3 files changed, 94 insertions(+), 5 deletions(-)
>  create mode 100644 tools/patman/test_settings.py

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

Re: [PATCH v5 6/6] patman: additionally honor a local .patman config file

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> This enables versioning a project specific patman configuration file.
> It also makes it possible to declare the project name, which is not a
> useful thing to do in $HOME/.patman.  A new test is added, along
> updated documentation.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> Changes in v5:
> - Use try/finally in cleared_command_line_args context manager
>
> Changes in v4:
> - Use tools.run to invoke git in test suite
>
> Changes in v3:
> - Clear command line arguments in test_git_local_config
> - Also test overriding a 'send' command line argument
>
> Changes in v2:
> - Explicitly provide an empty args list to parse_known_args in test
>
>  tools/patman/patman.rst       |  8 ++++-
>  tools/patman/settings.py      | 24 ++++++++++---
>  tools/patman/test_settings.py | 67 +++++++++++++++++++++++++++++++++++
>  3 files changed, 94 insertions(+), 5 deletions(-)
>  create mode 100644 tools/patman/test_settings.py

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

Applied to u-boot-dm/next, thanks!

Re: [PATCH v5 5/6] patman: fail early in Setup when provided config file does not exist

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Rationale: if the user explicitly provide this argument, they probably
> intend for it to be used.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/settings.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

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

Applied to u-boot-dm/next, thanks!

Re: [PATCH v5 3/6] patman: import gitutil module where it is needed

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Instead of propagating it from the module entry point (main script).
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/__main__.py | 3 +--
>  tools/patman/settings.py | 8 +++++---
>  2 files changed, 6 insertions(+), 5 deletions(-)

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

Applied to u-boot-dm/next, thanks!

Re: [PATCH v5 4/6] patman: set the default config_fname argument value to None

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> This better matches Python conventions, allowing to easily test
> whether the optional argument is provided.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/__main__.py | 2 +-
>  tools/patman/settings.py | 9 +++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)

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

Applied to u-boot-dm/next, thanks!

Re: [PATCH v5 2/6] patman: replace deprecated SafeConfigParser with ConfigParser

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> The SafeConfigParser class has been renamed in Python 3.2 to
> ConfigParser, and the old alias has been deprecated since.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/settings.py | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>

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

Applied to u-boot-dm/next, thanks!

Re: [PATCH v5 1/6] patman: fix pep8 warnings in settings module

[Simon Glass]

On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Remove extraneous imports, variables and comply to PEP 8 maximum line
> width, among other PEP 8 changes suggested by Pyflake.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/settings.py | 38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)

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

Applied to u-boot-dm/next, thanks!

Re: [PATCH v5 6/6] patman: additionally honor a local .patman config file

[Maxim Cournoyer]

Hi Simon,

Simon Glass <sjg@chromium.org> writes:

> On Mon, 19 Dec 2022 at 22:38, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>>
>> This enables versioning a project specific patman configuration file.
>> It also makes it possible to declare the project name, which is not a
>> useful thing to do in $HOME/.patman.  A new test is added, along
>> updated documentation.
>>
>> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
>> ---
>>
>> Changes in v5:
>> - Use try/finally in cleared_command_line_args context manager
>>
>> Changes in v4:
>> - Use tools.run to invoke git in test suite
>>
>> Changes in v3:
>> - Clear command line arguments in test_git_local_config
>> - Also test overriding a 'send' command line argument
>>
>> Changes in v2:
>> - Explicitly provide an empty args list to parse_known_args in test
>>
>>  tools/patman/patman.rst       |  8 ++++-
>>  tools/patman/settings.py      | 24 ++++++++++---
>>  tools/patman/test_settings.py | 67 +++++++++++++++++++++++++++++++++++
>>  3 files changed, 94 insertions(+), 5 deletions(-)
>>  create mode 100644 tools/patman/test_settings.py
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Applied to u-boot-dm/next, thanks!

Great, thank you for the review and seeing it through!

-- 
Thanks,
Maxim