Re: [Xen-devel] [PATCH for-4.11 v3 1/1] Add new add_maintainers.pl script to optimise the workflow when using git format-patch with get_maintainer.pl

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 05/04/2018 09:36 AM, Lars Kurth wrote:
> The tool covers step 2 of the following workflow
> 
>   Step 1: git format-patch ... -o <patchdir> ...
>   Step 2: ./scripts/add_maintainers.pl -d <patchdir>
>           This overwrites  *.patch files in <patchdir>
>   Step 3: git send-email -to xen-devel@xxxxxxxxxxxxxxxxxxxx 
> <patchdir>/*.patchxm
> 
> I manually tested all options and the most common combinations
> on Mac.
> 
> Changes since v1:
> - Added RAB (indicated by Juergen on IRC that this is OK)
> - Remove trailing whitespaces
> - Renamed --prefix to --reroll-count
> - Cleaned up short options -v, ... to be in line with git
> - Added --tags|-t option to add AB, RAB and RB emails to CC list
> - Added --insert|-i mode to allow for people adding CCs to commit message
>   instead of the e-mail header (the header is the default)
> - Moved common code into functions
> - Added logic, such that the tool only insert's To: and Cc: statements
>   which were not there before, allowing for running the tool multiple times
>   on the same <patchdir>
> 
> Changes since v2:
> - Deleted --version and related infrastructure
> - Added subroutine prototypes
> - Removed AT and @lists declaration and used \@ in literals
> - Changed usage message and options based on feedback
> - Improved error handling
> - Removed occurances of index() and replaced with regex
> - Removed non-perl idioms
> - Moved uniq statements to normalize and added info on what normalize does
> - Read L: tags from MAINTAINERS file instead of using heuristic
> - Fixed issues related to metacharacters in getmaintainers()
> - Allow multiple -a | --arg values (because of this renamed --args)
> - Identify tags via regex
> - CC's from tags are only inserted in the mail header, never the body
> - That is unless the new option --tagscc is used
> - Added policy processing which includes reworking insert()
> - Replaced -i|--insert with -p|--inspatch and -c|--inscover now using policies
> - Added new policies to cover for all user requests
> - Rewrote help message to center around usage of policies
> - Reordered some code (e.g. help string first to make code more easily 
> readable)
> 
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Signed-off-by: Lars Kurth <lars.kurth@xxxxxxxxxx>
> Release-acked-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  scripts/add_maintainers.pl | 512 
> +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 512 insertions(+)
>  create mode 100755 scripts/add_maintainers.pl
> 
> diff --git a/scripts/add_maintainers.pl b/scripts/add_maintainers.pl
> new file mode 100755
> index 0000000000..11ae60d888
> --- /dev/null
> +++ b/scripts/add_maintainers.pl
> @@ -0,0 +1,512 @@
> +#!/usr/bin/perl -w
> +# (c) 2018, Lars Kurth <lars.kurth@xxxxxxxxxx>
> +#
> +# Add maintainers to patches generated with git format-patch
> +#
> +# Usage: perl scripts/add_maintainers.pl [OPTIONS] -patchdir <patchdir>
> +#
> +# Prerequisites: Execute
> +#                git format-patch ... -o <patchdir> ...
> +#
> +#                ./scripts/get_maintainer.pl is present in the tree
> +#
> +# Licensed under the terms of the GNU GPL License version 2
> +
> +use strict;
> +
> +use Getopt::Long qw(:config no_auto_abbrev);
> +use File::Basename;
> +use List::MoreUtils qw(uniq);
> +
> +sub getmaintainers ($$$);
> +sub gettagsfrompatch ($$$;$);
> +sub normalize ($$);
> +sub insert ($$$$);
> +sub hastag ($$);
> +
> +# Tool Variables
> +my $tool = $0;
> +my $usage = <<EOT;
> +USAGE: $tool [options] (--patchdir | -d) <patchdir>
> +
> +OPTIONS:
> +--------
> +  --reroll-count <n> | -v <n>
> +    Choose patch files for specific version. This results into the
> +    following filters on <patchdir>
> +    0: default - *.patch
> +    >1: v<n>*.patch
> +  --inspatch (top|ccbody|cc---|none) | -p (top|ccbody|cc---|none)
> +    Insert email addresses into *.patch files according to the POLICY
> +    See section POLICY:
> +  --inscover (top|ccend|none) | -c (top|ccend|none)
> +    Insert email addresses into cover letteraccording to the POLICY
> +    See section PROCESSING POLICY:
> +  --tags | -t
> +    Read email addresses from tags and add to CC list.
> +    Note that git send-email does not do this. It will add the senders
> +    email adress to the CC list though
> +  --tagscc
> +    Same as tags, only that in this case CCs extracted from tags
> +    are treated like CCs that have come from the *.patch file

Not clear on the difference between these.

> +  --arg <argument> | -a <argument> ...
> +    Arguments passed on to get_maintainer.pl
> +    This option can be used multiple times, e.g. -a <a1> -a <a2> ...
> +  --verbose
> +    Show more output
> +  --help | -h
> +    Show this help information
> +
> +PROCESSING POLICY:

Why is this called 'policy'?  This seems to be definitions.

> +------------------
> +  *.patch files consist of several sections relevant to processing:
> +  <top>:   This is the email header containing email related information
> +           It ends with the Subject: line
> +  <body>:  This is the body that ends up in the commit message
> +           It ends with ---
> +  <--->:   This section contains the actual patches. CCs added here are
> +           processed by git send-email, but are not stored in the commit
> +           message. Some people add CCs into this section

<---> is not a normal name (how do you say it? "dash-dash-dash"?), and
worse yet might be confused with an option.  `--inspatch cc---` looks
like there was some sort of mistake.  "Top" would normally mean, "Top of
the body of the mail".

I think it would be better to call these sections <header>, <commit> and
"commit message", and <comment> and "reviewer comment section",
respectively.


> +  <ccend>: It ends with '-- '
> +
> +  Note that cover letters do not have the <body> section.
> +
> +  The following options specifiy how CCs are insertied into *.patch files
> +  top:    Insert CCs into the email header
> +          Insert CCs from *-by: tags and TOs from mailing lists into the 
> header
> +          (this is the default)
> +  ccbody: Insert CCs into body
> +          Insert CCs from *-by: tags and TOs from mailing lists into the 
> header
> +          unless specified otherwise (via --tagscc).
> +  cc---:  Insert CCs just after the --- line
> +          Insert CCs from *-by: tags and TOs from mailing lists into the 
> header
> +          unless specified otherwise (via --tagscc).
> +  ccend:  Insert CCs before the '-- ' line
> +          Insert CCs from *-by: tags and TOs from mailing lists into the 
> header
> +          unless specified otherwise (via --tagscc).
> +  none:   Neither insert TO, CCs from --tags nor other CCs

I don't really get this section.

What about having the functionality be something like this?  (Obvious
this would need some code changes as well.  Also I guessed what the
significance of the `-- ` is in the cover letter, so correct me if I'm
wrong.)

---
USAGE: $tool [options] (--patchdir | -d) <patchdir>

OPTIONS:
--------

  --reroll-count <n> | -v <n>
    Choose patch files for specific version. This results into the
    following filters on <patchdir>
    0: default - *.patch
    >1: v<n>*.patch

  --patchcc (top|commit|comment|none) | -p (top|commit|comment|none)

    Insert CC lines into *.patch files in the specified location.
    See LOCATIONS for a definition of the various locations.

    The default is `top`.

  --covercc (top|end|none) | -c (top|end|none)

    Insert CC lines into cover letter in the specified location. See
    LOCATIONS for a definition of the various locations.

    The default is `top`.

  --tags | -t

    In addition to the output of get_maintainer.pl, include email
    addresses from commit tags (e.g., Reviewed-by, Tested-by, &c) in
    the list of CC lines to insert.

  --arg <argument> | -a <argument> ...
    Arguments passed on to get_maintainer.pl
    This option can be used multiple times, e.g. -a <a1> -a <a2> ...

  --verbose
    Show more output

  --help | -h
    Show this help information

LOCATIONS
---------

  *.patch and cover letters files consist of several sections relevant
   to processing:

  <header>:  This is the email header containing email related information
             It ends with the Subject: line

  <commit>: This is the email body that ends up in the commit message.
             It ends with ---.  CC lines added here will be checked
             into the git tree on commit.  Only applicable to normal
             patch files.

  <comment>: This is the 'comment for reviewers' section, after the
             --- but before the diff actually starts. CCs added here
             are processed by git send-email, but are not checked into
             the git tree on commit.  Only applicable to normal patch
             files.

  <end>:     The part of a cover letter just before `-- ` (which normally
             begins a diffstat).  Only applicable to cover letters.

---

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel