[PATCH] checkpatch.pl: add support for checking patch from git repository

[PATCH] checkpatch.pl: add support for checking patch from git repository

[changbin.du]

From: "Du, Changbin" <changbin.du@intel.com>

This patch add "-g, --git" option that tread FILE as git commits
expression. You can specify the git commit hash ID expressions,
then these commits from your git repository will be checked.

This feature allows you can check your patches but no need to
generate temporary patch files. The flow git expressions are
supported.
<rev>
<rev>^
<rev>~n
<rev1>..<rev2>
<rev1>...<rev2>
<rev>-n (This is a speical one. For example, 'HEAD-3' means we
need check commits 'HEAD', 'HEAD~1' and HEAD~2'.)

Here is a example output:

changbin@linux$ scripts/checkpatch.pl -g 2a50009-2
----------------------------------------------------------------------------
Commit 4bfd2e6 ("net/mlx4_core: Avoid repeated calls to pci enable/disable")
----------------------------------------------------------------------------
total: 0 errors, 0 warnings, 100 lines checked

Commit 4bfd2e6 ("net/mlx4_core: Avoid repeated calls to pci
enable/disable") has no obvious style problems and is ready for
submission.
--------------------------------------------------------------------------------
Commit 2a50009 ("net/mlx4_core: Don't allow to VF change global pause settings")
--------------------------------------------------------------------------------
total: 0 errors, 0 warnings, 0 checks, 27 lines checked

Commit 2a50009 ("net/mlx4_core: Don't allow to VF change global pause
settings") has no obvious style problems and is ready for
submission.

Signed-off-by: Du, Changbin <changbin.du@intel.com>
---
 scripts/checkpatch.pl | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d574d13..9a8a340 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -27,6 +27,7 @@ my $emacs = 0;
 my $terse = 0;
 my $showfile = 0;
 my $file = 0;
+my $git = 0;
 my $check = 0;
 my $check_orig = 0;
 my $summary = 1;
@@ -68,6 +69,12 @@ Options:
   --emacs                    emacs compile window format
   --terse                    one line per report
   --showfile                 emit diffed file position, not input file position
+  -g, --git                  tread FILE as git commits expression. You can
+                             specify the git commit hash ID expressions, then
+                             these commits from your git repository will be
+                             checked. For example, when -g applied, [FILE]
+			     'HEAD-3' means we need check commits 'HEAD',
+                             'HEAD~1' and HEAD~2'.
   -f, --file                 treat FILE as regular source file
   --subjective, --strict     enable more subjective tests
   --types TYPE(,TYPE2...)    show only these comma separated message types
@@ -141,6 +148,7 @@ GetOptions(
 	'terse!'	=> \$terse,
 	'showfile!'	=> \$showfile,
 	'f|file!'	=> \$file,
+	'g|git!'	=> \$git,
 	'subjective!'	=> \$check,
 	'strict!'	=> \$check,
 	'ignore=s'	=> \@ignore,
@@ -752,10 +760,40 @@ my @fixed_inserted = ();
 my @fixed_deleted = ();
 my $fixlinenr = -1;
 
+# If input is git commits, extract all commits from the commit expressions.
+# For example, HEAD-3 means we need check 'HEAD, HEAD~1, HEAD~2'.
+if ($git) {
+	my @commits = ();
+	for my $commit_expr (@ARGV) {
+		if ($commit_expr =~ m/-/) {
+			my @tmp = split(/-/, $commit_expr);
+			die "$P: incorrect git commits expression".
+				"$commit_expr$!\n" if @tmp != 2;
+			for (my $i = 0; $i < $tmp[1]; $i++) {
+				my $sha = `git log -1 $tmp[0]~$i --pretty=format:'%H'`;
+				unshift(@commits, $sha);
+			}
+		} elsif ($commit_expr =~ m/\.\./) {
+			my $lines = `git log --pretty=format:'%H' $commit_expr`;
+			my $line;
+			foreach $line (split(/\n/, $lines)) {
+				unshift(@commits, $line);
+			}
+		} else {
+			unshift(@commits, $commit_expr);
+		}
+	}
+	die "$P: no git commits after extraction!\n" if @commits == 0;
+	@ARGV = @commits
+}
+
 my $vname;
 for my $filename (@ARGV) {
 	my $FILE;
-	if ($file) {
+	if ($git) {
+		open($FILE, '-|', "git format-patch --stdout -1 $filename") ||
+			die "$P: $filename: git format-patch failed - $!\n";
+	} elsif ($file) {
 		open($FILE, '-|', "diff -u /dev/null $filename") ||
 			die "$P: $filename: diff failed - $!\n";
 	} elsif ($filename eq '-') {
@@ -766,6 +804,8 @@ for my $filename (@ARGV) {
 	}
 	if ($filename eq '-') {
 		$vname = 'Your patch';
+	} elsif ($git) {
+		$vname = "Commit ". `git log -1 $filename --pretty=format:'%h("%s")'`;
 	} else {
 		$vname = $filename;
 	}
-- 
2.7.4

Re: [PATCH] checkpatch.pl: add support for checking patch from git repository

[Joe Perches]

On Sun, 2016-04-24 at 18:40 +0800, changbin.du@intel.com wrote:
> From: "Du, Changbin" <changbin.du@intel.com>
> 
> This patch add "-g, --git" option that tread FILE as git commits
> expression. You can specify the git commit hash ID expressions,
> then these commits from your git repository will be checked.

Why would anyone want to use checkpatch on commits already in git?

Re: [PATCH] checkpatch.pl: add support for checking patch from git repository

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

Hi,

On Sun, Apr 24, 2016 at 04:22:45AM -0700, Joe Perches wrote:
> On Sun, 2016-04-24 at 18:40 +0800, changbin.du@intel.com wrote:
> > From: "Du, Changbin" <changbin.du@intel.com>
> > 
> > This patch add "-g, --git" option that tread FILE as git commits
> > expression. You can specify the git commit hash ID expressions,
> > then these commits from your git repository will be checked.
> 
> Why would anyone want to use checkpatch on commits already in git?

It may be in some non-public development branch. Usually when I
write patches I open a file, change it and commit the result or even
interim result to have backups and other git features available as
soon as possible. All testing is done later.

So IMHO this is a really useful feature.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] checkpatch.pl: add support for checking patch from git repository

[Joe Perches]

On Sun, 2016-04-24 at 17:10 +0200, Sebastian Reichel wrote:
> On Sun, Apr 24, 2016 at 04:22:45AM -0700, Joe Perches wrote:
> > On Sun, 2016-04-24 at 18:40 +0800, changbin.du@intel.com wrote:
> > > From: "Du, Changbin" <changbin.du@intel.com>
> > > This patch add "-g, --git" option that tread FILE as git commits
> > > expression. You can specify the git commit hash ID expressions,
> > > then these commits from your git repository will be checked.
> > Why would anyone want to use checkpatch on commits already in git?
> It may be in some non-public development branch. Usually when I
> write patches I open a file, change it and commit the result or even
> interim result to have backups and other git features available as
> soon as possible. All testing is done later.
> 
> So IMHO this is a really useful feature.

I think it would be a more useful feature for
something like a git pull request rather than
a local git repository.

Re: [PATCH] checkpatch.pl: add support for checking patch from git repository

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

Hi,

On Sun, Apr 24, 2016 at 09:07:21AM -0700, Joe Perches wrote:
> On Sun, 2016-04-24 at 17:10 +0200, Sebastian Reichel wrote:
> > On Sun, Apr 24, 2016 at 04:22:45AM -0700, Joe Perches wrote:
> > > On Sun, 2016-04-24 at 18:40 +0800, changbin.du@intel.com wrote:
> > > > From: "Du, Changbin" <changbin.du@intel.com>
> > > > This patch add "-g, --git" option that tread FILE as git commits
> > > > expression. You can specify the git commit hash ID expressions,
> > > > then these commits from your git repository will be checked.
> > > Why would anyone want to use checkpatch on commits already in git?
> > It may be in some non-public development branch. Usually when I
> > write patches I open a file, change it and commit the result or even
> > interim result to have backups and other git features available as
> > soon as possible. All testing is done later.
> > 
> > So IMHO this is a really useful feature.
> 
> I think it would be a more useful feature for
> something like a git pull request rather than
> a local git repository.

There are basically two places, where one wants to check patches:

1. When one creates/modifies patches
2. When one wants to apply patches in some tree

I'm perfectly happy with checkpatch's current behaviour for
the second task. OTOH during development I would find it useful
if I can do something like "checkpatch --git HEAD~3..HEAD".

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] checkpatch.pl: add support for checking patch from git repository

[Joe Perches]

On Sun, 2016-04-24 at 19:30 +0200, Sebastian Reichel wrote:
> On Sun, Apr 24, 2016 at 09:07:21AM -0700, Joe Perches wrote:
> > On Sun, 2016-04-24 at 17:10 +0200, Sebastian Reichel wrote:
> > > On Sun, Apr 24, 2016 at 04:22:45AM -0700, Joe Perches wrote:
> > > > On Sun, 2016-04-24 at 18:40 +0800, changbin.du@intel.com wrote:
> > > > > This patch add "-g, --git" option that tread FILE as git commits
> > > > > expression. You can specify the git commit hash ID expressions,
> > > > > then these commits from your git repository will be checked.
> > > > Why would anyone want to use checkpatch on commits already in git?
> > > It may be in some non-public development branch. Usually when I
> > > write patches I open a file, change it and commit the result or even
> > > interim result to have backups and other git features available as
> > > soon as possible. All testing is done later.
> > > 
> > > So IMHO this is a really useful feature.
> > I think it would be a more useful feature for
> > something like a git pull request rather than
> > a local git repository.
> There are basically two places, where one wants to check patches:
> 
> 1. When one creates/modifies patches
> 2. When one wants to apply patches in some tree

3. when one wants to accept patches from a pull request.

> I'm perfectly happy with checkpatch's current behaviour for
> the second task. OTOH during development I would find it useful
> if I can do something like "checkpatch --git HEAD~3..HEAD".

So you can rework the patches that are already applied?
What would you do if it showed errors/defects?

Encouraging rework seems inefficient.

Re: [PATCH] checkpatch.pl: add support for checking patch from git repository

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

Hi,

On Sun, Apr 24, 2016 at 10:43:44AM -0700, Joe Perches wrote:
> On Sun, 2016-04-24 at 19:30 +0200, Sebastian Reichel wrote:
> > On Sun, Apr 24, 2016 at 09:07:21AM -0700, Joe Perches wrote:
> > > On Sun, 2016-04-24 at 17:10 +0200, Sebastian Reichel wrote:
> > > > On Sun, Apr 24, 2016 at 04:22:45AM -0700, Joe Perches wrote:
> > > > > On Sun, 2016-04-24 at 18:40 +0800, changbin.du@intel.com wrote:
> > > > > > This patch add "-g, --git" option that tread FILE as git commits
> > > > > > expression. You can specify the git commit hash ID expressions,
> > > > > > then these commits from your git repository will be checked.
> > > > > Why would anyone want to use checkpatch on commits already in git?
> > > > It may be in some non-public development branch. Usually when I
> > > > write patches I open a file, change it and commit the result or even
> > > > interim result to have backups and other git features available as
> > > > soon as possible. All testing is done later.
> > > > 
> > > > So IMHO this is a really useful feature.
> > > I think it would be a more useful feature for
> > > something like a git pull request rather than
> > > a local git repository.
> > There are basically two places, where one wants to check patches:
> > 
> > 1. When one creates/modifies patches
> > 2. When one wants to apply patches in some tree
> 
> 3. when one wants to accept patches from a pull request.
> 
> > I'm perfectly happy with checkpatch's current behaviour for
> > the second task. OTOH during development I would find it useful
> > if I can do something like "checkpatch --git HEAD~3..HEAD".
> 
> So you can rework the patches that are already applied?
> What would you do if it showed errors/defects?
> 
> Encouraging rework seems inefficient.

No, but I can do it with patches I'm currently writing. My workflow
when working on patches I write some stuff and commit it. When I
have reached a status where the result looks ok to me, I start to
prepare it for sending it to the mailinglists.

That involves squashing some patches, changing some descriptions,
running checkpatch, maybe reorder some patches. IMHO having a --git
option is not an encouragement, but makes it less annoying to run it
in this use case.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]