[PATCH 0/3] BaseTools: Add a script to use with 'git-send-email --cc-cmd'

[PATCH 0/3] BaseTools: Add a script to use with 'git-send-email --cc-cmd'

[Philippe Mathieu-Daudé]

Hi,

This series contains trivial fixes to the GetMaintainer.py script,
then adds a new script which allow git-send-email to automatically
fills the email addresses relevant to each patch, using the new
Maintainer.txt format (with the GetMaintainer.py script).

I sent this series adding:

[sendemail]
    ccCmd = /home/phil/source/edk2/BaseTools/Scripts/GitCcCmd.sh

To my /home/phil/source/edk2/.git/config

The sendemail.ccCmd entry is documented here:
https://git-scm.com/docs/git-send-email#Documentation/git-send-email.txt---cc-cmdltcommandgt

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  BaseTools: Let the GetMaintainer.py script be executable
  BaseTools: Add '--quiet' option to GetMaintainer.py script
  BaseTools: Add script to help git-send-email pick addresses to cc

 BaseTools/Scripts/GetMaintainer.py |  9 +++++--
 BaseTools/Scripts/GitCcCmd.sh      | 38 ++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 BaseTools/Scripts/GetMaintainer.py
 create mode 100755 BaseTools/Scripts/GitCcCmd.sh

-- 
2.21.0

[PATCH 1/3] BaseTools: Let the GetMaintainer.py script be executable

[Philippe Mathieu-Daudé]

The GetMaintainer.py script use the "if __name__ == '__main__'"
evaluation so it expects to be run as a standalone program.

We have other Python scripts with the executable permission, let
this one be executable too.

Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 BaseTools/Scripts/GetMaintainer.py | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 BaseTools/Scripts/GetMaintainer.py

diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
old mode 100644
new mode 100755
-- 
2.21.0

Re: [PATCH 1/3] BaseTools: Let the GetMaintainer.py script be executable

[Bob Feng]

Hi Phil,

To make python script to be executable, I think only change file permission would not be enough. There also need to be a shebang
#!/usr/bin/env python at the top of this script.

But from the patch 3/3, I see in the GitCcCmd.sh, GetMaintainer.py is called by "python ${EDK_TOOLS_PATH}/Scripts/GetMaintainer.py --quiet"
Python interpreter is called explicit, so I think it's not necessary to change GetMaintainer.py permission.

Thanks,
Bob


-----Original Message-----
From: Philippe Mathieu-Daude [mailto:philmd@redhat.com] 
Sent: Wednesday, October 16, 2019 10:10 PM
To: Leif Lindholm <leif.lindholm@linaro.org>; devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Philippe Mathieu-Daude <philmd@redhat.com>
Subject: [PATCH 1/3] BaseTools: Let the GetMaintainer.py script be executable

The GetMaintainer.py script use the "if __name__ == '__main__'"
evaluation so it expects to be run as a standalone program.

We have other Python scripts with the executable permission, let this one be executable too.

Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 BaseTools/Scripts/GetMaintainer.py | 0
 1 file changed, 0 insertions(+), 0 deletions(-)  mode change 100644 => 100755 BaseTools/Scripts/GetMaintainer.py

diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
old mode 100644
new mode 100755
--
2.21.0

[PATCH 2/3] BaseTools: Add '--quiet' option to GetMaintainer.py script

[Philippe Mathieu-Daudé]

To have GetMaintainer.py script output easily parsable by
scripts, add the '--quiet' option. Output will be formatted
as one email per line.

Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 BaseTools/Scripts/GetMaintainer.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
index fbc63522db77..f26f2c5ba584 100755
--- a/BaseTools/Scripts/GetMaintainer.py
+++ b/BaseTools/Scripts/GetMaintainer.py
@@ -157,6 +157,10 @@ def get_modified_files(repo, args):
 if __name__ == '__main__':
     PARSER = argparse.ArgumentParser(
         description='Retrieves information on who to cc for review on a given commit')
+    PARSER.add_argument('-q', '--quiet',
+                        action="store_true",
+                        help='Quiet mode, useful when piping the output to a script',
+                        required=False)
     PARSER.add_argument('commit',
                         action="store",
                         help='git revision to examine (default: HEAD)',
@@ -181,10 +185,11 @@ if __name__ == '__main__':
     ADDRESSES = []
 
     for file in FILES:
-        print(file)
+        if not ARGS.quiet:
+            print(file)
         addresslist = get_maintainers(file, SECTIONS)
         if addresslist:
             ADDRESSES += addresslist
 
     for address in list(OrderedDict.fromkeys(ADDRESSES)):
-        print('  %s' % address)
+        print('%s%s' % ("" if ARGS.quiet else "  ", address))
-- 
2.21.0

[PATCH 3/3] BaseTools: Add script to help git-send-email pick addresses to cc

[Philippe Mathieu-Daudé]

Add a new script GitCcCmd.sh that git-send-email can use to
automatically determine which addresses to cc on patch submission.

Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 BaseTools/Scripts/GitCcCmd.sh | 38 +++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100755 BaseTools/Scripts/GitCcCmd.sh

diff --git a/BaseTools/Scripts/GitCcCmd.sh b/BaseTools/Scripts/GitCcCmd.sh
new file mode 100755
index 000000000000..3afdf3f1e40b
--- /dev/null
+++ b/BaseTools/Scripts/GitCcCmd.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+###
+# @file
+# Shell script to be called by 'git-send-email --cc-cmd' to pick the correct
+# maintainer emails for each patch when sending a series.
+#
+# Copyright (C) 2019, Red Hat, Inc.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+# This script expects the following EDK2 environment variables defined:
+# - WORKSPACE
+# - EDK_TOOLS_PATH
+#
+###
+
+set -e
+
+#
+# git-send-email calls this script with a list of patches generated by
+# git-format-patch.
+#
+for arg in $@; do
+    case ${arg} in
+    *0000-cover-letter.patch)
+        break
+        ;;
+    *.patch)
+        test -e ${arg} && (
+            cd ${WORKSPACE} && \
+            head -1 < ${arg} \
+            | cut -d' ' -f2 \
+            | xargs python ${EDK_TOOLS_PATH}/Scripts/GetMaintainer.py --quiet
+        )
+        break
+        ;;
+    esac
+done
-- 
2.21.0

Re: [PATCH 3/3] BaseTools: Add script to help git-send-email pick addresses to cc

[Philippe Mathieu-Daudé]

On 10/16/19 4:09 PM, Philippe Mathieu-Daude wrote:
> Add a new script GitCcCmd.sh that git-send-email can use to
> automatically determine which addresses to cc on patch submission.
> 
> Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
> ---
>   BaseTools/Scripts/GitCcCmd.sh | 38 +++++++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
>   create mode 100755 BaseTools/Scripts/GitCcCmd.sh
> 
> diff --git a/BaseTools/Scripts/GitCcCmd.sh b/BaseTools/Scripts/GitCcCmd.sh
> new file mode 100755
> index 000000000000..3afdf3f1e40b
> --- /dev/null
> +++ b/BaseTools/Scripts/GitCcCmd.sh
> @@ -0,0 +1,38 @@
> +#!/bin/sh
> +###
> +# @file
> +# Shell script to be called by 'git-send-email --cc-cmd' to pick the correct
> +# maintainer emails for each patch when sending a series.
> +#
> +# Copyright (C) 2019, Red Hat, Inc.
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +# This script expects the following EDK2 environment variables defined:
> +# - WORKSPACE
> +# - EDK_TOOLS_PATH
> +#
> +###
> +
> +set -e
> +
> +#
> +# git-send-email calls this script with a list of patches generated by
> +# git-format-patch.
> +#
> +for arg in $@; do
> +    case ${arg} in
> +    *0000-cover-letter.patch)
> +        break
> +        ;;
> +    *.patch)
> +        test -e ${arg} && (
> +            cd ${WORKSPACE} && \
> +            head -1 < ${arg} \
> +            | cut -d' ' -f2 \
> +            | xargs python ${EDK_TOOLS_PATH}/Scripts/GetMaintainer.py --quiet
> +        )
> +        break
> +        ;;
> +    esac
> +done

I meant to tag this patch RFC but forgot about it before sending, so 
please consider it as RFC :)

Re: [PATCH 0/3] BaseTools: Add a script to use with 'git-send-email --cc-cmd'

[Laszlo Ersek]

Hi Phil,

On 10/16/19 16:09, Philippe Mathieu-Daude wrote:
> Hi,
> 
> This series contains trivial fixes to the GetMaintainer.py script,
> then adds a new script which allow git-send-email to automatically
> fills the email addresses relevant to each patch, using the new
> Maintainer.txt format (with the GetMaintainer.py script).
> 
> I sent this series adding:
> 
> [sendemail]
>     ccCmd = /home/phil/source/edk2/BaseTools/Scripts/GitCcCmd.sh
> 
> To my /home/phil/source/edk2/.git/config
> 
> The sendemail.ccCmd entry is documented here:
> https://git-scm.com/docs/git-send-email#Documentation/git-send-email.txt---cc-cmdltcommandgt

I'm neutral on this patch set; I defer to Leif and others for reviewing it.

I like to rely on explicit Cc: tags in the commit messages, and those
tags only. They give me better per-patch control, they work nicely
across rebases, and I like to see them in the git commit history too.
Furthermore, if someone else takes over a patch from a pending series,
for a rework, they get the Cc's too. If a reviewer expresses permanent
uninterest in a patch, I can drop them permanently.

That said, I don't mind if others use "sendemail.ccCmd"!

Thanks,
Laszlo

Re: [PATCH 0/3] BaseTools: Add a script to use with 'git-send-email --cc-cmd'

[Leif Lindholm]

Hi Phil,

This is definitely a manual step that could do with automation, so
many thanks for having a go. ...But, I have a few reservations
regarding the actual solution.

First of all, doing it as a .sh means it will not work for those
developers working primarily with Visual Studio. Rewriting it as
python script would resolve this, and also let you import
GetMaintainer.py and use functions from there instead of adding a
quiet mode to that. (I would very much like to not turn the
GetMaintainer.py output into an ABI.)

Secondly, I agree with Laszlo that I would rather see something that
operated with format-patch than send-email (since we want to keep the
Cc: tags in the commit messages). That command does not have the same
useful hook, however.

For this reason *I* would not use said script, but I'm not going to
say we shouldn't have it. I will say it shouldn't be enabled by
default by SetupGit.py though.

Best Regards,

Leif

On Wed, Oct 16, 2019 at 04:09:37PM +0200, Philippe Mathieu-Daude wrote:
> This series contains trivial fixes to the GetMaintainer.py script,
> then adds a new script which allow git-send-email to automatically
> fills the email addresses relevant to each patch, using the new
> Maintainer.txt format (with the GetMaintainer.py script).
> 
> I sent this series adding:
> 
> [sendemail]
>     ccCmd = /home/phil/source/edk2/BaseTools/Scripts/GitCcCmd.sh
> 
> To my /home/phil/source/edk2/.git/config
> 
> The sendemail.ccCmd entry is documented here:
> https://git-scm.com/docs/git-send-email#Documentation/git-send-email.txt---cc-cmdltcommandgt
>
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (3):
>   BaseTools: Let the GetMaintainer.py script be executable
>   BaseTools: Add '--quiet' option to GetMaintainer.py script
>   BaseTools: Add script to help git-send-email pick addresses to cc
> 
>  BaseTools/Scripts/GetMaintainer.py |  9 +++++--
>  BaseTools/Scripts/GitCcCmd.sh      | 38 ++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 2 deletions(-)
>  mode change 100644 => 100755 BaseTools/Scripts/GetMaintainer.py
>  create mode 100755 BaseTools/Scripts/GitCcCmd.sh
> 
> -- 
> 2.21.0
> 

Re: [PATCH 0/3] BaseTools: Add a script to use with 'git-send-email --cc-cmd'

[Philippe Mathieu-Daudé]

On 10/16/19 5:18 PM, Leif Lindholm wrote:
> Hi Phil,
> 
> This is definitely a manual step that could do with automation, so
> many thanks for having a go. ...But, I have a few reservations
> regarding the actual solution.
> 
> First of all, doing it as a .sh means it will not work for those
> developers working primarily with Visual Studio. Rewriting it as
> python script would resolve this, and also let you import
> GetMaintainer.py and use functions from there instead of adding a
> quiet mode to that. (I would very much like to not turn the
> GetMaintainer.py output into an ABI.)

OK. I was not expecting the .sh to get merged but wanted to have 
feedback as in a RFC, so I sent it mostly as a proof of concept.

> Secondly, I agree with Laszlo that I would rather see something that
> operated with format-patch than send-email (since we want to keep the
> Cc: tags in the commit messages). That command does not have the same
> useful hook, however.

Laszlo's reply and yours are the feedback I was expecting :)

> For this reason *I* would not use said script, but I'm not going to
> say we shouldn't have it. I will say it shouldn't be enabled by
> default by SetupGit.py though.

Well, I wrote it because I use git-publish in my workflow and wanted to 
use your GetMaintainer script output to select the recipients.

I'm not custom to the Cc tag (I don't use it). Maybe my problem is 
git-publish not handling correctly the Cc tag feature, so forget about 
this series for now, I'll see how to better integrate both tools in my 
workflow and eventually respin (without using your script as a stable ABI).

Regards,

Phil.

> On Wed, Oct 16, 2019 at 04:09:37PM +0200, Philippe Mathieu-Daude wrote:
>> This series contains trivial fixes to the GetMaintainer.py script,
>> then adds a new script which allow git-send-email to automatically
>> fills the email addresses relevant to each patch, using the new
>> Maintainer.txt format (with the GetMaintainer.py script).
>>
>> I sent this series adding:
>>
>> [sendemail]
>>      ccCmd = /home/phil/source/edk2/BaseTools/Scripts/GitCcCmd.sh
>>
>> To my /home/phil/source/edk2/.git/config
>>
>> The sendemail.ccCmd entry is documented here:
>> https://git-scm.com/docs/git-send-email#Documentation/git-send-email.txt---cc-cmdltcommandgt
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (3):
>>    BaseTools: Let the GetMaintainer.py script be executable
>>    BaseTools: Add '--quiet' option to GetMaintainer.py script
>>    BaseTools: Add script to help git-send-email pick addresses to cc
>>
>>   BaseTools/Scripts/GetMaintainer.py |  9 +++++--
>>   BaseTools/Scripts/GitCcCmd.sh      | 38 ++++++++++++++++++++++++++++++
>>   2 files changed, 45 insertions(+), 2 deletions(-)
>>   mode change 100644 => 100755 BaseTools/Scripts/GetMaintainer.py
>>   create mode 100755 BaseTools/Scripts/GitCcCmd.sh
>>
>> -- 
>> 2.21.0
>>

Re: [PATCH 0/3] BaseTools: Add a script to use with 'git-send-email --cc-cmd'

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]

On Wed, Oct 16, 2019 at 06:50:29PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/16/19 5:18 PM, Leif Lindholm wrote:
> > For this reason *I* would not use said script, but I'm not going to
> > say we shouldn't have it. I will say it shouldn't be enabled by
> > default by SetupGit.py though.
> 
> Well, I wrote it because I use git-publish in my workflow and wanted to use
> your GetMaintainer script output to select the recipients.
> 
> I'm not custom to the Cc tag (I don't use it). Maybe my problem is
> git-publish not handling correctly the Cc tag feature, so forget about this
> series for now, I'll see how to better integrate both tools in my workflow
> and eventually respin (without using your script as a stable ABI).

git-publish should handle Cc: tags unless you have set the --suppress-cc
option on the command-line or in git-config(1).

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]