| Message ID | 1355770579.13361.41.camel@joe-AO722 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2012-12-17 at 10:56 -0800, Joe Perches wrote: > On Mon, 2012-12-17 at 18:00 +0100, Borislav Petkov wrote: > > On Mon, Dec 17, 2012 at 07:35:44AM -0800, Joe Perches wrote: > > > Perhaps Cesar can use his script as a starting point to find those > > > pattern invalidating commits or maybe add the capability (or a > > > --strict check) to checkpatch. > > So, yeah, I can see how checkpatch saying: "you've just renamed a > > file and thusly invalidated a pattern in MAINTAINERS. Pls, consider > > correcting the pattern" could make sense. And I would even add it to > > default functionality since the MAINTAINERS patterns are something we > > want to always have up-to-date, IMO. > > Maybe something like this: A trivial correction: > scripts/checkpatch.pl | 20 ++++++++++++++++++-- > 1 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -1556,6 +1559,19 @@ sub process { > ERROR("MODIFIED_INCLUDE_ASM", > "do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n"); > } This needs a new test here to avoid chirping on files that aren't added, deleted or renamed. next if ($realfile eq $modifiedfile); > + > + my $action = "renames $modifiedfile to $realfile"; > + $action = "creates file $realfile" if ($modifiedfile =~ m@dev/null@); > + $action = "deletes file $modifiedfile" if ($realfile =~ m@dev/null@); > + > + CHK("MAINTAINERS", > + "Patch $action, update MAINTAINERS?\n"); -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 17, 2012 at 11:09:43AM -0800, Joe Perches wrote: > This needs a new test here to avoid chirping > on files that aren't added, deleted or renamed. > > next if ($realfile eq $modifiedfile); Hmm, I don't think that catches file renames when using the normal 'git diff' output: $ git mv README README.old And this detection should work in the default case with a pre-commit hook. For example, I have this in .git/hooks/pre-commit: if git rev-parse --verify HEAD >/dev/null 2>&1 then against=HEAD else # Initial commit: diff against an empty tree object against=4b825dc642cb6eb9a060e54bf8d69288fbee4904 fi git diff --cached $against -- | ./scripts/checkpatch.pl --strict --no-signoff - and when I do this, I get the changes checked: $ git commit -a README.old:5: trailing whitespace. +kernel, and what to do if something goes wrong. README.old:19: trailing whitespace. + accompanying COPYING file for more details. README.old:47: trailing whitespace. + these typically contain kernel-specific installation notes for some README.old:287: trailing whitespace. + - Keep a backup kernel handy in case something goes wrong. This is README.old:301: trailing whitespace. + to the place where your regular bootable kernel is found. README.old:314: trailing whitespace. + Reinstalling LILO is usually a matter of running /sbin/lilo. README.old:317: trailing whitespace. + work. See the LILO docs for more information. README.old:325: trailing whitespace. + recompile the kernel to change these parameters. README.old:327: trailing whitespace. + - Reboot with the new kernel and enjoy. README.old:394: trailing whitespace. + interesting one. README.old:412: new blank line at EOF. but not the MAINTAINERS check. git diff --cached HEAD gives: diff --git a/README b/README.old similarity index 100% rename from README rename to README.old so I'd say we're almost there. Thanks.
On Tue, 2012-12-18 at 19:28 +0100, Borislav Petkov wrote: > On Mon, Dec 17, 2012 at 11:09:43AM -0800, Joe Perches wrote: > > This needs a new test here to avoid chirping > > on files that aren't added, deleted or renamed. > > > > next if ($realfile eq $modifiedfile); > > Hmm, I don't think that catches file renames when using the normal 'git > diff' output: That depends on whether or not an actual patch follows. This looks for the +++ and --- lines in a patch. If no patch is attached, you should get ERROR: Does not appear to be a unified-diff format patch -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 725c596..a7dddc1 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1379,6 +1379,7 @@ sub process { # Trace the real file/line as we go. my $realfile = ''; my $realline = 0; + my $modifiedfile = ''; my $realcnt = 0; my $here = ''; my $in_comment = 0; @@ -1536,8 +1537,10 @@ sub process { $here = "#$realline: " if ($file); # extract the filename as it passes - if ($line =~ /^diff --git.*?(\S+)$/) { - $realfile = $1; + if ($line =~ /^diff --git\s+(\S+)\s+(\S+)$/) { + $modifiedfile = $1; + $realfile = $2; + $modifiedfile =~ s@^([^/]*)/@@; $realfile =~ s@^([^/]*)/@@; $in_commit_log = 0; } elsif ($line =~ /^\+\+\+\s+(\S+)/) { @@ -1556,6 +1559,19 @@ sub process { ERROR("MODIFIED_INCLUDE_ASM", "do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n"); } + + my $action = "renames $modifiedfile to $realfile"; + $action = "creates file $realfile" if ($modifiedfile =~ m@dev/null@); + $action = "deletes file $modifiedfile" if ($realfile =~ m@dev/null@); + + CHK("MAINTAINERS", + "Patch $action, update MAINTAINERS?\n"); + + next; + } elsif ($line =~ /^\-\-\-\s+(\S+)/) { + $modifiedfile = $1; + $modifiedfile =~ s@^([^/]*)/@@; + $in_commit_log = 0; next; }