[PATCH v2 1/7] Revert "scripts: mandate that new files have SPDX-License-Identifier"

Daniel P. Berrangé posted 7 patches 6 months, 1 week ago
[PATCH v2 1/7] Revert "scripts: mandate that new files have SPDX-License-Identifier"
Posted by Daniel P. Berrangé 6 months, 1 week ago
From: Daniel P. Berrangé <berrange@redhat.com>

This reverts commit fa4d79c64dae03ffa269e42e21822453856618b7.

The logic in this commit was flawed in two critical ways

 * It always failed to report SPDX validation on the last newly
   added file. IOW, it only worked if at least 2 new files were
   added in a commit

 * If an existing file change, followed a new file change, in
   the commit and the existing file context/changed lines
   included SPDX-License-Identifier, it would incorrectly
   associate this with the previous newly added file.

Simply reverting this commit will make it significantly easier to
understand the improved logic in the following commit.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/checkpatch.pl | 30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 365892de04..d355c0dad5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1442,8 +1442,6 @@ sub process {
 	my $in_imported_file = 0;
 	my $in_no_imported_file = 0;
 	my $non_utf8_charset = 0;
-	my $expect_spdx = 0;
-	my $expect_spdx_file;
 
 	our @report = ();
 	our $cnt_lines = 0;
@@ -1681,34 +1679,6 @@ sub process {
 			WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
 		}
 
-# All new files should have a SPDX-License-Identifier tag
-		if ($line =~ /^new file mode\s*\d+\s*$/) {
-		    if ($expect_spdx) {
-			if ($expect_spdx_file =~
-			    /\.(c|h|py|pl|sh|json|inc|Makefile)$/) {
-			    # source code files MUST have SPDX license declared
-			    ERROR("New file '$expect_spdx_file' requires " .
-				  "'SPDX-License-Identifier'");
-			} else {
-			    # Other files MAY have SPDX license if appropriate
-			    WARN("Does new file '$expect_spdx_file' need " .
-				 "'SPDX-License-Identifier'?");
-			}
-		    }
-		    $expect_spdx = 1;
-		    $expect_spdx_file = undef;
-		} elsif ($expect_spdx) {
-		    $expect_spdx_file = $realfile unless
-			defined $expect_spdx_file;
-
-		    # SPDX tags may occurr in comments which were
-		    # stripped from '$line', so use '$rawline'
-		    if ($rawline =~ /SPDX-License-Identifier/) {
-			$expect_spdx = 0;
-			$expect_spdx_file = undef;
-		    }
-		}
-
 # Check SPDX-License-Identifier references a permitted license
 		if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) {
 		    &checkspdx($realfile, $1);
-- 
2.49.0


Re: [PATCH v2 1/7] Revert "scripts: mandate that new files have 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>
> 
> This reverts commit fa4d79c64dae03ffa269e42e21822453856618b7.
> 
> The logic in this commit was flawed in two critical ways
> 
>   * It always failed to report SPDX validation on the last newly
>     added file. IOW, it only worked if at least 2 new files were
>     added in a commit
> 
>   * If an existing file change, followed a new file change, in
>     the commit and the existing file context/changed lines
>     included SPDX-License-Identifier, it would incorrectly
>     associate this with the previous newly added file.
> 
> Simply reverting this commit will make it significantly easier to
> understand the improved logic in the following commit.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>


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

Thanks,

C.


> ---
>   scripts/checkpatch.pl | 30 ------------------------------
>   1 file changed, 30 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 365892de04..d355c0dad5 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1442,8 +1442,6 @@ sub process {
>   	my $in_imported_file = 0;
>   	my $in_no_imported_file = 0;
>   	my $non_utf8_charset = 0;
> -	my $expect_spdx = 0;
> -	my $expect_spdx_file;
>   
>   	our @report = ();
>   	our $cnt_lines = 0;
> @@ -1681,34 +1679,6 @@ sub process {
>   			WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
>   		}
>   
> -# All new files should have a SPDX-License-Identifier tag
> -		if ($line =~ /^new file mode\s*\d+\s*$/) {
> -		    if ($expect_spdx) {
> -			if ($expect_spdx_file =~
> -			    /\.(c|h|py|pl|sh|json|inc|Makefile)$/) {
> -			    # source code files MUST have SPDX license declared
> -			    ERROR("New file '$expect_spdx_file' requires " .
> -				  "'SPDX-License-Identifier'");
> -			} else {
> -			    # Other files MAY have SPDX license if appropriate
> -			    WARN("Does new file '$expect_spdx_file' need " .
> -				 "'SPDX-License-Identifier'?");
> -			}
> -		    }
> -		    $expect_spdx = 1;
> -		    $expect_spdx_file = undef;
> -		} elsif ($expect_spdx) {
> -		    $expect_spdx_file = $realfile unless
> -			defined $expect_spdx_file;
> -
> -		    # SPDX tags may occurr in comments which were
> -		    # stripped from '$line', so use '$rawline'
> -		    if ($rawline =~ /SPDX-License-Identifier/) {
> -			$expect_spdx = 0;
> -			$expect_spdx_file = undef;
> -		    }
> -		}
> -
>   # Check SPDX-License-Identifier references a permitted license
>   		if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) {
>   		    &checkspdx($realfile, $1);