[PATCH 1/3] patman: Add --no-signoff to suppress adding signoffs
To enable use of patman with FSF/GNU projects, such as GCC or Binutils, no Signed-off-by may be added. This adds a command line flag '--no-signoff' to suppress adding signoffs in patman when processing commits.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu ---
tools/patman/control.py | 6 +++--- tools/patman/gitutil.py | 6 ++++-- tools/patman/main.py | 2 ++ 3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/tools/patman/control.py b/tools/patman/control.py index 2330682..ee9717c 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -20,7 +20,7 @@ def setup(): """Do required setup before doing anything""" gitutil.Setup()
-def prepare_patches(col, branch, count, start, end, ignore_binary): +def prepare_patches(col, branch, count, start, end, ignore_binary, signoff): """Figure out what patches to generate, then generate them
The patch files are written to the current directory, e.g. 0001_xxx.patch @@ -56,7 +56,7 @@ def prepare_patches(col, branch, count, start, end, ignore_binary): to_do = count - end series = patchstream.get_metadata(branch, start, to_do) cover_fname, patch_files = gitutil.CreatePatches( - branch, start, to_do, ignore_binary, series) + branch, start, to_do, ignore_binary, series, signoff)
# Fix up the patch files to our liking, and insert the cover letter patchstream.fix_patches(series, patch_files) @@ -163,7 +163,7 @@ def send(args): col = terminal.Color() series, cover_fname, patch_files = prepare_patches( col, args.branch, args.count, args.start, args.end, - args.ignore_binary) + args.ignore_binary, args.add_signoff) ok = check_patches(series, patch_files, args.check_patch, args.verbose)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index 31fb3b2..5736d37 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -305,7 +305,7 @@ def PruneWorktrees(git_dir): if result.return_code != 0: raise OSError('git worktree prune: %s' % result.stderr)
-def CreatePatches(branch, start, count, ignore_binary, series): +def CreatePatches(branch, start, count, ignore_binary, series, signoff = True): """Create a series of patches from the top of the current branch.
The patch files are written to the current directory using @@ -323,7 +323,9 @@ def CreatePatches(branch, start, count, ignore_binary, series): """ if series.get('version'): version = '%s ' % series['version'] - cmd = ['git', 'format-patch', '-M', '--signoff'] + cmd = ['git', 'format-patch', '-M' ] + if signoff: + cmd.append('--signoff') if ignore_binary: cmd.append('--no-binary') if series.get('cover'): diff --git a/tools/patman/main.py b/tools/patman/main.py index 342fd44..c4e4d80 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -81,6 +81,8 @@ send.add_argument('--no-check', action='store_false', dest='check_patch', help="Don't check for patch compliance") send.add_argument('--no-tags', action='store_false', dest='process_tags', default=True, help="Don't process subject tags as aliases") +send.add_argument('--no-signoff', action='store_false', dest='add_signoff', + default=True, help="Don't add Signed-off-by to patches") send.add_argument('--smtp-server', type=str, help="Specify the SMTP server to 'git send-email'")
Add defaults for FSF/GNU projects, such as gcc, that provide sensible settings for those projects.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu ---
tools/patman/settings.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/patman/settings.py b/tools/patman/settings.py index 8c10eab..bb3f868 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -23,7 +23,12 @@ _default_settings = { "u-boot": {}, "linux": { "process_tags": "False", - } + }, + "gcc": { + "process_tags": "False", + "add_signoff": "False", + "check_patch": "False", + }, }
class _ProjectConfigParser(ConfigParser.SafeConfigParser):
On Tue, 24 Nov 2020 at 10:15, Philipp Tomsich philipp.tomsich@vrull.eu wrote:
Add defaults for FSF/GNU projects, such as gcc, that provide sensible settings for those projects.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu
tools/patman/settings.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Tue, 24 Nov 2020 at 10:15, Philipp Tomsich philipp.tomsich@vrull.eu wrote:
Add defaults for FSF/GNU projects, such as gcc, that provide sensible settings for those projects.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu
tools/patman/settings.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
Project defaults (e.g. for linux and gcc) do not propagate into the subparsers. As both the processing of tags (e.g. in the defaults for the linux project) and supressing the signoff (in the defaults for the gcc project) are settings from subparsers, these would still require an explicit commandline option mirroring the (ignored) default.
This change ensures that defaults are updated in all parsers.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu ---
tools/patman/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/patman/settings.py b/tools/patman/settings.py index bb3f868..dc57b2f 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -266,7 +266,8 @@ def _UpdateDefaults(main_parser, config): print("WARNING: Unknown setting %s" % name)
# Set all the defaults (this propagates through all subparsers) - main_parser.set_defaults(**defaults) + for parser in parsers: + parser.set_defaults(**defaults)
def _ReadAliasFile(fname): """Read in the U-Boot git alias file if it exists.
Hi Philipp,
On Tue, 24 Nov 2020 at 10:15, Philipp Tomsich philipp.tomsich@vrull.eu wrote:
Project defaults (e.g. for linux and gcc) do not propagate into the subparsers. As both the processing of tags (e.g. in the defaults for the linux project) and supressing the signoff (in the defaults for the gcc project) are settings from subparsers, these would still require an explicit commandline option mirroring the (ignored) default.
This change ensures that defaults are updated in all parsers.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu
tools/patman/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/patman/settings.py b/tools/patman/settings.py index bb3f868..dc57b2f 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -266,7 +266,8 @@ def _UpdateDefaults(main_parser, config): print("WARNING: Unknown setting %s" % name)
# Set all the defaults (this propagates through all subparsers)
- main_parser.set_defaults(**defaults)
- for parser in parsers:
parser.set_defaults(**defaults)
According to the Python documentation and my testing, it should propagate. Do you know what is going wrong here? If there is a problem, we should update the comment.
def _ReadAliasFile(fname): """Read in the U-Boot git alias file if it exists. -- 1.8.3.1
Regards, Simon
Simon,
On Wed, 25 Nov 2020 at 00:41, Simon Glass sjg@chromium.org wrote:
Project defaults (e.g. for linux and gcc) do not propagate into the subparsers. As both the processing of tags (e.g. in the defaults for the linux project) and supressing the signoff (in the defaults for the gcc project) are settings from subparsers, these would still require an explicit commandline option mirroring the (ignored) default.
This change ensures that defaults are updated in all parsers.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu
tools/patman/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/patman/settings.py b/tools/patman/settings.py index bb3f868..dc57b2f 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -266,7 +266,8 @@ def _UpdateDefaults(main_parser, config): print("WARNING: Unknown setting %s" % name)
# Set all the defaults (this propagates through all subparsers)
- main_parser.set_defaults(**defaults)
- for parser in parsers:
parser.set_defaults(**defaults)According to the Python documentation and my testing, it should propagate. Do you know what is going wrong here? If there is a problem, we should update the comment.
I haven't dug down to find the root-cause, but I see the following behavior both on Debian 10.6 and CentOS 7 when adding a print(args) in main.py just before the __name__ == "__main__" check...
With this commit: $ tools/patman/patman -p linux -c1 send -n Namespace(add_maintainers=True, branch=None, cc_cmd=None, check_patch=True, cmd='send', count=1, debug=False, dest_branch=None, dry_run=True, end=0, force=False, full_help=False, ignore_bad_tags=False, ignore_binary=False, ignore_errors=False, in_reply_to=None, limit=None, patchfiles=['linux'], patchwork_url='https://patchwork.ozlabs.org', process_tags=False, project='linux', show_comments=False, smtp_server=None, start=0, testname='linux', thread=False, verbose=False)
=> process_tags=False
Without this commit: Namespace(add_maintainers=True, branch=None, cc_cmd=None, check_patch=True, cmd='send', count=1, debug=False, dest_branch=None, dry_run=True, end=0, force=False, full_help=False, ignore_bad_tags=False, ignore_binary=False, ignore_errors=False, in_reply_to=None, limit=None, patchfiles=[], patchwork_url='https://patchwork.ozlabs.org', process_tags=True, project='linux', show_comments=False, smtp_server=None, start=0, testname='linux', thread=False, verbose=False)
=> process_tags=True
So in my testing, the problem reproduces reliably across distributions.
Regards, Philipp
Simon,
On Wed, 25 Nov 2020 at 00:41, Simon Glass sjg@chromium.org wrote:
According to the Python documentation and my testing, it should propagate. Do you know what is going wrong here? If there is a problem, we should update the comment.
I don't see any code for propagating this in the argparse module: https://github.com/python/cpython/blob/85c84920f511d0d73a133336daeaf715a022c... https://github.com/python/cpython/blob/85c84920f511d0d73a133336daeaf715a022c...
Philipp.
Hi Philipp,
On Wed, 25 Nov 2020 at 06:02, Philipp Tomsich philipp.tomsich@vrull.eu wrote:
Simon,
On Wed, 25 Nov 2020 at 00:41, Simon Glass sjg@chromium.org wrote:
According to the Python documentation and my testing, it should propagate. Do you know what is going wrong here? If there is a problem, we should update the comment.
I don't see any code for propagating this in the argparse module: https://github.com/python/cpython/blob/85c84920f511d0d73a133336daeaf715a022c... https://github.com/python/cpython/blob/85c84920f511d0d73a133336daeaf715a022c...
Thanks for dingging into this.
I'll give it another try and try to figure out why it worked for me, but I expect I'll find the same problem.
Here is a pointer to the docs I saw:
https://docs.python.org/3/library/argparse.html#argparse.ArgumentParser.set_...
"Parser-level defaults can be particularly useful when working with multiple parsers. See the add_subparsers() method for an example of this type."
Regards, Simon
Simon,
On Wed, 25 Nov 2020 at 16:30, Simon Glass sjg@chromium.org wrote:
Here is a pointer to the docs I saw:
https://docs.python.org/3/library/argparse.html#argparse.ArgumentParser.set_...
"Parser-level defaults can be particularly useful when working with multiple parsers. See the add_subparsers() method for an example of this type."
I had looked at the same documentation and needed to carefully read the example to infer the original author's intended messaging...
The only mention of this in the subparser-docs was:
One particularly effective way of handling sub-commands is to combine the use of the add_subparsers() method with calls to set_defaults() so that each subparser knows which Python function it should execute.
This example just demonstrated that two different subparsers could create a different default for the same argument name. I couldn't find any example of a set_defaults() on a higher-level parser propagating and the argparse-source doesn't seem to try to propagate defaults either.
I am starting to think that the correct fix for this would be more along the lines of:
diff --git a/tools/patman/settings.py b/tools/patman/settings.py index bb3f868..525c69e 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -266,7 +266,11 @@ def _UpdateDefaults(main_parser, config): print("WARNING: Unknown setting %s" % name)
# Set all the defaults (this propagates through all subparsers)
- main_parser.set_defaults(**defaults)
- for parser in parsers:
for name, val in defaults.items():if parser.get_default(name) and val:parser.set_defaults(name=val)def _ReadAliasFile(fname): """Read in the U-Boot git alias file if it exists.
than of what I sent out earlier.
An interesting aside: it looks as if the double-parsing of the args leads to 'defaults' being installed for all arguments that are passed in the first cycle (e.g. count, project and patchwork_url suddenly have the values loaded from the config files and parsed from the args populated with 'default' values).
Philipp.
Hi Philipp,
On Wed, 25 Nov 2020 at 12:41, Philipp Tomsich philipp.tomsich@vrull.eu wrote:
Simon,
On Wed, 25 Nov 2020 at 16:30, Simon Glass sjg@chromium.org wrote:
Here is a pointer to the docs I saw:
https://docs.python.org/3/library/argparse.html#argparse.ArgumentParser.set_...
"Parser-level defaults can be particularly useful when working with multiple parsers. See the add_subparsers() method for an example of this type."
I had looked at the same documentation and needed to carefully read the example to infer the original author's intended messaging...
The only mention of this in the subparser-docs was:
One particularly effective way of handling sub-commands is to combine the use of the add_subparsers() method with calls to set_defaults() so that each subparser knows which Python function it should execute.
This example just demonstrated that two different subparsers could create a different default for the same argument name. I couldn't find any example of a set_defaults() on a higher-level parser propagating and the argparse-source doesn't seem to try to propagate defaults either.
I am starting to think that the correct fix for this would be more along the lines of:
diff --git a/tools/patman/settings.py b/tools/patman/settings.py index bb3f868..525c69e 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -266,7 +266,11 @@ def _UpdateDefaults(main_parser, config): print("WARNING: Unknown setting %s" % name)
# Set all the defaults (this propagates through all subparsers)
- main_parser.set_defaults(**defaults)
- for parser in parsers:
for name, val in defaults.items():if parser.get_default(name) and val:parser.set_defaults(name=val)def _ReadAliasFile(fname): """Read in the U-Boot git alias file if it exists.
than of what I sent out earlier.
Can you check u-boot/next for this? I have been testing it a bit and from what I can tell the code there does propagate things down to parsers.
An interesting aside: it looks as if the double-parsing of the args leads to 'defaults' being installed for all arguments that are passed in the first cycle (e.g. count, project and patchwork_url suddenly have the values loaded from the config files and parsed from the args populated with 'default' values).
Yes, it isn't very elegant. If you have a better way, it would be nice to improve it.
Regards, Simon
On Tue, 24 Nov 2020 at 10:15, Philipp Tomsich philipp.tomsich@vrull.eu wrote:
To enable use of patman with FSF/GNU projects, such as GCC or Binutils, no Signed-off-by may be added. This adds a command line flag '--no-signoff' to suppress adding signoffs in patman when processing commits.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu
tools/patman/control.py | 6 +++--- tools/patman/gitutil.py | 6 ++++-- tools/patman/main.py | 2 ++ 3 files changed, 9 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Tue, 24 Nov 2020 at 10:15, Philipp Tomsich philipp.tomsich@vrull.eu wrote:
To enable use of patman with FSF/GNU projects, such as GCC or Binutils, no Signed-off-by may be added. This adds a command line flag '--no-signoff' to suppress adding signoffs in patman when processing commits.
Signed-off-by: Philipp Tomsich philipp.tomsich@vrull.eu
tools/patman/control.py | 6 +++--- tools/patman/gitutil.py | 6 ++++-- tools/patman/main.py | 2 ++ 3 files changed, 9 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
participants (2)
-
Philipp Tomsich -
Simon Glass