[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