scripts: checkpatch: Revert SPDX license identifier checks

[stephanosio]

This commit reverts the Linux-style SPDX license identifier checks that
were merged as part of 5b10fac.

These checks require the SPDX license identifier to be placed in the
line 1, which is not the convention used by Zephyr.

Moreover, the Zephyr CI checks the SPDX license identifier in a
separate job, so it is not strictly necessary for the checkpatch to
validate it.

Signed-off-by: Stephanos Ioannidis root@stephanos.io

Superseded by #24930

[stephanosio]

p.s. I marked this a hotfix because CI currently gives the following warnings and this can be both confusing and misleading for many (unless, of course, we are now enforcing the Linux-style 1st line SPDX license identifier placement):

-:22: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#22: FILE: drivers/interrupt_controller/intc_gicv3.c:1:
+/*

-:25: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#25: FILE: drivers/interrupt_controller/intc_gicv3.c:4:
+ * SPDX-License-Identifier: Apache-2.0

[gmarull]

Wouldn't --ignore SPDX_LICENSE_TAG to .checkpatch.conf just ignore the warning?

[stephanosio]

Wouldn't --ignore SPDX_LICENSE_TAG to .checkpatch.conf just ignore the warning?

Yes, that would ignore the warning; but, why do that when we can just remove the source of it? (especially when the checks that generate the warnings are not applicable/compliant to the Zephyr coding style)

AFAIU, it is not our goal to use the Linux checkpatch.pl as is.

[gmarull]

I think it's easier to maintain checkpatch.pl by ignoring non-relevant warnings. Otherwise it will need to be patched every time it is updated from Kernel sources.

[stephanosio]

Otherwise it will need to be patched every time it is updated from Kernel sources.

We do that already. There are many irreconcilable differences between Linux and Zephyr and using the Linux checkpatch as is is already out of question (see checkpatch.pl history).

[gmarull]

Yes, I've seen that, but I understand that a patch would be useful if we change the behavior of the check (e.g. to check that Zephyr sources include SPDX tag at the expected location). If the warning is completely ignored, I think it's more maintainable to just use .checkpatch.conf.

[stephanosio]

I understand that a patch would be useful if we change the behavior of the check (e.g. to check that Zephyr sources include SPDX tag at the expected location)

Sure, feel free to submit a PR if you would like to look into it. I will close this PR in favour of that.

If the warning is completely ignored, I think it's more maintainable to just use .checkpatch.conf.

Sure, it would slightly improve maintainability, but given that merging Zephyr and Linux checkpatch.pl already requires full manual intervention with knowledge of what (not) to be merged, I doubt doing so would make all that much of a difference (TBH, I think it makes more sense to think of Zephyr checkpatch.pl as a separate thing from the Linux equivalent, and we should only take what we actually need from the Linux version).

Also, the point is that this change should not have been merged from the Linux checkpatch.pl in the first place, and this patch is simply reverting those hunks.

[nashif]

also prefer --ignore SPDX_LICENSE_TAG, this avoid conflicts in the future when we try to sync with upstream.

[pabigot]

I wasn't aware this was going to break things, but I do believe that Zephyr's copy of checkpatch should be maintained in a way that deviations from Linux are clearly identified in the file's history.

So it's good that it was added, as the decision to exclude it is documented, either by this commit removing it or by adding an ignore directive.

[stephanosio]

also prefer --ignore SPDX_LICENSE_TAG, this avoid conflicts in the future when we try to sync with upstream.

https://gist.github.com/stephanosio/a721472458e052c187836c51250128c4

I already see a number of differences that would classify as "conflicts" when diffing the Linux and Zephyr checkpatch.pl, and their origin can be clarified only by looking at the checkpatch.pl commit log anyway (i.e. we cannot determine if the differences are due to upstream changes or the Zephyr-specific patches), so I am not sure if that argument is fully valid in this case.

But, this is ultimately up to the maintainer's choice, and I will close this PR in favour of #24930.

p.s. if we intend to keep syncing with the Linux checkpatch.pl, we should really maintain a set of .patch files somewhere, since going through the commit log manually to cherry-pick only the valid and applicable Zephyr-specific patches is not so easy.