[PATCH v4 0/6] add support for repository-local .patman config file
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 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 | 65 ++++++++++++++++++++++ 4 files changed, 137 insertions(+), 40 deletions(-) create mode 100644 tools/patman/test_settings.py
base-commit: 609e301d41284dbca3e6d03fe038ea0807b3df93
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()
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:
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)
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):
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')
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 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 | 65 +++++++++++++++++++++++++++++++++++ 3 files changed, 92 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..6c87fdc601 --- /dev/null +++ b/tools/patman/test_settings.py @@ -0,0 +1,65 @@ +# 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]] + yield + 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
participants (1)
-
Maxim Cournoyer