[Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for script files

Lukas Bulwahn lukas.bulwahn at gmail.com
Sun Jul 26 06:12:57 UTC 2020



On Sat, 25 Jul 2020, Mrinal Pandey wrote:

> In all the script files(except yaml), SPDX license identifier is expected

Why do believe a yaml file is a script file? Where did you read that?

To me, this reads like:

All fish(except zebras) live in water...

I think you can just drop that note on yaml.

> on the second line, the first line being the shebang. Sometimes, the
> diff content includes the SPDX licensing information but excludes the
> shebang when a commit is made to a script file.
> 

You can be more explicit here. Sometimes? You can exactly say when and how 
that happens.

> This can be seen in the commits like commit 37f8173dd849 ("locking/atomics:
> Flip fallbacks and instrumentation") and commit 075c8aa79d54 ("selftests:
> forwarding: tc_actions.sh: add matchall mirror test"). In these cases
> checkpatch issues a false positive warning:
> "Misplaced SPDX-License-Identifier tag - use line 1 instead".
> 
> I noticed this false positive, while running checkpatch on the set of
> commits from v5.7 to v5.8-rc1 of the kernel, on the said commits.
> This false positive exists in checkpatch since commit a8da38a9cf0e
> ("checkpatch: add test for SPDX-License-Identifier on wrong line #")
> when the corresponding rule was first added.
> 
> The existing logic looks for a shebang in the patch/file being checked
> and if a shebang is encountered it directs checkpatch to expect
> license on the second line by setting `$checklicenseline` to `2`.
>

You can drop those `quotes` around variable names and numbers.

This can be written shorter:
Currently, if checkpatch find a shebang in line 1, it expects the 
license identifier in line 2 instead of line 1.
 
> However, this approach doesn't work when we don't have a shebang in the
> patch and `$checklicensline` continues to be `1` in this case leading to
> the unnecessary warning.
> 

However, this only works when both line 1 and 2 are part of the patch.
 
> Fix this by setting `$checklicenseline` to `2` by checking if the patch
> comes from a script for the cases when the shebang doesn't appear in the
> patch.
>

You are not really fixing it. You are improving the heuristics.

So:

Reduce the false warnings for such patches on scripts, by checking file 
extension.

Alternatively, you could actually also check the file permissions of the 
file.



Improve the patch; if it looks good then, it could be suitable for the 
larger discussion on lkml.

Lukas

> Signed-off-by: Mrinal Pandey <mrinalmni at gmail.com>
> ---
>  scripts/checkpatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 4c820607540b..80dfa83ed0fb 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3167,7 +3167,7 @@ sub process {
>  
>  # check for using SPDX license tag at beginning of files
>  		if ($realline == $checklicenseline) {
> -			if ($rawline =~ /^[ \+]\s*\#\!\s*\//) {
> +			if ($rawline =~ /^[ \+]\s*\#\!\s*\// || $realfile =~ /.*\.\(py\|sh\|pl\|awk\|tc\)/) {
>  				$checklicenseline = 2;
>  			} elsif ($rawline =~ /^\+/) {
>  				my $comment = "";
> -- 
> 2.25.1
> 
> 


More information about the Linux-kernel-mentees mailing list