OSes have additional maintainers that should be cc'd on patches or may
want to circulate internal patches.
Parse the .get_maintainer.MAINTAINERS file. Entries in the file
can begin with a '+' to indicate the email and list entries should be
added to the exiting MAINTAINERS output, or a '-' to indicate that the
entries should override the existing MAINTAINERS file.
Also add a help entry for the .get_maintainers.ignore file.
Signed-off-by: Prarit Bhargava <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
scripts/get_maintainer.pl | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index c87fa734e3e1..239f4d2ce972 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -384,6 +384,32 @@ sub find_ignore_git {
read_all_maintainer_files();
sub read_all_maintainer_files {
+ my $conf = which_conf(".get_maintainer.MAINTAINERS");
+ if ( -f $conf) {
+ my @conf_args;
+ my $add = 0;
+ open(my $conffile, '<', "$conf")
+ or warn "$P: Can't find a readable .get_maintainer.MAINTAINERS file $!\n";
+ while (<$conffile>) {
+ my $line = $_;
+ if ($line =~ m/^\+/ ) {
+ $add = 1;
+ }
+ next if ($line =~ m/^\s*#/);
+ $line =~ s/^\+//g;
+ $line =~ s/^\-//g;
+ $line =~ s/\s*\n?$//;
+ push(@mfiles, $line);
+ }
+ close($conffile);
+ if ($add eq 0) {
+ foreach my $file (@mfiles) {
+ read_maintainer_file("$file");
+ }
+ return;
+ }
+ }
+
if (-d "${lk_path}MAINTAINERS") {
opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
my @files = readdir(DIR);
@@ -1068,6 +1094,14 @@ Notes:
Entries in this file can be any command line argument.
This file is prepended to any additional command line arguments.
Multiple lines and # comments are allowed.
+ File ".get_maintainer.ignore", if it exists in the linux kernel source root
+ directory, can contain a list of email addresses to ignore. Multiple
+ lines and # comments are allowed.
+ File ".get_maintainer.MAINTAINERS", if it exists in the linux kernel source
+ root directory, can change the location of the MAINTAINERS file.
+ Entries beginning with a '+' are added to the default list, and
+ entries beginning with a '-' override the existing MAINTAINERS list
+ lookup. Multiple lines and # comments are allowed.
Most options have both positive and negative forms.
The negative forms for --<foo> are --no<foo> and --no-<foo>.
--
2.14.4
On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
> OSes have additional maintainers that should be cc'd on patches or may
> want to circulate internal patches.
>
> Parse the .get_maintainer.MAINTAINERS file. Entries in the file
> can begin with a '+' to indicate the email and list entries should be
> added to the exiting MAINTAINERS output, or a '-' to indicate that the
> entries should override the existing MAINTAINERS file.
>
> Also add a help entry for the .get_maintainers.ignore file.
I see no reason for this patch to be applied.
Why should it?
Why shouldn't this be in your private repository?
> Signed-off-by: Prarit Bhargava <[email protected]>
> Cc: Joe Perches <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> scripts/get_maintainer.pl | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index c87fa734e3e1..239f4d2ce972 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -384,6 +384,32 @@ sub find_ignore_git {
> read_all_maintainer_files();
>
> sub read_all_maintainer_files {
> + my $conf = which_conf(".get_maintainer.MAINTAINERS");
> + if ( -f $conf) {
> + my @conf_args;
> + my $add = 0;
> + open(my $conffile, '<', "$conf")
> + or warn "$P: Can't find a readable .get_maintainer.MAINTAINERS file $!\n";
> + while (<$conffile>) {
> + my $line = $_;
> + if ($line =~ m/^\+/ ) {
> + $add = 1;
> + }
> + next if ($line =~ m/^\s*#/);
> + $line =~ s/^\+//g;
> + $line =~ s/^\-//g;
> + $line =~ s/\s*\n?$//;
> + push(@mfiles, $line);
> + }
> + close($conffile);
> + if ($add eq 0) {
> + foreach my $file (@mfiles) {
> + read_maintainer_file("$file");
> + }
> + return;
> + }
> + }
> +
> if (-d "${lk_path}MAINTAINERS") {
> opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
> my @files = readdir(DIR);
> @@ -1068,6 +1094,14 @@ Notes:
> Entries in this file can be any command line argument.
> This file is prepended to any additional command line arguments.
> Multiple lines and # comments are allowed.
> + File ".get_maintainer.ignore", if it exists in the linux kernel source root
> + directory, can contain a list of email addresses to ignore. Multiple
> + lines and # comments are allowed.
> + File ".get_maintainer.MAINTAINERS", if it exists in the linux kernel source
> + root directory, can change the location of the MAINTAINERS file.
> + Entries beginning with a '+' are added to the default list, and
> + entries beginning with a '-' override the existing MAINTAINERS list
> + lookup. Multiple lines and # comments are allowed.
> Most options have both positive and negative forms.
> The negative forms for --<foo> are --no<foo> and --no-<foo>.
>
On 06/26/2018 04:16 PM, Joe Perches wrote:
> On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
>> OSes have additional maintainers that should be cc'd on patches or may
>> want to circulate internal patches.
>>
>> Parse the .get_maintainer.MAINTAINERS file. Entries in the file
>> can begin with a '+' to indicate the email and list entries should be
>> added to the exiting MAINTAINERS output, or a '-' to indicate that the
>> entries should override the existing MAINTAINERS file.
>>
>> Also add a help entry for the .get_maintainers.ignore file.
>
> I see no reason for this patch to be applied.
> Why should it?
The kernel has other vendor/OS changes like my patch, for example, 4efb442cc12e
("kernel/panic.c: add TAINT_AUX"). From that commit message
Add an auxiliary taint flag to be used by distros and others. This
obviates the need to forward-port whatever internal solutions people
have in favor of a single flag which they can map arbitrarily to a
definition of their pleasing.
The same principle should be applied to my patch in that distros no longer would
need to forward-port internal solutions similar to this.
> Why shouldn't this be in your private repository?
If you don't want it I'll carry it forward but that's a loss for both of us, and
as pointed out in the above commit, other distros. If you do want to reject the
patch please let me know and I'll only submit the "get_maintainer.ignore" help
chunk.
P.
>
>> Signed-off-by: Prarit Bhargava <[email protected]>
>> Cc: Joe Perches <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> scripts/get_maintainer.pl | 34 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 34 insertions(+)
>>
>> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
>> index c87fa734e3e1..239f4d2ce972 100755
>> --- a/scripts/get_maintainer.pl
>> +++ b/scripts/get_maintainer.pl
>> @@ -384,6 +384,32 @@ sub find_ignore_git {
>> read_all_maintainer_files();
>>
>> sub read_all_maintainer_files {
>> + my $conf = which_conf(".get_maintainer.MAINTAINERS");
>> + if ( -f $conf) {
>> + my @conf_args;
>> + my $add = 0;
>> + open(my $conffile, '<', "$conf")
>> + or warn "$P: Can't find a readable .get_maintainer.MAINTAINERS file $!\n";
>> + while (<$conffile>) {
>> + my $line = $_;
>> + if ($line =~ m/^\+/ ) {
>> + $add = 1;
>> + }
>> + next if ($line =~ m/^\s*#/);
>> + $line =~ s/^\+//g;
>> + $line =~ s/^\-//g;
>> + $line =~ s/\s*\n?$//;
>> + push(@mfiles, $line);
>> + }
>> + close($conffile);
>> + if ($add eq 0) {
>> + foreach my $file (@mfiles) {
>> + read_maintainer_file("$file");
>> + }
>> + return;
>> + }
>> + }
>> +
>> if (-d "${lk_path}MAINTAINERS") {
>> opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
>> my @files = readdir(DIR);
>> @@ -1068,6 +1094,14 @@ Notes:
>> Entries in this file can be any command line argument.
>> This file is prepended to any additional command line arguments.
>> Multiple lines and # comments are allowed.
>> + File ".get_maintainer.ignore", if it exists in the linux kernel source root
>> + directory, can contain a list of email addresses to ignore. Multiple
>> + lines and # comments are allowed.
>> + File ".get_maintainer.MAINTAINERS", if it exists in the linux kernel source
>> + root directory, can change the location of the MAINTAINERS file.
>> + Entries beginning with a '+' are added to the default list, and
>> + entries beginning with a '-' override the existing MAINTAINERS list
>> + lookup. Multiple lines and # comments are allowed.
>> Most options have both positive and negative forms.
>> The negative forms for --<foo> are --no<foo> and --no-<foo>.
>>
On 06/26/2018 07:04 PM, Joe Perches wrote:
> On Tue, 2018-06-26 at 18:52 -0400, Prarit Bhargava wrote:
>>
>> On 06/26/2018 04:16 PM, Joe Perches wrote:
>>> On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
>>>> OSes have additional maintainers that should be cc'd on patches or may
>>>> want to circulate internal patches.
>>>>
>>>> Parse the .get_maintainer.MAINTAINERS file. Entries in the file
>>>> can begin with a '+' to indicate the email and list entries should be
>>>> added to the exiting MAINTAINERS output, or a '-' to indicate that the
>>>> entries should override the existing MAINTAINERS file.
>>>>
>>>> Also add a help entry for the .get_maintainers.ignore file.
>>>
>>> I see no reason for this patch to be applied.
>>> Why should it?
>>
>> The kernel has other vendor/OS changes like my patch, for example, 4efb442cc12e
>> ("kernel/panic.c: add TAINT_AUX"). From that commit message
>>
>> Add an auxiliary taint flag to be used by distros and others. This
>> obviates the need to forward-port whatever internal solutions people
>> have in favor of a single flag which they can map arbitrarily to a
>> definition of their pleasing.
>>
>> The same principle should be applied to my patch in that distros no longer would
>> need to forward-port internal solutions similar to this.
>>
>>> Why shouldn't this be in your private repository?
>>
>> If you don't want it I'll carry it forward but that's a loss for both of us, and
>> as pointed out in the above commit, other distros. If you do want to reject the
>> patch please let me know and I'll only submit the "get_maintainer.ignore" help
>> chunk.
>
> I doubt it's a really a loss for others as whatever
> .get_maintainers.<foo> files would likely need to
> be customized for each distro.
That's the point of the patch. Each distro would customize their own internal
.get_maintainers.MAINTAINERS file.
In any case, consider it dropped.
P.
>
> I think the whole thing should be ignored.
>
On Tue, 2018-06-26 at 18:52 -0400, Prarit Bhargava wrote:
>
> On 06/26/2018 04:16 PM, Joe Perches wrote:
> > On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
> > > OSes have additional maintainers that should be cc'd on patches or may
> > > want to circulate internal patches.
> > >
> > > Parse the .get_maintainer.MAINTAINERS file. Entries in the file
> > > can begin with a '+' to indicate the email and list entries should be
> > > added to the exiting MAINTAINERS output, or a '-' to indicate that the
> > > entries should override the existing MAINTAINERS file.
> > >
> > > Also add a help entry for the .get_maintainers.ignore file.
> >
> > I see no reason for this patch to be applied.
> > Why should it?
>
> The kernel has other vendor/OS changes like my patch, for example, 4efb442cc12e
> ("kernel/panic.c: add TAINT_AUX"). From that commit message
>
> Add an auxiliary taint flag to be used by distros and others. This
> obviates the need to forward-port whatever internal solutions people
> have in favor of a single flag which they can map arbitrarily to a
> definition of their pleasing.
>
> The same principle should be applied to my patch in that distros no longer would
> need to forward-port internal solutions similar to this.
>
> > Why shouldn't this be in your private repository?
>
> If you don't want it I'll carry it forward but that's a loss for both of us, and
> as pointed out in the above commit, other distros. If you do want to reject the
> patch please let me know and I'll only submit the "get_maintainer.ignore" help
> chunk.
I doubt it's a really a loss for others as whatever
.get_maintainers.<foo> files would likely need to
be customized for each distro.
I think the whole thing should be ignored.
On Tue, Jun 26, 2018 at 01:16:11PM -0700, Joe Perches wrote:
> On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
> > OSes have additional maintainers that should be cc'd on patches or may
> > want to circulate internal patches.
> >
> > Parse the .get_maintainer.MAINTAINERS file. Entries in the file
> > can begin with a '+' to indicate the email and list entries should be
> > added to the exiting MAINTAINERS output, or a '-' to indicate that the
> > entries should override the existing MAINTAINERS file.
> >
> > Also add a help entry for the .get_maintainers.ignore file.
>
> I see no reason for this patch to be applied.
> Why should it?
> Why shouldn't this be in your private repository?
Hi Joe,
Would you be open to a '--mfile=<path>/MAINTAINERS' option that would
override the default ./MAINTAINERS file? Then we could just add that to our
.get_maintainers.conf file.
Just trying to find ways to minimize our collection of private patches.
Cheers,
Don
>
> > Signed-off-by: Prarit Bhargava <[email protected]>
> > Cc: Joe Perches <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > scripts/get_maintainer.pl | 34 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 34 insertions(+)
> >
> > diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> > index c87fa734e3e1..239f4d2ce972 100755
> > --- a/scripts/get_maintainer.pl
> > +++ b/scripts/get_maintainer.pl
> > @@ -384,6 +384,32 @@ sub find_ignore_git {
> > read_all_maintainer_files();
> >
> > sub read_all_maintainer_files {
> > + my $conf = which_conf(".get_maintainer.MAINTAINERS");
> > + if ( -f $conf) {
> > + my @conf_args;
> > + my $add = 0;
> > + open(my $conffile, '<', "$conf")
> > + or warn "$P: Can't find a readable .get_maintainer.MAINTAINERS file $!\n";
> > + while (<$conffile>) {
> > + my $line = $_;
> > + if ($line =~ m/^\+/ ) {
> > + $add = 1;
> > + }
> > + next if ($line =~ m/^\s*#/);
> > + $line =~ s/^\+//g;
> > + $line =~ s/^\-//g;
> > + $line =~ s/\s*\n?$//;
> > + push(@mfiles, $line);
> > + }
> > + close($conffile);
> > + if ($add eq 0) {
> > + foreach my $file (@mfiles) {
> > + read_maintainer_file("$file");
> > + }
> > + return;
> > + }
> > + }
> > +
> > if (-d "${lk_path}MAINTAINERS") {
> > opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
> > my @files = readdir(DIR);
> > @@ -1068,6 +1094,14 @@ Notes:
> > Entries in this file can be any command line argument.
> > This file is prepended to any additional command line arguments.
> > Multiple lines and # comments are allowed.
> > + File ".get_maintainer.ignore", if it exists in the linux kernel source root
> > + directory, can contain a list of email addresses to ignore. Multiple
> > + lines and # comments are allowed.
> > + File ".get_maintainer.MAINTAINERS", if it exists in the linux kernel source
> > + root directory, can change the location of the MAINTAINERS file.
> > + Entries beginning with a '+' are added to the default list, and
> > + entries beginning with a '-' override the existing MAINTAINERS list
> > + lookup. Multiple lines and # comments are allowed.
> > Most options have both positive and negative forms.
> > The negative forms for --<foo> are --no<foo> and --no-<foo>.
> >
On Fri, 2018-07-06 at 13:54 -0400, Don Zickus wrote:
> On Tue, Jun 26, 2018 at 01:16:11PM -0700, Joe Perches wrote:
> > On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
> > > OSes have additional maintainers that should be cc'd on patches or may
> > > want to circulate internal patches.
> > >
> > > Parse the .get_maintainer.MAINTAINERS file. Entries in the file
> > > can begin with a '+' to indicate the email and list entries should be
> > > added to the exiting MAINTAINERS output, or a '-' to indicate that the
> > > entries should override the existing MAINTAINERS file.
> > >
> > > Also add a help entry for the .get_maintainers.ignore file.
> >
> > I see no reason for this patch to be applied.
> > Why should it?
> > Why shouldn't this be in your private repository?
>
> Hi Joe,
>
> Would you be open to a '--mfile=<path>/MAINTAINERS' option that would
> override the default ./MAINTAINERS file? Then we could just add that to our
> .get_maintainers.conf file.
Hi Don.
Sure.
And that kinda already exists in mainline with
--find-maintainer-files where any subdirectory
that contains a MAINTAINER file is also read.
> Just trying to find ways to minimize our collection of private patches.
Perhaps that could be extended for your purpose
with some additional argument like a specific
optional directory/path where every subdirectory
would be found.
> Cheers,
> Don
cheers back, Joe
> > > + $line =~ s/\s*\n?$//;
> > > + push(@mfiles, $line);
> > > + }
> > > + close($conffile);
> > > + if ($add eq 0) {
> > > + foreach my $file (@mfiles) {
> > > + read_maintainer_file("$file");
> > > + }
> > > + return;
> > > + }
> > > + }
> > > +
> > > if (-d "${lk_path}MAINTAINERS") {
> > > opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
> > > my @files = readdir(DIR);
> > > @@ -1068,6 +1094,14 @@ Notes:
> > > Entries in this file can be any command line argument.
> > > This file is prepended to any additional command line arguments.
> > > Multiple lines and # comments are allowed.
> > > + File ".get_maintainer.ignore", if it exists in the linux kernel source root
> > > + directory, can contain a list of email addresses to ignore. Multiple
> > > + lines and # comments are allowed.
> > > + File ".get_maintainer.MAINTAINERS", if it exists in the linux kernel source
> > > + root directory, can change the location of the MAINTAINERS file.
> > > + Entries beginning with a '+' are added to the default list, and
> > > + entries beginning with a '-' override the existing MAINTAINERS list
> > > + lookup. Multiple lines and # comments are allowed.
> > > Most options have both positive and negative forms.
> > > The negative forms for --<foo> are --no<foo> and --no-<foo>.
> > >
On Fri, Jul 06, 2018 at 11:31:13AM -0700, Joe Perches wrote:
> On Fri, 2018-07-06 at 13:54 -0400, Don Zickus wrote:
> > On Tue, Jun 26, 2018 at 01:16:11PM -0700, Joe Perches wrote:
> > > On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
> > > > OSes have additional maintainers that should be cc'd on patches or may
> > > > want to circulate internal patches.
> > > >
> > > > Parse the .get_maintainer.MAINTAINERS file. Entries in the file
> > > > can begin with a '+' to indicate the email and list entries should be
> > > > added to the exiting MAINTAINERS output, or a '-' to indicate that the
> > > > entries should override the existing MAINTAINERS file.
> > > >
> > > > Also add a help entry for the .get_maintainers.ignore file.
> > >
> > > I see no reason for this patch to be applied.
> > > Why should it?
> > > Why shouldn't this be in your private repository?
> >
> > Hi Joe,
> >
> > Would you be open to a '--mfile=<path>/MAINTAINERS' option that would
> > override the default ./MAINTAINERS file? Then we could just add that to our
> > .get_maintainers.conf file.
>
> Hi Don.
>
> Sure.
>
> And that kinda already exists in mainline with
> --find-maintainer-files where any subdirectory
> that contains a MAINTAINER file is also read.
Hi Joe,
Yes, I saw and played with it. My only quirk with it was that option still
found and added ./MAINTAINERS to the list which I/we were trying to avoid
(we have our own private MAINTAINERS copy).
But yes, it easily found our private MAINTAINERS file.
>
> > Just trying to find ways to minimize our collection of private patches.
>
> Perhaps that could be extended for your purpose
> with some additional argument like a specific
> optional directory/path where every subdirectory
> would be found.
So something like --find-maintainer-files=<dir> ? I think that could work.
Cheers,
Don
>
> > Cheers,
> > Don
>
> cheers back, Joe
>
> > > > + $line =~ s/\s*\n?$//;
> > > > + push(@mfiles, $line);
> > > > + }
> > > > + close($conffile);
> > > > + if ($add eq 0) {
> > > > + foreach my $file (@mfiles) {
> > > > + read_maintainer_file("$file");
> > > > + }
> > > > + return;
> > > > + }
> > > > + }
> > > > +
> > > > if (-d "${lk_path}MAINTAINERS") {
> > > > opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
> > > > my @files = readdir(DIR);
> > > > @@ -1068,6 +1094,14 @@ Notes:
> > > > Entries in this file can be any command line argument.
> > > > This file is prepended to any additional command line arguments.
> > > > Multiple lines and # comments are allowed.
> > > > + File ".get_maintainer.ignore", if it exists in the linux kernel source root
> > > > + directory, can contain a list of email addresses to ignore. Multiple
> > > > + lines and # comments are allowed.
> > > > + File ".get_maintainer.MAINTAINERS", if it exists in the linux kernel source
> > > > + root directory, can change the location of the MAINTAINERS file.
> > > > + Entries beginning with a '+' are added to the default list, and
> > > > + entries beginning with a '-' override the existing MAINTAINERS list
> > > > + lookup. Multiple lines and # comments are allowed.
> > > > Most options have both positive and negative forms.
> > > > The negative forms for --<foo> are --no<foo> and --no-<foo>.
> > > >
On 07/06/2018 02:44 PM, Don Zickus wrote:
> On Fri, Jul 06, 2018 at 11:31:13AM -0700, Joe Perches wrote:
>> On Fri, 2018-07-06 at 13:54 -0400, Don Zickus wrote:
>>> On Tue, Jun 26, 2018 at 01:16:11PM -0700, Joe Perches wrote:
>>>> On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
>>>>> OSes have additional maintainers that should be cc'd on patches or may
>>>>> want to circulate internal patches.
>>>>>
>>>>> Parse the .get_maintainer.MAINTAINERS file. Entries in the file
>>>>> can begin with a '+' to indicate the email and list entries should be
>>>>> added to the exiting MAINTAINERS output, or a '-' to indicate that the
>>>>> entries should override the existing MAINTAINERS file.
>>>>>
>>>>> Also add a help entry for the .get_maintainers.ignore file.
>>>>
>>>> I see no reason for this patch to be applied.
>>>> Why should it?
>>>> Why shouldn't this be in your private repository?
>>>
>>> Hi Joe,
>>>
>>> Would you be open to a '--mfile=<path>/MAINTAINERS' option that would
>>> override the default ./MAINTAINERS file? Then we could just add that to our
>>> .get_maintainers.conf file.
>>
>> Hi Don.
>>
>> Sure.
>>
>> And that kinda already exists in mainline with
>> --find-maintainer-files where any subdirectory
>> that contains a MAINTAINER file is also read.
>
> Hi Joe,
>
> Yes, I saw and played with it. My only quirk with it was that option still
> found and added ./MAINTAINERS to the list which I/we were trying to avoid
> (we have our own private MAINTAINERS copy).
>
> But yes, it easily found our private MAINTAINERS file.
>
>>
>>> Just trying to find ways to minimize our collection of private patches.
>>
>> Perhaps that could be extended for your purpose
>> with some additional argument like a specific
>> optional directory/path where every subdirectory
>> would be found.
>
> So something like --find-maintainer-files=<dir> ? I think that could work.
So --find-maintainers-files=./kernel/pci
would only look for MAINTAINERS files under kernel/pci?
P.
>
> Cheers,
> Don
>
>>
>>> Cheers,
>>> Don
>>
>> cheers back, Joe
>>
>>>>> + $line =~ s/\s*\n?$//;
>>>>> + push(@mfiles, $line);
>>>>> + }
>>>>> + close($conffile);
>>>>> + if ($add eq 0) {
>>>>> + foreach my $file (@mfiles) {
>>>>> + read_maintainer_file("$file");
>>>>> + }
>>>>> + return;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> if (-d "${lk_path}MAINTAINERS") {
>>>>> opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
>>>>> my @files = readdir(DIR);
>>>>> @@ -1068,6 +1094,14 @@ Notes:
>>>>> Entries in this file can be any command line argument.
>>>>> This file is prepended to any additional command line arguments.
>>>>> Multiple lines and # comments are allowed.
>>>>> + File ".get_maintainer.ignore", if it exists in the linux kernel source root
>>>>> + directory, can contain a list of email addresses to ignore. Multiple
>>>>> + lines and # comments are allowed.
>>>>> + File ".get_maintainer.MAINTAINERS", if it exists in the linux kernel source
>>>>> + root directory, can change the location of the MAINTAINERS file.
>>>>> + Entries beginning with a '+' are added to the default list, and
>>>>> + entries beginning with a '-' override the existing MAINTAINERS list
>>>>> + lookup. Multiple lines and # comments are allowed.
>>>>> Most options have both positive and negative forms.
>>>>> The negative forms for --<foo> are --no<foo> and --no-<foo>.
>>>>>
On Fri, Jul 06, 2018 at 11:31:13AM -0700, Joe Perches wrote:
> On Fri, 2018-07-06 at 13:54 -0400, Don Zickus wrote:
> > On Tue, Jun 26, 2018 at 01:16:11PM -0700, Joe Perches wrote:
> > > On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
> > > > OSes have additional maintainers that should be cc'd on patches or may
> > > > want to circulate internal patches.
> > > >
> > > > Parse the .get_maintainer.MAINTAINERS file. Entries in the file
> > > > can begin with a '+' to indicate the email and list entries should be
> > > > added to the exiting MAINTAINERS output, or a '-' to indicate that the
> > > > entries should override the existing MAINTAINERS file.
> > > >
> > > > Also add a help entry for the .get_maintainers.ignore file.
> > >
> > > I see no reason for this patch to be applied.
> > > Why should it?
> > > Why shouldn't this be in your private repository?
> >
> > Hi Joe,
> >
> > Would you be open to a '--mfile=<path>/MAINTAINERS' option that would
> > override the default ./MAINTAINERS file? Then we could just add that to our
> > .get_maintainers.conf file.
>
> Hi Don.
>
> Sure.
>
> And that kinda already exists in mainline with
> --find-maintainer-files where any subdirectory
> that contains a MAINTAINER file is also read.
>
> > Just trying to find ways to minimize our collection of private patches.
>
> Perhaps that could be extended for your purpose
> with some additional argument like a specific
> optional directory/path where every subdirectory
> would be found.
Would you be ok with something like this? This seems to solve our problem
cleanly. If you are ok with it or need some tweaks, I will send a more
formal patch.
Cheers,
Don
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 8e8ac50f018c..ced7fd664862 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -60,7 +60,7 @@ my $pattern_depth = 0;
my $self_test = undef;
my $version = 0;
my $help = 0;
-my $find_maintainer_files = 0;
+my $find_maintainer_files = '';
my $rh_only = 1;
my $vcs_used = 0;
@@ -263,7 +263,7 @@ if (!GetOptions(
'sections!' => \$sections,
'fe|file-emails!' => \$file_emails,
'f|file' => \$from_filename,
- 'find-maintainer-files' => \$find_maintainer_files,
+ 'find-maintainer-files=s' => \$find_maintainer_files,
'self-test:s' => \$self_test,
'v|version' => \$version,
'h|help|usage' => \$help,
@@ -425,7 +425,7 @@ sub read_all_maintainer_files {
find( { wanted => \&find_is_maintainer_file,
preprocess => \&find_ignore_git,
no_chdir => 1,
- }, "${lk_path}");
+ }, "${lk_path}$find_maintainer_files");
} else {
push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
}
On Fri, 2018-07-06 at 15:39 -0400, Prarit Bhargava wrote:
>
> On 07/06/2018 02:44 PM, Don Zickus wrote:
> > On Fri, Jul 06, 2018 at 11:31:13AM -0700, Joe Perches wrote:
> > > On Fri, 2018-07-06 at 13:54 -0400, Don Zickus wrote:
> > > > On Tue, Jun 26, 2018 at 01:16:11PM -0700, Joe Perches wrote:
> > > > > On Tue, 2018-06-26 at 14:25 -0400, Prarit Bhargava wrote:
> > > > > > OSes have additional maintainers that should be cc'd on patches or may
> > > > > > want to circulate internal patches.
> > > > > >
> > > > > > Parse the .get_maintainer.MAINTAINERS file. Entries in the file
> > > > > > can begin with a '+' to indicate the email and list entries should be
> > > > > > added to the exiting MAINTAINERS output, or a '-' to indicate that the
> > > > > > entries should override the existing MAINTAINERS file.
> > > > > >
> > > > > > Also add a help entry for the .get_maintainers.ignore file.
> > > > >
> > > > > I see no reason for this patch to be applied.
> > > > > Why should it?
> > > > > Why shouldn't this be in your private repository?
> > > >
> > > > Hi Joe,
> > > >
> > > > Would you be open to a '--mfile=<path>/MAINTAINERS' option that would
> > > > override the default ./MAINTAINERS file? Then we could just add that to our
> > > > .get_maintainers.conf file.
> > >
> > > Hi Don.
> > >
> > > Sure.
> > >
> > > And that kinda already exists in mainline with
> > > --find-maintainer-files where any subdirectory
> > > that contains a MAINTAINER file is also read.
> >
> > Hi Joe,
> >
> > Yes, I saw and played with it. My only quirk with it was that option still
> > found and added ./MAINTAINERS to the list which I/we were trying to avoid
> > (we have our own private MAINTAINERS copy).
> >
> > But yes, it easily found our private MAINTAINERS file.
> >
> > >
> > > > Just trying to find ways to minimize our collection of private patches.
> > >
> > > Perhaps that could be extended for your purpose
> > > with some additional argument like a specific
> > > optional directory/path where every subdirectory
> > > would be found.
> >
> > So something like --find-maintainer-files=<dir> ? I think that could work.
>
> So --find-maintainers-files=./kernel/pci
>
> would only look for MAINTAINERS files under kernel/pci?
Well, perhaps yes. Perhaps it would also read
a top level MAINTAINERS file. Dunno. What seems
right to you?
I don't have an objection to
--find-maintainer-files=<path_or_file> where
the existing behavior of --find-maintainer-files
without <path_or_file> is all subdirs.
Perhaps something like the below
(some of this is whitespace change only)
---
scripts/get_maintainer.pl | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index c87fa734e3e1..2eb11b5c8f34 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -60,7 +60,7 @@ my $pattern_depth = 0;
my $self_test = undef;
my $version = 0;
my $help = 0;
-my $find_maintainer_files = 0;
+my $find_maintainer_files;
my $vcs_used = 0;
@@ -262,7 +262,7 @@ if (!GetOptions(
'sections!' => \$sections,
'fe|file-emails!' => \$file_emails,
'f|file' => \$from_filename,
- 'find-maintainer-files' => \$find_maintainer_files,
+ 'find-maintainer-files:s' => \$find_maintainer_files,
'self-test:s' => \$self_test,
'v|version' => \$version,
'h|help|usage' => \$help,
@@ -384,26 +384,30 @@ sub find_ignore_git {
read_all_maintainer_files();
sub read_all_maintainer_files {
- if (-d "${lk_path}MAINTAINERS") {
- opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
- my @files = readdir(DIR);
- closedir(DIR);
- foreach my $file (@files) {
- push(@mfiles, "${lk_path}MAINTAINERS/$file") if ($file !~ /^\./);
- }
- }
-
- if ($find_maintainer_files) {
- find( { wanted => \&find_is_maintainer_file,
- preprocess => \&find_ignore_git,
- no_chdir => 1,
- }, "${lk_path}");
+ my $path = defined $find_maintainer_files && $find_maintainer_files ne ""
+ ? $find_maintainer_files : $lk_path;
+ if (-d "${path}MAINTAINERS") {
+ opendir(DIR, "${path}MAINTAINERS") or die $!;
+ my @files = readdir(DIR);
+ closedir(DIR);
+ foreach my $file (@files) {
+ push(@mfiles, "${path}MAINTAINERS/$file") if ($file !~ /^\./);
+ }
+ }
+
+ if (defined $find_maintainer_files) {
+ die "$P: invalid find-maintainer-files <$path>" if (!-d $path);
+ find( { wanted => \&find_is_maintainer_file,
+ preprocess => \&find_ignore_git,
+ no_chdir => 1,
+ }, "${path}");
} else {
- push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
+ push(@mfiles, "${path}MAINTAINERS") if -f "${path}MAINTAINERS";
}
+ die "$P: No MAINTAINER files found in $path\n" if (scalar(@mfiles) == 0);
foreach my $file (@mfiles) {
- read_maintainer_file("$file");
+ read_maintainer_file("$file");
}
}
On Fri, Jul 06, 2018 at 02:36:28PM -0700, Joe Perches wrote:
> > > > > Just trying to find ways to minimize our collection of private patches.
> > > >
> > > > Perhaps that could be extended for your purpose
> > > > with some additional argument like a specific
> > > > optional directory/path where every subdirectory
> > > > would be found.
> > >
> > > So something like --find-maintainer-files=<dir> ? I think that could work.
> >
> > So --find-maintainers-files=./kernel/pci
> >
> > would only look for MAINTAINERS files under kernel/pci?
>
> Well, perhaps yes. Perhaps it would also read
> a top level MAINTAINERS file. Dunno. What seems
> right to you?
>
> I don't have an objection to
> --find-maintainer-files=<path_or_file> where
> the existing behavior of --find-maintainer-files
> without <path_or_file> is all subdirs.
>
> Perhaps something like the below
> (some of this is whitespace change only)
> ---
> scripts/get_maintainer.pl | 40 ++++++++++++++++++++++------------------
> 1 file changed, 22 insertions(+), 18 deletions(-)
>
<snip>
> +
> + if (defined $find_maintainer_files) {
> + die "$P: invalid find-maintainer-files <$path>" if (!-d $path);
^^^^^
Hi Joe,
Thanks for the patch!! If I remove the above 'die' line I can pass in a
file or a dir. Otherwise the 'die' line prevents a 'file' from being used.
Not sure if that is important for this patch or not.
We have an internal use case of multiple MAINTAINER files, some folks have
more rights to patches than others so they are not allowed to be cc'd (think
embargoed stuff).
So the 'file' usage would be useful for us. But if you are against it, we can
easily patch it out on our end.
Prarit,
I believe this patch still does what we want (and handles your pci example).
Cheers,
Don
> + find( { wanted => \&find_is_maintainer_file,
> + preprocess => \&find_ignore_git,
> + no_chdir => 1,
> + }, "${path}");
> } else {
> - push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
> + push(@mfiles, "${path}MAINTAINERS") if -f "${path}MAINTAINERS";
> }
>
> + die "$P: No MAINTAINER files found in $path\n" if (scalar(@mfiles) == 0);
> foreach my $file (@mfiles) {
> - read_maintainer_file("$file");
> + read_maintainer_file("$file");
> }
> }
>
On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> On Fri, Jul 06, 2018 at 02:36:28PM -0700, Joe Perches wrote:
> > > > > > Just trying to find ways to minimize our collection of private patches.
> > > > >
> > > > > Perhaps that could be extended for your purpose
> > > > > with some additional argument like a specific
> > > > > optional directory/path where every subdirectory
> > > > > would be found.
> > > >
> > > > So something like --find-maintainer-files=<dir> ? I think that could work.
> > >
> > > So --find-maintainers-files=./kernel/pci
> > >
> > > would only look for MAINTAINERS files under kernel/pci?
> >
> > Well, perhaps yes. Perhaps it would also read
> > a top level MAINTAINERS file. Dunno. What seems
> > right to you?
> >
> > I don't have an objection to
> > --find-maintainer-files=<path_or_file> where
> > the existing behavior of --find-maintainer-files
> > without <path_or_file> is all subdirs.
> >
> > Perhaps something like the below
> > (some of this is whitespace change only)
> > ---
> > scripts/get_maintainer.pl | 40 ++++++++++++++++++++++------------------
> > 1 file changed, 22 insertions(+), 18 deletions(-)
> >
>
> <snip>
>
> > +
> > + if (defined $find_maintainer_files) {
> > + die "$P: invalid find-maintainer-files <$path>" if (!-d $path);
>
> ^^^^^
>
> Hi Joe,
>
> Thanks for the patch!! If I remove the above 'die' line I can pass in a
> file or a dir. Otherwise the 'die' line prevents a 'file' from being used.
Change it to (!-e $file), I was just spitballing.
> Not sure if that is important for this patch or not.
It's not.
> We have an internal use case of multiple MAINTAINER files, some folks have
> more rights to patches than others so they are not allowed to be cc'd (think
> embargoed stuff).
>
> So the 'file' usage would be useful for us. But if you are against it, we can
> easily patch it out on our end.
cheers, Joe
On Fri, Jul 06, 2018 at 03:09:17PM -0700, Joe Perches wrote:
> On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> > On Fri, Jul 06, 2018 at 02:36:28PM -0700, Joe Perches wrote:
> > > > > > > Just trying to find ways to minimize our collection of private patches.
> > > > > >
> > > > > > Perhaps that could be extended for your purpose
> > > > > > with some additional argument like a specific
> > > > > > optional directory/path where every subdirectory
> > > > > > would be found.
> > > > >
> > > > > So something like --find-maintainer-files=<dir> ? I think that could work.
> > > >
> > > > So --find-maintainers-files=./kernel/pci
> > > >
> > > > would only look for MAINTAINERS files under kernel/pci?
> > >
> > > Well, perhaps yes. Perhaps it would also read
> > > a top level MAINTAINERS file. Dunno. What seems
> > > right to you?
> > >
> > > I don't have an objection to
> > > --find-maintainer-files=<path_or_file> where
> > > the existing behavior of --find-maintainer-files
> > > without <path_or_file> is all subdirs.
> > >
> > > Perhaps something like the below
> > > (some of this is whitespace change only)
> > > ---
> > > scripts/get_maintainer.pl | 40 ++++++++++++++++++++++------------------
> > > 1 file changed, 22 insertions(+), 18 deletions(-)
> > >
> >
> > <snip>
> >
> > > +
> > > + if (defined $find_maintainer_files) {
> > > + die "$P: invalid find-maintainer-files <$path>" if (!-d $path);
> >
> > ^^^^^
> >
> > Hi Joe,
> >
> > Thanks for the patch!! If I remove the above 'die' line I can pass in a
> > file or a dir. Otherwise the 'die' line prevents a 'file' from being used.
>
> Change it to (!-e $file), I was just spitballing.
Perfect. Works for me! :-)
Cheers,
Don
>
> > Not sure if that is important for this patch or not.
>
> It's not.
>
> > We have an internal use case of multiple MAINTAINER files, some folks have
> > more rights to patches than others so they are not allowed to be cc'd (think
> > embargoed stuff).
> >
> > So the 'file' usage would be useful for us. But if you are against it, we can
> > easily patch it out on our end.
>
> cheers, Joe
On Fri, 2018-07-06 at 15:09 -0700, Joe Perches wrote:
> On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> > We have an internal use case of multiple MAINTAINER files, some folks have
> > more rights to patches than others so they are not allowed to be cc'd (think
> > embargoed stuff).
How about:
---
scripts/get_maintainer.pl | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index c87fa734e3e1..f7a7d46340a8 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -60,7 +60,7 @@ my $pattern_depth = 0;
my $self_test = undef;
my $version = 0;
my $help = 0;
-my $find_maintainer_files = 0;
+my $find_maintainer_files;
my $vcs_used = 0;
@@ -262,7 +262,7 @@ if (!GetOptions(
'sections!' => \$sections,
'fe|file-emails!' => \$file_emails,
'f|file' => \$from_filename,
- 'find-maintainer-files' => \$find_maintainer_files,
+ 'find-maintainer-files:s' => \$find_maintainer_files,
'self-test:s' => \$self_test,
'v|version' => \$version,
'h|help|usage' => \$help,
@@ -384,26 +384,29 @@ sub find_ignore_git {
read_all_maintainer_files();
sub read_all_maintainer_files {
- if (-d "${lk_path}MAINTAINERS") {
- opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
- my @files = readdir(DIR);
- closedir(DIR);
- foreach my $file (@files) {
- push(@mfiles, "${lk_path}MAINTAINERS/$file") if ($file !~ /^\./);
- }
- }
-
- if ($find_maintainer_files) {
- find( { wanted => \&find_is_maintainer_file,
- preprocess => \&find_ignore_git,
- no_chdir => 1,
- }, "${lk_path}");
+ my $path = defined $find_maintainer_files && $find_maintainer_files ne ""
+ ? $find_maintainer_files : $lk_path;
+ if (-d "${path}MAINTAINERS") {
+ opendir(DIR, "${path}MAINTAINERS") or die $!;
+ my @files = readdir(DIR);
+ closedir(DIR);
+ foreach my $file (@files) {
+ push(@mfiles, "${path}MAINTAINERS/$file") if ($file !~ /^\./);
+ }
+ }
+
+ if (defined $find_maintainer_files && (-d $find_maintainer_files)) {
+ find( { wanted => \&find_is_maintainer_file,
+ preprocess => \&find_ignore_git,
+ no_chdir => 1,
+ }, "${path}");
} else {
- push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
+ push(@mfiles, "${path}MAINTAINERS") if -f "${path}MAINTAINERS";
}
+ die "$P: No MAINTAINER files found in $path\n" if (scalar(@mfiles) == 0);
foreach my $file (@mfiles) {
- read_maintainer_file("$file");
+ read_maintainer_file("$file");
}
}
On Fri, Jul 06, 2018 at 03:14:28PM -0700, Joe Perches wrote:
> On Fri, 2018-07-06 at 15:09 -0700, Joe Perches wrote:
> > On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> > > We have an internal use case of multiple MAINTAINER files, some folks have
> > > more rights to patches than others so they are not allowed to be cc'd (think
> > > embargoed stuff).
>
> How about:
I think you tried to optimized and it broke my passed in file.
See below.
> ---
> scripts/get_maintainer.pl | 39 +++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index c87fa734e3e1..f7a7d46340a8 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -60,7 +60,7 @@ my $pattern_depth = 0;
> my $self_test = undef;
> my $version = 0;
> my $help = 0;
> -my $find_maintainer_files = 0;
> +my $find_maintainer_files;
>
> my $vcs_used = 0;
>
> @@ -262,7 +262,7 @@ if (!GetOptions(
> 'sections!' => \$sections,
> 'fe|file-emails!' => \$file_emails,
> 'f|file' => \$from_filename,
> - 'find-maintainer-files' => \$find_maintainer_files,
> + 'find-maintainer-files:s' => \$find_maintainer_files,
> 'self-test:s' => \$self_test,
> 'v|version' => \$version,
> 'h|help|usage' => \$help,
> @@ -384,26 +384,29 @@ sub find_ignore_git {
> read_all_maintainer_files();
>
> sub read_all_maintainer_files {
> - if (-d "${lk_path}MAINTAINERS") {
> - opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
> - my @files = readdir(DIR);
> - closedir(DIR);
> - foreach my $file (@files) {
> - push(@mfiles, "${lk_path}MAINTAINERS/$file") if ($file !~ /^\./);
> - }
> - }
> -
> - if ($find_maintainer_files) {
> - find( { wanted => \&find_is_maintainer_file,
> - preprocess => \&find_ignore_git,
> - no_chdir => 1,
> - }, "${lk_path}");
> + my $path = defined $find_maintainer_files && $find_maintainer_files ne ""
> + ? $find_maintainer_files : $lk_path;
> + if (-d "${path}MAINTAINERS") {
> + opendir(DIR, "${path}MAINTAINERS") or die $!;
> + my @files = readdir(DIR);
> + closedir(DIR);
> + foreach my $file (@files) {
> + push(@mfiles, "${path}MAINTAINERS/$file") if ($file !~ /^\./);
> + }
> + }
> +
> + if (defined $find_maintainer_files && (-d $find_maintainer_files)) {
> + find( { wanted => \&find_is_maintainer_file,
> + preprocess => \&find_ignore_git,
> + no_chdir => 1,
> + }, "${path}");
> } else {
> - push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
> + push(@mfiles, "${path}MAINTAINERS") if -f "${path}MAINTAINERS";
> }
>
> + die "$P: No MAINTAINER files found in $path\n" if (scalar(@mfiles) == 0);
^^^ I see this 'die' when using --find-maintainer-files=<file>
I suspect the '-d $find_maintainer_files' should be a '-e' but that kinda
breaks your optimization to use the 'else'??
the 'else' fails because it has $path==<file> and the 'else' appends MAINTAINERS
to <file>, which fails the -f check.
You almost need a
} else {
push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
+ push(@mfiles, "${path}") if -f "${path}";
}
but that might be kinda kludgy.
Cheers,
Don
> foreach my $file (@mfiles) {
> - read_maintainer_file("$file");
> + read_maintainer_file("$file");
> }
> }
>
On Fri, Jul 06, 2018 at 03:14:28PM -0700, Joe Perches wrote:
> On Fri, 2018-07-06 at 15:09 -0700, Joe Perches wrote:
> > On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> > > We have an internal use case of multiple MAINTAINER files, some folks have
> > > more rights to patches than others so they are not allowed to be cc'd (think
> > > embargoed stuff).
>
> How about:
Oh by the way, you can probably mimic my testing by doing
mkdir test
cp MAINTAINERS test/
./scripts/get_maintainer.pl --find-maintainer-files test/ <patch>
./scripts/get_maintainer.pl --find-maintainer-files test/MAINTAINERS <patch>
If that helps.
Cheers,
Don
> ---
> scripts/get_maintainer.pl | 39 +++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index c87fa734e3e1..f7a7d46340a8 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -60,7 +60,7 @@ my $pattern_depth = 0;
> my $self_test = undef;
> my $version = 0;
> my $help = 0;
> -my $find_maintainer_files = 0;
> +my $find_maintainer_files;
>
> my $vcs_used = 0;
>
> @@ -262,7 +262,7 @@ if (!GetOptions(
> 'sections!' => \$sections,
> 'fe|file-emails!' => \$file_emails,
> 'f|file' => \$from_filename,
> - 'find-maintainer-files' => \$find_maintainer_files,
> + 'find-maintainer-files:s' => \$find_maintainer_files,
> 'self-test:s' => \$self_test,
> 'v|version' => \$version,
> 'h|help|usage' => \$help,
> @@ -384,26 +384,29 @@ sub find_ignore_git {
> read_all_maintainer_files();
>
> sub read_all_maintainer_files {
> - if (-d "${lk_path}MAINTAINERS") {
> - opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
> - my @files = readdir(DIR);
> - closedir(DIR);
> - foreach my $file (@files) {
> - push(@mfiles, "${lk_path}MAINTAINERS/$file") if ($file !~ /^\./);
> - }
> - }
> -
> - if ($find_maintainer_files) {
> - find( { wanted => \&find_is_maintainer_file,
> - preprocess => \&find_ignore_git,
> - no_chdir => 1,
> - }, "${lk_path}");
> + my $path = defined $find_maintainer_files && $find_maintainer_files ne ""
> + ? $find_maintainer_files : $lk_path;
> + if (-d "${path}MAINTAINERS") {
> + opendir(DIR, "${path}MAINTAINERS") or die $!;
> + my @files = readdir(DIR);
> + closedir(DIR);
> + foreach my $file (@files) {
> + push(@mfiles, "${path}MAINTAINERS/$file") if ($file !~ /^\./);
> + }
> + }
> +
> + if (defined $find_maintainer_files && (-d $find_maintainer_files)) {
> + find( { wanted => \&find_is_maintainer_file,
> + preprocess => \&find_ignore_git,
> + no_chdir => 1,
> + }, "${path}");
> } else {
> - push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
> + push(@mfiles, "${path}MAINTAINERS") if -f "${path}MAINTAINERS";
> }
>
> + die "$P: No MAINTAINER files found in $path\n" if (scalar(@mfiles) == 0);
> foreach my $file (@mfiles) {
> - read_maintainer_file("$file");
> + read_maintainer_file("$file");
> }
> }
>
On Fri, Jul 06, 2018 at 03:14:28PM -0700, Joe Perches wrote:
> On Fri, 2018-07-06 at 15:09 -0700, Joe Perches wrote:
> > On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> > > We have an internal use case of multiple MAINTAINER files, some folks have
> > > more rights to patches than others so they are not allowed to be cc'd (think
> > > embargoed stuff).
>
> How about:
Hi Joe,
You are probably busy with stuff, but wanted to softly poke you to see what
is going on with this patch and if there is anything we can help with?
Cheers,
Don
> ---
> scripts/get_maintainer.pl | 39 +++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index c87fa734e3e1..f7a7d46340a8 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -60,7 +60,7 @@ my $pattern_depth = 0;
> my $self_test = undef;
> my $version = 0;
> my $help = 0;
> -my $find_maintainer_files = 0;
> +my $find_maintainer_files;
>
> my $vcs_used = 0;
>
> @@ -262,7 +262,7 @@ if (!GetOptions(
> 'sections!' => \$sections,
> 'fe|file-emails!' => \$file_emails,
> 'f|file' => \$from_filename,
> - 'find-maintainer-files' => \$find_maintainer_files,
> + 'find-maintainer-files:s' => \$find_maintainer_files,
> 'self-test:s' => \$self_test,
> 'v|version' => \$version,
> 'h|help|usage' => \$help,
> @@ -384,26 +384,29 @@ sub find_ignore_git {
> read_all_maintainer_files();
>
> sub read_all_maintainer_files {
> - if (-d "${lk_path}MAINTAINERS") {
> - opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
> - my @files = readdir(DIR);
> - closedir(DIR);
> - foreach my $file (@files) {
> - push(@mfiles, "${lk_path}MAINTAINERS/$file") if ($file !~ /^\./);
> - }
> - }
> -
> - if ($find_maintainer_files) {
> - find( { wanted => \&find_is_maintainer_file,
> - preprocess => \&find_ignore_git,
> - no_chdir => 1,
> - }, "${lk_path}");
> + my $path = defined $find_maintainer_files && $find_maintainer_files ne ""
> + ? $find_maintainer_files : $lk_path;
> + if (-d "${path}MAINTAINERS") {
> + opendir(DIR, "${path}MAINTAINERS") or die $!;
> + my @files = readdir(DIR);
> + closedir(DIR);
> + foreach my $file (@files) {
> + push(@mfiles, "${path}MAINTAINERS/$file") if ($file !~ /^\./);
> + }
> + }
> +
> + if (defined $find_maintainer_files && (-d $find_maintainer_files)) {
> + find( { wanted => \&find_is_maintainer_file,
> + preprocess => \&find_ignore_git,
> + no_chdir => 1,
> + }, "${path}");
> } else {
> - push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
> + push(@mfiles, "${path}MAINTAINERS") if -f "${path}MAINTAINERS";
> }
>
> + die "$P: No MAINTAINER files found in $path\n" if (scalar(@mfiles) == 0);
> foreach my $file (@mfiles) {
> - read_maintainer_file("$file");
> + read_maintainer_file("$file");
> }
> }
>
On Fri, 2018-07-13 at 14:51 -0400, Don Zickus wrote:
> On Fri, Jul 06, 2018 at 03:14:28PM -0700, Joe Perches wrote:
> > On Fri, 2018-07-06 at 15:09 -0700, Joe Perches wrote:
> > > On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> > > > We have an internal use case of multiple MAINTAINER files, some folks have
> > > > more rights to patches than others so they are not allowed to be cc'd (think
> > > > embargoed stuff).
> >
> > How about:
>
> Hi Joe,
>
> You are probably busy with stuff, but wanted to softly poke you to see what
> is going on with this patch and if there is anything we can help with?
Hey Don. No worries.
Tell me again if the --find-maintainer-files=<path or file> should
be exclusive or should it also read any existing MAINTAINERS file?
What if anything is wrong with what I suggested?
cheers, Joe
On Fri, Jul 13, 2018 at 05:11:58PM -0700, Joe Perches wrote:
> On Fri, 2018-07-13 at 14:51 -0400, Don Zickus wrote:
> > On Fri, Jul 06, 2018 at 03:14:28PM -0700, Joe Perches wrote:
> > > On Fri, 2018-07-06 at 15:09 -0700, Joe Perches wrote:
> > > > On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> > > > > We have an internal use case of multiple MAINTAINER files, some folks have
> > > > > more rights to patches than others so they are not allowed to be cc'd (think
> > > > > embargoed stuff).
> > >
> > > How about:
> >
> > Hi Joe,
> >
> > You are probably busy with stuff, but wanted to softly poke you to see what
> > is going on with this patch and if there is anything we can help with?
>
> Hey Don. No worries.
>
> Tell me again if the --find-maintainer-files=<path or file> should
> be exclusive or should it also read any existing MAINTAINERS file?
Hi Joe,
We were looking for it to be exclusive. If we backport a patch, we want our
internal maintainers to be cc'd not the upstream maintainers. I don't think
upstream needs any more spam. :-)
>
> What if anything is wrong with what I suggested?
You posted multiple versions of a similar patch, each needed some
tweaks. But were pretty close to working.
If I had a <dir> with a MAINTAINERS directory beneath it with files behind
that, I just wanted to run:
get_maintainers.pl --find-maintainer-files <dir>/ # find <dir>/MAINTAINERS/
get_maintainers.pl --find-maintainer-files <dir>/MAINTAINER/<file> # restricted list
That's it. I attached a patch you provided that worked for me. I added a
+ push(@mfiles, "${path}") if -f "${path}";
to your last version of the patch to get things working for me. You may
want a different solution there.
Cheers,
Don
---
scripts/get_maintainer.pl | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index c87fa734e3e1..f7a7d46340a8 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -60,7 +60,7 @@ my $pattern_depth = 0;
my $self_test = undef;
my $version = 0;
my $help = 0;
-my $find_maintainer_files = 0;
+my $find_maintainer_files;
my $vcs_used = 0;
@@ -262,7 +262,7 @@ if (!GetOptions(
'sections!' => \$sections,
'fe|file-emails!' => \$file_emails,
'f|file' => \$from_filename,
- 'find-maintainer-files' => \$find_maintainer_files,
+ 'find-maintainer-files:s' => \$find_maintainer_files,
'self-test:s' => \$self_test,
'v|version' => \$version,
'h|help|usage' => \$help,
@@ -384,26 +384,30 @@ sub find_ignore_git {
read_all_maintainer_files();
sub read_all_maintainer_files {
- if (-d "${lk_path}MAINTAINERS") {
- opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
- my @files = readdir(DIR);
- closedir(DIR);
- foreach my $file (@files) {
- push(@mfiles, "${lk_path}MAINTAINERS/$file") if ($file !~ /^\./);
- }
- }
-
- if ($find_maintainer_files) {
- find( { wanted => \&find_is_maintainer_file,
- preprocess => \&find_ignore_git,
- no_chdir => 1,
- }, "${lk_path}");
+ my $path = defined $find_maintainer_files && $find_maintainer_files ne ""
+ ? $find_maintainer_files : $lk_path;
+ if (-d "${path}MAINTAINERS") {
+ opendir(DIR, "${path}MAINTAINERS") or die $!;
+ my @files = readdir(DIR);
+ closedir(DIR);
+ foreach my $file (@files) {
+ push(@mfiles, "${path}MAINTAINERS/$file") if ($file !~ /^\./);
+ }
+ }
+
+ if (defined $find_maintainer_files && (-d $find_maintainer_files)) {
+ find( { wanted => \&find_is_maintainer_file,
+ preprocess => \&find_ignore_git,
+ no_chdir => 1,
+ }, "${path}");
} else {
- push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
+ push(@mfiles, "${path}MAINTAINERS") if -f "${path}MAINTAINERS";
+ push(@mfiles, "${path}") if -f "${path}";
}
+ die "$P: No MAINTAINER files found in $path\n" if (scalar(@mfiles) == 0);
foreach my $file (@mfiles) {
- read_maintainer_file("$file");
+ read_maintainer_file("$file");
}
}
On Mon, Jul 16, 2018 at 05:20:19PM -0400, Don Zickus wrote:
> On Fri, Jul 13, 2018 at 05:11:58PM -0700, Joe Perches wrote:
> > On Fri, 2018-07-13 at 14:51 -0400, Don Zickus wrote:
> > > On Fri, Jul 06, 2018 at 03:14:28PM -0700, Joe Perches wrote:
> > > > On Fri, 2018-07-06 at 15:09 -0700, Joe Perches wrote:
> > > > > On Fri, 2018-07-06 at 17:58 -0400, Don Zickus wrote:
> > > > > > We have an internal use case of multiple MAINTAINER files, some folks have
> > > > > > more rights to patches than others so they are not allowed to be cc'd (think
> > > > > > embargoed stuff).
> > > >
> > > > How about:
> > >
> > > Hi Joe,
> > >
> > > You are probably busy with stuff, but wanted to softly poke you to see what
> > > is going on with this patch and if there is anything we can help with?
> >
> > Hey Don. No worries.
> >
> > Tell me again if the --find-maintainer-files=<path or file> should
> > be exclusive or should it also read any existing MAINTAINERS file?
>
> Hi Joe,
>
> We were looking for it to be exclusive. If we backport a patch, we want our
> internal maintainers to be cc'd not the upstream maintainers. I don't think
> upstream needs any more spam. :-)
Hi Joe,
ping? Any feedback here?
Cheers,
Don
>
> >
> > What if anything is wrong with what I suggested?
>
> You posted multiple versions of a similar patch, each needed some
> tweaks. But were pretty close to working.
>
> If I had a <dir> with a MAINTAINERS directory beneath it with files behind
> that, I just wanted to run:
>
> get_maintainers.pl --find-maintainer-files <dir>/ # find <dir>/MAINTAINERS/
> get_maintainers.pl --find-maintainer-files <dir>/MAINTAINER/<file> # restricted list
>
> That's it. I attached a patch you provided that worked for me. I added a
>
> + push(@mfiles, "${path}") if -f "${path}";
>
> to your last version of the patch to get things working for me. You may
> want a different solution there.
>
> Cheers,
> Don
>
> ---
> scripts/get_maintainer.pl | 39 +++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index c87fa734e3e1..f7a7d46340a8 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -60,7 +60,7 @@ my $pattern_depth = 0;
> my $self_test = undef;
> my $version = 0;
> my $help = 0;
> -my $find_maintainer_files = 0;
> +my $find_maintainer_files;
>
> my $vcs_used = 0;
>
> @@ -262,7 +262,7 @@ if (!GetOptions(
> 'sections!' => \$sections,
> 'fe|file-emails!' => \$file_emails,
> 'f|file' => \$from_filename,
> - 'find-maintainer-files' => \$find_maintainer_files,
> + 'find-maintainer-files:s' => \$find_maintainer_files,
> 'self-test:s' => \$self_test,
> 'v|version' => \$version,
> 'h|help|usage' => \$help,
> @@ -384,26 +384,30 @@ sub find_ignore_git {
> read_all_maintainer_files();
>
> sub read_all_maintainer_files {
> - if (-d "${lk_path}MAINTAINERS") {
> - opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
> - my @files = readdir(DIR);
> - closedir(DIR);
> - foreach my $file (@files) {
> - push(@mfiles, "${lk_path}MAINTAINERS/$file") if ($file !~ /^\./);
> - }
> - }
> -
> - if ($find_maintainer_files) {
> - find( { wanted => \&find_is_maintainer_file,
> - preprocess => \&find_ignore_git,
> - no_chdir => 1,
> - }, "${lk_path}");
> + my $path = defined $find_maintainer_files && $find_maintainer_files ne ""
> + ? $find_maintainer_files : $lk_path;
> + if (-d "${path}MAINTAINERS") {
> + opendir(DIR, "${path}MAINTAINERS") or die $!;
> + my @files = readdir(DIR);
> + closedir(DIR);
> + foreach my $file (@files) {
> + push(@mfiles, "${path}MAINTAINERS/$file") if ($file !~ /^\./);
> + }
> + }
> +
> + if (defined $find_maintainer_files && (-d $find_maintainer_files)) {
> + find( { wanted => \&find_is_maintainer_file,
> + preprocess => \&find_ignore_git,
> + no_chdir => 1,
> + }, "${path}");
> } else {
> - push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
> + push(@mfiles, "${path}MAINTAINERS") if -f "${path}MAINTAINERS";
> + push(@mfiles, "${path}") if -f "${path}";
> }
>
> + die "$P: No MAINTAINER files found in $path\n" if (scalar(@mfiles) == 0);
> foreach my $file (@mfiles) {
> - read_maintainer_file("$file");
> + read_maintainer_file("$file");
> }
> }
>
Add the ability to have an override for the location of the
MAINTAINERS file.
Miscellanea:
o Properly indent a few lines with leading spaces
Suggested-by: Don Zickus <[email protected]>
Signed-off-by: Joe Perches <[email protected]>
---
scripts/get_maintainer.pl | 48 +++++++++++++++++++++++++++++------------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 82a482f858ee..0ebdefe74d5b 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -62,7 +62,7 @@ my $self_test = undef;
my $version = 0;
my $help = 0;
my $find_maintainer_files = 0;
-
+my $maintainer_path;
my $vcs_used = 0;
my $exit = 0;
@@ -265,6 +265,7 @@ if (!GetOptions(
'fe|file-emails!' => \$file_emails,
'f|file' => \$from_filename,
'find-maintainer-files' => \$find_maintainer_files,
+ 'mpath|maintainer-path=s' => \$maintainer_path,
'self-test:s' => \$self_test,
'v|version' => \$version,
'h|help|usage' => \$help,
@@ -386,26 +387,37 @@ sub find_ignore_git {
read_all_maintainer_files();
sub read_all_maintainer_files {
- if (-d "${lk_path}MAINTAINERS") {
- opendir(DIR, "${lk_path}MAINTAINERS") or die $!;
- my @files = readdir(DIR);
- closedir(DIR);
- foreach my $file (@files) {
- push(@mfiles, "${lk_path}MAINTAINERS/$file") if ($file !~ /^\./);
- }
- }
-
- if ($find_maintainer_files) {
- find( { wanted => \&find_is_maintainer_file,
- preprocess => \&find_ignore_git,
- no_chdir => 1,
- }, "${lk_path}");
+ my $path = "${lk_path}MAINTAINERS";
+ if (defined $maintainer_path) {
+ $path = $maintainer_path;
+ # Perl Cookbook tilde expansion if necessary
+ $path =~ s@^~([^/]*)@ $1 ? (getpwnam($1))[7] : ( $ENV{HOME} || $ENV{LOGDIR} || (getpwuid($<))[7])@ex;
+ }
+
+ if (-d $path) {
+ $path .= '/' if ($path !~ m@/$@);
+ if ($path eq "${lk_path}MAINTAINERS/") {
+ opendir(DIR, "$path") or die $!;
+ my @files = readdir(DIR);
+ closedir(DIR);
+ foreach my $file (@files) {
+ push(@mfiles, "$path$file") if ($file !~ /^\./);
+ }
+ }
+ if ($find_maintainer_files) {
+ find( { wanted => \&find_is_maintainer_file,
+ preprocess => \&find_ignore_git,
+ no_chdir => 1,
+ }, "$path");
+ }
+ } elsif (-f "$path") {
+ push(@mfiles, "$path");
} else {
- push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
+ die "$P: MAINTAINER file not found '$path'\n";
}
-
+ die "$P: No MAINTAINER files found in '$path'\n" if (scalar(@mfiles) == 0);
foreach my $file (@mfiles) {
- read_maintainer_file("$file");
+ read_maintainer_file("$file");
}
}