Re: [Xen-devel] [PATCH v2] Allow get_maintainer.pl / add_maintainers.pl scripts to be called outside of xen.git

[Lars Kurth <lars.kurth@xxxxxxxxxx>]

On 16/08/2019, 11:01, "Julien Grall" <julien.grall@xxxxxxx> wrote:

    Hi Lars,
    
    Thank you for the patch.
    
    On 15/08/2019 18:22, Lars Kurth wrote:
    > Use-case: Allow using both scripts on xen repositories such as
    > mini-os.git, osstest.git,
    > 
    > Tool changes:
    > * add_maintainers.pl: $get_maintainer inherits path from $0
    > * get_maintainer.pl: warn (instead fo die) when called
    >    from a different tree
    > 
    > Assumptions: the repository contains a MAINTAINERS file that
    > follows the same conventions as the file in xen.git
    > 
    > A suggested template
    > 
    > ========================================================
    > This file follows the same conventions as outlined in
    > xen.git:MAINTAINERS. Please refer to the file in xen.git
    > for more information.
    > 
    > THE REST
    > M:        MAINTAINER1 <maintainer1@xxxxxxxxx>
    > M:      MAINTAINER2 <maintainer2@xxxxxxxxx>
    > L:        xen-devel@xxxxxxxxxxxxxxxxxxxx
    > S:        Supported
    > F:        */
    > ========================================================
    > 
    > Signed-off-by: Lars Kurth <lars.kurth@xxxxxxxxxx>
    > ---
    >   scripts/add_maintainers.pl |  4 ++--
    >   scripts/get_maintainer.pl  | 13 +++++++++++--
    >   2 files changed, 13 insertions(+), 4 deletions(-)
    > 
    > diff --git a/scripts/add_maintainers.pl b/scripts/add_maintainers.pl
    > index 09e9f6609f..5a6d0f631b 100755
    > --- a/scripts/add_maintainers.pl
    > +++ b/scripts/add_maintainers.pl
    > @@ -26,9 +26,9 @@ sub insert ($$$$);
    >   sub hastag ($$);
    >   
    >   # Tool Variables
    > -my $get_maintainer      = "./scripts/get_maintainer.pl";
    > -
    >   my $tool = $0;
    > +my $get_maintainer = $tool;
    > +$get_maintainer =~ s/add_maintainers/get_maintainer/;
    >   my $usage = <<EOT;
    >   OPTIONS:
    >   --------
    > diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
    > index 174dfb7e40..f1e9c904ee 100755
    > --- a/scripts/get_maintainer.pl
    > +++ b/scripts/get_maintainer.pl
    > @@ -266,8 +266,17 @@ if ($email &&
    >   }
    >   
    >   if (!top_of_tree($xen_path)) {
    > -    die "$P: The current directory does not appear to be "
    > - . "a Xen source tree.\n";
    > +    # Do not exit, but print an error message to STDERR to allow calling
    > +    # the tool from xen-related repos such as mini-os.git,
    > +    # live patch-build-tools.git, etc
    > +    print STDERR "$P:\n".
    > +          "=====================================================\n".
    > +          "WARNING: The current directory does not appear to be \n".
    > +   "the xen.git source tree.\n\n".
    > +          "The tool works outside of the xen.git tree, if the\n".
    > +          "MAINTAINERS file follows the same format as that of\n".
    > +          "xen.git. Use at your own peril.\n".
    > +          "=====================================================\n";
    
     From my understanding, any use on mini-os.git & co will be legitimate. 
However, 
    we still print the WARNING in those cases.
    
    Usually WARNING means something needs attention. As most of the users will 
    likely copy/paste from the wiki, we are going to have report asking why the 
    WARNING is there.
    
    I think it would make sense to try to downgrade the message a bit when 
possible. 
    For instance, we could check if the section "THE REST" is present in the 
file 
    MAINTAINERS. If not, this is likely not a file we are able to support.
    
I thought about this and it is not as easy as it seems, because the script only 
parses
M: ... &c lines

Maybe the best way to address this would be to include some identifier into the
MAINTAINERS file (after the header with all the definitions). 

FORMAT: xen-project-maintainers <version>  
(note that this is not currently picked up by the tool)

Or

V: xen-project-maintainers <version>  
(note that this would be picked up by the tool)

Then any compliant version is easily identifiable and the warning can be 
supressed.

That would make maintainers look like 

============
....
V:  xen-project-maintainers 1.6

ACPI
M:      Jan Beulich <jbeulich@xxxxxxxx>  
...
============

This seems to be more of a can of worms than I thought.

Cheers
Lars


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