| Message ID | 20170912165139.16212-1-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] scripts: let checkpatch.pl process an entire GIT branch | expand |
On 12/09/2017 18:51, Daniel P. Berrange wrote: > Currently before submitting a series, devs should run checkpatch.pl > across each patch to be submitted. This can be automated using a > command such as: > > git rebase -i master -x 'git show | ./scripts/checkpatch.pl -' > > This is rather long winded to type, so this patch introduces a way > to tell checkpatch.pl to validate a series of GIT revisions. > > If checkpatch.pl is given a single argument that contains a literal > '..', this is interpreted as a GIT revision list. > > For example: > > $ ./scripts/checkpatch.pl master.. > total: 0 errors, 0 warnings, 297 lines checked > > b886d352a2bf58f0996471fb3991a138373a2957 has no obvious style problems and is ready for submission. > total: 0 errors, 0 warnings, 182 lines checked > > 2a731f9a9ce145e0e0df6d42dd2a3ce4dfc543fa has no obvious style problems and is ready for submission. > total: 0 errors, 0 warnings, 102 lines checked > > 11844169bcc0c8ed4449eb3744a69877ed329dd7 has no obvious style problems and is ready for submission. > > If a genuine patch filename contains the characters '..' it is > possible to force interpretation of the arg as a patch > > $ ./scripts/checkpatch.pl --patch master.. > > will force it to load a patch file called "master.." Looks good, but why no --branch anymore? :) (I can also try adding it back on top). Paolo > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > scripts/checkpatch.pl | 102 +++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 77 insertions(+), 25 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index fa478074b8..213777d488 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -18,7 +18,8 @@ use Getopt::Long qw(:config no_auto_abbrev); > my $quiet = 0; > my $tree = 1; > my $chk_signoff = 1; > -my $chk_patch = 1; > +my $chk_patch = undef; > +my $chk_branch = undef; > my $tst_only; > my $emacs = 0; > my $terse = 0; > @@ -35,7 +36,11 @@ sub help { > my ($exitcode) = @_; > > print << "EOM"; > -Usage: $P [OPTION]... [FILE]... > +Usage: > + > + $P [OPTION]... [FILE]... > + $P [OPTION]... [GIT-REV-LIST] > + > Version: $V > > Options: > @@ -88,6 +93,19 @@ help(0) if ($help); > > my $exit = 0; > > +if (!defined $chk_patch) { > + if (!$file) { > + if ($#ARGV == 0 && $ARGV[0] =~ /\.\./) { > + $chk_branch = $ARGV[0]; > + $chk_patch = 0; > + } else { > + $chk_patch = 1; > + } > + } else { > + $chk_patch = 0; > + } > +} > + > if ($#ARGV < 0) { > print "$P: no input files\n"; > exit(1); > @@ -251,32 +269,66 @@ $chk_signoff = 0 if ($file); > my @rawlines = (); > my @lines = (); > my $vname; > -for my $filename (@ARGV) { > - my $FILE; > - if ($file) { > - open($FILE, '-|', "diff -u /dev/null $filename") || > - die "$P: $filename: diff failed - $!\n"; > - } elsif ($filename eq '-') { > - open($FILE, '<&STDIN'); > - } else { > - open($FILE, '<', "$filename") || > - die "$P: $filename: open failed - $!\n"; > - } > - if ($filename eq '-') { > - $vname = 'Your patch'; > - } else { > - $vname = $filename; > - } > - while (<$FILE>) { > +if ($chk_branch) { > + my @patches; > + my $HASH; > + open($HASH, "-|", "git", "log", "--format=%H", $chk_branch) || > + die "$P: git log --format=%H $chk_branch failed - $!\n"; > + > + while (<$HASH>) { > chomp; > - push(@rawlines, $_); > + push @patches, $_; > } > - close($FILE); > - if (!process($filename)) { > - $exit = 1; > + > + close $HASH; > + > + die "$P: no revisions returned for revlist '$chk_branch'\n" > + unless @patches; > + > + for my $hash (@patches) { > + my $FILE; > + open($FILE, '-|', "git", "show", $hash) || > + die "$P: git show $hash - $!\n"; > + $vname = $hash; > + while (<$FILE>) { > + chomp; > + push(@rawlines, $_); > + } > + close($FILE); > + if (!process($hash)) { > + $exit = 1; > + } > + @rawlines = (); > + @lines = (); > + } > +} else { > + for my $filename (@ARGV) { > + my $FILE; > + if ($file) { > + open($FILE, '-|', "diff -u /dev/null $filename") || > + die "$P: $filename: diff failed - $!\n"; > + } elsif ($filename eq '-') { > + open($FILE, '<&STDIN'); > + } else { > + open($FILE, '<', "$filename") || > + die "$P: $filename: open failed - $!\n"; > + } > + if ($filename eq '-') { > + $vname = 'Your patch'; > + } else { > + $vname = $filename; > + } > + while (<$FILE>) { > + chomp; > + push(@rawlines, $_); > + } > + close($FILE); > + if (!process($filename)) { > + $exit = 1; > + } > + @rawlines = (); > + @lines = (); > } > - @rawlines = (); > - @lines = (); > } > > exit($exit); >
On Tue, Sep 12, 2017 at 07:11:34PM +0200, Paolo Bonzini wrote: > On 12/09/2017 18:51, Daniel P. Berrange wrote: > > Currently before submitting a series, devs should run checkpatch.pl > > across each patch to be submitted. This can be automated using a > > command such as: > > > > git rebase -i master -x 'git show | ./scripts/checkpatch.pl -' > > > > This is rather long winded to type, so this patch introduces a way > > to tell checkpatch.pl to validate a series of GIT revisions. > > > > If checkpatch.pl is given a single argument that contains a literal > > '..', this is interpreted as a GIT revision list. > > > > For example: > > > > $ ./scripts/checkpatch.pl master.. > > total: 0 errors, 0 warnings, 297 lines checked > > > > b886d352a2bf58f0996471fb3991a138373a2957 has no obvious style problems and is ready for submission. > > total: 0 errors, 0 warnings, 182 lines checked > > > > 2a731f9a9ce145e0e0df6d42dd2a3ce4dfc543fa has no obvious style problems and is ready for submission. > > total: 0 errors, 0 warnings, 102 lines checked > > > > 11844169bcc0c8ed4449eb3744a69877ed329dd7 has no obvious style problems and is ready for submission. > > > > If a genuine patch filename contains the characters '..' it is > > possible to force interpretation of the arg as a patch > > > > $ ./scripts/checkpatch.pl --patch master.. > > > > will force it to load a patch file called "master.." > > Looks good, but why no --branch anymore? :) (I can also try adding it > back on top). It felt redundant to me. I guess it could be used to force interpretation of the arg as a git revlist, even if it lacks '..' ? Regards, Daniel
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index fa478074b8..213777d488 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -18,7 +18,8 @@ use Getopt::Long qw(:config no_auto_abbrev); my $quiet = 0; my $tree = 1; my $chk_signoff = 1; -my $chk_patch = 1; +my $chk_patch = undef; +my $chk_branch = undef; my $tst_only; my $emacs = 0; my $terse = 0; @@ -35,7 +36,11 @@ sub help { my ($exitcode) = @_; print << "EOM"; -Usage: $P [OPTION]... [FILE]... +Usage: + + $P [OPTION]... [FILE]... + $P [OPTION]... [GIT-REV-LIST] + Version: $V Options: @@ -88,6 +93,19 @@ help(0) if ($help); my $exit = 0; +if (!defined $chk_patch) { + if (!$file) { + if ($#ARGV == 0 && $ARGV[0] =~ /\.\./) { + $chk_branch = $ARGV[0]; + $chk_patch = 0; + } else { + $chk_patch = 1; + } + } else { + $chk_patch = 0; + } +} + if ($#ARGV < 0) { print "$P: no input files\n"; exit(1); @@ -251,32 +269,66 @@ $chk_signoff = 0 if ($file); my @rawlines = (); my @lines = (); my $vname; -for my $filename (@ARGV) { - my $FILE; - if ($file) { - open($FILE, '-|', "diff -u /dev/null $filename") || - die "$P: $filename: diff failed - $!\n"; - } elsif ($filename eq '-') { - open($FILE, '<&STDIN'); - } else { - open($FILE, '<', "$filename") || - die "$P: $filename: open failed - $!\n"; - } - if ($filename eq '-') { - $vname = 'Your patch'; - } else { - $vname = $filename; - } - while (<$FILE>) { +if ($chk_branch) { + my @patches; + my $HASH; + open($HASH, "-|", "git", "log", "--format=%H", $chk_branch) || + die "$P: git log --format=%H $chk_branch failed - $!\n"; + + while (<$HASH>) { chomp; - push(@rawlines, $_); + push @patches, $_; } - close($FILE); - if (!process($filename)) { - $exit = 1; + + close $HASH; + + die "$P: no revisions returned for revlist '$chk_branch'\n" + unless @patches; + + for my $hash (@patches) { + my $FILE; + open($FILE, '-|', "git", "show", $hash) || + die "$P: git show $hash - $!\n"; + $vname = $hash; + while (<$FILE>) { + chomp; + push(@rawlines, $_); + } + close($FILE); + if (!process($hash)) { + $exit = 1; + } + @rawlines = (); + @lines = (); + } +} else { + for my $filename (@ARGV) { + my $FILE; + if ($file) { + open($FILE, '-|', "diff -u /dev/null $filename") || + die "$P: $filename: diff failed - $!\n"; + } elsif ($filename eq '-') { + open($FILE, '<&STDIN'); + } else { + open($FILE, '<', "$filename") || + die "$P: $filename: open failed - $!\n"; + } + if ($filename eq '-') { + $vname = 'Your patch'; + } else { + $vname = $filename; + } + while (<$FILE>) { + chomp; + push(@rawlines, $_); + } + close($FILE); + if (!process($filename)) { + $exit = 1; + } + @rawlines = (); + @lines = (); } - @rawlines = (); - @lines = (); } exit($exit);
Currently before submitting a series, devs should run checkpatch.pl across each patch to be submitted. This can be automated using a command such as: git rebase -i master -x 'git show | ./scripts/checkpatch.pl -' This is rather long winded to type, so this patch introduces a way to tell checkpatch.pl to validate a series of GIT revisions. If checkpatch.pl is given a single argument that contains a literal '..', this is interpreted as a GIT revision list. For example: $ ./scripts/checkpatch.pl master.. total: 0 errors, 0 warnings, 297 lines checked b886d352a2bf58f0996471fb3991a138373a2957 has no obvious style problems and is ready for submission. total: 0 errors, 0 warnings, 182 lines checked 2a731f9a9ce145e0e0df6d42dd2a3ce4dfc543fa has no obvious style problems and is ready for submission. total: 0 errors, 0 warnings, 102 lines checked 11844169bcc0c8ed4449eb3744a69877ed329dd7 has no obvious style problems and is ready for submission. If a genuine patch filename contains the characters '..' it is possible to force interpretation of the arg as a patch $ ./scripts/checkpatch.pl --patch master.. will force it to load a patch file called "master.." Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- scripts/checkpatch.pl | 102 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 77 insertions(+), 25 deletions(-)