* [PATCH] scripts: warn about invalid MAINTAINERS patterns
@ 2017-10-31 21:37 Tom Saeger
2017-11-01 15:32 ` Joe Perches
2017-11-01 16:50 ` Joe Perches
0 siblings, 2 replies; 9+ messages in thread
From: Tom Saeger @ 2017-10-31 21:37 UTC (permalink / raw)
To: Joe Perches, linux-kernel; +Cc: Tom Saeger
Add "--pattern-checks" option to get_maintainer.pl to warn about invalid
"F" and "X" patterns found in MAINTAINERS file(s).
Signed-off-by: Tom Saeger <tom.saeger@oracle.com>
Cc: Joe Perches <joe@perches.com>
---
scripts/get_maintainer.pl | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index bc443201d3ef..ab741b022405 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -57,6 +57,7 @@ my $sections = 0;
my $file_emails = 0;
my $from_filename = 0;
my $pattern_depth = 0;
+my $pattern_checks = 0;
my $version = 0;
my $help = 0;
my $find_maintainer_files = 0;
@@ -138,6 +139,7 @@ my %VCS_cmds_git = (
"subject_pattern" => "^GitSubject: (.*)",
"stat_pattern" => "^(\\d+)\\t(\\d+)\\t\$file\$",
"file_exists_cmd" => "git ls-files \$file",
+ "list_files_cmd" => "git ls-files \$file",
);
my %VCS_cmds_hg = (
@@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
"subject_pattern" => "^HgSubject: (.*)",
"stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
"file_exists_cmd" => "hg files \$file",
+ "list_files_cmd" => "hg files \$file",
);
my $conf = which_conf(".get_maintainer.conf");
@@ -252,6 +255,7 @@ if (!GetOptions(
'fe|file-emails!' => \$file_emails,
'f|file' => \$from_filename,
'find-maintainer-files' => \$find_maintainer_files,
+ 'pattern-checks' => \$pattern_checks,
'v|version' => \$version,
'h|help|usage' => \$help,
)) {
@@ -311,12 +315,14 @@ if (!top_of_kernel_tree($lk_path)) {
my @typevalue = ();
my %keyword_hash;
my @mfiles = ();
+my @pattern_checks_info = ();
sub read_maintainer_file {
my ($file) = @_;
open (my $maint, '<', "$file")
or die "$P: Can't open MAINTAINERS file '$file': $!\n";
+ my $i = 1;
while (<$maint>) {
my $line = $_;
@@ -333,6 +339,9 @@ sub read_maintainer_file {
if ((-d $value)) {
$value =~ s@([^/])$@$1/@;
}
+ if ($pattern_checks) {
+ push(@pattern_checks_info, {file=>$file, line=>$line, linenr=>$i, pat=>$value});
+ }
} elsif ($type eq "K") {
$keyword_hash{@typevalue} = $value;
}
@@ -341,6 +350,7 @@ sub read_maintainer_file {
$line =~ s/\n$//g;
push(@typevalue, $line);
}
+ $i++;
}
close($maint);
}
@@ -543,6 +553,11 @@ foreach my $file (@ARGV) {
}
}
+if ($pattern_checks) {
+ check_maintainers_patterns();
+ exit 0;
+}
+
@file_emails = uniq(@file_emails);
my %email_hash_name;
@@ -586,6 +601,20 @@ if ($web) {
exit($exit);
+sub check_maintainers_patterns {
+ my @lsfiles = ();
+
+ @lsfiles = vcs_list_files($lk_path);
+
+ for my $x (@pattern_checks_info) {
+ if (!grep(m@^$x->{pat}@, @lsfiles)) {
+ my $line = $x->{line};
+ chomp($line);
+ print(STDERR "$x->{file}:$x->{linenr}\twarning: no matches\t$line\n");
+ }
+ }
+}
+
sub ignore_email_address {
my ($address) = @_;
@@ -863,6 +892,7 @@ Other options:
--sections => print all of the subsystem sections with pattern matches
--letters => print all matching 'letter' types from all matching sections
--mailmap => use .mailmap file (default: $email_use_mailmap)
+ --pattern-checks => warn about invalid "F" and "X" patterns in MAINTAINERS file
--version => show version
--help => show this help information
@@ -2192,6 +2222,23 @@ sub vcs_file_exists {
return $exists;
}
+sub vcs_list_files {
+ my ($file) = @_;
+
+ my @lsfiles = ();
+
+ my $vcs_used = vcs_exists();
+ return 0 if (!$vcs_used);
+
+ my $cmd = $VCS_cmds{"list_files_cmd"};
+ $cmd =~ s/(\$\w+)/$1/eeg; # interpolate $cmd
+ @lsfiles = &{$VCS_cmds{"execute_cmd"}}($cmd);
+
+ return () if ($? != 0);
+
+ return @lsfiles;
+}
+
sub uniq {
my (@parms) = @_;
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts: warn about invalid MAINTAINERS patterns
2017-10-31 21:37 [PATCH] scripts: warn about invalid MAINTAINERS patterns Tom Saeger
@ 2017-11-01 15:32 ` Joe Perches
2017-11-01 16:05 ` Tom Saeger
2017-11-01 16:50 ` Joe Perches
1 sibling, 1 reply; 9+ messages in thread
From: Joe Perches @ 2017-11-01 15:32 UTC (permalink / raw)
To: Tom Saeger, linux-kernel
On Tue, 2017-10-31 at 16:37 -0500, Tom Saeger wrote:
> Add "--pattern-checks" option to get_maintainer.pl to warn about invalid
> "F" and "X" patterns found in MAINTAINERS file(s).
Hey Tom.
I've come around to this addition, but I think a few
changes are useful.
o Change --pattern-checks to --self-test so future checks
can be added (valid email address, .mailmap uses, existence
of git trees, etc...)
o Do not require an unnecessary argument with --self-test
o Validate --self-test if it is the only command line argument
o Use emacs filename:line: style output for easier linking
o --self-test emits to STDOUT not STDERR
---
scripts/get_maintainer.pl | 59 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 57 insertions(+), 2 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index bc443201d3ef..128bc90f7034 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -57,6 +57,7 @@ my $sections = 0;
my $file_emails = 0;
my $from_filename = 0;
my $pattern_depth = 0;
+my $self_test = 0;
my $version = 0;
my $help = 0;
my $find_maintainer_files = 0;
@@ -138,6 +139,7 @@ my %VCS_cmds_git = (
"subject_pattern" => "^GitSubject: (.*)",
"stat_pattern" => "^(\\d+)\\t(\\d+)\\t\$file\$",
"file_exists_cmd" => "git ls-files \$file",
+ "list_files_cmd" => "git ls-files \$file",
);
my %VCS_cmds_hg = (
@@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
"subject_pattern" => "^HgSubject: (.*)",
"stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
"file_exists_cmd" => "hg files \$file",
+ "list_files_cmd" => "hg files \$file",
);
my $conf = which_conf(".get_maintainer.conf");
@@ -216,6 +219,14 @@ if (-f $ignore_file) {
close($ignore);
}
+if ($#ARGV > 0) {
+ foreach (@ARGV) {
+ if ($_ eq "-self-test" || $_ eq "--self-test") {
+ die "$P: using --self-test does not allow any other option or argument\n";
+ }
+ }
+}
+
if (!GetOptions(
'email!' => \$email,
'git!' => \$email_git,
@@ -252,6 +263,7 @@ if (!GetOptions(
'fe|file-emails!' => \$file_emails,
'f|file' => \$from_filename,
'find-maintainer-files' => \$find_maintainer_files,
+ 'self-test' => \$self_test,
'v|version' => \$version,
'h|help|usage' => \$help,
)) {
@@ -268,7 +280,7 @@ if ($version != 0) {
exit 0;
}
-if (-t STDIN && !@ARGV) {
+if (!$self_test && -t STDIN && !@ARGV) {
# We're talking to a terminal, but have no command line arguments.
die "$P: missing patchfile or -f file - use --help if necessary\n";
}
@@ -311,12 +323,14 @@ if (!top_of_kernel_tree($lk_path)) {
my @typevalue = ();
my %keyword_hash;
my @mfiles = ();
+my @check_pattern_info = ();
sub read_maintainer_file {
my ($file) = @_;
open (my $maint, '<', "$file")
or die "$P: Can't open MAINTAINERS file '$file': $!\n";
+ my $i = 1;
while (<$maint>) {
my $line = $_;
@@ -333,6 +347,9 @@ sub read_maintainer_file {
if ((-d $value)) {
$value =~ s@([^/])$@$1/@;
}
+ if ($self_test) {
+ push(@check_pattern_info, {file=>$file, linenr=>$i, line=>$line, pat=>$value});
+ }
} elsif ($type eq "K") {
$keyword_hash{@typevalue} = $value;
}
@@ -341,6 +358,7 @@ sub read_maintainer_file {
$line =~ s/\n$//g;
push(@typevalue, $line);
}
+ $i++;
}
close($maint);
}
@@ -464,7 +482,7 @@ my @range = ();
my @keyword_tvi = ();
my @file_emails = ();
-if (!@ARGV) {
+if (!$self_test && !@ARGV) {
push(@ARGV, "&STDIN");
}
@@ -543,6 +561,11 @@ foreach my $file (@ARGV) {
}
}
+if ($self_test) {
+ check_maintainers_patterns();
+ exit 0;
+}
+
@file_emails = uniq(@file_emails);
my %email_hash_name;
@@ -586,6 +609,20 @@ if ($web) {
exit($exit);
+sub check_maintainers_patterns {
+ my @lsfiles = ();
+
+ @lsfiles = vcs_list_files($lk_path);
+
+ for my $x (@check_pattern_info) {
+ if (!grep(m@^$x->{pat}@, @lsfiles)) {
+ my $line = $x->{line};
+ chomp($line);
+ print("$x->{file}:$x->{linenr}:\twarning: no matches\t$line\n");
+ }
+ }
+}
+
sub ignore_email_address {
my ($address) = @_;
@@ -863,6 +900,7 @@ Other options:
--sections => print all of the subsystem sections with pattern matches
--letters => print all matching 'letter' types from all matching sections
--mailmap => use .mailmap file (default: $email_use_mailmap)
+ --self-test => show possible invalid MAINTAINERS file content
--version => show version
--help => show this help information
@@ -2192,6 +2230,23 @@ sub vcs_file_exists {
return $exists;
}
+sub vcs_list_files {
+ my ($file) = @_;
+
+ my @lsfiles = ();
+
+ my $vcs_used = vcs_exists();
+ return 0 if (!$vcs_used);
+
+ my $cmd = $VCS_cmds{"list_files_cmd"};
+ $cmd =~ s/(\$\w+)/$1/eeg; # interpolate $cmd
+ @lsfiles = &{$VCS_cmds{"execute_cmd"}}($cmd);
+
+ return () if ($? != 0);
+
+ return @lsfiles;
+}
+
sub uniq {
my (@parms) = @_;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts: warn about invalid MAINTAINERS patterns
2017-11-01 15:32 ` Joe Perches
@ 2017-11-01 16:05 ` Tom Saeger
0 siblings, 0 replies; 9+ messages in thread
From: Tom Saeger @ 2017-11-01 16:05 UTC (permalink / raw)
To: Joe Perches; +Cc: Tom Saeger, linux-kernel
On Wed, Nov 01, 2017 at 08:32:51AM -0700, Joe Perches wrote:
> On Tue, 2017-10-31 at 16:37 -0500, Tom Saeger wrote:
> > Add "--pattern-checks" option to get_maintainer.pl to warn about invalid
> > "F" and "X" patterns found in MAINTAINERS file(s).
>
> Hey Tom.
>
> I've come around to this addition, but I think a few
> changes are useful.
>
> o Change --pattern-checks to --self-test so future checks
> can be added (valid email address, .mailmap uses, existence
> of git trees, etc...)
Ok.
Had similar thoughts. Was just looking at git trees.
> o Do not require an unnecessary argument with --self-test
Ok.
> o Validate --self-test if it is the only command line argument
Ok.
> o Use emacs filename:line: style output for easier linking
Ok.
> o --self-test emits to STDOUT not STDERR
Ok - I debated this one, I'll change it back.
Thanks for your input Joe, I'll send a v2.
--Tom
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts: warn about invalid MAINTAINERS patterns
2017-10-31 21:37 [PATCH] scripts: warn about invalid MAINTAINERS patterns Tom Saeger
2017-11-01 15:32 ` Joe Perches
@ 2017-11-01 16:50 ` Joe Perches
2017-11-01 17:11 ` Tom Saeger
2017-11-01 18:13 ` [PATCH v2 1/1] " Tom Saeger
1 sibling, 2 replies; 9+ messages in thread
From: Joe Perches @ 2017-11-01 16:50 UTC (permalink / raw)
To: Tom Saeger, linux-kernel; +Cc: xen-devel, mercurial-devel
(add mercurial-devel and xen-devel to cc's)
On Tue, 2017-10-31 at 16:37 -0500, Tom Saeger wrote:
> Add "--pattern-checks" option to get_maintainer.pl to warn about invalid
> "F" and "X" patterns found in MAINTAINERS file(s).
Hey again Tom.
About mercurial/hg.
While as far as I know there hasn't been a mercurial tree
for the linux kernel sources in many years, I believe the
mercurial command to list files should be different.
> my %VCS_cmds_hg = (
> @@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
> "subject_pattern" => "^HgSubject: (.*)",
> "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
> "file_exists_cmd" => "hg files \$file",
> + "list_files_cmd" => "hg files \$file",
I think this should be
"list_files_cmd" => "hg manifest -R \$file",
It seems to work on a XEN test branch but does anyone
really care about hg support in get_maintainers?
btw: to the XEN maintainers
The XEN mercurial branch for MAINTAINERS has a few odd
entries and a few missing file patterns
I think the XEN MAINTAINERS file should be updated to:
---
diff -r c60f04b73240 MAINTAINERS
--- a/MAINTAINERS Mon Oct 16 15:24:44 2017 +0100
+++ b/MAINTAINERS Wed Nov 01 09:39:34 2017 -0700
@@ -246,7 +246,8 @@
KCONFIG
M: Doug Goldstein <cardoe@cardoe.com>
S: Supported
-F: docs/misc/kconfig{,-language}.txt
+F: docs/misc/kconfig.txt
+F: docs/misc/kconfig-language.txt
F: xen/tools/kconfig/
KDD DEBUGGER
@@ -257,8 +258,8 @@
KEXEC
M: Andrew Cooper <andrew.cooper3@citrix.com>
S: Supported
-F: xen/common/{kexec,kimage}.c
-F: xen/include/{kexec,kimage}.h
+F: xen/common/kexec.[ch]
+F: xen/common/kimage.[ch]
F: xen/arch/x86/machine_kexec.c
F: xen/arch/x86/x86_64/kexec_reloc.S
---
After the patch above is applied, --self-test shows:
$ ~/linux/next/scripts/get_maintainer.pl --self-test
./MAINTAINERS:403: warning: no matches F: drivers/xen/usb*/
./MAINTAINERS:415: warning: no matches F: xen/arch/x88/hvm/vm_event.c
./MAINTAINERS:429: warning: no matches F: extras/mini-os/tpm*
./MAINTAINERS:430: warning: no matches F: extras/mini-os/include/tpm*
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts: warn about invalid MAINTAINERS patterns
2017-11-01 16:50 ` Joe Perches
@ 2017-11-01 17:11 ` Tom Saeger
2017-11-01 20:05 ` Augie Fackler
2017-11-01 18:13 ` [PATCH v2 1/1] " Tom Saeger
1 sibling, 1 reply; 9+ messages in thread
From: Tom Saeger @ 2017-11-01 17:11 UTC (permalink / raw)
To: Joe Perches; +Cc: Tom Saeger, linux-kernel, xen-devel, mercurial-devel
On Wed, Nov 01, 2017 at 09:50:05AM -0700, Joe Perches wrote:
> (add mercurial-devel and xen-devel to cc's)
>
> On Tue, 2017-10-31 at 16:37 -0500, Tom Saeger wrote:
> > Add "--pattern-checks" option to get_maintainer.pl to warn about invalid
> > "F" and "X" patterns found in MAINTAINERS file(s).
>
> Hey again Tom.
>
> About mercurial/hg.
>
> While as far as I know there hasn't been a mercurial tree
> for the linux kernel sources in many years, I believe the
> mercurial command to list files should be different.
>
> > my %VCS_cmds_hg = (
> > @@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
> > "subject_pattern" => "^HgSubject: (.*)",
> > "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
> > "file_exists_cmd" => "hg files \$file",
> > + "list_files_cmd" => "hg files \$file",
>
> I think this should be
>
> "list_files_cmd" => "hg manifest -R \$file",
Ok - I'll add to v2.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/1] scripts: warn about invalid MAINTAINERS patterns
2017-11-01 16:50 ` Joe Perches
2017-11-01 17:11 ` Tom Saeger
@ 2017-11-01 18:13 ` Tom Saeger
2017-11-01 18:33 ` Joe Perches
1 sibling, 1 reply; 9+ messages in thread
From: Tom Saeger @ 2017-11-01 18:13 UTC (permalink / raw)
To: Joe Perches, linux-kernel; +Cc: Tom Saeger, xen-devel, mercurial-devel
Add "--self-test" option to get_maintainer.pl to show potential
issues in MAINTAINERS file(s) content.
Pattern check warnings are shown for "F" and "X" patterns found in
MAINTAINERS file(s) which do not match any files known by git.
Signed-off-by: Tom Saeger <tom.saeger@oracle.com>
Cc: Joe Perches <joe@perches.com>
---
v2:
Incorporated suggestions from Joe Perches:
- changed "--pattern-checks" to "--self-test" to allow for future work.
- fixed vcs command "list_files_cmd" for mercurial.
- "--self-test" option is all or nothing.
- output to STDOUT
- output format in emacs-style "filename:line: message"
- changed self-test help to:
--self-test => show potential issues with MAINTAINERS file content
(Joe, I slightly reworded in hopes this rendition is clear and future proof).
- Moved execution of $self_test to just after $help and $version.
This prompted encapsulating main content code to read MAINTAINERS files into
a function (read_all_maintainer_files) callable from $self_test. This
has the side benefit of not having to special case for "$self_test" in other parts
of main program flow.
scripts/get_maintainer.pl | 94 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 77 insertions(+), 17 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index bc443201d3ef..c68a5d1ba709 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -57,6 +57,7 @@ my $sections = 0;
my $file_emails = 0;
my $from_filename = 0;
my $pattern_depth = 0;
+my $self_test = 0;
my $version = 0;
my $help = 0;
my $find_maintainer_files = 0;
@@ -138,6 +139,7 @@ my %VCS_cmds_git = (
"subject_pattern" => "^GitSubject: (.*)",
"stat_pattern" => "^(\\d+)\\t(\\d+)\\t\$file\$",
"file_exists_cmd" => "git ls-files \$file",
+ "list_files_cmd" => "git ls-files \$file",
);
my %VCS_cmds_hg = (
@@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
"subject_pattern" => "^HgSubject: (.*)",
"stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
"file_exists_cmd" => "hg files \$file",
+ "list_files_cmd" => "hg manifest -R \$file",
);
my $conf = which_conf(".get_maintainer.conf");
@@ -216,6 +219,14 @@ if (-f $ignore_file) {
close($ignore);
}
+if ($#ARGV > 0) {
+ foreach (@ARGV) {
+ if ($_ eq "-self-test" || $_ eq "--self-test") {
+ die "$P: using --self-test does not allow any other option or argument\n";
+ }
+ }
+}
+
if (!GetOptions(
'email!' => \$email,
'git!' => \$email_git,
@@ -252,6 +263,7 @@ if (!GetOptions(
'fe|file-emails!' => \$file_emails,
'f|file' => \$from_filename,
'find-maintainer-files' => \$find_maintainer_files,
+ 'self-test' => \$self_test,
'v|version' => \$version,
'h|help|usage' => \$help,
)) {
@@ -268,6 +280,12 @@ if ($version != 0) {
exit 0;
}
+if ($self_test) {
+ read_all_maintainer_files();
+ check_maintainers_patterns();
+ exit 0;
+}
+
if (-t STDIN && !@ARGV) {
# We're talking to a terminal, but have no command line arguments.
die "$P: missing patchfile or -f file - use --help if necessary\n";
@@ -311,12 +329,14 @@ if (!top_of_kernel_tree($lk_path)) {
my @typevalue = ();
my %keyword_hash;
my @mfiles = ();
+my @self_test_pattern_info = ();
sub read_maintainer_file {
my ($file) = @_;
open (my $maint, '<', "$file")
or die "$P: Can't open MAINTAINERS file '$file': $!\n";
+ my $i = 1;
while (<$maint>) {
my $line = $_;
@@ -333,6 +353,9 @@ sub read_maintainer_file {
if ((-d $value)) {
$value =~ s@([^/])$@$1/@;
}
+ if ($self_test) {
+ push(@self_test_pattern_info, {file=>$file, line=>$line, linenr=>$i, pat=>$value});
+ }
} elsif ($type eq "K") {
$keyword_hash{@typevalue} = $value;
}
@@ -341,6 +364,7 @@ sub read_maintainer_file {
$line =~ s/\n$//g;
push(@typevalue, $line);
}
+ $i++;
}
close($maint);
}
@@ -357,26 +381,30 @@ sub find_ignore_git {
return grep { $_ !~ /^\.git$/; } @_;
}
-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 !~ /^\./);
+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}");
-} else {
- push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
-}
+ if ($find_maintainer_files) {
+ find( { wanted => \&find_is_maintainer_file,
+ preprocess => \&find_ignore_git,
+ no_chdir => 1,
+ }, "${lk_path}");
+ } else {
+ push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
+ }
-foreach my $file (@mfiles) {
- read_maintainer_file("$file");
+ foreach my $file (@mfiles) {
+ read_maintainer_file("$file");
+ }
}
#
@@ -586,6 +614,20 @@ if ($web) {
exit($exit);
+sub check_maintainers_patterns {
+ my @lsfiles = ();
+
+ @lsfiles = vcs_list_files($lk_path);
+
+ for my $x (@self_test_pattern_info) {
+ if (!grep(m@^$x->{pat}@, @lsfiles)) {
+ my $line = $x->{line};
+ chomp($line);
+ print("$x->{file}:$x->{linenr}: warning: no matches $line\n");
+ }
+ }
+}
+
sub ignore_email_address {
my ($address) = @_;
@@ -863,6 +905,7 @@ Other options:
--sections => print all of the subsystem sections with pattern matches
--letters => print all matching 'letter' types from all matching sections
--mailmap => use .mailmap file (default: $email_use_mailmap)
+ --self-test => show potential issues with MAINTAINERS file content
--version => show version
--help => show this help information
@@ -2192,6 +2235,23 @@ sub vcs_file_exists {
return $exists;
}
+sub vcs_list_files {
+ my ($file) = @_;
+
+ my @lsfiles = ();
+
+ my $vcs_used = vcs_exists();
+ return 0 if (!$vcs_used);
+
+ my $cmd = $VCS_cmds{"list_files_cmd"};
+ $cmd =~ s/(\$\w+)/$1/eeg; # interpolate $cmd
+ @lsfiles = &{$VCS_cmds{"execute_cmd"}}($cmd);
+
+ return () if ($? != 0);
+
+ return @lsfiles;
+}
+
sub uniq {
my (@parms) = @_;
--
2.14.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/1] scripts: warn about invalid MAINTAINERS patterns
2017-11-01 18:13 ` [PATCH v2 1/1] " Tom Saeger
@ 2017-11-01 18:33 ` Joe Perches
0 siblings, 0 replies; 9+ messages in thread
From: Joe Perches @ 2017-11-01 18:33 UTC (permalink / raw)
To: Tom Saeger, linux-kernel, Andrew Morton; +Cc: xen-devel, mercurial-devel
On Wed, 2017-11-01 at 13:13 -0500, Tom Saeger wrote:
> Add "--self-test" option to get_maintainer.pl to show potential
> issues in MAINTAINERS file(s) content.
This patch subject should be:
get_maintainer: Add --self-test for internal consistency tests
Andrew, can you please change the subject if/when you add it
to quilt?
> Pattern check warnings are shown for "F" and "X" patterns found in
> MAINTAINERS file(s) which do not match any files known by git.
>
> Signed-off-by: Tom Saeger <tom.saeger@oracle.com>
> Cc: Joe Perches <joe@perches.com>
Otherwise,
Acked-by: Joe Perches <joe@perches.com>
> ---
>
> v2:
>
> Incorporated suggestions from Joe Perches:
> - changed "--pattern-checks" to "--self-test" to allow for future work.
> - fixed vcs command "list_files_cmd" for mercurial.
> - "--self-test" option is all or nothing.
> - output to STDOUT
> - output format in emacs-style "filename:line: message"
> - changed self-test help to:
>
> --self-test => show potential issues with MAINTAINERS file content
>
> (Joe, I slightly reworded in hopes this rendition is clear and future proof).
>
> - Moved execution of $self_test to just after $help and $version.
> This prompted encapsulating main content code to read MAINTAINERS files into
> a function (read_all_maintainer_files) callable from $self_test. This
> has the side benefit of not having to special case for "$self_test" in other parts
> of main program flow.
This makes sense to me and is better program flow, thanks.
cheers, Joe
[v2 patch quoted below]
> scripts/get_maintainer.pl | 94 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 77 insertions(+), 17 deletions(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index bc443201d3ef..c68a5d1ba709 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -57,6 +57,7 @@ my $sections = 0;
> my $file_emails = 0;
> my $from_filename = 0;
> my $pattern_depth = 0;
> +my $self_test = 0;
> my $version = 0;
> my $help = 0;
> my $find_maintainer_files = 0;
> @@ -138,6 +139,7 @@ my %VCS_cmds_git = (
> "subject_pattern" => "^GitSubject: (.*)",
> "stat_pattern" => "^(\\d+)\\t(\\d+)\\t\$file\$",
> "file_exists_cmd" => "git ls-files \$file",
> + "list_files_cmd" => "git ls-files \$file",
> );
>
> my %VCS_cmds_hg = (
> @@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
> "subject_pattern" => "^HgSubject: (.*)",
> "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
> "file_exists_cmd" => "hg files \$file",
> + "list_files_cmd" => "hg manifest -R \$file",
> );
>
> my $conf = which_conf(".get_maintainer.conf");
> @@ -216,6 +219,14 @@ if (-f $ignore_file) {
> close($ignore);
> }
>
> +if ($#ARGV > 0) {
> + foreach (@ARGV) {
> + if ($_ eq "-self-test" || $_ eq "--self-test") {
> + die "$P: using --self-test does not allow any other option or argument\n";
> + }
> + }
> +}
> +
> if (!GetOptions(
> 'email!' => \$email,
> 'git!' => \$email_git,
> @@ -252,6 +263,7 @@ if (!GetOptions(
> 'fe|file-emails!' => \$file_emails,
> 'f|file' => \$from_filename,
> 'find-maintainer-files' => \$find_maintainer_files,
> + 'self-test' => \$self_test,
> 'v|version' => \$version,
> 'h|help|usage' => \$help,
> )) {
> @@ -268,6 +280,12 @@ if ($version != 0) {
> exit 0;
> }
>
> +if ($self_test) {
> + read_all_maintainer_files();
> + check_maintainers_patterns();
> + exit 0;
> +}
> +
> if (-t STDIN && !@ARGV) {
> # We're talking to a terminal, but have no command line arguments.
> die "$P: missing patchfile or -f file - use --help if necessary\n";
> @@ -311,12 +329,14 @@ if (!top_of_kernel_tree($lk_path)) {
> my @typevalue = ();
> my %keyword_hash;
> my @mfiles = ();
> +my @self_test_pattern_info = ();
>
> sub read_maintainer_file {
> my ($file) = @_;
>
> open (my $maint, '<', "$file")
> or die "$P: Can't open MAINTAINERS file '$file': $!\n";
> + my $i = 1;
> while (<$maint>) {
> my $line = $_;
>
> @@ -333,6 +353,9 @@ sub read_maintainer_file {
> if ((-d $value)) {
> $value =~ s@([^/])$@$1/@;
> }
> + if ($self_test) {
> + push(@self_test_pattern_info, {file=>$file, line=>$line, linenr=>$i, pat=>$value});
> + }
> } elsif ($type eq "K") {
> $keyword_hash{@typevalue} = $value;
> }
> @@ -341,6 +364,7 @@ sub read_maintainer_file {
> $line =~ s/\n$//g;
> push(@typevalue, $line);
> }
> + $i++;
> }
> close($maint);
> }
> @@ -357,26 +381,30 @@ sub find_ignore_git {
> return grep { $_ !~ /^\.git$/; } @_;
> }
>
> -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 !~ /^\./);
> +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}");
> -} else {
> - push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
> -}
> + if ($find_maintainer_files) {
> + find( { wanted => \&find_is_maintainer_file,
> + preprocess => \&find_ignore_git,
> + no_chdir => 1,
> + }, "${lk_path}");
> + } else {
> + push(@mfiles, "${lk_path}MAINTAINERS") if -f "${lk_path}MAINTAINERS";
> + }
>
> -foreach my $file (@mfiles) {
> - read_maintainer_file("$file");
> + foreach my $file (@mfiles) {
> + read_maintainer_file("$file");
> + }
> }
>
> #
> @@ -586,6 +614,20 @@ if ($web) {
>
> exit($exit);
>
> +sub check_maintainers_patterns {
> + my @lsfiles = ();
> +
> + @lsfiles = vcs_list_files($lk_path);
> +
> + for my $x (@self_test_pattern_info) {
> + if (!grep(m@^$x->{pat}@, @lsfiles)) {
> + my $line = $x->{line};
> + chomp($line);
> + print("$x->{file}:$x->{linenr}: warning: no matches $line\n");
> + }
> + }
> +}
> +
> sub ignore_email_address {
> my ($address) = @_;
>
> @@ -863,6 +905,7 @@ Other options:
> --sections => print all of the subsystem sections with pattern matches
> --letters => print all matching 'letter' types from all matching sections
> --mailmap => use .mailmap file (default: $email_use_mailmap)
> + --self-test => show potential issues with MAINTAINERS file content
> --version => show version
> --help => show this help information
>
> @@ -2192,6 +2235,23 @@ sub vcs_file_exists {
> return $exists;
> }
>
> +sub vcs_list_files {
> + my ($file) = @_;
> +
> + my @lsfiles = ();
> +
> + my $vcs_used = vcs_exists();
> + return 0 if (!$vcs_used);
> +
> + my $cmd = $VCS_cmds{"list_files_cmd"};
> + $cmd =~ s/(\$\w+)/$1/eeg; # interpolate $cmd
> + @lsfiles = &{$VCS_cmds{"execute_cmd"}}($cmd);
> +
> + return () if ($? != 0);
> +
> + return @lsfiles;
> +}
> +
> sub uniq {
> my (@parms) = @_;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts: warn about invalid MAINTAINERS patterns
2017-11-01 17:11 ` Tom Saeger
@ 2017-11-01 20:05 ` Augie Fackler
2017-11-01 20:24 ` Joe Perches
0 siblings, 1 reply; 9+ messages in thread
From: Augie Fackler @ 2017-11-01 20:05 UTC (permalink / raw)
To: Tom Saeger; +Cc: Joe Perches, xen-devel, mercurial-devel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1248 bytes --]
> On Nov 1, 2017, at 13:11, Tom Saeger <tom.saeger@oracle.com> wrote:
>
> On Wed, Nov 01, 2017 at 09:50:05AM -0700, Joe Perches wrote:
>> (add mercurial-devel and xen-devel to cc's)
>>
>> On Tue, 2017-10-31 at 16:37 -0500, Tom Saeger wrote:
>>> Add "--pattern-checks" option to get_maintainer.pl to warn about invalid
>>> "F" and "X" patterns found in MAINTAINERS file(s).
>>
>> Hey again Tom.
>>
>> About mercurial/hg.
>>
>> While as far as I know there hasn't been a mercurial tree
>> for the linux kernel sources in many years, I believe the
>> mercurial command to list files should be different.
>>
>>> my %VCS_cmds_hg = (
>>> @@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
>>> "subject_pattern" => "^HgSubject: (.*)",
>>> "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
>>> "file_exists_cmd" => "hg files \$file",
>>> + "list_files_cmd" => "hg files \$file",
>>
>> I think this should be
>>
>> "list_files_cmd" => "hg manifest -R \$file",
>
> Ok - I'll add to v2.
Actually, I'd recommend `hg files` over `hg manifest` by a wide margin.
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] scripts: warn about invalid MAINTAINERS patterns
2017-11-01 20:05 ` Augie Fackler
@ 2017-11-01 20:24 ` Joe Perches
0 siblings, 0 replies; 9+ messages in thread
From: Joe Perches @ 2017-11-01 20:24 UTC (permalink / raw)
To: Augie Fackler, Tom Saeger; +Cc: xen-devel, mercurial-devel, linux-kernel
On Wed, 2017-11-01 at 16:05 -0400, Augie Fackler wrote:
> > On Nov 1, 2017, at 13:11, Tom Saeger <tom.saeger@oracle.com> wrote:
> >
> > On Wed, Nov 01, 2017 at 09:50:05AM -0700, Joe Perches wrote:
> > > (add mercurial-devel and xen-devel to cc's)
> > >
> > > On Tue, 2017-10-31 at 16:37 -0500, Tom Saeger wrote:
> > > > Add "--pattern-checks" option to get_maintainer.pl to warn about invalid
> > > > "F" and "X" patterns found in MAINTAINERS file(s).
> > >
> > > Hey again Tom.
> > >
> > > About mercurial/hg.
> > >
> > > While as far as I know there hasn't been a mercurial tree
> > > for the linux kernel sources in many years, I believe the
> > > mercurial command to list files should be different.
> > >
> > > > my %VCS_cmds_hg = (
> > > > @@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
> > > > "subject_pattern" => "^HgSubject: (.*)",
> > > > "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
> > > > "file_exists_cmd" => "hg files \$file",
> > > > + "list_files_cmd" => "hg files \$file",
> > >
> > > I think this should be
> > >
> > > "list_files_cmd" => "hg manifest -R \$file",
> >
> > Ok - I'll add to v2.
>
> Actually, I'd recommend `hg files` over `hg manifest` by a wide margin.
why?
hg files -R <path> prefixes all the output
hg manifest -R <path> output is unprefixed
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-01 20:24 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-31 21:37 [PATCH] scripts: warn about invalid MAINTAINERS patterns Tom Saeger
2017-11-01 15:32 ` Joe Perches
2017-11-01 16:05 ` Tom Saeger
2017-11-01 16:50 ` Joe Perches
2017-11-01 17:11 ` Tom Saeger
2017-11-01 20:05 ` Augie Fackler
2017-11-01 20:24 ` Joe Perches
2017-11-01 18:13 ` [PATCH v2 1/1] " Tom Saeger
2017-11-01 18:33 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).