* [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts
@ 2020-07-13 9:57 Mrinal Pandey
2020-07-13 19:46 ` Lukas Bulwahn
0 siblings, 1 reply; 14+ messages in thread
From: Mrinal Pandey @ 2020-07-13 9:57 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: Linux-kernel-mentees
[-- Attachment #1.1: Type: text/plain, Size: 1776 bytes --]
In all the scripts, the SPDX license should be on the second line,
the first line being the "sh-bang", but checkpatch issues a warning
"Misplaced SPDX-License-Identifier tag - use line 1 instead" for the
scripts that have SPDX license in the second line.
However, this warning is not issued when checkpatch is run on a file using
`-f` option. The case for files has been handled gracefully by changing
`$checklicenseline` to `2` but a corresponding check when running checkpatch
on a commit hash is missing.
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 commits which modified
a script file.
This check is missing in checkpatch since commit a8da38a9cf0e
("checkpatch: add test for SPDX-License-Identifier on wrong line #")
when the corresponding rule was first commited.
Fix this by setting `$checklicenseline` to `2` when the diff content that
is being checked originates from a script, thus, informing checkpatch that
the SPDX license should be on the second line.
Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com>
---
scripts/checkpatch.pl | 3 +++
1 file changed, 3 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4c820607540b..bbffd0c4449d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3218,6 +3218,9 @@ sub process {
next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/);
# check for using SPDX-License-Identifier on the wrong line number
+ if ($realfile =~ /^scripts/) {
+ $checklicenseline = 2;
+ }
if ($realline != $checklicenseline &&
$rawline =~ /\bSPDX-License-Identifier:/ &&
substr($line, @-, @+ - @-) eq "$;" x (@+ - @-)) {
--
2.25.1
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 201 bytes --]
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-13 9:57 [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts Mrinal Pandey @ 2020-07-13 19:46 ` Lukas Bulwahn 2020-07-14 5:35 ` Mrinal Pandey 0 siblings, 1 reply; 14+ messages in thread From: Lukas Bulwahn @ 2020-07-13 19:46 UTC (permalink / raw) To: Mrinal Pandey; +Cc: Linux-kernel-mentees On Mon, 13 Jul 2020, Mrinal Pandey wrote: > In all the scripts, the SPDX license should be on the second line, > the first line being the "sh-bang", but checkpatch issues a warning > "Misplaced SPDX-License-Identifier tag - use line 1 instead" for the > scripts that have SPDX license in the second line. > > However, this warning is not issued when checkpatch is run on a file using > `-f` option. The case for files has been handled gracefully by changing > `$checklicenseline` to `2` but a corresponding check when running checkpatch > on a commit hash is missing. > > 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 commits which modified > a script file. > > This check is missing in checkpatch since commit a8da38a9cf0e > ("checkpatch: add test for SPDX-License-Identifier on wrong line #") > when the corresponding rule was first commited. > > Fix this by setting `$checklicenseline` to `2` when the diff content that > is being checked originates from a script, thus, informing checkpatch that > the SPDX license should be on the second line. > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> > --- > scripts/checkpatch.pl | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 4c820607540b..bbffd0c4449d 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -3218,6 +3218,9 @@ sub process { > next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); > > # check for using SPDX-License-Identifier on the wrong line number > + if ($realfile =~ /^scripts/) { > + $checklicenseline = 2; > + } I think this is somehow wrong here. The check for checklicenseline = 2 looks very different above. Why does -f work and using a patch file not work? Lukas > if ($realline != $checklicenseline && > $rawline =~ /\bSPDX-License-Identifier:/ && > substr($line, @-, @+ - @-) eq "$;" x (@+ - @-)) { > -- > 2.25.1 > > _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-13 19:46 ` Lukas Bulwahn @ 2020-07-14 5:35 ` Mrinal Pandey 2020-07-14 6:03 ` Lukas Bulwahn 0 siblings, 1 reply; 14+ messages in thread From: Mrinal Pandey @ 2020-07-14 5:35 UTC (permalink / raw) To: Lukas Bulwahn; +Cc: Linux-kernel-mentees [-- Attachment #1.1: Type: text/plain, Size: 3910 bytes --] On Tue, Jul 14, 2020 at 1:16 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > On Mon, 13 Jul 2020, Mrinal Pandey wrote: > > > In all the scripts, the SPDX license should be on the second line, > > the first line being the "sh-bang", but checkpatch issues a warning > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" for the > > scripts that have SPDX license in the second line. > > > > However, this warning is not issued when checkpatch is run on a file > using > > `-f` option. The case for files has been handled gracefully by changing > > `$checklicenseline` to `2` but a corresponding check when running > checkpatch > > on a commit hash is missing. > > > > 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 commits which modified > > a script file. > > > > This check is missing in checkpatch since commit a8da38a9cf0e > > ("checkpatch: add test for SPDX-License-Identifier on wrong line #") > > when the corresponding rule was first commited. > > > > Fix this by setting `$checklicenseline` to `2` when the diff content that > > is being checked originates from a script, thus, informing checkpatch > that > > the SPDX license should be on the second line. > > > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> > > --- > > scripts/checkpatch.pl | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 4c820607540b..bbffd0c4449d 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -3218,6 +3218,9 @@ sub process { > > next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); > > > > # check for using SPDX-License-Identifier on the wrong line number > > + if ($realfile =~ /^scripts/) { > > + $checklicenseline = 2; > > + } > > I think this is somehow wrong here. The check for checklicenseline = 2 > looks very different above. > > Why does -f work and using a patch file not work? > Sir, I am going to explain my observation based on file `scripts/atomic/gen-atomic-fallback.sh` and commit hash `37f8173dd849`. If we are checking against the file, `checklicenseline` is set to 1 and when `realline` is 1 the above `if` block is triggered, then we check if this line is of the form `#!/` using the regular expression `^[ \+]\s*\#\!\s*\/`. If this is the case we set `checklicenseline` to `2` informing checkpatch that it should expect license on the second line and this works all fine for a file. The `if` block below my proposed changes evaluates to false in this case and thus it emits no false warning. However, If we are checking a diff content, the above `if` block is not triggered at all. This is because `realline` stores the actual line number of the line we are checking currently out of diff content. This value is 2 because SPDX identifier is indeed at the second line in the file but `checklicenseline` is still `1`. `realline` will never become equal to 1 again and thus the above `if` condition will never be true in this case. Even if the above `if` block is triggered it would not update `checklicenseline` to 2 as the regular expression is not satisfied since we don't have sh-bang in diff content and just the SPDX tag. If we don't do this, the `if` block below evaluates to true when `realline` is 2 and `checklicensline` is `1` leading to the emission of a false warning. So, what I did was to check if the diff content we are checking actually comes from a script, if yes, we can set `checklicenseline` to `2` to avoid this confusion. Please let me know if this is reasonable. Thank you. > > Lukas > > > > if ($realline != $checklicenseline && > > $rawline =~ /\bSPDX-License-Identifier:/ && > > substr($line, @-, @+ - @-) eq "$;" x (@+ - @-)) { > > -- > > 2.25.1 > > > > > [-- Attachment #1.2: Type: text/html, Size: 5533 bytes --] [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-14 5:35 ` Mrinal Pandey @ 2020-07-14 6:03 ` Lukas Bulwahn 2020-07-16 5:15 ` Mrinal Pandey 0 siblings, 1 reply; 14+ messages in thread From: Lukas Bulwahn @ 2020-07-14 6:03 UTC (permalink / raw) To: Mrinal Pandey; +Cc: Linux-kernel-mentees [-- Attachment #1: Type: text/plain, Size: 5153 bytes --] On Tue, 14 Jul 2020, Mrinal Pandey wrote: > > > On Tue, Jul 14, 2020 at 1:16 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > On Mon, 13 Jul 2020, Mrinal Pandey wrote: > > > In all the scripts, the SPDX license should be on the second line, > > the first line being the "sh-bang", but checkpatch issues a warning > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" for the > > scripts that have SPDX license in the second line. > > > > However, this warning is not issued when checkpatch is run on a file using > > `-f` option. The case for files has been handled gracefully by changing > > `$checklicenseline` to `2` but a corresponding check when running checkpatch > > on a commit hash is missing. > > > > 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 commits which modified > > a script file. > > > > This check is missing in checkpatch since commit a8da38a9cf0e > > ("checkpatch: add test for SPDX-License-Identifier on wrong line #") > > when the corresponding rule was first commited. > > > > Fix this by setting `$checklicenseline` to `2` when the diff content that > > is being checked originates from a script, thus, informing checkpatch that > > the SPDX license should be on the second line. > > > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> > > --- > > scripts/checkpatch.pl | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 4c820607540b..bbffd0c4449d 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -3218,6 +3218,9 @@ sub process { > > next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); > > > > # check for using SPDX-License-Identifier on the wrong line number > > + if ($realfile =~ /^scripts/) { > > + $checklicenseline = 2; > > + } > > I think this is somehow wrong here. The check for checklicenseline = 2 > looks very different above. > > Why does -f work and using a patch file not work? > > > Sir, > > I am going to explain my observation based on file `scripts/atomic/gen-atomic-fallback.sh` and > commit hash `37f8173dd849`. > > If we are checking against the file, `checklicenseline` is set to 1 and when `realline` is 1 the above > `if` block is triggered, then we check if this line is of the form `#!/` using the regular expression > `^[ \+]\s*\#\!\s*\/`. If this is the case we set `checklicenseline` to `2` informing checkpatch that it should > expect license on the second line and this works all fine for a file. > The `if` block below my proposed changes evaluates to false in this case and thus it emits no false warning. > > However, If we are checking a diff content, the above `if` block is not triggered at all. This is > because `realline` stores the actual line number of the line we are checking currently out of diff content. > This value is 2 because SPDX identifier is indeed at the second line in the file but `checklicenseline` is still > `1`. > `realline` will never become equal to 1 again and thus the above `if` condition will never be true in this case. > Even if the above `if` block is triggered it would not update `checklicenseline` to 2 as the regular expression > is not satisfied since we don't have sh-bang in diff content and just the SPDX tag. > If we don't do this, the `if` block below evaluates to true when `realline` is 2 and `checklicensline` is `1` > leading > to the emission of a false warning. > So, maybe this whole logic needs to be reworked. If you do not know the first line, you need to have a different criteria in the first place to determine if you expect the license tag in the first or the second, e.g., the file extension, and then checking line 1 for a shebang is just sanity checking. If it is of a specific file extension, you know line 1 and it is not a shebang, that is probably worth noting as a different recommendation in checkpatch.pl anyway. Lukas > So, what I did was to check if the diff content we are checking actually comes from a script, if yes, we can set > `checklicenseline` to `2` to avoid this confusion. > Why would you think that scripts are only in scripts? How about first listing all files where the SPDX tag is in line 2 in the current repository, e.g., v5.8-rc5? Then, we look at that list and determine a suitable criteria for looking in line 2 for the SPDX tag. Lukas > Please let me know if this is reasonable. > > Thank you. > > > Lukas > > > > if ($realline != $checklicenseline && > > $rawline =~ /\bSPDX-License-Identifier:/ && > > substr($line, @-, @+ - @-) eq "$;" x (@+ - @-)) { > > -- > > 2.25.1 > > > > > > > [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-14 6:03 ` Lukas Bulwahn @ 2020-07-16 5:15 ` Mrinal Pandey 2020-07-16 5:31 ` Lukas Bulwahn 0 siblings, 1 reply; 14+ messages in thread From: Mrinal Pandey @ 2020-07-16 5:15 UTC (permalink / raw) To: Lukas Bulwahn, Shuah Khan, Linux-kernel-mentees [-- Attachment #1.1: Type: text/plain, Size: 6503 bytes --] On Tue, Jul 14, 2020 at 11:33 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > On Tue, 14 Jul 2020, Mrinal Pandey wrote: > > > > > > > On Tue, Jul 14, 2020 at 1:16 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> > wrote: > > > > > > On Mon, 13 Jul 2020, Mrinal Pandey wrote: > > > > > In all the scripts, the SPDX license should be on the second > line, > > > the first line being the "sh-bang", but checkpatch issues a > warning > > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" for > the > > > scripts that have SPDX license in the second line. > > > > > > However, this warning is not issued when checkpatch is run on a > file using > > > `-f` option. The case for files has been handled gracefully by > changing > > > `$checklicenseline` to `2` but a corresponding check when > running checkpatch > > > on a commit hash is missing. > > > > > > 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 commits which > modified > > > a script file. > > > > > > This check is missing in checkpatch since commit a8da38a9cf0e > > > ("checkpatch: add test for SPDX-License-Identifier on wrong line > #") > > > when the corresponding rule was first commited. > > > > > > Fix this by setting `$checklicenseline` to `2` when the diff > content that > > > is being checked originates from a script, thus, informing > checkpatch that > > > the SPDX license should be on the second line. > > > > > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> > > > --- > > > scripts/checkpatch.pl | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > > index 4c820607540b..bbffd0c4449d 100755 > > > --- a/scripts/checkpatch.pl > > > +++ b/scripts/checkpatch.pl > > > @@ -3218,6 +3218,9 @@ sub process { > > > next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); > > > > > > # check for using SPDX-License-Identifier on the wrong line > number > > > + if ($realfile =~ /^scripts/) { > > > + $checklicenseline = 2; > > > + } > > > > I think this is somehow wrong here. The check for checklicenseline > = 2 > > looks very different above. > > > > Why does -f work and using a patch file not work? > > > > > > Sir, > > > > I am going to explain my observation based on file > `scripts/atomic/gen-atomic-fallback.sh` and > > commit hash `37f8173dd849`. > > > > If we are checking against the file, `checklicenseline` is set to 1 and > when `realline` is 1 the above > > `if` block is triggered, then we check if this line is of the form `#!/` > using the regular expression > > `^[ \+]\s*\#\!\s*\/`. If this is the case we set `checklicenseline` to > `2` informing checkpatch that it should > > expect license on the second line and this works all fine for a file. > > The `if` block below my proposed changes evaluates to false in this case > and thus it emits no false warning. > > > > However, If we are checking a diff content, the above `if` block is not > triggered at all. This is > > because `realline` stores the actual line number of the line we are > checking currently out of diff content. > > This value is 2 because SPDX identifier is indeed at the second line in > the file but `checklicenseline` is still > > `1`. > > `realline` will never become equal to 1 again and thus the above `if` > condition will never be true in this case. > > Even if the above `if` block is triggered it would not update > `checklicenseline` to 2 as the regular expression > > is not satisfied since we don't have sh-bang in diff content and just > the SPDX tag. > > If we don't do this, the `if` block below evaluates to true when > `realline` is 2 and `checklicensline` is `1` > > leading > > to the emission of a false warning. > > > > So, maybe this whole logic needs to be reworked. If you do not know the > first line, you need to have a different criteria in the first place > to determine if you expect the license tag in the first or the second, > e.g., the file extension, and then checking line 1 for a shebang is just > sanity checking. If it is of a specific file extension, you know line 1 > and it is not a shebang, that is probably worth noting as a different > recommendation in checkpatch.pl anyway. > Sir, When we know the first line, i.e. we are running checkpatch against a file, the existing logic works fine. We probably don't want to induce any changes there. But when we don't know the first line, if am not wrong, it would go somewhat like: if (the file is a script) { if (the first line is shebang) { if (the second line is SPDX) { All good } else { Issue a misplaced or missing SPDX tag warning } } else { Issue a missing shebang warning } } else { if (the first line is SPDX) { All good } else { Issue a misplaced or missing SPDX tag warning } } Lukas > > > > So, what I did was to check if the diff content we are checking actually > comes from a script, if yes, we can set > > `checklicenseline` to `2` to avoid this confusion. > > > > Why would you think that scripts are only in scripts? > > How about first listing all files where the SPDX tag is in line 2 in the > current repository, e.g., v5.8-rc5? > > Then, we look at that list and determine a suitable criteria for looking > in line 2 for the SPDX tag. > Yes, the scripts are not only in scripts. I have listed all the files where the SPDX tag should be on the second line. I've attached the list for reference. We should probably be checking the file extension to determine if the tag needs to be on the second line or not. The documentation says the SPDX tag should be present in all source files. Do these source files include Documentation files too? Thank you. > > Lukas > > > Please let me know if this is reasonable. > > > > Thank you. > > > > > > Lukas > > > > > > > if ($realline != $checklicenseline && > > > $rawline =~ /\bSPDX-License-Identifier:/ && > > > substr($line, @-, @+ - @-) eq "$;" x (@+ - > @-)) { > > > -- > > > 2.25.1 > > > > > > > > > > > > [-- Attachment #1.2: Type: text/html, Size: 9209 bytes --] [-- Attachment #2: scripts --] [-- Type: application/octet-stream, Size: 31498 bytes --] ./Documentation/networking/cxacru-cf.py ./Documentation/features/scripts/features-refresh.sh ./Documentation/features/list-arch.sh ./Documentation/trace/postprocess/trace-pagealloc-postprocess.pl ./Documentation/trace/postprocess/trace-vmscan-postprocess.pl ./Documentation/trace/postprocess/decode_msr.py ./Documentation/arm64/kasan-offsets.sh ./Documentation/s390/config3270.sh ./Documentation/admin-guide/aoe/status.sh ./Documentation/admin-guide/aoe/udev-install.sh ./Documentation/admin-guide/aoe/autoload.sh ./Documentation/admin-guide/cifs/winucase_convert.pl ./Documentation/sphinx/kerneldoc.py ./Documentation/sphinx/parse-headers.pl ./Documentation/sphinx/rstFlatTable.py ./Documentation/sphinx/kernellog.py ./Documentation/sphinx/load_config.py ./Documentation/sphinx/kfigure.py ./Documentation/sphinx/cdomain.py ./Documentation/sphinx/maintainers_include.py ./Documentation/sphinx/kernel_include.py ./Documentation/sphinx/automarkup.py ./Documentation/sphinx/parallel-wrapper.sh ./Documentation/sound/cards/multisound.sh ./Documentation/conf.py ./Documentation/target/tcm_mod_builder.py ./Documentation/userspace-api/media/conf_nitpick.py ./vmlinux-gdb.py ./kernel/gen_kheaders.sh ./scripts/checkpatch.pl ./scripts/kconfig/tests/err_recursive_dep/__init__.py ./scripts/kconfig/tests/err_recursive_inc/__init__.py ./scripts/kconfig/tests/choice_value_with_m_dep/__init__.py ./scripts/kconfig/tests/choice/__init__.py ./scripts/kconfig/tests/auto_submenu/__init__.py ./scripts/kconfig/tests/conftest.py ./scripts/kconfig/tests/new_choice_with_dep/__init__.py ./scripts/kconfig/tests/preprocess/variable/__init__.py ./scripts/kconfig/tests/preprocess/escape/__init__.py ./scripts/kconfig/tests/preprocess/builtin_func/__init__.py ./scripts/kconfig/tests/preprocess/circular_expansion/__init__.py ./scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py ./scripts/kconfig/tests/inter_choice/__init__.py ./scripts/kconfig/qconf-cfg.sh ./scripts/kconfig/gconf-cfg.sh ./scripts/kconfig/streamline_config.pl ./scripts/kconfig/nconf-cfg.sh ./scripts/kconfig/merge_config.sh ./scripts/kconfig/mconf-cfg.sh ./scripts/gcc-version.sh ./scripts/gcc-plugin.sh ./scripts/gcc-goto.sh ./scripts/gcc-x86_32-has-stack-protector.sh ./scripts/extract-module-sig.pl ./scripts/tools-support-relr.sh ./scripts/get_abi.pl ./scripts/clang-version.sh ./scripts/extract-sys-certs.pl ./scripts/dtc/update-dtc-source.sh ./scripts/tracing/draw_functrace.py ./scripts/tracing/ftrace-bisect.sh ./scripts/recordmcount.pl ./scripts/headerdep.pl ./scripts/split-man.pl ./scripts/selinux/install_policy.sh ./scripts/checkincludes.pl ./scripts/link-vmlinux.sh ./scripts/checkversion.pl ./scripts/mkuboot.sh ./scripts/headers_check.pl ./scripts/get_maintainer.pl ./scripts/atomic/atomic-tbl.sh ./scripts/atomic/gen-atomic-fallback.sh ./scripts/atomic/check-atomics.sh ./scripts/atomic/gen-atomic-long.sh ./scripts/atomic/gen-atomics.sh ./scripts/atomic/gen-atomic-instrumented.sh ./scripts/gcc-plugins/gen-random-seed.sh ./scripts/find-unused-docs.sh ./scripts/depmod.sh ./scripts/gen_compile_commands.py ./scripts/tags.sh ./scripts/cc-can-link.sh ./scripts/bootgraph.pl ./scripts/gdb/vmlinux-gdb.py ./scripts/gdb/linux/rbtree.py ./scripts/gdb/linux/timerlist.py ./scripts/gdb/linux/utils.py ./scripts/gdb/linux/cpus.py ./scripts/gdb/linux/tasks.py ./scripts/gdb/linux/constants.py ./scripts/gdb/linux/device.py ./scripts/gdb/linux/__init__.py ./scripts/gdb/linux/proc.py ./scripts/gdb/linux/lists.py ./scripts/gdb/linux/modules.py ./scripts/gdb/linux/dmesg.py ./scripts/gdb/linux/clk.py ./scripts/gdb/linux/symbols.py ./scripts/gdb/linux/genpd.py ./scripts/gdb/linux/config.py ./scripts/xz_wrap.sh ./scripts/gen_autoksyms.sh ./scripts/check_extable.sh ./scripts/markup_oops.pl ./scripts/gcc-x86_64-has-stack-protector.sh ./scripts/headers_install.sh ./scripts/extract_xc3028.pl ./scripts/gen_ksymdeps.sh ./scripts/adjust_autoksyms.sh ./scripts/spdxcheck-test.sh ./scripts/parse-maintainers.pl ./scripts/leaking_addresses.pl ./scripts/modules-check.sh ./scripts/xen-hypercalls.sh ./scripts/bpf_helpers_doc.py ./scripts/file-size.sh ./scripts/checkkconfigsymbols.py ./scripts/spdxcheck.py ./scripts/ld-version.sh ./scripts/checkstack.pl ./scripts/export_report.pl ./scripts/decode_stacktrace.sh ./scripts/checksyscalls.sh ./scripts/namespace.pl ./scripts/profile2linkerlist.pl ./drivers/scsi/script_asm.pl ./drivers/usb/serial/ezusb_convert.pl ./drivers/staging/comedi/drivers/ni_routing/tools/csv_collection.py ./drivers/staging/comedi/drivers/ni_routing/tools/ni_names.py ./drivers/staging/comedi/drivers/ni_routing/tools/make_blank_csv.py ./drivers/staging/comedi/drivers/ni_routing/tools/convert_py_to_csv.py ./drivers/staging/comedi/drivers/ni_routing/tools/convert_csv_to_c.py ./drivers/crypto/vmx/ppc-xlate.pl ./drivers/crypto/vmx/ghashp8-ppc.pl ./drivers/crypto/vmx/aesp8-ppc.pl ./tools/bootconfig/test-bootconfig.sh ./tools/objtool/sync-check.sh ./tools/memory-model/scripts/cmplitmushist.sh ./tools/memory-model/scripts/checkalllitmus.sh ./tools/memory-model/scripts/checkghlitmus.sh ./tools/memory-model/scripts/runlitmushist.sh ./tools/memory-model/scripts/checklitmus.sh ./tools/memory-model/scripts/parseargs.sh ./tools/memory-model/scripts/judgelitmus.sh ./tools/memory-model/scripts/initlitmushist.sh ./tools/memory-model/scripts/checklitmushist.sh ./tools/memory-model/scripts/newlitmushist.sh ./tools/perf/python/tracepoint.py ./tools/perf/python/twatch.py ./tools/perf/tests/attr.py ./tools/perf/tests/shell/record+probe_libc_inet_pton.sh ./tools/perf/tests/shell/trace+probe_vfs_getname.sh ./tools/perf/tests/shell/probe_vfs_getname.sh ./tools/perf/tests/shell/record+script_probe_vfs_getname.sh ./tools/perf/tests/shell/record+zstd_comp_decomp.sh ./tools/perf/tests/shell/lib/probe_vfs_getname.sh ./tools/perf/tests/shell/lib/probe.sh ./tools/perf/scripts/python/check-perf-trace.py ./tools/perf/scripts/python/mem-phys-addr.py ./tools/perf/scripts/python/syscall-counts-by-pid.py ./tools/perf/scripts/python/stat-cpi.py ./tools/perf/scripts/python/powerpc-hcalls.py ./tools/perf/scripts/python/net_dropmonitor.py ./tools/perf/scripts/python/flamegraph.py ./tools/perf/scripts/python/sctop.py ./tools/perf/scripts/python/exported-sql-viewer.py ./tools/perf/scripts/python/event_analyzing_sample.py ./tools/perf/scripts/python/sched-migration.py ./tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/EventClass.py ./tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py ./tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/SchedGui.py ./tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Core.py ./tools/perf/scripts/python/syscall-counts.py ./tools/perf/scripts/python/export-to-sqlite.py ./tools/perf/scripts/python/failed-syscalls-by-pid.py ./tools/perf/scripts/python/export-to-postgresql.py ./tools/perf/scripts/python/netdev-times.py ./tools/perf/scripts/python/compaction-times.py ./tools/perf/scripts/python/stackcollapse.py ./tools/perf/scripts/python/futex-contention.py ./tools/perf/scripts/python/intel-pt-events.py ./tools/perf/scripts/perl/check-perf-trace.pl ./tools/perf/scripts/perl/wakeup-latency.pl ./tools/perf/scripts/perl/rw-by-pid.pl ./tools/perf/scripts/perl/failed-syscalls.pl ./tools/perf/scripts/perl/rw-by-file.pl ./tools/perf/scripts/perl/rwtop.pl ./tools/perf/trace/beauty/fadvise.sh ./tools/perf/trace/beauty/fsmount.sh ./tools/perf/trace/beauty/mmap_flags.sh ./tools/perf/trace/beauty/x86_arch_prctl.sh ./tools/perf/trace/beauty/usbdevfs_ioctl.sh ./tools/perf/trace/beauty/sync_file_range.sh ./tools/perf/trace/beauty/rename_flags.sh ./tools/perf/trace/beauty/kvm_ioctl.sh ./tools/perf/trace/beauty/tracepoints/x86_irq_vectors.sh ./tools/perf/trace/beauty/tracepoints/x86_msr.sh ./tools/perf/trace/beauty/fspick.sh ./tools/perf/trace/beauty/madvise_behavior.sh ./tools/perf/trace/beauty/drm_ioctl.sh ./tools/perf/trace/beauty/prctl_option.sh ./tools/perf/trace/beauty/mount_flags.sh ./tools/perf/trace/beauty/socket_ipproto.sh ./tools/perf/trace/beauty/arch_errno_names.sh ./tools/perf/trace/beauty/perf_ioctl.sh ./tools/perf/trace/beauty/fsconfig.sh ./tools/perf/trace/beauty/sndrv_pcm_ioctl.sh ./tools/perf/trace/beauty/sndrv_ctl_ioctl.sh ./tools/perf/trace/beauty/vhost_virtio_ioctl.sh ./tools/perf/trace/beauty/pkey_alloc_access_rights.sh ./tools/perf/trace/beauty/kcmp_type.sh ./tools/perf/trace/beauty/move_mount_flags.sh ./tools/perf/perf-with-kcore.sh ./tools/perf/util/setup.py ./tools/perf/util/generate-cmdlist.sh ./tools/perf/perf-completion.sh ./tools/perf/arch/x86/tests/gen-insn-x86-dat.sh ./tools/perf/arch/x86/entry/syscalls/syscalltbl.sh ./tools/perf/check-headers.sh ./tools/perf/perf-archive.sh ./tools/nfsd/inject_fault.sh ./tools/build/tests/run.sh ./tools/testing/fault-injection/failcmd.sh ./tools/testing/kunit/kunit_config.py ./tools/testing/kunit/kunit_kernel.py ./tools/testing/kunit/kunit_parser.py ./tools/testing/kunit/kunit_tool_test.py ./tools/testing/kunit/kunit.py ./tools/testing/selftests/firmware/fw_lib.sh ./tools/testing/selftests/firmware/fw_fallback.sh ./tools/testing/selftests/firmware/fw_filesystem.sh ./tools/testing/selftests/firmware/fw_run_tests.sh ./tools/testing/selftests/powerpc/eeh/eeh-functions.sh ./tools/testing/selftests/powerpc/eeh/eeh-basic.sh ./tools/testing/selftests/powerpc/scripts/hmi.sh ./tools/testing/selftests/powerpc/nx-gzip/nx-gzip-test.sh ./tools/testing/selftests/android/run.sh ./tools/testing/selftests/android/ion/ion_test.sh ./tools/testing/selftests/netfilter/ipvs.sh ./tools/testing/selftests/netfilter/nft_nat.sh ./tools/testing/selftests/netfilter/nft_conntrack_helper.sh ./tools/testing/selftests/netfilter/nft_queue.sh ./tools/testing/selftests/netfilter/conntrack_icmp_related.sh ./tools/testing/selftests/netfilter/bridge_brouter.sh ./tools/testing/selftests/netfilter/nft_concat_range.sh ./tools/testing/selftests/netfilter/nft_flowtable.sh ./tools/testing/selftests/netfilter/nft_trans_stress.sh ./tools/testing/selftests/memfd/run_fuse_test.sh ./tools/testing/selftests/memfd/run_hugetlbfs_test.sh ./tools/testing/selftests/gen_kselftest_tar.sh ./tools/testing/selftests/rcutorture/bin/kvm-recheck.sh ./tools/testing/selftests/rcutorture/bin/kcsan-collapse.sh ./tools/testing/selftests/rcutorture/bin/configinit.sh ./tools/testing/selftests/rcutorture/bin/kvm-test-1-run.sh ./tools/testing/selftests/rcutorture/bin/parse-console.sh ./tools/testing/selftests/rcutorture/bin/mkinitrd.sh ./tools/testing/selftests/rcutorture/bin/cpus2use.sh ./tools/testing/selftests/rcutorture/bin/jitter.sh ./tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf.sh ./tools/testing/selftests/rcutorture/bin/configcheck.sh ./tools/testing/selftests/rcutorture/bin/configNR_CPUS.sh ./tools/testing/selftests/rcutorture/bin/functions.sh ./tools/testing/selftests/rcutorture/bin/kvm-recheck-lock.sh ./tools/testing/selftests/rcutorture/bin/kvm.sh ./tools/testing/selftests/rcutorture/bin/kvm-find-errors.sh ./tools/testing/selftests/rcutorture/bin/parse-build.sh ./tools/testing/selftests/rcutorture/bin/kvm-recheck-rcu.sh ./tools/testing/selftests/rcutorture/bin/kvm-recheck-rcuperf-ftrace.sh ./tools/testing/selftests/rcutorture/bin/kvm-build.sh ./tools/testing/selftests/rcutorture/bin/config_override.sh ./tools/testing/selftests/rcutorture/formal/srcu-cbmc/tests/test_script.sh ./tools/testing/selftests/rcutorture/configs/rcu/ver_functions.sh ./tools/testing/selftests/rcutorture/configs/rcuperf/ver_functions.sh ./tools/testing/selftests/rcutorture/configs/lock/ver_functions.sh ./tools/testing/selftests/kselftest/prefix.pl ./tools/testing/selftests/kselftest/runner.sh ./tools/testing/selftests/kselftest/module.sh ./tools/testing/selftests/bpf/test_tcp_check_syncookie.sh ./tools/testing/selftests/bpf/test_skb_cgroup_id.sh ./tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh ./tools/testing/selftests/bpf/benchs/run_bench_rename.sh ./tools/testing/selftests/bpf/benchs/run_bench_trigger.sh ./tools/testing/selftests/bpf/with_addr.sh ./tools/testing/selftests/bpf/test_bpftool.sh ./tools/testing/selftests/bpf/test_xdping.sh ./tools/testing/selftests/bpf/test_tunnel.sh ./tools/testing/selftests/bpf/test_xdp_redirect.sh ./tools/testing/selftests/bpf/test_xdp_vlan.sh ./tools/testing/selftests/bpf/test_xdp_meta.sh ./tools/testing/selftests/bpf/test_bpftool_build.sh ./tools/testing/selftests/bpf/test_lirc_mode2.sh ./tools/testing/selftests/bpf/test_xdp_vlan_mode_native.sh ./tools/testing/selftests/bpf/test_flow_dissector.sh ./tools/testing/selftests/bpf/test_lwt_seg6local.sh ./tools/testing/selftests/bpf/tcp_server.py ./tools/testing/selftests/bpf/test_kmod.sh ./tools/testing/selftests/bpf/test_ftrace.sh ./tools/testing/selftests/bpf/with_tunnels.sh ./tools/testing/selftests/bpf/test_bpftool.py ./tools/testing/selftests/bpf/test_tc_tunnel.sh ./tools/testing/selftests/bpf/test_sock_addr.sh ./tools/testing/selftests/bpf/test_xdp_vlan_mode_generic.sh ./tools/testing/selftests/bpf/test_offload.py ./tools/testing/selftests/bpf/test_tc_edt.sh ./tools/testing/selftests/bpf/tcp_client.py ./tools/testing/selftests/bpf/test_xdp_veth.sh ./tools/testing/selftests/bpf/test_lwt_ip_encap.sh ./tools/testing/selftests/tc-testing/TdcPlugin.py ./tools/testing/selftests/tc-testing/tdc.py ./tools/testing/selftests/tc-testing/TdcResults.py ./tools/testing/selftests/tc-testing/tdc_config_local_template.py ./tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py ./tools/testing/selftests/tc-testing/plugin-lib/valgrindPlugin.py ./tools/testing/selftests/tc-testing/plugin-lib/rootPlugin.py ./tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py ./tools/testing/selftests/tc-testing/plugin-lib/buildebpfPlugin.py ./tools/testing/selftests/tc-testing/tdc_helper.py ./tools/testing/selftests/tc-testing/tdc_multibatch.py ./tools/testing/selftests/tc-testing/plugins/__init__.py ./tools/testing/selftests/tc-testing/tdc_config.py ./tools/testing/selftests/tc-testing/tdc_batch.py ./tools/testing/selftests/ntb/ntb_test.sh ./tools/testing/selftests/static_keys/test_static_keys.sh ./tools/testing/selftests/ptp/phc.sh ./tools/testing/selftests/drivers/gpu/drm_mm.sh ./tools/testing/selftests/drivers/gpu/i915.sh ./tools/testing/selftests/drivers/usb/usbip/usbip_test.sh ./tools/testing/selftests/drivers/net/netdevsim/fib.sh ./tools/testing/selftests/drivers/net/netdevsim/devlink.sh ./tools/testing/selftests/drivers/net/netdevsim/devlink_in_netns.sh ./tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh ./tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh ./tools/testing/selftests/drivers/net/mlxsw/sch_tbf_ets.sh ./tools/testing/selftests/drivers/net/mlxsw/tc_action_hw_stats.sh ./tools/testing/selftests/drivers/net/mlxsw/fib.sh ./tools/testing/selftests/drivers/net/mlxsw/vxlan.sh ./tools/testing/selftests/drivers/net/mlxsw/tc_flower_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/sharedbuffer.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l3_exceptions.sh ./tools/testing/selftests/drivers/net/mlxsw/vxlan_fdb_veto.sh ./tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum-2/mirror_gre_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_flower.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum-2/router_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/fib_offload.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap_control.sh ./tools/testing/selftests/drivers/net/mlxsw/one_armed_router.sh ./tools/testing/selftests/drivers/net/mlxsw/sharedbuffer_configuration.py ./tools/testing/selftests/drivers/net/mlxsw/qos_ets_strict.sh ./tools/testing/selftests/drivers/net/mlxsw/mlxsw_lib.sh ./tools/testing/selftests/drivers/net/mlxsw/sch_red_prio.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap_acl_drops.sh ./tools/testing/selftests/drivers/net/mlxsw/qos_defprio.sh ./tools/testing/selftests/drivers/net/mlxsw/mirror_gre.sh ./tools/testing/selftests/drivers/net/mlxsw/sch_red_root.sh ./tools/testing/selftests/drivers/net/mlxsw/tc_restrictions.sh ./tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap_policer.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l2_drops.sh ./tools/testing/selftests/drivers/net/mlxsw/mirror_gre_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap_tunnel_ipip.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap_tunnel_vxlan.sh ./tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh ./tools/testing/selftests/drivers/net/mlxsw/rtnetlink.sh ./tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l3_drops.sh ./tools/testing/selftests/drivers/net/mlxsw/router_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh ./tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh ./tools/testing/selftests/drivers/net/mlxsw/blackhole_routes.sh ./tools/testing/selftests/drivers/net/mlxsw/sch_tbf_root.sh ./tools/testing/selftests/drivers/net/mlxsw/vxlan_flooding.sh ./tools/testing/selftests/drivers/net/mlxsw/sch_tbf_prio.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum/tc_flower_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum/mirror_gre_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum/router_scale.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum/devlink_lib_spectrum.sh ./tools/testing/selftests/drivers/net/mlxsw/spectrum/devlink_resources.sh ./tools/testing/selftests/drivers/net/mlxsw/qos_lib.sh ./tools/testing/selftests/drivers/net/mlxsw/extack.sh ./tools/testing/selftests/kexec/test_kexec_file_load.sh ./tools/testing/selftests/kexec/test_kexec_load.sh ./tools/testing/selftests/kexec/kexec_common_lib.sh ./tools/testing/selftests/kselftest_deps.sh ./tools/testing/selftests/arm64/tags/run_tags_test.sh ./tools/testing/selftests/tpm2/tpm2_tests.py ./tools/testing/selftests/tpm2/test_smoke.sh ./tools/testing/selftests/tpm2/test_space.sh ./tools/testing/selftests/tpm2/tpm2.py ./tools/testing/selftests/kmod/kmod.sh ./tools/testing/selftests/zram/zram02.sh ./tools/testing/selftests/zram/zram.sh ./tools/testing/selftests/zram/zram01.sh ./tools/testing/selftests/zram/zram_lib.sh ./tools/testing/selftests/ir/ir_loopback.sh ./tools/testing/selftests/x86/check_cc.sh ./tools/testing/selftests/cgroup/with_stress.sh ./tools/testing/selftests/cgroup/test_stress.sh ./tools/testing/selftests/rseq/run_param_test.sh ./tools/testing/selftests/futex/run.sh ./tools/testing/selftests/futex/functional/run.sh ./tools/testing/selftests/intel_pstate/run.sh ./tools/testing/selftests/gpio/gpio-mockup.sh ./tools/testing/selftests/gpio/gpio-mockup-sysfs.sh ./tools/testing/selftests/sysctl/sysctl.sh ./tools/testing/selftests/livepatch/test-state.sh ./tools/testing/selftests/livepatch/test-shadow-vars.sh ./tools/testing/selftests/livepatch/test-livepatch.sh ./tools/testing/selftests/livepatch/test-ftrace.sh ./tools/testing/selftests/livepatch/functions.sh ./tools/testing/selftests/livepatch/test-callbacks.sh ./tools/testing/selftests/mount/run_tests.sh ./tools/testing/selftests/locking/ww_mutex.sh ./tools/testing/selftests/sparc64/drivers/drivers_test.sh ./tools/testing/selftests/sparc64/run.sh ./tools/testing/selftests/splice/default_file_splice_read.sh ./tools/testing/selftests/kselftest_install.sh ./tools/testing/selftests/cpufreq/cpufreq.sh ./tools/testing/selftests/cpufreq/governor.sh ./tools/testing/selftests/cpufreq/main.sh ./tools/testing/selftests/cpufreq/cpu.sh ./tools/testing/selftests/cpufreq/module.sh ./tools/testing/selftests/cpufreq/special-tests.sh ./tools/testing/selftests/net/forwarding/mirror_lib.sh ./tools/testing/selftests/net/forwarding/mirror_gre_lag_lacp.sh ./tools/testing/selftests/net/forwarding/sch_ets.sh ./tools/testing/selftests/net/forwarding/mirror_gre_nh.sh ./tools/testing/selftests/net/forwarding/mirror_gre_lib.sh ./tools/testing/selftests/net/forwarding/lib.sh ./tools/testing/selftests/net/forwarding/sch_tbf_ets.sh ./tools/testing/selftests/net/forwarding/devlink_lib.sh ./tools/testing/selftests/net/forwarding/mirror_vlan.sh ./tools/testing/selftests/net/forwarding/fib_offload_lib.sh ./tools/testing/selftests/net/forwarding/router.sh ./tools/testing/selftests/net/forwarding/tc_flower_router.sh ./tools/testing/selftests/net/forwarding/sch_ets_core.sh ./tools/testing/selftests/net/forwarding/bridge_igmp.sh ./tools/testing/selftests/net/forwarding/mirror_gre_vlan.sh ./tools/testing/selftests/net/forwarding/ipip_hier_gre_keys.sh ./tools/testing/selftests/net/forwarding/ip6gre_inner_v6_multipath.sh ./tools/testing/selftests/net/forwarding/tc_vlan_modify.sh ./tools/testing/selftests/net/forwarding/mirror_gre_topo_lib.sh ./tools/testing/selftests/net/forwarding/mirror_gre_flower.sh ./tools/testing/selftests/net/forwarding/vxlan_asymmetric.sh ./tools/testing/selftests/net/forwarding/tc_common.sh ./tools/testing/selftests/net/forwarding/router_broadcast.sh ./tools/testing/selftests/net/forwarding/router_bridge.sh ./tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh ./tools/testing/selftests/net/forwarding/tc_shblocks.sh ./tools/testing/selftests/net/forwarding/router_vid_1.sh ./tools/testing/selftests/net/forwarding/bridge_port_isolation.sh ./tools/testing/selftests/net/forwarding/ipip_flat_gre.sh ./tools/testing/selftests/net/forwarding/ethtool_lib.sh ./tools/testing/selftests/net/forwarding/vxlan_bridge_1q_port_8472.sh ./tools/testing/selftests/net/forwarding/vxlan_bridge_1q.sh ./tools/testing/selftests/net/forwarding/vxlan_bridge_1d.sh ./tools/testing/selftests/net/forwarding/mirror_gre_neigh.sh ./tools/testing/selftests/net/forwarding/mirror_gre.sh ./tools/testing/selftests/net/forwarding/tc_actions.sh ./tools/testing/selftests/net/forwarding/gre_inner_v6_multipath.sh ./tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d_vlan.sh ./tools/testing/selftests/net/forwarding/sch_tbf_etsprio.sh ./tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh ./tools/testing/selftests/net/forwarding/ipip_lib.sh ./tools/testing/selftests/net/forwarding/router_multipath.sh ./tools/testing/selftests/net/forwarding/router_bridge_vlan.sh ./tools/testing/selftests/net/forwarding/bridge_sticky_fdb.sh ./tools/testing/selftests/net/forwarding/ipip_flat_gre_key.sh ./tools/testing/selftests/net/forwarding/tc_flower.sh ./tools/testing/selftests/net/forwarding/mirror_gre_bound.sh ./tools/testing/selftests/net/forwarding/tc_chains.sh ./tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q.sh ./tools/testing/selftests/net/forwarding/ipip_hier_gre.sh ./tools/testing/selftests/net/forwarding/gre_multipath.sh ./tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh ./tools/testing/selftests/net/forwarding/mirror_gre_bridge_1d.sh ./tools/testing/selftests/net/forwarding/mirror_topo_lib.sh ./tools/testing/selftests/net/forwarding/router_multicast.sh ./tools/testing/selftests/net/forwarding/gre_inner_v4_multipath.sh ./tools/testing/selftests/net/forwarding/sch_tbf_root.sh ./tools/testing/selftests/net/forwarding/bridge_vlan_unaware.sh ./tools/testing/selftests/net/forwarding/mirror_gre_changes.sh ./tools/testing/selftests/net/forwarding/ipip_flat_gre_keys.sh ./tools/testing/selftests/net/forwarding/ethtool.sh ./tools/testing/selftests/net/forwarding/sch_tbf_prio.sh ./tools/testing/selftests/net/forwarding/sch_tbf_core.sh ./tools/testing/selftests/net/forwarding/ipip_hier_gre_key.sh ./tools/testing/selftests/net/forwarding/pedit_dsfield.sh ./tools/testing/selftests/net/forwarding/router_mpath_nh.sh ./tools/testing/selftests/net/forwarding/sch_ets_tests.sh ./tools/testing/selftests/net/forwarding/vxlan_bridge_1d_port_8472.sh ./tools/testing/selftests/net/forwarding/ip6gre_inner_v4_multipath.sh ./tools/testing/selftests/net/forwarding/loopback.sh ./tools/testing/selftests/net/forwarding/skbedit_priority.sh ./tools/testing/selftests/net/forwarding/vxlan_symmetric.sh ./tools/testing/selftests/net/fcnal-test.sh ./tools/testing/selftests/net/reuseport_addr_any.sh ./tools/testing/selftests/net/udpgro.sh ./tools/testing/selftests/net/ip6_gre_headroom.sh ./tools/testing/selftests/net/test_vxlan_fdb_changelink.sh ./tools/testing/selftests/net/fib_nexthop_multiprefix.sh ./tools/testing/selftests/net/test_bpf.sh ./tools/testing/selftests/net/vrf-xfrm-tests.sh ./tools/testing/selftests/net/udpgro_bench.sh ./tools/testing/selftests/net/ip_defrag.sh ./tools/testing/selftests/net/in_netns.sh ./tools/testing/selftests/net/test_vxlan_under_vrf.sh ./tools/testing/selftests/net/netdevice.sh ./tools/testing/selftests/net/l2tp.sh ./tools/testing/selftests/net/altnames.sh ./tools/testing/selftests/net/fib_nexthops.sh ./tools/testing/selftests/net/udpgso.sh ./tools/testing/selftests/net/fin_ack_lat.sh ./tools/testing/selftests/net/xfrm_policy.sh ./tools/testing/selftests/net/mptcp/pm_netlink.sh ./tools/testing/selftests/net/mptcp/mptcp_connect.sh ./tools/testing/selftests/net/mptcp/mptcp_join.sh ./tools/testing/selftests/net/rtnetlink.sh ./tools/testing/selftests/net/fib_rule_tests.sh ./tools/testing/selftests/net/so_txtime.sh ./tools/testing/selftests/net/txtimestamp.sh ./tools/testing/selftests/net/udpgso_bench.sh ./tools/testing/selftests/net/route_localnet.sh ./tools/testing/selftests/net/pmtu.sh ./tools/testing/selftests/net/traceroute.sh ./tools/testing/selftests/net/psock_snd.sh ./tools/testing/selftests/net/msg_zerocopy.sh ./tools/testing/selftests/net/tcp_fastopen_backup_key.sh ./tools/testing/selftests/net/fib-onlink-tests.sh ./tools/testing/selftests/net/icmp_redirect.sh ./tools/testing/selftests/net/fib_tests.sh ./tools/testing/selftests/net/test_blackhole_dev.sh ./tools/testing/selftests/net/reuseaddr_ports_exhausted.sh ./tools/testing/selftests/net/ipv6_flowlabel.sh ./tools/testing/selftests/lkdtm/run.sh ./tools/testing/selftests/lib/strscpy.sh ./tools/testing/selftests/lib/prime_numbers.sh ./tools/testing/selftests/lib/printf.sh ./tools/testing/selftests/lib/bitmap.sh ./tools/testing/selftests/user/test_user_copy.sh ./tools/testing/selftests/vm/charge_reserved_hugetlb.sh ./tools/testing/selftests/vm/test_vmalloc.sh ./tools/testing/selftests/vm/hugetlb_reparenting_test.sh ./tools/testing/selftests/vm/write_hugetlb_memory.sh ./tools/testing/selftests/vm/test_hmm.sh ./tools/testing/selftests/memory-hotplug/mem-on-off-test.sh ./tools/testing/selftests/efivarfs/efivarfs.sh ./tools/testing/selftests/safesetid/safesetid-test.sh ./tools/testing/selftests/media_tests/open_loop_test.sh ./tools/testing/selftests/media_tests/bind_unbind_sample.sh ./tools/testing/selftests/media_tests/media_dev_allocator.sh ./tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh ./tools/testing/selftests/wireguard/netns.sh ./tools/testing/ktest/compare-ktest-sample.pl ./tools/testing/ktest/ktest.pl ./tools/testing/ktest/config-bisect.pl ./tools/cgroup/iocost_monitor.py ./tools/cgroup/iocost_coef_gen.py ./tools/usb/hcd-tests.sh ./tools/usb/usbip/vudc/vudc_server_example.sh ./tools/usb/usbip/autogen.sh ./tools/usb/usbip/cleanup.sh ./tools/leds/get_led_device_info.sh ./tools/hv/hv_get_dhcp_info.sh ./tools/hv/hv_get_dns_info.sh ./tools/hv/hv_set_ifconfig.sh ./tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py ./tools/power/pm-graph/sleepgraph.py ./tools/power/pm-graph/bootgraph.py ./tools/power/cpupower/bench/cpufreq-bench_plot.sh ./tools/power/cpupower/bench/cpufreq-bench_script.sh ./tools/power/cpupower/utils/version-gen.sh ./tools/power/cpupower/cpupower-completion.sh ./tools/lib/lockdep/tests/ABCDBDDA.sh ./tools/lib/lockdep/tests/AA.sh ./tools/lib/lockdep/tests/ABBCCA.sh ./tools/lib/lockdep/tests/ABA.sh ./tools/lib/lockdep/tests/ABBA.sh ./tools/lib/lockdep/tests/ABCABC.sh ./tools/lib/lockdep/tests/ABCDBCDA.sh ./tools/lib/lockdep/tests/WW.sh ./tools/lib/lockdep/tests/ABBCCDDA.sh ./tools/lib/lockdep/tests/unlock_balance.sh ./tools/lib/lockdep/tests/ABBA_2threads.sh ./tools/lib/lockdep/run_tests.sh ./tools/vm/slabinfo-gnuplot.sh ./tools/pci/pcitest.sh ./tools/virtio/ringtest/run-on-all.sh ./tools/time/udelay_test.sh ./samples/bpf/run_cookie_uid_helper_example.sh ./samples/bpf/test_cls_bpf.sh ./samples/bpf/test_cgrp2_tc.sh ./samples/bpf/test_override_return.sh ./samples/bpf/test_cgrp2_sock2.sh ./samples/bpf/test_lwt_bpf.sh ./samples/bpf/do_hbm_test.sh ./samples/bpf/test_ipip.sh ./samples/bpf/xdp2skb_meta.sh ./samples/bpf/lwt_len_hist.sh ./samples/bpf/tc_l2_redirect.sh ./samples/bpf/test_cgrp2_sock.sh ./samples/pktgen/pktgen_sample03_burst_single_flow.sh ./samples/pktgen/pktgen_sample02_multiqueue.sh ./samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh ./samples/pktgen/functions.sh ./samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh ./samples/pktgen/pktgen_sample01_simple.sh ./samples/pktgen/pktgen_sample05_flow_per_thread.sh ./samples/pktgen/pktgen_sample04_many_flows.sh ./samples/pktgen/parameters.sh ./samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh ./usr/gen_initramfs.sh ./arch/xtensa/kernel/syscalls/syscalltbl.sh ./arch/xtensa/kernel/syscalls/syscallhdr.sh ./arch/parisc/kernel/syscalls/syscalltbl.sh ./arch/parisc/kernel/syscalls/syscallhdr.sh ./arch/parisc/boot/install.sh ./arch/parisc/install.sh ./arch/powerpc/kernel/syscalls/syscalltbl.sh ./arch/powerpc/kernel/syscalls/syscallhdr.sh ./arch/powerpc/kernel/prom_init_check.sh ./arch/powerpc/kernel/systbl_chk.sh ./arch/powerpc/tools/gcc-check-mprofile-kernel.sh ./arch/powerpc/tools/unrel_branch_check.sh ./arch/powerpc/tools/head_check.sh ./arch/powerpc/tools/relocs_check.sh ./arch/powerpc/tools/checkpatch.sh ./arch/powerpc/boot/install.sh ./arch/sparc/kernel/syscalls/syscalltbl.sh ./arch/sparc/kernel/syscalls/syscallhdr.sh ./arch/sparc/vdso/checkundef.sh ./arch/sparc/boot/install.sh ./arch/arm/tools/syscallnr.sh ./arch/arm/tools/syscalltbl.sh ./arch/arm/tools/syscallhdr.sh ./arch/arm/boot/deflate_xip_data.sh ./arch/arm/boot/install.sh ./arch/arm/crypto/sha256-armv4.pl ./arch/arm/crypto/sha512-armv4.pl ./arch/arm/crypto/poly1305-armv4.pl ./arch/mips/kernel/syscalls/syscallnr.sh ./arch/mips/kernel/syscalls/syscalltbl.sh ./arch/mips/kernel/syscalls/syscallhdr.sh ./arch/mips/tools/generic-board-config.sh ./arch/mips/crypto/poly1305-mips.pl ./arch/m68k/kernel/syscalls/syscalltbl.sh ./arch/m68k/kernel/syscalls/syscallhdr.sh ./arch/m68k/install.sh ./arch/arm64/kernel/vdso/gen_vdso_offsets.sh ./arch/arm64/boot/install.sh ./arch/arm64/crypto/poly1305-armv8.pl ./arch/arm64/crypto/sha512-armv8.pl ./arch/s390/boot/install.sh ./arch/x86/kernel/cpu/mkcapflags.sh ./arch/x86/um/vdso/checkundef.sh ./arch/x86/boot/genimage.sh ./arch/x86/boot/install.sh ./arch/x86/crypto/poly1305-x86_64-cryptogams.pl ./arch/x86/entry/syscalls/syscalltbl.sh ./arch/x86/entry/syscalls/syscallhdr.sh ./arch/x86/entry/vdso/checkundef.sh ./arch/nds32/kernel/vdso/gen_vdso_offsets.sh ./arch/ia64/kernel/syscalls/syscalltbl.sh ./arch/ia64/kernel/syscalls/syscallhdr.sh ./arch/ia64/scripts/unwcheck.py ./arch/ia64/install.sh ./arch/microblaze/kernel/syscalls/syscalltbl.sh ./arch/microblaze/kernel/syscalls/syscallhdr.sh ./arch/alpha/kernel/syscalls/syscalltbl.sh ./arch/alpha/kernel/syscalls/syscallhdr.sh ./arch/nios2/boot/install.sh ./arch/riscv/boot/install.sh ./arch/sh/kernel/syscalls/syscalltbl.sh ./arch/sh/kernel/syscalls/syscallhdr.sh ./arch/sh/boot/compressed/install.sh [-- Attachment #3: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-16 5:15 ` Mrinal Pandey @ 2020-07-16 5:31 ` Lukas Bulwahn 2020-07-17 9:54 ` Mrinal Pandey 0 siblings, 1 reply; 14+ messages in thread From: Lukas Bulwahn @ 2020-07-16 5:31 UTC (permalink / raw) To: Mrinal Pandey; +Cc: Linux-kernel-mentees [-- Attachment #1.1: Type: text/plain, Size: 7238 bytes --] On Thu, Jul 16, 2020 at 7:15 AM Mrinal Pandey <mrinalmni@gmail.com> wrote: > > > On Tue, Jul 14, 2020 at 11:33 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> > wrote: > >> >> >> On Tue, 14 Jul 2020, Mrinal Pandey wrote: >> >> > >> > >> > On Tue, Jul 14, 2020 at 1:16 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> >> wrote: >> > >> > >> > On Mon, 13 Jul 2020, Mrinal Pandey wrote: >> > >> > > In all the scripts, the SPDX license should be on the second >> line, >> > > the first line being the "sh-bang", but checkpatch issues a >> warning >> > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" >> for the >> > > scripts that have SPDX license in the second line. >> > > >> > > However, this warning is not issued when checkpatch is run on a >> file using >> > > `-f` option. The case for files has been handled gracefully by >> changing >> > > `$checklicenseline` to `2` but a corresponding check when >> running checkpatch >> > > on a commit hash is missing. >> > > >> > > 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 commits >> which modified >> > > a script file. >> > > >> > > This check is missing in checkpatch since commit a8da38a9cf0e >> > > ("checkpatch: add test for SPDX-License-Identifier on wrong >> line #") >> > > when the corresponding rule was first commited. >> > > >> > > Fix this by setting `$checklicenseline` to `2` when the diff >> content that >> > > is being checked originates from a script, thus, informing >> checkpatch that >> > > the SPDX license should be on the second line. >> > > >> > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> >> > > --- >> > > scripts/checkpatch.pl | 3 +++ >> > > 1 file changed, 3 insertions(+) >> > > >> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >> > > index 4c820607540b..bbffd0c4449d 100755 >> > > --- a/scripts/checkpatch.pl >> > > +++ b/scripts/checkpatch.pl >> > > @@ -3218,6 +3218,9 @@ sub process { >> > > next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); >> > > >> > > # check for using SPDX-License-Identifier on the wrong line >> number >> > > + if ($realfile =~ /^scripts/) { >> > > + $checklicenseline = 2; >> > > + } >> > >> > I think this is somehow wrong here. The check for >> checklicenseline = 2 >> > looks very different above. >> > >> > Why does -f work and using a patch file not work? >> > >> > >> > Sir, >> > >> > I am going to explain my observation based on file >> `scripts/atomic/gen-atomic-fallback.sh` and >> > commit hash `37f8173dd849`. >> > >> > If we are checking against the file, `checklicenseline` is set to 1 and >> when `realline` is 1 the above >> > `if` block is triggered, then we check if this line is of the form >> `#!/` using the regular expression >> > `^[ \+]\s*\#\!\s*\/`. If this is the case we set `checklicenseline` to >> `2` informing checkpatch that it should >> > expect license on the second line and this works all fine for a file. >> > The `if` block below my proposed changes evaluates to false in this >> case and thus it emits no false warning. >> > >> > However, If we are checking a diff content, the above `if` block is not >> triggered at all. This is >> > because `realline` stores the actual line number of the line we are >> checking currently out of diff content. >> > This value is 2 because SPDX identifier is indeed at the second line in >> the file but `checklicenseline` is still >> > `1`. >> > `realline` will never become equal to 1 again and thus the above `if` >> condition will never be true in this case. >> > Even if the above `if` block is triggered it would not update >> `checklicenseline` to 2 as the regular expression >> > is not satisfied since we don't have sh-bang in diff content and just >> the SPDX tag. >> > If we don't do this, the `if` block below evaluates to true when >> `realline` is 2 and `checklicensline` is `1` >> > leading >> > to the emission of a false warning. >> > >> >> So, maybe this whole logic needs to be reworked. If you do not know the >> first line, you need to have a different criteria in the first place >> to determine if you expect the license tag in the first or the second, >> e.g., the file extension, and then checking line 1 for a shebang is just >> sanity checking. If it is of a specific file extension, you know line 1 >> and it is not a shebang, that is probably worth noting as a different >> recommendation in checkpatch.pl anyway. >> > > Sir, > > When we know the first line, i.e. we are running checkpatch against a > file, the existing logic > works fine. We probably don't want to induce any changes there. > > Why not? Do you think we would break things there? Then we should not touch the code at all. Do you think we cannot test it properly after the change? Then we should think about how we make a proper regression test suite for that. But when we don't know the first line, if am not wrong, it would go > somewhat like: > if (the file is a script) { > if (the first line is shebang) { > if (the second line is SPDX) { > All good > } else { > Issue a misplaced or missing SPDX tag warning > } > } else { > Issue a missing shebang warning > } > } else { > if (the first line is SPDX) { > All good > } else { > Issue a misplaced or missing SPDX tag warning > } > } > > Basically agree, but that logic applies when you know the first line as well (and only, right?). What if you do not know the first line, how would you check "the first line is shebang" if you do not know the first line? The missing shebang warning probably needs to go elsewhere in the whole script. > Lukas >> >> >> > So, what I did was to check if the diff content we are checking >> actually comes from a script, if yes, we can set >> > `checklicenseline` to `2` to avoid this confusion. >> > >> >> Why would you think that scripts are only in scripts? >> >> How about first listing all files where the SPDX tag is in line 2 in the >> current repository, e.g., v5.8-rc5? >> >> Then, we look at that list and determine a suitable criteria for looking >> in line 2 for the SPDX tag. >> > > Yes, the scripts are not only in scripts. I have listed all the files > where the SPDX tag should be > on the second line. I've attached the list for reference. We should > probably be checking the file > extension to determine if the tag needs to be on the second line or not. > The documentation says the SPDX tag should be present in all source files. > Do these source files include > Documentation files too? > > How did you create that list? Agree (if the way you created that list makes sense). File extension seems to cover all cases, and checking for the directory 'scripts' does not. We might also add a further sanity check in checkpatch.pl if someone adds an executable file that is not with extension sh, pl, or py. Lukas > [-- Attachment #1.2: Type: text/html, Size: 10487 bytes --] [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-16 5:31 ` Lukas Bulwahn @ 2020-07-17 9:54 ` Mrinal Pandey 2020-07-17 11:47 ` Lukas Bulwahn 0 siblings, 1 reply; 14+ messages in thread From: Mrinal Pandey @ 2020-07-17 9:54 UTC (permalink / raw) To: Lukas Bulwahn, Shuah Khan, Linux-kernel-mentees [-- Attachment #1.1: Type: text/plain, Size: 8630 bytes --] On Thu, Jul 16, 2020, 11:01 Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > On Thu, Jul 16, 2020 at 7:15 AM Mrinal Pandey <mrinalmni@gmail.com> wrote: > >> >> >> On Tue, Jul 14, 2020 at 11:33 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> >> wrote: >> >>> >>> >>> On Tue, 14 Jul 2020, Mrinal Pandey wrote: >>> >>> > >>> > >>> > On Tue, Jul 14, 2020 at 1:16 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> >>> wrote: >>> > >>> > >>> > On Mon, 13 Jul 2020, Mrinal Pandey wrote: >>> > >>> > > In all the scripts, the SPDX license should be on the second >>> line, >>> > > the first line being the "sh-bang", but checkpatch issues a >>> warning >>> > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" >>> for the >>> > > scripts that have SPDX license in the second line. >>> > > >>> > > However, this warning is not issued when checkpatch is run on >>> a file using >>> > > `-f` option. The case for files has been handled gracefully by >>> changing >>> > > `$checklicenseline` to `2` but a corresponding check when >>> running checkpatch >>> > > on a commit hash is missing. >>> > > >>> > > 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 commits >>> which modified >>> > > a script file. >>> > > >>> > > This check is missing in checkpatch since commit a8da38a9cf0e >>> > > ("checkpatch: add test for SPDX-License-Identifier on wrong >>> line #") >>> > > when the corresponding rule was first commited. >>> > > >>> > > Fix this by setting `$checklicenseline` to `2` when the diff >>> content that >>> > > is being checked originates from a script, thus, informing >>> checkpatch that >>> > > the SPDX license should be on the second line. >>> > > >>> > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> >>> > > --- >>> > > scripts/checkpatch.pl | 3 +++ >>> > > 1 file changed, 3 insertions(+) >>> > > >>> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >>> > > index 4c820607540b..bbffd0c4449d 100755 >>> > > --- a/scripts/checkpatch.pl >>> > > +++ b/scripts/checkpatch.pl >>> > > @@ -3218,6 +3218,9 @@ sub process { >>> > > next if ($realfile !~ >>> /\.(h|c|s|S|sh|dtsi|dts)$/); >>> > > >>> > > # check for using SPDX-License-Identifier on the wrong line >>> number >>> > > + if ($realfile =~ /^scripts/) { >>> > > + $checklicenseline = 2; >>> > > + } >>> > >>> > I think this is somehow wrong here. The check for >>> checklicenseline = 2 >>> > looks very different above. >>> > >>> > Why does -f work and using a patch file not work? >>> > >>> > >>> > Sir, >>> > >>> > I am going to explain my observation based on file >>> `scripts/atomic/gen-atomic-fallback.sh` and >>> > commit hash `37f8173dd849`. >>> > >>> > If we are checking against the file, `checklicenseline` is set to 1 >>> and when `realline` is 1 the above >>> > `if` block is triggered, then we check if this line is of the form >>> `#!/` using the regular expression >>> > `^[ \+]\s*\#\!\s*\/`. If this is the case we set `checklicenseline` to >>> `2` informing checkpatch that it should >>> > expect license on the second line and this works all fine for a file. >>> > The `if` block below my proposed changes evaluates to false in this >>> case and thus it emits no false warning. >>> > >>> > However, If we are checking a diff content, the above `if` block is >>> not triggered at all. This is >>> > because `realline` stores the actual line number of the line we are >>> checking currently out of diff content. >>> > This value is 2 because SPDX identifier is indeed at the second line >>> in the file but `checklicenseline` is still >>> > `1`. >>> > `realline` will never become equal to 1 again and thus the above `if` >>> condition will never be true in this case. >>> > Even if the above `if` block is triggered it would not update >>> `checklicenseline` to 2 as the regular expression >>> > is not satisfied since we don't have sh-bang in diff content and just >>> the SPDX tag. >>> > If we don't do this, the `if` block below evaluates to true when >>> `realline` is 2 and `checklicensline` is `1` >>> > leading >>> > to the emission of a false warning. >>> > >>> >>> So, maybe this whole logic needs to be reworked. If you do not know the >>> first line, you need to have a different criteria in the first place >>> to determine if you expect the license tag in the first or the second, >>> e.g., the file extension, and then checking line 1 for a shebang is just >>> sanity checking. If it is of a specific file extension, you know line 1 >>> and it is not a shebang, that is probably worth noting as a different >>> recommendation in checkpatch.pl anyway. >>> >> >> Sir, >> >> When we know the first line, i.e. we are running checkpatch against a >> file, the existing logic >> works fine. We probably don't want to induce any changes there. >> >> > Why not? Do you think we would break things there? Then we should not > touch the code at all. > Do you think we cannot test it properly after the change? Then we should > think about how we make a proper regression test suite for that. > Sir, No, breaking code or not being able to test is not why I suggest this. I feel that the existing logic handles the case of "Improper or malformed SPDX tag" and "Misplaced SPDX tag" for files i.e. when the first line is known. Anyway, the logic for "Misplaced SPDX tag" is written as a different rule. We just need to add in the logic for patches there. I tried to do this by checking for the scripts directory which was wrong. If I check instead for the file being a script that would make much more sense. Please let me know if you suggest something else. > > But when we don't know the first line, if am not wrong, it would go >> somewhat like: >> if (the file is a script) { >> if (the first line is shebang) { >> if (the second line is SPDX) { >> All good >> } else { >> Issue a misplaced or missing SPDX tag warning >> } >> } else { >> Issue a missing shebang warning >> } >> } else { >> if (the first line is SPDX) { >> All good >> } else { >> Issue a misplaced or missing SPDX tag warning >> } >> } >> >> > Basically agree, but that logic applies when you know the first line as > well (and only, right?). What if you do not know the first line, how would > you check "the first line is shebang" if you do not know the first line? > > > The missing shebang warning probably needs to go elsewhere in the whole > script. > By not knowing the first line I mean to say that the first line doesn't show up in diff content of the patch but what if we open the file at that point in the commit history and check for the first line to be a shebang? Would it be okay to do that? Once we check the first line we can then continue as suggested. > > > >> Lukas >>> >>> >>> > So, what I did was to check if the diff content we are checking >>> actually comes from a script, if yes, we can set >>> > `checklicenseline` to `2` to avoid this confusion. >>> > >>> >>> Why would you think that scripts are only in scripts? >>> >>> How about first listing all files where the SPDX tag is in line 2 in the >>> current repository, e.g., v5.8-rc5? >>> >>> Then, we look at that list and determine a suitable criteria for looking >>> in line 2 for the SPDX tag. >>> >> >> Yes, the scripts are not only in scripts. I have listed all the files >> where the SPDX tag should be >> on the second line. I've attached the list for reference. We should >> probably be checking the file >> extension to determine if the tag needs to be on the second line or not. >> The documentation says the SPDX tag should be present in all source >> files. Do these source files include >> Documentation files too? >> >> > How did you create that list? > Agree (if the way you created that list makes sense). File extension seems > to cover all cases, and checking for the directory 'scripts' does not. > > I issued the command `find . -regex ".*\.\(py\|sh\|pl\)"` to make this list. I should have included awk, YAML and tc files too since they are scripts too. We might also add a further sanity check in checkpatch.pl if someone adds > an executable file that is not with extension sh, pl, or py. > Yes, this check should be present. Thank you. > > Lukas > >> [-- Attachment #1.2: Type: text/html, Size: 13362 bytes --] [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-17 9:54 ` Mrinal Pandey @ 2020-07-17 11:47 ` Lukas Bulwahn 2020-07-19 6:27 ` Mrinal Pandey 0 siblings, 1 reply; 14+ messages in thread From: Lukas Bulwahn @ 2020-07-17 11:47 UTC (permalink / raw) To: Mrinal Pandey; +Cc: Linux-kernel-mentees [-- Attachment #1.1: Type: text/plain, Size: 9669 bytes --] On Fri, Jul 17, 2020 at 11:54 AM Mrinal Pandey <mrinalmni@gmail.com> wrote: > > > On Thu, Jul 16, 2020, 11:01 Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > >> >> >> On Thu, Jul 16, 2020 at 7:15 AM Mrinal Pandey <mrinalmni@gmail.com> >> wrote: >> >>> >>> >>> On Tue, Jul 14, 2020 at 11:33 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> >>> wrote: >>> >>>> >>>> >>>> On Tue, 14 Jul 2020, Mrinal Pandey wrote: >>>> >>>> > >>>> > >>>> > On Tue, Jul 14, 2020 at 1:16 AM Lukas Bulwahn < >>>> lukas.bulwahn@gmail.com> wrote: >>>> > >>>> > >>>> > On Mon, 13 Jul 2020, Mrinal Pandey wrote: >>>> > >>>> > > In all the scripts, the SPDX license should be on the second >>>> line, >>>> > > the first line being the "sh-bang", but checkpatch issues a >>>> warning >>>> > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" >>>> for the >>>> > > scripts that have SPDX license in the second line. >>>> > > >>>> > > However, this warning is not issued when checkpatch is run on >>>> a file using >>>> > > `-f` option. The case for files has been handled gracefully >>>> by changing >>>> > > `$checklicenseline` to `2` but a corresponding check when >>>> running checkpatch >>>> > > on a commit hash is missing. >>>> > > >>>> > > 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 commits >>>> which modified >>>> > > a script file. >>>> > > >>>> > > This check is missing in checkpatch since commit a8da38a9cf0e >>>> > > ("checkpatch: add test for SPDX-License-Identifier on wrong >>>> line #") >>>> > > when the corresponding rule was first commited. >>>> > > >>>> > > Fix this by setting `$checklicenseline` to `2` when the diff >>>> content that >>>> > > is being checked originates from a script, thus, informing >>>> checkpatch that >>>> > > the SPDX license should be on the second line. >>>> > > >>>> > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> >>>> > > --- >>>> > > scripts/checkpatch.pl | 3 +++ >>>> > > 1 file changed, 3 insertions(+) >>>> > > >>>> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >>>> > > index 4c820607540b..bbffd0c4449d 100755 >>>> > > --- a/scripts/checkpatch.pl >>>> > > +++ b/scripts/checkpatch.pl >>>> > > @@ -3218,6 +3218,9 @@ sub process { >>>> > > next if ($realfile !~ >>>> /\.(h|c|s|S|sh|dtsi|dts)$/); >>>> > > >>>> > > # check for using SPDX-License-Identifier on the wrong line >>>> number >>>> > > + if ($realfile =~ /^scripts/) { >>>> > > + $checklicenseline = 2; >>>> > > + } >>>> > >>>> > I think this is somehow wrong here. The check for >>>> checklicenseline = 2 >>>> > looks very different above. >>>> > >>>> > Why does -f work and using a patch file not work? >>>> > >>>> > >>>> > Sir, >>>> > >>>> > I am going to explain my observation based on file >>>> `scripts/atomic/gen-atomic-fallback.sh` and >>>> > commit hash `37f8173dd849`. >>>> > >>>> > If we are checking against the file, `checklicenseline` is set to 1 >>>> and when `realline` is 1 the above >>>> > `if` block is triggered, then we check if this line is of the form >>>> `#!/` using the regular expression >>>> > `^[ \+]\s*\#\!\s*\/`. If this is the case we set `checklicenseline` >>>> to `2` informing checkpatch that it should >>>> > expect license on the second line and this works all fine for a file. >>>> > The `if` block below my proposed changes evaluates to false in this >>>> case and thus it emits no false warning. >>>> > >>>> > However, If we are checking a diff content, the above `if` block is >>>> not triggered at all. This is >>>> > because `realline` stores the actual line number of the line we are >>>> checking currently out of diff content. >>>> > This value is 2 because SPDX identifier is indeed at the second line >>>> in the file but `checklicenseline` is still >>>> > `1`. >>>> > `realline` will never become equal to 1 again and thus the above `if` >>>> condition will never be true in this case. >>>> > Even if the above `if` block is triggered it would not update >>>> `checklicenseline` to 2 as the regular expression >>>> > is not satisfied since we don't have sh-bang in diff content and just >>>> the SPDX tag. >>>> > If we don't do this, the `if` block below evaluates to true when >>>> `realline` is 2 and `checklicensline` is `1` >>>> > leading >>>> > to the emission of a false warning. >>>> > >>>> >>>> So, maybe this whole logic needs to be reworked. If you do not know the >>>> first line, you need to have a different criteria in the first place >>>> to determine if you expect the license tag in the first or the second, >>>> e.g., the file extension, and then checking line 1 for a shebang is just >>>> sanity checking. If it is of a specific file extension, you know line 1 >>>> and it is not a shebang, that is probably worth noting as a different >>>> recommendation in checkpatch.pl anyway. >>>> >>> >>> Sir, >>> >>> When we know the first line, i.e. we are running checkpatch against a >>> file, the existing logic >>> works fine. We probably don't want to induce any changes there. >>> >>> >> Why not? Do you think we would break things there? Then we should not >> touch the code at all. >> Do you think we cannot test it properly after the change? Then we should >> think about how we make a proper regression test suite for that. >> > > Sir, > > No, breaking code or not being able to test is not why I suggest this. I > feel that the existing logic handles the case of > "Improper or malformed SPDX tag" and "Misplaced SPDX tag" for files i.e. > when the first line is known. Anyway, the logic > for "Misplaced SPDX tag" is written as a different rule. We just need to > add in the logic for patches there. > I tried to do this by checking for the scripts directory which was wrong. > If I check instead for the file being a script that would make much more > sense. > Please let me know if you suggest something else. > Well, you are going to add a different way of checking as you suggested, right? So are you suggesting to have two duplicated ways of checking for the same thing? That seems strange to me. Go ahead, make a suitable first proposal, then we will see if and how to refactor. > >> But when we don't know the first line, if am not wrong, it would go >>> somewhat like: >>> if (the file is a script) { >>> if (the first line is shebang) { >>> if (the second line is SPDX) { >>> All good >>> } else { >>> Issue a misplaced or missing SPDX tag warning >>> } >>> } else { >>> Issue a missing shebang warning >>> } >>> } else { >>> if (the first line is SPDX) { >>> All good >>> } else { >>> Issue a misplaced or missing SPDX tag warning >>> } >>> } >>> >>> >> Basically agree, but that logic applies when you know the first line as >> well (and only, right?). What if you do not know the first line, how would >> you check "the first line is shebang" if you do not know the first line? >> >> >> The missing shebang warning probably needs to go elsewhere in the whole >> script. >> > > By not knowing the first line I mean to say that the first line doesn't > show up in diff content of the patch but > what if we open the file at that point in the commit history and check for > the first line to be a shebang? > Would it be okay to do that? Once we check the first line we can then > continue as suggested. > I think that is essentially against the general design decision of checkpatch.pl; checkpatch.pl takes the patch but it does not check anything in the current working tree, nor does it know what the "parent" of that patch in the git history really is. Maybe Shuah can confirm? Otherwise, I suggest to look if you find checking a file beyond the patch happening anywhere in the current code of checkpatch.pl and documented in the sparse documentation on checkpatch.pl. So, I believe you need to make it work without checking the first line (if that is not in the patch). > >>>> > So, what I did was to check if the diff content we are checking >>>> actually comes from a script, if yes, we can set >>>> > `checklicenseline` to `2` to avoid this confusion. >>>> > >>>> >>>> Why would you think that scripts are only in scripts? >>>> >>>> How about first listing all files where the SPDX tag is in line 2 in >>>> the >>>> current repository, e.g., v5.8-rc5? >>>> >>>> Then, we look at that list and determine a suitable criteria for >>>> looking >>>> in line 2 for the SPDX tag. >>>> >>> >>> Yes, the scripts are not only in scripts. I have listed all the files >>> where the SPDX tag should be >>> on the second line. I've attached the list for reference. We should >>> probably be checking the file >>> extension to determine if the tag needs to be on the second line or not. >>> The documentation says the SPDX tag should be present in all source >>> files. Do these source files include >>> Documentation files too? >>> >>> >> How did you create that list? >> Agree (if the way you created that list makes sense). File extension >> seems to cover all cases, and checking for the directory 'scripts' does not. >> >> I issued the command `find . -regex ".*\.\(py\|sh\|pl\)"` to make this > list. I should have included awk, YAML and tc files too since they are > scripts too. > > Why not look for all files that have a shebang in the first line? Lukas > [-- Attachment #1.2: Type: text/html, Size: 15133 bytes --] [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-17 11:47 ` Lukas Bulwahn @ 2020-07-19 6:27 ` Mrinal Pandey 2020-07-19 7:13 ` Lukas Bulwahn 0 siblings, 1 reply; 14+ messages in thread From: Mrinal Pandey @ 2020-07-19 6:27 UTC (permalink / raw) To: Lukas Bulwahn, Shuah Khan, Linux-kernel-mentees [-- Attachment #1.1: Type: text/plain, Size: 10876 bytes --] On Fri, Jul 17, 2020 at 5:18 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > On Fri, Jul 17, 2020 at 11:54 AM Mrinal Pandey <mrinalmni@gmail.com> > wrote: > >> >> >> On Thu, Jul 16, 2020, 11:01 Lukas Bulwahn <lukas.bulwahn@gmail.com> >> wrote: >> >>> >>> >>> On Thu, Jul 16, 2020 at 7:15 AM Mrinal Pandey <mrinalmni@gmail.com> >>> wrote: >>> >>>> >>>> >>>> On Tue, Jul 14, 2020 at 11:33 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> >>>> wrote: >>>> >>>>> >>>>> >>>>> On Tue, 14 Jul 2020, Mrinal Pandey wrote: >>>>> >>>>> > >>>>> > >>>>> > On Tue, Jul 14, 2020 at 1:16 AM Lukas Bulwahn < >>>>> lukas.bulwahn@gmail.com> wrote: >>>>> > >>>>> > >>>>> > On Mon, 13 Jul 2020, Mrinal Pandey wrote: >>>>> > >>>>> > > In all the scripts, the SPDX license should be on the second >>>>> line, >>>>> > > the first line being the "sh-bang", but checkpatch issues a >>>>> warning >>>>> > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" >>>>> for the >>>>> > > scripts that have SPDX license in the second line. >>>>> > > >>>>> > > However, this warning is not issued when checkpatch is run >>>>> on a file using >>>>> > > `-f` option. The case for files has been handled gracefully >>>>> by changing >>>>> > > `$checklicenseline` to `2` but a corresponding check when >>>>> running checkpatch >>>>> > > on a commit hash is missing. >>>>> > > >>>>> > > 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 commits >>>>> which modified >>>>> > > a script file. >>>>> > > >>>>> > > This check is missing in checkpatch since commit a8da38a9cf0e >>>>> > > ("checkpatch: add test for SPDX-License-Identifier on wrong >>>>> line #") >>>>> > > when the corresponding rule was first commited. >>>>> > > >>>>> > > Fix this by setting `$checklicenseline` to `2` when the diff >>>>> content that >>>>> > > is being checked originates from a script, thus, informing >>>>> checkpatch that >>>>> > > the SPDX license should be on the second line. >>>>> > > >>>>> > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> >>>>> > > --- >>>>> > > scripts/checkpatch.pl | 3 +++ >>>>> > > 1 file changed, 3 insertions(+) >>>>> > > >>>>> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >>>>> > > index 4c820607540b..bbffd0c4449d 100755 >>>>> > > --- a/scripts/checkpatch.pl >>>>> > > +++ b/scripts/checkpatch.pl >>>>> > > @@ -3218,6 +3218,9 @@ sub process { >>>>> > > next if ($realfile !~ >>>>> /\.(h|c|s|S|sh|dtsi|dts)$/); >>>>> > > >>>>> > > # check for using SPDX-License-Identifier on the wrong line >>>>> number >>>>> > > + if ($realfile =~ /^scripts/) { >>>>> > > + $checklicenseline = 2; >>>>> > > + } >>>>> > >>>>> > I think this is somehow wrong here. The check for >>>>> checklicenseline = 2 >>>>> > looks very different above. >>>>> > >>>>> > Why does -f work and using a patch file not work? >>>>> > >>>>> > >>>>> > Sir, >>>>> > >>>>> > I am going to explain my observation based on file >>>>> `scripts/atomic/gen-atomic-fallback.sh` and >>>>> > commit hash `37f8173dd849`. >>>>> > >>>>> > If we are checking against the file, `checklicenseline` is set to 1 >>>>> and when `realline` is 1 the above >>>>> > `if` block is triggered, then we check if this line is of the form >>>>> `#!/` using the regular expression >>>>> > `^[ \+]\s*\#\!\s*\/`. If this is the case we set `checklicenseline` >>>>> to `2` informing checkpatch that it should >>>>> > expect license on the second line and this works all fine for a file. >>>>> > The `if` block below my proposed changes evaluates to false in this >>>>> case and thus it emits no false warning. >>>>> > >>>>> > However, If we are checking a diff content, the above `if` block is >>>>> not triggered at all. This is >>>>> > because `realline` stores the actual line number of the line we are >>>>> checking currently out of diff content. >>>>> > This value is 2 because SPDX identifier is indeed at the second line >>>>> in the file but `checklicenseline` is still >>>>> > `1`. >>>>> > `realline` will never become equal to 1 again and thus the above >>>>> `if` condition will never be true in this case. >>>>> > Even if the above `if` block is triggered it would not update >>>>> `checklicenseline` to 2 as the regular expression >>>>> > is not satisfied since we don't have sh-bang in diff content and >>>>> just the SPDX tag. >>>>> > If we don't do this, the `if` block below evaluates to true when >>>>> `realline` is 2 and `checklicensline` is `1` >>>>> > leading >>>>> > to the emission of a false warning. >>>>> > >>>>> >>>>> So, maybe this whole logic needs to be reworked. If you do not know >>>>> the >>>>> first line, you need to have a different criteria in the first place >>>>> to determine if you expect the license tag in the first or the second, >>>>> e.g., the file extension, and then checking line 1 for a shebang is >>>>> just >>>>> sanity checking. If it is of a specific file extension, you know line 1 >>>>> and it is not a shebang, that is probably worth noting as a different >>>>> recommendation in checkpatch.pl anyway. >>>>> >>>> >>>> Sir, >>>> >>>> When we know the first line, i.e. we are running checkpatch against a >>>> file, the existing logic >>>> works fine. We probably don't want to induce any changes there. >>>> >>>> >>> Why not? Do you think we would break things there? Then we should not >>> touch the code at all. >>> Do you think we cannot test it properly after the change? Then we should >>> think about how we make a proper regression test suite for that. >>> >> >> Sir, >> >> No, breaking code or not being able to test is not why I suggest this. I >> feel that the existing logic handles the case of >> "Improper or malformed SPDX tag" and "Misplaced SPDX tag" for files i.e. >> when the first line is known. Anyway, the logic >> for "Misplaced SPDX tag" is written as a different rule. We just need to >> add in the logic for patches there. >> I tried to do this by checking for the scripts directory which was wrong. >> If I check instead for the file being a script that would make much more >> sense. >> Please let me know if you suggest something else. >> > > Well, you are going to add a different way of checking as you suggested, > right? So are you suggesting to have two duplicated ways of checking for > the same thing? That seems strange to me. > > Go ahead, make a suitable first proposal, then we will see if and how to > refactor. > Sir, No, I would not want to duplicate things. Yes, let me first send a patch to you first, and then we can refactor it if needed. > > >> >>> But when we don't know the first line, if am not wrong, it would go >>>> somewhat like: >>>> if (the file is a script) { >>>> if (the first line is shebang) { >>>> if (the second line is SPDX) { >>>> All good >>>> } else { >>>> Issue a misplaced or missing SPDX tag warning >>>> } >>>> } else { >>>> Issue a missing shebang warning >>>> } >>>> } else { >>>> if (the first line is SPDX) { >>>> All good >>>> } else { >>>> Issue a misplaced or missing SPDX tag warning >>>> } >>>> } >>>> >>>> >>> Basically agree, but that logic applies when you know the first line as >>> well (and only, right?). What if you do not know the first line, how would >>> you check "the first line is shebang" if you do not know the first line? >>> >>> >>> The missing shebang warning probably needs to go elsewhere in the whole >>> script. >>> >> >> By not knowing the first line I mean to say that the first line doesn't >> show up in diff content of the patch but >> what if we open the file at that point in the commit history and check >> for the first line to be a shebang? >> Would it be okay to do that? Once we check the first line we can then >> continue as suggested. >> > > I think that is essentially against the general design decision of > checkpatch.pl; checkpatch.pl takes the patch but it does not check > anything in the current working tree, nor does it know what the "parent" of > that patch in the git history really is. > > Maybe Shuah can confirm? Otherwise, I suggest to look if you find checking > a file beyond the patch happening anywhere in the current code of > checkpatch.pl and documented in the sparse documentation on checkpatch.pl. > > So, I believe you need to make it work without checking the first line (if > that is not in the patch). > Yes. We can't open a file for checking, you are correct here but we need to check for shebang to be on the first line only if it appears in the diff content or when we check a complete file, otherwise, we should anyway not chek it since checkpatch only checks the patch or the complete file. Am I correct here? > > >> >>>>> > So, what I did was to check if the diff content we are checking >>>>> actually comes from a script, if yes, we can set >>>>> > `checklicenseline` to `2` to avoid this confusion. >>>>> > >>>>> >>>>> Why would you think that scripts are only in scripts? >>>>> >>>>> How about first listing all files where the SPDX tag is in line 2 in >>>>> the >>>>> current repository, e.g., v5.8-rc5? >>>>> >>>>> Then, we look at that list and determine a suitable criteria for >>>>> looking >>>>> in line 2 for the SPDX tag. >>>>> >>>> >>>> Yes, the scripts are not only in scripts. I have listed all the files >>>> where the SPDX tag should be >>>> on the second line. I've attached the list for reference. We should >>>> probably be checking the file >>>> extension to determine if the tag needs to be on the second line or not. >>>> The documentation says the SPDX tag should be present in all source >>>> files. Do these source files include >>>> Documentation files too? >>>> >>>> >>> How did you create that list? >>> Agree (if the way you created that list makes sense). File extension >>> seems to cover all cases, and checking for the directory 'scripts' does not. >>> >>> I issued the command `find . -regex ".*\.\(py\|sh\|pl\)"` to make this >> list. I should have included awk, YAML and tc files too since they are >> scripts too. >> >> > Why not look for all files that have a shebang in the first line? > We could be checking a file or a patch. I suggest we take the name of the file we are checking(or the name of the file from which the diff content comes from) and then run a regex, similar to above, on it to determine if it is a script. If we instead go for checking the first line to be shebang in the current file would it not require to cat or open the file? Thank you. > > Lukas > >> [-- Attachment #1.2: Type: text/html, Size: 17473 bytes --] [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-19 6:27 ` Mrinal Pandey @ 2020-07-19 7:13 ` Lukas Bulwahn 0 siblings, 0 replies; 14+ messages in thread From: Lukas Bulwahn @ 2020-07-19 7:13 UTC (permalink / raw) To: Mrinal Pandey; +Cc: Linux-kernel-mentees [-- Attachment #1.1: Type: text/plain, Size: 11336 bytes --] On So., 19. Juli 2020 at 08:28, Mrinal Pandey <mrinalmni@gmail.com> wrote: > > > On Fri, Jul 17, 2020 at 5:18 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> > wrote: > >> >> >> On Fri, Jul 17, 2020 at 11:54 AM Mrinal Pandey <mrinalmni@gmail.com> >> wrote: >> >>> >>> >>> On Thu, Jul 16, 2020, 11:01 Lukas Bulwahn <lukas.bulwahn@gmail.com> >>> wrote: >>> >>>> >>>> >>>> On Thu, Jul 16, 2020 at 7:15 AM Mrinal Pandey <mrinalmni@gmail.com> >>>> wrote: >>>> >>>>> >>>>> >>>>> On Tue, Jul 14, 2020 at 11:33 AM Lukas Bulwahn < >>>>> lukas.bulwahn@gmail.com> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Tue, 14 Jul 2020, Mrinal Pandey wrote: >>>>>> >>>>>> > >>>>>> > >>>>>> > On Tue, Jul 14, 2020 at 1:16 AM Lukas Bulwahn < >>>>>> lukas.bulwahn@gmail.com> wrote: >>>>>> > >>>>>> > >>>>>> > On Mon, 13 Jul 2020, Mrinal Pandey wrote: >>>>>> > >>>>>> > > In all the scripts, the SPDX license should be on the >>>>>> second line, >>>>>> > > the first line being the "sh-bang", but checkpatch issues a >>>>>> warning >>>>>> > > "Misplaced SPDX-License-Identifier tag - use line 1 >>>>>> instead" for the >>>>>> > > scripts that have SPDX license in the second line. >>>>>> > > >>>>>> > > However, this warning is not issued when checkpatch is run >>>>>> on a file using >>>>>> > > `-f` option. The case for files has been handled gracefully >>>>>> by changing >>>>>> > > `$checklicenseline` to `2` but a corresponding check when >>>>>> running checkpatch >>>>>> > > on a commit hash is missing. >>>>>> > > >>>>>> > > 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 commits >>>>>> which modified >>>>>> > > a script file. >>>>>> > > >>>>>> > > This check is missing in checkpatch since commit >>>>>> a8da38a9cf0e >>>>>> > > ("checkpatch: add test for SPDX-License-Identifier on wrong >>>>>> line #") >>>>>> > > when the corresponding rule was first commited. >>>>>> > > >>>>>> > > Fix this by setting `$checklicenseline` to `2` when the >>>>>> diff content that >>>>>> > > is being checked originates from a script, thus, informing >>>>>> checkpatch that >>>>>> > > the SPDX license should be on the second line. >>>>>> > > >>>>>> > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> >>>>>> > > --- >>>>>> > > scripts/checkpatch.pl | 3 +++ >>>>>> > > 1 file changed, 3 insertions(+) >>>>>> > > >>>>>> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >>>>>> > > index 4c820607540b..bbffd0c4449d 100755 >>>>>> > > --- a/scripts/checkpatch.pl >>>>>> > > +++ b/scripts/checkpatch.pl >>>>>> > > @@ -3218,6 +3218,9 @@ sub process { >>>>>> > > next if ($realfile !~ >>>>>> /\.(h|c|s|S|sh|dtsi|dts)$/); >>>>>> > > >>>>>> > > # check for using SPDX-License-Identifier on the wrong >>>>>> line number >>>>>> > > + if ($realfile =~ /^scripts/) { >>>>>> > > + $checklicenseline = 2; >>>>>> > > + } >>>>>> > >>>>>> > I think this is somehow wrong here. The check for >>>>>> checklicenseline = 2 >>>>>> > looks very different above. >>>>>> > >>>>>> > Why does -f work and using a patch file not work? >>>>>> > >>>>>> > >>>>>> > Sir, >>>>>> > >>>>>> > I am going to explain my observation based on file >>>>>> `scripts/atomic/gen-atomic-fallback.sh` and >>>>>> > commit hash `37f8173dd849`. >>>>>> > >>>>>> > If we are checking against the file, `checklicenseline` is set to 1 >>>>>> and when `realline` is 1 the above >>>>>> > `if` block is triggered, then we check if this line is of the form >>>>>> `#!/` using the regular expression >>>>>> > `^[ \+]\s*\#\!\s*\/`. If this is the case we set `checklicenseline` >>>>>> to `2` informing checkpatch that it should >>>>>> > expect license on the second line and this works all fine for a >>>>>> file. >>>>>> > The `if` block below my proposed changes evaluates to false in this >>>>>> case and thus it emits no false warning. >>>>>> > >>>>>> > However, If we are checking a diff content, the above `if` block is >>>>>> not triggered at all. This is >>>>>> > because `realline` stores the actual line number of the line we are >>>>>> checking currently out of diff content. >>>>>> > This value is 2 because SPDX identifier is indeed at the second >>>>>> line in the file but `checklicenseline` is still >>>>>> > `1`. >>>>>> > `realline` will never become equal to 1 again and thus the above >>>>>> `if` condition will never be true in this case. >>>>>> > Even if the above `if` block is triggered it would not update >>>>>> `checklicenseline` to 2 as the regular expression >>>>>> > is not satisfied since we don't have sh-bang in diff content and >>>>>> just the SPDX tag. >>>>>> > If we don't do this, the `if` block below evaluates to true when >>>>>> `realline` is 2 and `checklicensline` is `1` >>>>>> > leading >>>>>> > to the emission of a false warning. >>>>>> > >>>>>> >>>>>> So, maybe this whole logic needs to be reworked. If you do not know >>>>>> the >>>>>> first line, you need to have a different criteria in the first place >>>>>> to determine if you expect the license tag in the first or the >>>>>> second, >>>>>> e.g., the file extension, and then checking line 1 for a shebang is >>>>>> just >>>>>> sanity checking. If it is of a specific file extension, you know line >>>>>> 1 >>>>>> and it is not a shebang, that is probably worth noting as a different >>>>>> recommendation in checkpatch.pl anyway. >>>>>> >>>>> >>>>> Sir, >>>>> >>>>> When we know the first line, i.e. we are running checkpatch against a >>>>> file, the existing logic >>>>> works fine. We probably don't want to induce any changes there. >>>>> >>>>> >>>> Why not? Do you think we would break things there? Then we should not >>>> touch the code at all. >>>> Do you think we cannot test it properly after the change? Then we >>>> should think about how we make a proper regression test suite for that. >>>> >>> >>> Sir, >>> >>> No, breaking code or not being able to test is not why I suggest this. I >>> feel that the existing logic handles the case of >>> "Improper or malformed SPDX tag" and "Misplaced SPDX tag" for files i.e. >>> when the first line is known. Anyway, the logic >>> for "Misplaced SPDX tag" is written as a different rule. We just need to >>> add in the logic for patches there. >>> I tried to do this by checking for the scripts directory which was >>> wrong. If I check instead for the file being a script that would make much >>> more sense. >>> Please let me know if you suggest something else. >>> >> >> Well, you are going to add a different way of checking as you suggested, >> right? So are you suggesting to have two duplicated ways of checking for >> the same thing? That seems strange to me. >> > >> Go ahead, make a suitable first proposal, then we will see if and how to >> refactor. >> > > Sir, > > No, I would not want to duplicate things. Yes, let me first send a patch > to you first, and then we can refactor it if needed. > >> >> >>> >>>> But when we don't know the first line, if am not wrong, it would go >>>>> somewhat like: >>>>> if (the file is a script) { >>>>> if (the first line is shebang) { >>>>> if (the second line is SPDX) { >>>>> All good >>>>> } else { >>>>> Issue a misplaced or missing SPDX tag warning >>>>> } >>>>> } else { >>>>> Issue a missing shebang warning >>>>> } >>>>> } else { >>>>> if (the first line is SPDX) { >>>>> All good >>>>> } else { >>>>> Issue a misplaced or missing SPDX tag warning >>>>> } >>>>> } >>>>> >>>>> >>>> Basically agree, but that logic applies when you know the first line as >>>> well (and only, right?). What if you do not know the first line, how would >>>> you check "the first line is shebang" if you do not know the first line? >>>> >>>> >>>> The missing shebang warning probably needs to go elsewhere in the whole >>>> script. >>>> >>> >>> By not knowing the first line I mean to say that the first line doesn't >>> show up in diff content of the patch but >>> what if we open the file at that point in the commit history and check >>> for the first line to be a shebang? >>> Would it be okay to do that? Once we check the first line we can then >>> continue as suggested. >>> >> >> I think that is essentially against the general design decision of >> checkpatch.pl; checkpatch.pl takes the patch but it does not check >> anything in the current working tree, nor does it know what the "parent" of >> that patch in the git history really is. >> >> Maybe Shuah can confirm? Otherwise, I suggest to look if you find >> checking a file beyond the patch happening anywhere in the current code of >> checkpatch.pl and documented in the sparse documentation on checkpatch.pl >> . >> >> So, I believe you need to make it work without checking the first line >> (if that is not in the patch). >> > > Yes. We can't open a file for checking, you are correct here but we need > to check for shebang to be on the first line only if it appears > in the diff content or when we check a complete file, otherwise, we should > anyway not chek it since checkpatch only checks the patch or the complete > file. > Am I correct here? > >> >> >>> >>>>>> > So, what I did was to check if the diff content we are checking >>>>>> actually comes from a script, if yes, we can set >>>>>> > `checklicenseline` to `2` to avoid this confusion. >>>>>> > >>>>>> >>>>>> Why would you think that scripts are only in scripts? >>>>>> >>>>>> How about first listing all files where the SPDX tag is in line 2 in >>>>>> the >>>>>> current repository, e.g., v5.8-rc5? >>>>>> >>>>>> Then, we look at that list and determine a suitable criteria for >>>>>> looking >>>>>> in line 2 for the SPDX tag. >>>>>> >>>>> >>>>> Yes, the scripts are not only in scripts. I have listed all the files >>>>> where the SPDX tag should be >>>>> on the second line. I've attached the list for reference. We should >>>>> probably be checking the file >>>>> extension to determine if the tag needs to be on the second line or >>>>> not. >>>>> The documentation says the SPDX tag should be present in all source >>>>> files. Do these source files include >>>>> Documentation files too? >>>>> >>>>> >>>> How did you create that list? >>>> Agree (if the way you created that list makes sense). File extension >>>> seems to cover all cases, and checking for the directory 'scripts' does not. >>>> >>>> I issued the command `find . -regex ".*\.\(py\|sh\|pl\)"` to make this >>> list. I should have included awk, YAML and tc files too since they are >>> scripts too. >>> >>> >> Why not look for all files that have a shebang in the first line? >> > > We could be checking a file or a patch. I suggest we take the name of the > file we are checking(or the name of the file from which the diff content > comes from) > and then run a regex, similar to above, on it to determine if it is a > script. If we instead go for checking the first line to be shebang in the > current file would it not require to cat or open > the file? > All good. Create a patch and then we will see. Lukas > [-- Attachment #1.2: Type: text/html, Size: 17842 bytes --] [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts
@ 2020-07-21 5:44 Mrinal Pandey
2020-07-22 10:20 ` Lukas Bulwahn
0 siblings, 1 reply; 14+ messages in thread
From: Mrinal Pandey @ 2020-07-21 5:44 UTC (permalink / raw)
To: lukas.bulwahn, skhan, Linux-kernel-mentees
[-- Attachment #1.1: Type: text/plain, Size: 2122 bytes --]
In all the scripts, the SPDX license should be on the second line,
the first line being the shebang, but checkpatch issues a warning
"Misplaced SPDX-License-Identifier tag - use line 1 instead" for the
scripts that have SPDX license in the second line.
However, this warning is not issued when checkpatch is run on a file.
The case for files has been handled gracefully by checking first line of
the file to be a shebang and then setting `$checklicenseline` to `2`but
this doesn't work when we don't have shebang in diff content of a patch
and `$checklicenseline` continues to be `1` in such cases. Therefore,
checkpatch expects the line `1` to contain the SPDX license when it
should have been `2` 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 commits which modified
a script file.
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 commited.
Fix this by setting `$checklicenseline` to `2` whenever the file or diff
content we are checking comes from a script instead of checking first
line to be a shebang, thus, informing checkpatch that the SPDX license
should be expected on the second line.
Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com>
---
scripts/checkpatch.pl | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4c820607540b..bdd2f9a80891 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3166,10 +3166,11 @@ sub process {
}
# check for using SPDX license tag at beginning of files
+ if ($realfile =~ /.*\.\(py\|sh\|pl\|awk\|tc\|yaml\)/) {
+ $checklicenseline = 2;
+ }
if ($realline == $checklicenseline) {
- if ($rawline =~ /^[ \+]\s*\#\!\s*\//) {
- $checklicenseline = 2;
- } elsif ($rawline =~ /^\+/) {
+ if ($rawline =~ /^\+/) {
my $comment = "";
if ($realfile =~ /\.(h|s|S)$/) {
$comment = '/*';
--
2.25.1
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 201 bytes --]
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-21 5:44 Mrinal Pandey @ 2020-07-22 10:20 ` Lukas Bulwahn 2020-07-24 7:02 ` Mrinal Pandey 0 siblings, 1 reply; 14+ messages in thread From: Lukas Bulwahn @ 2020-07-22 10:20 UTC (permalink / raw) To: Mrinal Pandey; +Cc: Linux-kernel-mentees On Tue, 21 Jul 2020, Mrinal Pandey wrote: > In all the scripts, the SPDX license should be on the second line, > the first line being the shebang, but checkpatch issues a warning > "Misplaced SPDX-License-Identifier tag - use line 1 instead" for the > scripts that have SPDX license in the second line. > That what you wrote is actually wrong. Checkpatch only wrongly warns if the first line is not included in the diff, right? Otherwise, it does not warn as intended. > However, this warning is not issued when checkpatch is run on a file. > The case for files has been handled gracefully by checking first line of > the file to be a shebang and then setting `$checklicenseline` to `2`but > this doesn't work when we don't have shebang in diff content of a patch > and `$checklicenseline` continues to be `1` in such cases. Therefore, > checkpatch expects the line `1` to contain the SPDX license when it > should have been `2` instead. > It is described very complicated here. Maybe think about rephrasing it, so it becomes more clear. > 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 commits which modified > a script file. > How about naming the commits and providing a statistics how often that happens? > 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 commited. > > Fix this by setting `$checklicenseline` to `2` whenever the file or diff > content we are checking comes from a script instead of checking first > line to be a shebang, thus, informing checkpatch that the SPDX license > should be expected on the second line. > Well, now the critical part: It seems that this now makes the situation worse compared to before. I quickly ran this evaluation: $ cat first-line.sh #!/bin/bash for file in $(git ls-files) do firstline="$(head --silent -n 1 $file)" echo "$file: $firstline" done $ sh ./first-line.sh > first-lines $ grep "#!" first-lines | wc -l 810 $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | grep -v "\." $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | grep -v "\." | wc -l 120 $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | rev | cut -d"/" -f1 | rev | grep "\." | cut -d"." -f2 | sort | uniq -c | sort -nr 509 sh 91 tc 45 pl 37 py 6 awk 1 config So, in 120 cases you actually would now warn on line 1 where the should be a shebang as intended. Also, I cross-checked .yaml files: $ grep "\.yaml:" first-lines | grep "SPDX-License-Identifier" | wc -l 1035 $ grep "\.yaml:" first-lines | wc -l 1037 So, for yaml, SPDX is actually in line 1. I guess you need to think about this a bit more. > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> > --- > scripts/checkpatch.pl | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 4c820607540b..bdd2f9a80891 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -3166,10 +3166,11 @@ sub process { > } > > # check for using SPDX license tag at beginning of files > + if ($realfile =~ /.*\.\(py\|sh\|pl\|awk\|tc\|yaml\)/) { > + $checklicenseline = 2; > + } > if ($realline == $checklicenseline) { > - if ($rawline =~ /^[ \+]\s*\#\!\s*\//) { > - $checklicenseline = 2; > - } elsif ($rawline =~ /^\+/) { > + if ($rawline =~ /^\+/) { > my $comment = ""; > if ($realfile =~ /\.(h|s|S)$/) { > $comment = '/*'; > -- > 2.25.1 > > _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-22 10:20 ` Lukas Bulwahn @ 2020-07-24 7:02 ` Mrinal Pandey 2020-07-24 8:09 ` Lukas Bulwahn 0 siblings, 1 reply; 14+ messages in thread From: Mrinal Pandey @ 2020-07-24 7:02 UTC (permalink / raw) To: Lukas Bulwahn, Shuah Khan, Linux-kernel-mentees [-- Attachment #1.1: Type: text/plain, Size: 4885 bytes --] On Wed, Jul 22, 2020 at 3:51 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > On Tue, 21 Jul 2020, Mrinal Pandey wrote: > > > In all the scripts, the SPDX license should be on the second line, > > the first line being the shebang, but checkpatch issues a warning > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" for the > > scripts that have SPDX license in the second line. > > > > That what you wrote is actually wrong. Checkpatch only wrongly warns if > the first line is not included in the diff, right? Otherwise, it does not > warn as intended. > Sir, Yes. I would modify the description accordingly. > > > However, this warning is not issued when checkpatch is run on a file. > > The case for files has been handled gracefully by checking first line of > > the file to be a shebang and then setting `$checklicenseline` to `2`but > > this doesn't work when we don't have shebang in diff content of a patch > > and `$checklicenseline` continues to be `1` in such cases. Therefore, > > checkpatch expects the line `1` to contain the SPDX license when it > > should have been `2` instead. > > > > It is described very complicated here. Maybe think about rephrasing it, so > it becomes more clear. > > > 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 commits which modified > > a script file. > > > > How about naming the commits and providing a statistics how often that > happens? > > > 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 commited. > > > > Fix this by setting `$checklicenseline` to `2` whenever the file or diff > > content we are checking comes from a script instead of checking first > > line to be a shebang, thus, informing checkpatch that the SPDX license > > should be expected on the second line. > > > > Well, now the critical part: > > It seems that this now makes the situation worse compared to before. > > I quickly ran this evaluation: > > $ cat first-line.sh > #!/bin/bash > > for file in $(git ls-files) > do > firstline="$(head --silent -n 1 $file)" > echo "$file: $firstline" > done > > $ sh ./first-line.sh > first-lines > > $ grep "#!" first-lines | wc -l > 810 > > $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | grep -v "\." > > $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | grep -v "\." | wc > -l > 120 > > $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | rev | cut -d"/" > -f1 | rev | grep "\." | cut -d"." -f2 | sort | uniq -c | sort -nr > 509 sh > 91 tc > 45 pl > 37 py > 6 awk > 1 config > > So, in 120 cases you actually would now warn on line 1 where the should be > a shebang as intended. > > Also, I cross-checked .yaml files: > > $ grep "\.yaml:" first-lines | grep "SPDX-License-Identifier" | wc -l > 1035 > > $ grep "\.yaml:" first-lines | wc -l > 1037 > > > So, for yaml, SPDX is actually in line 1. > Yes, I should remove the `yaml` files from this check. > > > I guess you need to think about this a bit more. > If I am correct now we have an issue with all those files which are scripts without an extension. In this case, the earlier logic which I removed was working fine(now I get an idea of why it was like that in the first place). I think the best way is to keep both my suggested change and the logic that already exists so that we can at least avoid those false positives when we know the extension without essentially duplicating the logic. Checking for files with no extension, no shebang and only SPDX in diff is tricky since we can't actually open the file. Please let me know your views. Thank you. > > > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> > > --- > > scripts/checkpatch.pl | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 4c820607540b..bdd2f9a80891 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -3166,10 +3166,11 @@ sub process { > > } > > > > # check for using SPDX license tag at beginning of files > > + if ($realfile =~ /.*\.\(py\|sh\|pl\|awk\|tc\|yaml\)/) { > > + $checklicenseline = 2; > > + } > > if ($realline == $checklicenseline) { > > - if ($rawline =~ /^[ \+]\s*\#\!\s*\//) { > > - $checklicenseline = 2; > > - } elsif ($rawline =~ /^\+/) { > > + if ($rawline =~ /^\+/) { > > my $comment = ""; > > if ($realfile =~ /\.(h|s|S)$/) { > > $comment = '/*'; > > -- > > 2.25.1 > > > > > [-- Attachment #1.2: Type: text/html, Size: 7288 bytes --] [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts 2020-07-24 7:02 ` Mrinal Pandey @ 2020-07-24 8:09 ` Lukas Bulwahn 0 siblings, 0 replies; 14+ messages in thread From: Lukas Bulwahn @ 2020-07-24 8:09 UTC (permalink / raw) To: Mrinal Pandey; +Cc: Linux-kernel-mentees [-- Attachment #1: Type: text/plain, Size: 6305 bytes --] On Fri, 24 Jul 2020, Mrinal Pandey wrote: > > > On Wed, Jul 22, 2020 at 3:51 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote: > > > On Tue, 21 Jul 2020, Mrinal Pandey wrote: > > > In all the scripts, the SPDX license should be on the second line, > > the first line being the shebang, but checkpatch issues a warning > > "Misplaced SPDX-License-Identifier tag - use line 1 instead" for the > > scripts that have SPDX license in the second line. > > > > That what you wrote is actually wrong. Checkpatch only wrongly warns if > the first line is not included in the diff, right? Otherwise, it does not > warn as intended. > > > Sir, > > Yes. I would modify the description accordingly. > > > However, this warning is not issued when checkpatch is run on a file. > > The case for files has been handled gracefully by checking first line of > > the file to be a shebang and then setting `$checklicenseline` to `2`but > > this doesn't work when we don't have shebang in diff content of a patch > > and `$checklicenseline` continues to be `1` in such cases. Therefore, > > checkpatch expects the line `1` to contain the SPDX license when it > > should have been `2` instead. > > > > It is described very complicated here. Maybe think about rephrasing it, so > it becomes more clear. > > > > 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 commits which modified > > a script file. > > > > How about naming the commits and providing a statistics how often that > happens? > > > 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 commited. > > > > Fix this by setting `$checklicenseline` to `2` whenever the file or diff > > content we are checking comes from a script instead of checking first > > line to be a shebang, thus, informing checkpatch that the SPDX license > > should be expected on the second line. > > > > Well, now the critical part: > > It seems that this now makes the situation worse compared to before. > > I quickly ran this evaluation: > > $ cat first-line.sh > #!/bin/bash > > for file in $(git ls-files) > do > firstline="$(head --silent -n 1 $file)" > echo "$file: $firstline" > done > > $ sh ./first-line.sh > first-lines > > $ grep "#!" first-lines | wc -l > 810 > > $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | grep -v "\." > > $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | grep -v "\." | wc > -l > 120 > > $ grep "#!" first-lines | sed 's/^\(.*[^\]*\):.*$/\1/' | rev | cut -d"/" > -f1 | rev | grep "\." | cut -d"." -f2 | sort | uniq -c | sort -nr > 509 sh > 91 tc > 45 pl > 37 py > 6 awk > 1 config > > So, in 120 cases you actually would now warn on line 1 where the should be > a shebang as intended. > > Also, I cross-checked .yaml files: > > $ grep "\.yaml:" first-lines | grep "SPDX-License-Identifier" | wc -l > 1035 > > $ grep "\.yaml:" first-lines | wc -l > 1037 > > > So, for yaml, SPDX is actually in line 1. > > > Yes, I should remove the `yaml` files from this check. > > > I guess you need to think about this a bit more. > > > If I am correct now we have an issue with all those files which are scripts without an extension. Yes, correct. > In this case, the earlier logic which I removed was working fine(now I get an idea of why it was like > that in the first place). I think the best way is to keep both my suggested change and the logic that > already exists so that we can at least avoid those false positives when we know the extension without > essentially duplicating the logic. You need to combine the previous heuristic with your further extension, making it overall better and try to keep the logic checking as simple as possible. If the first line is there and a shebang, use it to make a decision. If the first line is not available, use another heuristic. > Checking for files with no extension, no shebang and only SPDX in diff is tricky since we can't actually open the file. You cannot open the file, but if a first line is available, you need to use that information. I suggest that you evaluate your patch on a set of use cases with a clear report what was reported before and after applying your patch. Lukas > Please let me know your views. > > Thank you. > > > > Signed-off-by: Mrinal Pandey <mrinalmni@gmail.com> > > --- > > scripts/checkpatch.pl | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 4c820607540b..bdd2f9a80891 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -3166,10 +3166,11 @@ sub process { > > } > > > > # check for using SPDX license tag at beginning of files > > + if ($realfile =~ /.*\.\(py\|sh\|pl\|awk\|tc\|yaml\)/) { > > + $checklicenseline = 2; > > + } > > if ($realline == $checklicenseline) { > > - if ($rawline =~ /^[ \+]\s*\#\!\s*\//) { > > - $checklicenseline = 2; > > - } elsif ($rawline =~ /^\+/) { > > + if ($rawline =~ /^\+/) { > > my $comment = ""; > > if ($realfile =~ /\.(h|s|S)$/) { > > $comment = '/*'; > > -- > > 2.25.1 > > > > > > > [-- Attachment #2: Type: text/plain, Size: 201 bytes --] _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-07-24 8:09 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-13 9:57 [Linux-kernel-mentees] [PATCH] checkpatch: Fix SPDX license check for scripts Mrinal Pandey 2020-07-13 19:46 ` Lukas Bulwahn 2020-07-14 5:35 ` Mrinal Pandey 2020-07-14 6:03 ` Lukas Bulwahn 2020-07-16 5:15 ` Mrinal Pandey 2020-07-16 5:31 ` Lukas Bulwahn 2020-07-17 9:54 ` Mrinal Pandey 2020-07-17 11:47 ` Lukas Bulwahn 2020-07-19 6:27 ` Mrinal Pandey 2020-07-19 7:13 ` Lukas Bulwahn 2020-07-21 5:44 Mrinal Pandey 2020-07-22 10:20 ` Lukas Bulwahn 2020-07-24 7:02 ` Mrinal Pandey 2020-07-24 8:09 ` Lukas Bulwahn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).