Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for script files
From: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Date: 2020-07-26 06:13:11
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
quoted hunk ↗ jump to hunk
Signed-off-by: Mrinal Pandey <redacted> --- 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
_______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees