[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GI
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] scripts: let checkpatch.pl process an entire GIT branch |
Date: |
Tue, 12 Sep 2017 18:14:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
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 <address@hidden>
>>> ---
>>> 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
>