[1/3] checkpatch: tweak the files in which TABs are checked

Include Python and shell scripts, and make an exception for Perl
scripts we imported from Linux or elsewhere.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/checkpatch.pl | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

On Tue, 08/09 17:47, Paolo Bonzini wrote:
> Include Python and shell scripts, and make an exception for Perl
> scripts we imported from Linux or elsewhere.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/checkpatch.pl | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b7cb4ab..7ccf6a8 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1351,8 +1351,10 @@ sub process {
>  			WARN("adding a line without newline at end of file\n" . $herecurr);
>  		}
>  
> -# check we are in a valid source file C or perl if not then ignore this hunk
> -		next if ($realfile !~ /\.(h|c|cpp|pl)$/);
> +# check we are in a valid source file; if not then tabs are allowed.
> +# make an exception from some scripts imported from other projects.

s/from some/for some/ ?

> +		next if ($realfile !~ /\.(h|c|cpp|pl|py|sh)$/);
> +		next if ($realfile =~ /(checkpatch|get_maintainer|texi2pod)\.pl$/);
>  
>  # in QEMU, no tabs are allowed
>  		if ($rawline =~ /^\+.*\t/) {
> -- 
> 2.7.4
> 
> 
>

Paolo Bonzini <pbonzini@redhat.com> writes:

> Include Python and shell scripts, and make an exception for Perl
> scripts we imported from Linux or elsewhere.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/checkpatch.pl | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b7cb4ab..7ccf6a8 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1351,8 +1351,10 @@ sub process {
>  			WARN("adding a line without newline at end of file\n" . $herecurr);
>  		}
>  
> -# check we are in a valid source file C or perl if not then ignore this hunk
> -		next if ($realfile !~ /\.(h|c|cpp|pl)$/);
> +# check we are in a valid source file; if not then tabs are allowed.
> +# make an exception from some scripts imported from other projects.
> +		next if ($realfile !~ /\.(h|c|cpp|pl|py|sh)$/);
> +		next if ($realfile =~ /(checkpatch|get_maintainer|texi2pod)\.pl$/);
>  
>  # in QEMU, no tabs are allowed
>  		if ($rawline =~ /^\+.*\t/) {

I'm afraid "if not then tabs are allowed" is confusing.  We're obviously
skipping more than just the tabs check: RCS/CVS revision markers, and a
whole bunch of C style checks.  Makes sense, we don't want to do these
checks for imported files.  "if not then tabs are allowed" starts to
make some sense only once you've stared at the next "next if ..." line
for a while.  Let's avoid that.  Minimal change:

   # check we are in a valid source file; if not then ignore this hunk
   # make an exception from some scripts imported from other projects.

Radim's "[PATCH] checkpatch: ignore automatically imported Linux
headers" adds a similar exception for other imported files in a
different place:

  @@ -1312,6 +1312,9 @@ sub process {
   # ignore non-hunk lines and lines being removed
                  next if (!$hunk_line || $line =~ /^-/);

  +# ignore files that are being periodically imported from Linux
  +		next if ($realfile =~ /^(linux-headers|include\/standard-headers)\//);
  +
   #trailing whitespace
                  if ($line =~ /^\+.*\015/) {
                          my $herevet = "$here\n" . cat_vet($rawline) . "\n";

Should both exceptions be in the same place?

On Wed, 10 Aug 2016 08:46:07 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> I'm afraid "if not then tabs are allowed" is confusing.  We're obviously
> skipping more than just the tabs check: RCS/CVS revision markers, and a
> whole bunch of C style checks.  Makes sense, we don't want to do these
> checks for imported files.  "if not then tabs are allowed" starts to
> make some sense only once you've stared at the next "next if ..." line
> for a while.  Let's avoid that.  Minimal change:
> 
>    # check we are in a valid source file; if not then ignore this hunk
>    # make an exception from some scripts imported from other projects.
> 
> Radim's "[PATCH] checkpatch: ignore automatically imported Linux
> headers" adds a similar exception for other imported files in a
> different place:
> 
>   @@ -1312,6 +1312,9 @@ sub process {
>    # ignore non-hunk lines and lines being removed
>                   next if (!$hunk_line || $line =~ /^-/);
> 
>   +# ignore files that are being periodically imported from Linux
>   +		next if ($realfile =~ /^(linux-headers|include\/standard-headers)\//);
>   +
>    #trailing whitespace
>                   if ($line =~ /^\+.*\015/) {
>                           my $herevet = "$here\n" . cat_vet($rawline) . "\n";
> 
> Should both exceptions be in the same place?

There are two cases of 'imported file':
(a) Things like the headers update where we want to copy whatever we
got from the kernel: If coding style is already messed up there, we
still want to copy.
(b) Things like checkpatch.pl which we tweak ourselves: We want to keep
close to the existing coding style, but don't want to mess up things
further.

I think in the long run it would make sense to skip any checks for case
(a) but still keep a subset of checks (like trailing whitespace) for
(b). For the sake of silencing the checkpatch bot, just skipping (a)
and (b) makes sense for now.