Re: [Qemu-devel] [PULL 44/50] scripts: let checkpatch.pl process an entire GIT branch

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Oct 04, 2017 at 04:11:07PM +0200, Paolo Bonzini wrote:
> On 04/10/2017 15:17, Alex Williamson wrote:
> > Yes, it works with the new --patch option.  I use stgit which does
> > not add a filename extension to the patch name on export, nor have I
> > ever known patch files to have any accepted standard file extension.
> > checkpatch.pl has always assumed it's operating on a patch file, as
> > implied by the name of the script, with an option to check regular
> > files.  I'd suggest maintaining behavior consistent with the script name
> > using more comprehensive heuristics than a non-standard file extension.
> > Thanks,
> 
> We are already limiting source files to only a handful of extensions, 
> perhaps we can reuse that in the DWIM mode selection code.  While at it,
> do not match "../foo" as a branch name:
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3c0a28e644..9eca0f8458 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -11,6 +11,8 @@ use warnings;
>  my $P = $0;
>  $P =~ s@.*/@@g;
>  
> +our $SrcFile    = qr{\.(?:h|c|cpp|s|S|pl|py|sh)$};
> +
>  my $V = '0.31';
>  
>  use Getopt::Long qw(:config no_auto_abbrev);
> @@ -101,30 +103,29 @@ if ($#ARGV < 0) {
>  }
>  
>  if (!defined $chk_branch && !defined $chk_patch && !defined $file) {
> -	$chk_branch = $ARGV[0] =~ /\.\./ ? 1 : 0;
> -	$chk_patch = $chk_branch ? 0 :
> -		$ARGV[0] =~ /\.patch$/ || $ARGV[0] eq "-" ? 1 : 0;
> -	$file = $chk_branch || $chk_patch ? 0 : 1;
> +	$chk_branch = $ARGV[0] =~ /.\.\./ ? 1 : 0;
> +	$file = $ARGV[0] =~ /$SrcFile/ ? 1 : 0;
> +	$chk_patch = $chk_branch || $file ? 0 : 1;
>  } elsif (!defined $chk_branch && !defined $chk_patch) {
>  	if ($file) {
>  		$chk_branch = $chk_patch = 0;
>  	} else {
> -		$chk_branch = $ARGV[0] =~ /\.\./ ? 1 : 0;
> +		$chk_branch = $ARGV[0] =~ /.\.\./ ? 1 : 0;
>  		$chk_patch = $chk_branch ? 0 : 1;
>  	}
>  } elsif (!defined $chk_branch && !defined $file) {
>  	if ($chk_patch) {
>  		$chk_branch = $file = 0;
>  	} else {
> -		$chk_branch = $ARGV[0] =~ /\.\./ ? 1 : 0;
> +		$chk_branch = $ARGV[0] =~ /.\.\./ ? 1 : 0;
>  		$file = $chk_branch ? 0 : 1;
>  	}
>  } elsif (!defined $chk_patch && !defined $file) {
>  	if ($chk_branch) {
>  		$chk_patch = $file = 0;
>  	} else {
> -		$chk_patch = $ARGV[0] =~ /\.patch$/ || $ARGV[0] eq "-" ? 1 : 0;
> -		$file = $chk_patch ? 0 : 1;
> +		$file = $ARGV[0] =~ /$SrcFile/ ? 1 : 0;
> +		$chk_patch = $file ? 0 : 1;
>  	}
>  } elsif (!defined $chk_branch) {
>  	$chk_branch = $chk_patch || $file ? 0 : 1;
> @@ -1442,7 +1443,7 @@ sub process {
>  		}
>  
>  # check we are in a valid source file if not then ignore this hunk
> -		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
> +		next if ($realfile !~ /$SrcFile/);
>  
>  #90 column limit
>  		if ($line =~ /^\+/ &&

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

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 :|