[PATCH v2 7/7] scripts/checkpatch: reimplement mandate for SPDX-License-Identifier

Daniel P. Berrangé posted 7 patches 6 months, 1 week ago
[PATCH v2 7/7] scripts/checkpatch: reimplement mandate for SPDX-License-Identifier
Posted by Daniel P. Berrangé 6 months, 1 week ago
From: Daniel P. Berrangé <berrange@redhat.com>

Going forward we want all newly created source files to have an
SPDX-License-Identifier tag present.

Initially mandate this for C, Python, Perl, Shell source files,
as well as JSON (QAPI) and Makefiles, while encouraging users
to consider it for other file types.

The new attempt at detecting missing SPDX-License-Identifier relies
on the hooks for relying triggering logic at the end of scanning a
new file in the diff.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/checkpatch.pl | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d416a6dcf9..95609ca010 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1482,6 +1482,20 @@ sub process_start_of_file {
 # Called at the end of processing a diff hunk for a file
 sub process_end_of_file {
 	my $fileinfo = shift;
+
+	if ($fileinfo->{action} eq "new" &&
+	    !exists $fileinfo->{facts}->{sawspdx}) {
+		if ($fileinfo->{filenew} =~
+		    /\.(c|h|py|pl|sh|json|inc|Makefile)$/) {
+			# source code files MUST have SPDX license declared
+			ERROR("New file '" . $fileinfo->{filenew} .
+			      "' requires 'SPDX-License-Identifier'");
+		} else {
+			# Other files MAY have SPDX license if appropriate
+			WARN("Does new file '" . $fileinfo->{filenew} .
+			     "' need 'SPDX-License-Identifier'?");
+		}
+	}
 }
 
 sub process {
@@ -1780,6 +1794,7 @@ sub process {
 
 # Check SPDX-License-Identifier references a permitted license
 		if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) {
+			$fileinfo->{facts}->{sawspdx} = 1;
 			&checkspdx($realfile, $1);
 		}
 
-- 
2.49.0


Re: [PATCH v2 7/7] scripts/checkpatch: reimplement mandate for SPDX-License-Identifier
Posted by Daniel P. Berrangé 6 months ago
On Mon, May 12, 2025 at 07:24:47PM +0100, Daniel P. Berrangé wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> Going forward we want all newly created source files to have an
> SPDX-License-Identifier tag present.
> 
> Initially mandate this for C, Python, Perl, Shell source files,
> as well as JSON (QAPI) and Makefiles, while encouraging users
> to consider it for other file types.
> 
> The new attempt at detecting missing SPDX-License-Identifier relies
> on the hooks for relying triggering logic at the end of scanning a
> new file in the diff.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  scripts/checkpatch.pl | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index d416a6dcf9..95609ca010 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1482,6 +1482,20 @@ sub process_start_of_file {
>  # Called at the end of processing a diff hunk for a file
>  sub process_end_of_file {
>  	my $fileinfo = shift;
> +
> +	if ($fileinfo->{action} eq "new" &&
> +	    !exists $fileinfo->{facts}->{sawspdx}) {
> +		if ($fileinfo->{filenew} =~
> +		    /\.(c|h|py|pl|sh|json|inc|Makefile)$/) {

Slight tweak needed here as 'Makefile' is a filename base
not an extension:

                /(\.(c|h|py|pl|sh|json|inc)|Makefile.*)$/) {


> +			# source code files MUST have SPDX license declared
> +			ERROR("New file '" . $fileinfo->{filenew} .
> +			      "' requires 'SPDX-License-Identifier'");
> +		} else {
> +			# Other files MAY have SPDX license if appropriate
> +			WARN("Does new file '" . $fileinfo->{filenew} .
> +			     "' need 'SPDX-License-Identifier'?");
> +		}
> +	}
>  }
>  
>  sub process {
> @@ -1780,6 +1794,7 @@ sub process {
>  
>  # Check SPDX-License-Identifier references a permitted license
>  		if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) {
> +			$fileinfo->{facts}->{sawspdx} = 1;
>  			&checkspdx($realfile, $1);
>  		}
>  
> -- 
> 2.49.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v2 7/7] scripts/checkpatch: reimplement mandate for SPDX-License-Identifier
Posted by Cédric Le Goater 6 months ago
On 5/12/25 20:24, Daniel P. Berrangé wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> Going forward we want all newly created source files to have an
> SPDX-License-Identifier tag present.
> 
> Initially mandate this for C, Python, Perl, Shell source files,
> as well as JSON (QAPI) and Makefiles, while encouraging users
> to consider it for other file types.
> 
> The new attempt at detecting missing SPDX-License-Identifier relies
> on the hooks for relying triggering logic at the end of scanning a
> new file in the diff.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   scripts/checkpatch.pl | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index d416a6dcf9..95609ca010 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1482,6 +1482,20 @@ sub process_start_of_file {
>   # Called at the end of processing a diff hunk for a file
>   sub process_end_of_file {
>   	my $fileinfo = shift;
> +
> +	if ($fileinfo->{action} eq "new" &&
> +	    !exists $fileinfo->{facts}->{sawspdx}) {
> +		if ($fileinfo->{filenew} =~
> +		    /\.(c|h|py|pl|sh|json|inc|Makefile)$/) {
> +			# source code files MUST have SPDX license declared
> +			ERROR("New file '" . $fileinfo->{filenew} .
> +			      "' requires 'SPDX-License-Identifier'");
> +		} else {
> +			# Other files MAY have SPDX license if appropriate
> +			WARN("Does new file '" . $fileinfo->{filenew} .
> +			     "' need 'SPDX-License-Identifier'?");
> +		}
> +	}
>   }
>   
>   sub process {
> @@ -1780,6 +1794,7 @@ sub process {
>   
>   # Check SPDX-License-Identifier references a permitted license
>   		if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) {
> +			$fileinfo->{facts}->{sawspdx} = 1;
>   			&checkspdx($realfile, $1);
>   		}
>   

When run on patch :

   https://lore.kernel.org/qemu-devel/20250512091014.3454083-2-kane_chen@aspeedtech.com/

./scripts/checkpatch.pl outputs :
   
   ERROR: New file 'hw/misc/aspeed_otpmem.c' requires 'SPDX-License-Identifier'
   WARNING: line over 80 characters
   #288: FILE: include/hw/misc/aspeed_otpmem.h:22:
   +    void (*prog)(AspeedOTPMemState *s, uint32_t addr, uint32_t val, Error **errp);
   
   WARNING: line over 80 characters
   #289: FILE: include/hw/misc/aspeed_otpmem.h:23:
   +    void (*set_default)(AspeedOTPMemState *s, uint32_t addr, uint32_t val, Error **errp);
   
   ERROR: New file 'include/hw/misc/aspeed_otpmem.h' requires 'SPDX-License-Identifier'
   WARNING: added, moved or deleted file(s):
   
     hw/misc/aspeed_otpmem.c
     include/hw/misc/aspeed_otpmem.h
   
   Does MAINTAINERS need updating?
   
   total: 2 errors, 3 warnings, 255 lines checked
   
Looks good.


Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.