[PATCH 0/1] Add add-maintainer.py script

[PATCH 0/1] Add add-maintainer.py script

[Guru Das Srinagesh]

When pushing patches to upstream, the `get_maintainer.pl` script is used to
determine whom to send the patches to. Instead of having to manually process
the output of the script, add a wrapper script to do that for you.

The add-maintainer.py script adds maintainers (and mailing lists) to a patch,
editing it in-place.

Thanks to Bjorn for being a sounding board to this idea and for his valuable
suggestions.

I referred to the following links during development of this script:
- https://stackoverflow.com/questions/4427542/how-to-do-sed-like-text-replace-with-python
- https://stackoverflow.com/questions/4146009/python-get-list-indexes-using-regular-expression
- https://stackoverflow.com/questions/10507230/insert-line-at-middle-of-file-with-python


Guru Das Srinagesh (1):
  scripts: Add add-maintainer.py

 scripts/add-maintainer.py | 82 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100755 scripts/add-maintainer.py


base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
-- 
2.40.0

[PATCH 1/1] scripts: Add add-maintainer.py

[Guru Das Srinagesh]

This script runs get_maintainer.py on a given patch file and adds its
output to the patch file in place with the appropriate email headers
"To: " or "Cc: " as the case may be. These new headers are added after
the "From: " line in the patch.

Currently, maintainers are added as "To: ", mailing lists are added as
"Cc: " and all others are addded as "Cc: " as well.

The script is quiet by default (only prints errors) and its verbosity
can be adjusted via an optional parameter.

Signed-off-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
---
 scripts/add-maintainer.py | 82 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100755 scripts/add-maintainer.py

diff --git a/scripts/add-maintainer.py b/scripts/add-maintainer.py
new file mode 100755
index 000000000000..942d331cf8d4
--- /dev/null
+++ b/scripts/add-maintainer.py
@@ -0,0 +1,82 @@
+#! /usr/bin/env python3
+
+import argparse
+import logging
+import os
+import sys
+import subprocess
+import re
+
+def add_maintainers_to_file(patch_file):
+    # Run get_maintainer.pl on patch file
+    logging.info("Patch: {}".format(os.path.basename(patch_file)))
+    cmd = ['scripts/get_maintainer.pl']
+    cmd.extend([patch_file])
+    p = subprocess.run(cmd, stdout=subprocess.PIPE, check=True)
+    logging.debug("\n{}".format(p.stdout.decode()))
+
+    entries = p.stdout.decode().splitlines()
+
+    maintainers = []
+    lists = []
+    others = []
+
+    for entry in entries:
+        entity = entry.split('(')[0].strip()
+        if "maintainer" in entry:
+            maintainers.append(entity)
+        elif "list" in entry:
+            lists.append(entity)
+        else:
+            others.append(entity)
+
+    # Specify email headers appropriately
+    to_maintainers  = ["To: " + m for m in maintainers]
+    cc_lists        = ["Cc: " + l for l in lists]
+    cc_others       = ["Cc: " + o for o in others]
+    logging.debug("To Maintainers:\n{}".format('\n'.join(to_maintainers)))
+    logging.debug("Cc Lists:\n{}".format('\n'.join(cc_lists)))
+    logging.debug("Cc Others:\n{}".format('\n'.join(cc_others)))
+
+    # Edit patch file in place to add maintainers
+    with open(patch_file, "r") as pf:
+        lines = pf.readlines()
+
+    from_line = [i for i, line in enumerate(lines) if re.search("From: ", line)]
+    if len(from_line) > 1:
+        logging.error("Only one From: line is allowed in a patch file")
+        sys.exit(1)
+
+    next_line_after_from = from_line[0] + 1
+
+    # Reverse lists to maintain same order as in get_maintainer.py output
+    # as we will be insert()-ing the elements
+    to_maintainers.reverse()
+    cc_lists.reverse()
+    cc_others.reverse()
+
+    for o in cc_others:
+        lines.insert(next_line_after_from, o + "\n")
+    for l in cc_lists:
+        lines.insert(next_line_after_from, l + "\n")
+    for m in to_maintainers:
+        lines.insert(next_line_after_from, m + "\n")
+
+    with open(patch_file, "w") as pf:
+        pf.writelines(lines)
+
+def main():
+    parser = argparse.ArgumentParser(description='Add the respective maintainers and mailing lists to patch files')
+    parser.add_argument('patches', nargs='*', help="One or more patch files")
+    parser.add_argument('--verbosity', choices=['debug', 'info', 'error'], default='error', help="Verbosity level of script output")
+    args = parser.parse_args()
+
+    logging.basicConfig(level=args.verbosity.upper(), format='%(levelname)s: %(message)s')
+
+    for patch in args.patches:
+        add_maintainers_to_file(patch)
+
+    logging.info("Maintainers added to all patch files successfully")
+
+if __name__ == "__main__":
+    main()
-- 
2.40.0

Re: [PATCH 1/1] scripts: Add add-maintainer.py

[Pavan Kondeti]

On Tue, Aug 01, 2023 at 09:54:29PM -0700, Guru Das Srinagesh wrote:
> This script runs get_maintainer.py on a given patch file and adds its
> output to the patch file in place with the appropriate email headers
> "To: " or "Cc: " as the case may be. These new headers are added after
> the "From: " line in the patch.
> 
> Currently, maintainers are added as "To: ", mailing lists are added as
> "Cc: " and all others are addded as "Cc: " as well.
> 
> The script is quiet by default (only prints errors) and its verbosity
> can be adjusted via an optional parameter.
> 
> Signed-off-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
> ---
>  scripts/add-maintainer.py | 82 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100755 scripts/add-maintainer.py
> 
> diff --git a/scripts/add-maintainer.py b/scripts/add-maintainer.py
> new file mode 100755
> index 000000000000..942d331cf8d4
> --- /dev/null
> +++ b/scripts/add-maintainer.py
> @@ -0,0 +1,82 @@
> +#! /usr/bin/env python3
> +
> +import argparse
> +import logging
> +import os
> +import sys
> +import subprocess
> +import re
> +
> +def add_maintainers_to_file(patch_file):
> +    # Run get_maintainer.pl on patch file
> +    logging.info("Patch: {}".format(os.path.basename(patch_file)))
> +    cmd = ['scripts/get_maintainer.pl']
> +    cmd.extend([patch_file])
> +    p = subprocess.run(cmd, stdout=subprocess.PIPE, check=True)
> +    logging.debug("\n{}".format(p.stdout.decode()))
> +
> +    entries = p.stdout.decode().splitlines()
> +
> +    maintainers = []
> +    lists = []
> +    others = []
> +
> +    for entry in entries:
> +        entity = entry.split('(')[0].strip()
> +        if "maintainer" in entry:
> +            maintainers.append(entity)
> +        elif "list" in entry:
> +            lists.append(entity)
> +        else:
> +            others.append(entity)
> +
> +    # Specify email headers appropriately
> +    to_maintainers  = ["To: " + m for m in maintainers]
> +    cc_lists        = ["Cc: " + l for l in lists]
> +    cc_others       = ["Cc: " + o for o in others]
> +    logging.debug("To Maintainers:\n{}".format('\n'.join(to_maintainers)))
> +    logging.debug("Cc Lists:\n{}".format('\n'.join(cc_lists)))
> +    logging.debug("Cc Others:\n{}".format('\n'.join(cc_others)))
> +
> +    # Edit patch file in place to add maintainers
> +    with open(patch_file, "r") as pf:
> +        lines = pf.readlines()
> +
> +    from_line = [i for i, line in enumerate(lines) if re.search("From: ", line)]
> +    if len(from_line) > 1:
> +        logging.error("Only one From: line is allowed in a patch file")
> +        sys.exit(1)
> +
> +    next_line_after_from = from_line[0] + 1
> +
> +    # Reverse lists to maintain same order as in get_maintainer.py output
> +    # as we will be insert()-ing the elements
> +    to_maintainers.reverse()
> +    cc_lists.reverse()
> +    cc_others.reverse()
> +
> +    for o in cc_others:
> +        lines.insert(next_line_after_from, o + "\n")
> +    for l in cc_lists:
> +        lines.insert(next_line_after_from, l + "\n")
> +    for m in to_maintainers:
> +        lines.insert(next_line_after_from, m + "\n")
> +
> +    with open(patch_file, "w") as pf:
> +        pf.writelines(lines)
> +
> +def main():
> +    parser = argparse.ArgumentParser(description='Add the respective maintainers and mailing lists to patch files')
> +    parser.add_argument('patches', nargs='*', help="One or more patch files")
> +    parser.add_argument('--verbosity', choices=['debug', 'info', 'error'], default='error', help="Verbosity level of script output")
> +    args = parser.parse_args()
> +
> +    logging.basicConfig(level=args.verbosity.upper(), format='%(levelname)s: %(message)s')
> +
> +    for patch in args.patches:
> +        add_maintainers_to_file(patch)
> +
> +    logging.info("Maintainers added to all patch files successfully")
> +
> +if __name__ == "__main__":
> +    main()
> -- 
> 2.40.0

Thanks Guru for submitting this script. In my limited testing, I don't
find any issues. It works as expected.

If I run this script on a series which spans across different lists,
there is a possiblity that we send partial series to certain lists. What
is the best way to deal with this? one way is to include union of all
lists in CC while sending the series with primary maintainer in TO. Can
the script print those values for easier workflow?

Thanks,
Pavan

Re: [PATCH 1/1] scripts: Add add-maintainer.py

[Guru Das Srinagesh]

On Aug 02 2023 15:35, Pavan Kondeti wrote:
> Thanks Guru for submitting this script. In my limited testing, I don't
> find any issues. It works as expected.

Great! Thank you so much for testing this script.

> If I run this script on a series which spans across different lists,
> there is a possiblity that we send partial series to certain lists. What
> is the best way to deal with this? one way is to include union of all
> lists in CC while sending the series with primary maintainer in TO. Can
> the script print those values for easier workflow?

Thanks for bringing this up - I was hoping to start a conversation on this.

Your suggestion seems logical to me. I can update v2 of this script adding an
`-a` flag that will add all maintainers to all patches following the scheme of
gathering the union of all maintainers, lists and "others" separately.

Hoping to get feedback from other maintainers on this point as well. Should I
expand the audience of this patch some more? I addressed this to whomever
`get_maintainer.pl -f ./scripts/` listed, plus Bjorn.

Guru Das.