[PATCH 1/3] patman: Add --no-signoff to suppress adding signoffs

[PATCH 1/3] patman: Add --no-signoff to suppress adding signoffs

[Philipp Tomsich]

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'")
 
-- 
1.8.3.1

[PATCH 2/3] patman: Add project-default for 'gcc'

[Philipp Tomsich]

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

[PATCH 3/3] patman: fix project-defaults not propagating into parsers

[Philipp Tomsich]

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.
-- 
1.8.3.1

[PATCH 1/3] patman: Add --no-signoff to suppress adding signoffs

[Simon Glass]

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>

[PATCH 2/3] patman: Add project-default for 'gcc'

[Simon Glass]

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>

[PATCH 3/3] patman: fix project-defaults not propagating into parsers

[Simon Glass]

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

[PATCH 3/3] patman: fix project-defaults not propagating into parsers

[Philipp Tomsich]

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

[PATCH 3/3] patman: fix project-defaults not propagating into parsers

[Philipp Tomsich]

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/85c84920f511d0d73a133336daeaf715a022cd64/Lib/argparse.py#L1763
  https://github.com/python/cpython/blob/85c84920f511d0d73a133336daeaf715a022cd64/Lib/argparse.py#L1363

Philipp.

[PATCH 3/3] patman: fix project-defaults not propagating into parsers

[Simon Glass]

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/85c84920f511d0d73a133336daeaf715a022cd64/Lib/argparse.py#L1763
>   https://github.com/python/cpython/blob/85c84920f511d0d73a133336daeaf715a022cd64/Lib/argparse.py#L1363

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_defaults

"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

[PATCH 3/3] patman: fix project-defaults not propagating into parsers

[Philipp Tomsich]

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_defaults
>
> "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.

[PATCH 3/3] patman: fix project-defaults not propagating into parsers

[Simon Glass]

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_defaults
> >
> > "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

[PATCH 2/3] patman: Add project-default for 'gcc'

[Simon Glass]

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!

[PATCH 1/3] patman: Add --no-signoff to suppress adding signoffs

[Simon Glass]

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!