* [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch
@ 2017-09-12 10:46 Daniel P. Berrange
2017-09-12 15:52 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2017-09-12 10:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Markus Armbruster, Daniel P. Berrange
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 new
flag '--branch' to checkpatch.pl which instructs it to check every
patch on the current GIT branch.
For example:
$ ./scripts/checkpatch.pl --branch
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.
By default it checks every patch identified by 'master..', however,
an alternative origin can be given if desired, if the current branch
is rebased to another non-master branch:
$ ./scripts/checkpatch.pl --branch somebranch..
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
scripts/checkpatch.pl | 97 +++++++++++++++++++++++++++++++++++++--------------
1 file changed, 71 insertions(+), 26 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fa478074b8..f8d080441f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -19,6 +19,8 @@ my $quiet = 0;
my $tree = 1;
my $chk_signoff = 1;
my $chk_patch = 1;
+my $chk_branch = 0;
+my $revlist = "master..";
my $tst_only;
my $emacs = 0;
my $terse = 0;
@@ -43,6 +45,7 @@ Options:
--no-tree run without a kernel tree
--no-signoff do not check for 'Signed-off-by' line
--patch treat FILE as patchfile (default)
+ --branch check all patches on branch since master
--emacs emacs compile window format
--terse one line per report
-f, --file treat FILE as regular source file
@@ -69,6 +72,7 @@ GetOptions(
'tree!' => \$tree,
'signoff!' => \$chk_signoff,
'patch!' => \$chk_patch,
+ 'branch' => \$chk_branch,
'emacs!' => \$emacs,
'terse!' => \$terse,
'f|file!' => \$file,
@@ -88,9 +92,19 @@ help(0) if ($help);
my $exit = 0;
-if ($#ARGV < 0) {
- print "$P: no input files\n";
- exit(1);
+if ($chk_branch) {
+ if ($#ARGV > 0) {
+ print "$P: expected zero or one origni revisions\n";
+ exit(1);
+ }
+ if ($#ARGV == 0) {
+ $revlist = shift @ARGV;
+ }
+} else {
+ if ($#ARGV < 0) {
+ print "$P: no input files\n";
+ exit(1);
+ }
}
my $dbg_values = 0;
@@ -251,32 +265,63 @@ $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", $revlist) ||
+ die "$P: git log --format=%H $revlist failed - $!\n";
+
+ while (<$HASH>) {
chomp;
- push(@rawlines, $_);
+ push @patches, $_;
}
- close($FILE);
- if (!process($filename)) {
- $exit = 1;
+
+ close $HASH;
+
+ 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);
--
2.13.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch
2017-09-12 10:46 [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch Daniel P. Berrange
@ 2017-09-12 15:52 ` Paolo Bonzini
2017-09-12 16:12 ` Daniel P. Berrange
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2017-09-12 15:52 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Markus Armbruster
On 12/09/2017 12:46, 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 new
> flag '--branch' to checkpatch.pl which instructs it to check every
> patch on the current GIT branch.
Great idea, though I'm not sure about having a default. And to keep it
easy to invoke, having a sole argument that ends with ".." might DWIM
and enable --branch too...
Paolo
> For example:
>
> $ ./scripts/checkpatch.pl --branch
> 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.
>
> By default it checks every patch identified by 'master..', however,
> an alternative origin can be given if desired, if the current branch
> is rebased to another non-master branch:
>
> $ ./scripts/checkpatch.pl --branch somebranch..
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> scripts/checkpatch.pl | 97 +++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 71 insertions(+), 26 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fa478074b8..f8d080441f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -19,6 +19,8 @@ my $quiet = 0;
> my $tree = 1;
> my $chk_signoff = 1;
> my $chk_patch = 1;
> +my $chk_branch = 0;
> +my $revlist = "master..";
> my $tst_only;
> my $emacs = 0;
> my $terse = 0;
> @@ -43,6 +45,7 @@ Options:
> --no-tree run without a kernel tree
> --no-signoff do not check for 'Signed-off-by' line
> --patch treat FILE as patchfile (default)
> + --branch check all patches on branch since master
> --emacs emacs compile window format
> --terse one line per report
> -f, --file treat FILE as regular source file
> @@ -69,6 +72,7 @@ GetOptions(
> 'tree!' => \$tree,
> 'signoff!' => \$chk_signoff,
> 'patch!' => \$chk_patch,
> + 'branch' => \$chk_branch,
> 'emacs!' => \$emacs,
> 'terse!' => \$terse,
> 'f|file!' => \$file,
> @@ -88,9 +92,19 @@ help(0) if ($help);
>
> my $exit = 0;
>
> -if ($#ARGV < 0) {
> - print "$P: no input files\n";
> - exit(1);
> +if ($chk_branch) {
> + if ($#ARGV > 0) {
> + print "$P: expected zero or one origni revisions\n";
> + exit(1);
> + }
> + if ($#ARGV == 0) {
> + $revlist = shift @ARGV;
> + }
> +} else {
> + if ($#ARGV < 0) {
> + print "$P: no input files\n";
> + exit(1);
> + }
> }
>
> my $dbg_values = 0;
> @@ -251,32 +265,63 @@ $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", $revlist) ||
> + die "$P: git log --format=%H $revlist failed - $!\n";
> +
> + while (<$HASH>) {
> chomp;
> - push(@rawlines, $_);
> + push @patches, $_;
> }
> - close($FILE);
> - if (!process($filename)) {
> - $exit = 1;
> +
> + close $HASH;
> +
> + 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);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch
2017-09-12 15:52 ` Paolo Bonzini
@ 2017-09-12 16:12 ` Daniel P. Berrange
2017-09-12 16:14 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2017-09-12 16:12 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Markus Armbruster
On Tue, Sep 12, 2017 at 05:52:18PM +0200, Paolo Bonzini wrote:
> On 12/09/2017 12:46, 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 new
> > flag '--branch' to checkpatch.pl which instructs it to check every
> > patch on the current GIT branch.
>
> Great idea, though I'm not sure about having a default. And to keep it
> easy to invoke, having a sole argument that ends with ".." might DWIM
> and enable --branch too...
I think it is beneficial to have a default, as I figure the majority
of contributors are working on a branch that's rebased against master..
Half as many characters to type in the common case :-)
Sometimes people might write patches against a particular subsystem
staging branch (eg kevin/block), but I don't think there's downside
in assuming 'master..' by default.
>
> Paolo
>
> > For example:
> >
> > $ ./scripts/checkpatch.pl --branch
> > 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.
> >
> > By default it checks every patch identified by 'master..', however,
> > an alternative origin can be given if desired, if the current branch
> > is rebased to another non-master branch:
> >
> > $ ./scripts/checkpatch.pl --branch somebranch..
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > scripts/checkpatch.pl | 97 +++++++++++++++++++++++++++++++++++++--------------
> > 1 file changed, 71 insertions(+), 26 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index fa478074b8..f8d080441f 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -19,6 +19,8 @@ my $quiet = 0;
> > my $tree = 1;
> > my $chk_signoff = 1;
> > my $chk_patch = 1;
> > +my $chk_branch = 0;
> > +my $revlist = "master..";
> > my $tst_only;
> > my $emacs = 0;
> > my $terse = 0;
> > @@ -43,6 +45,7 @@ Options:
> > --no-tree run without a kernel tree
> > --no-signoff do not check for 'Signed-off-by' line
> > --patch treat FILE as patchfile (default)
> > + --branch check all patches on branch since master
> > --emacs emacs compile window format
> > --terse one line per report
> > -f, --file treat FILE as regular source file
> > @@ -69,6 +72,7 @@ GetOptions(
> > 'tree!' => \$tree,
> > 'signoff!' => \$chk_signoff,
> > 'patch!' => \$chk_patch,
> > + 'branch' => \$chk_branch,
> > 'emacs!' => \$emacs,
> > 'terse!' => \$terse,
> > 'f|file!' => \$file,
> > @@ -88,9 +92,19 @@ help(0) if ($help);
> >
> > my $exit = 0;
> >
> > -if ($#ARGV < 0) {
> > - print "$P: no input files\n";
> > - exit(1);
> > +if ($chk_branch) {
> > + if ($#ARGV > 0) {
> > + print "$P: expected zero or one origni revisions\n";
> > + exit(1);
> > + }
> > + if ($#ARGV == 0) {
> > + $revlist = shift @ARGV;
> > + }
> > +} else {
> > + if ($#ARGV < 0) {
> > + print "$P: no input files\n";
> > + exit(1);
> > + }
> > }
> >
> > my $dbg_values = 0;
> > @@ -251,32 +265,63 @@ $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", $revlist) ||
> > + die "$P: git log --format=%H $revlist failed - $!\n";
> > +
> > + while (<$HASH>) {
> > chomp;
> > - push(@rawlines, $_);
> > + push @patches, $_;
> > }
> > - close($FILE);
> > - if (!process($filename)) {
> > - $exit = 1;
> > +
> > + close $HASH;
> > +
> > + 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);
> >
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch
2017-09-12 16:12 ` Daniel P. Berrange
@ 2017-09-12 16:14 ` Paolo Bonzini
2017-09-12 16:22 ` Daniel P. Berrange
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2017-09-12 16:14 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: qemu-devel, Markus Armbruster
On 12/09/2017 18:12, Daniel P. Berrange wrote:
> On Tue, Sep 12, 2017 at 05:52:18PM +0200, Paolo Bonzini wrote:
>> On 12/09/2017 12:46, 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 new
>>> flag '--branch' to checkpatch.pl which instructs it to check every
>>> patch on the current GIT branch.
>>
>> Great idea, though I'm not sure about having a default. And to keep it
>> easy to invoke, having a sole argument that ends with ".." might DWIM
>> and enable --branch too...
>
> I think it is beneficial to have a default, as I figure the majority
> of contributors are working on a branch that's rebased against master..
> Half as many characters to type in the common case :-)
With the DWIM option "--branch" and "master.." are exactly the same
length. :)
> Sometimes people might write patches against a particular subsystem
> staging branch (eg kevin/block), but I don't think there's downside
> in assuming 'master..' by default.
What about "origin/master.." instead?
Paolo
>>
>> Paolo
>>
>>> For example:
>>>
>>> $ ./scripts/checkpatch.pl --branch
>>> 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.
>>>
>>> By default it checks every patch identified by 'master..', however,
>>> an alternative origin can be given if desired, if the current branch
>>> is rebased to another non-master branch:
>>>
>>> $ ./scripts/checkpatch.pl --branch somebranch..
>>>
>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>> ---
>>> scripts/checkpatch.pl | 97 +++++++++++++++++++++++++++++++++++++--------------
>>> 1 file changed, 71 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index fa478074b8..f8d080441f 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -19,6 +19,8 @@ my $quiet = 0;
>>> my $tree = 1;
>>> my $chk_signoff = 1;
>>> my $chk_patch = 1;
>>> +my $chk_branch = 0;
>>> +my $revlist = "master..";
>>> my $tst_only;
>>> my $emacs = 0;
>>> my $terse = 0;
>>> @@ -43,6 +45,7 @@ Options:
>>> --no-tree run without a kernel tree
>>> --no-signoff do not check for 'Signed-off-by' line
>>> --patch treat FILE as patchfile (default)
>>> + --branch check all patches on branch since master
>>> --emacs emacs compile window format
>>> --terse one line per report
>>> -f, --file treat FILE as regular source file
>>> @@ -69,6 +72,7 @@ GetOptions(
>>> 'tree!' => \$tree,
>>> 'signoff!' => \$chk_signoff,
>>> 'patch!' => \$chk_patch,
>>> + 'branch' => \$chk_branch,
>>> 'emacs!' => \$emacs,
>>> 'terse!' => \$terse,
>>> 'f|file!' => \$file,
>>> @@ -88,9 +92,19 @@ help(0) if ($help);
>>>
>>> my $exit = 0;
>>>
>>> -if ($#ARGV < 0) {
>>> - print "$P: no input files\n";
>>> - exit(1);
>>> +if ($chk_branch) {
>>> + if ($#ARGV > 0) {
>>> + print "$P: expected zero or one origni revisions\n";
>>> + exit(1);
>>> + }
>>> + if ($#ARGV == 0) {
>>> + $revlist = shift @ARGV;
>>> + }
>>> +} else {
>>> + if ($#ARGV < 0) {
>>> + print "$P: no input files\n";
>>> + exit(1);
>>> + }
>>> }
>>>
>>> my $dbg_values = 0;
>>> @@ -251,32 +265,63 @@ $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", $revlist) ||
>>> + die "$P: git log --format=%H $revlist failed - $!\n";
>>> +
>>> + while (<$HASH>) {
>>> chomp;
>>> - push(@rawlines, $_);
>>> + push @patches, $_;
>>> }
>>> - close($FILE);
>>> - if (!process($filename)) {
>>> - $exit = 1;
>>> +
>>> + close $HASH;
>>> +
>>> + 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);
>>>
>>
>
> Regards,
> Daniel
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch
2017-09-12 16:14 ` Paolo Bonzini
@ 2017-09-12 16:22 ` Daniel P. Berrange
2017-09-12 16:24 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2017-09-12 16:22 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Markus Armbruster
On Tue, Sep 12, 2017 at 06:14:57PM +0200, Paolo Bonzini wrote:
> On 12/09/2017 18:12, Daniel P. Berrange wrote:
> > On Tue, Sep 12, 2017 at 05:52:18PM +0200, Paolo Bonzini wrote:
> >> On 12/09/2017 12:46, 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 new
> >>> flag '--branch' to checkpatch.pl which instructs it to check every
> >>> patch on the current GIT branch.
> >>
> >> Great idea, though I'm not sure about having a default. And to keep it
> >> easy to invoke, having a sole argument that ends with ".." might DWIM
> >> and enable --branch too...
> >
> > I think it is beneficial to have a default, as I figure the majority
> > of contributors are working on a branch that's rebased against master..
> > Half as many characters to type in the common case :-)
>
> With the DWIM option "--branch" and "master.." are exactly the same
> length. :)
Oh hang on. I think I misunderstood what you suggested. I thought you
meant 'checkpatch.pl --branch master..', but IIUC you actually mean
'checkpatch.pl master..' with no flag. That would work with me.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch
2017-09-12 16:22 ` Daniel P. Berrange
@ 2017-09-12 16:24 ` Paolo Bonzini
2017-09-12 18:05 ` John Snow
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2017-09-12 16:24 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: qemu-devel, Markus Armbruster
On 12/09/2017 18:22, Daniel P. Berrange wrote:
> On Tue, Sep 12, 2017 at 06:14:57PM +0200, Paolo Bonzini wrote:
>> On 12/09/2017 18:12, Daniel P. Berrange wrote:
>>> On Tue, Sep 12, 2017 at 05:52:18PM +0200, Paolo Bonzini wrote:
>>>> On 12/09/2017 12:46, 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 new
>>>>> flag '--branch' to checkpatch.pl which instructs it to check every
>>>>> patch on the current GIT branch.
>>>>
>>>> Great idea, though I'm not sure about having a default. And to keep it
>>>> easy to invoke, having a sole argument that ends with ".." might DWIM
>>>> and enable --branch too...
>>>
>>> I think it is beneficial to have a default, as I figure the majority
>>> of contributors are working on a branch that's rebased against master..
>>> Half as many characters to type in the common case :-)
>>
>> With the DWIM option "--branch" and "master.." are exactly the same
>> length. :)
>
> Oh hang on. I think I misunderstood what you suggested. I thought you
> meant 'checkpatch.pl --branch master..', but IIUC you actually mean
> 'checkpatch.pl master..' with no flag. That would work with me.
Yes, basically if length(argv) == 1 and argv[0] ends with ".." then
enable branch. The default for --branch with no ARGV could be
"origin/master.."---or it could ask git-config for the upstream tracking
branch but maybe that's too much to ask.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch
2017-09-12 16:24 ` Paolo Bonzini
@ 2017-09-12 18:05 ` John Snow
0 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2017-09-12 18:05 UTC (permalink / raw)
To: Paolo Bonzini, Daniel P. Berrange; +Cc: qemu-devel, Markus Armbruster
On 09/12/2017 12:24 PM, Paolo Bonzini wrote:
> On 12/09/2017 18:22, Daniel P. Berrange wrote:
>> On Tue, Sep 12, 2017 at 06:14:57PM +0200, Paolo Bonzini wrote:
>>> On 12/09/2017 18:12, Daniel P. Berrange wrote:
>>>> On Tue, Sep 12, 2017 at 05:52:18PM +0200, Paolo Bonzini wrote:
>>>>> On 12/09/2017 12:46, 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 new
>>>>>> flag '--branch' to checkpatch.pl which instructs it to check every
>>>>>> patch on the current GIT branch.
>>>>>
>>>>> Great idea, though I'm not sure about having a default. And to keep it
>>>>> easy to invoke, having a sole argument that ends with ".." might DWIM
>>>>> and enable --branch too...
>>>>
>>>> I think it is beneficial to have a default, as I figure the majority
>>>> of contributors are working on a branch that's rebased against master..
>>>> Half as many characters to type in the common case :-)
>>>
>>> With the DWIM option "--branch" and "master.." are exactly the same
>>> length. :)
>>
>> Oh hang on. I think I misunderstood what you suggested. I thought you
>> meant 'checkpatch.pl --branch master..', but IIUC you actually mean
>> 'checkpatch.pl master..' with no flag. That would work with me.
>
> Yes, basically if length(argv) == 1 and argv[0] ends with ".." then
> enable branch. The default for --branch with no ARGV could be
> "origin/master.."---or it could ask git-config for the upstream tracking
> branch but maybe that's too much to ask.
>
> Paolo
>
~Crazy suggestion~ is that the default could actually be "@{upstream}.."
which will default to whatever you've configured the upstream to be in
the branch you're working in.
Downside is that if you don't set the tracking branch during branch
creation (`git checkout -b myTopic origin/master`) or at a later date
(`git branch --set-upstream-to=origin/master`) that this reference won't
resolve.
You can check and see if it resolves to anything programmatically, though:
jhuston@probe (review) ~/s/qemu> git rev-parse "@{upstream}"
04ef33052c205170c92df21ca0b4be4f3b102188
jhuston@probe (review) ~/s/qemu> echo $status
0
jhuston@probe (master) ~/s/qemu> git checkout -b foobar
Switched to a new branch 'foobar'
jhuston@probe (foobar) ~/s/qemu> git rev-parse "@{upstream}"
fatal: no upstream configured for branch 'foobar'
jhuston@probe (foobar) ~/s/qemu> echo $status
128
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-12 18:05 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-12 10:46 [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch Daniel P. Berrange
2017-09-12 15:52 ` Paolo Bonzini
2017-09-12 16:12 ` Daniel P. Berrange
2017-09-12 16:14 ` Paolo Bonzini
2017-09-12 16:22 ` Daniel P. Berrange
2017-09-12 16:24 ` Paolo Bonzini
2017-09-12 18:05 ` John Snow
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).