[PATCH v3 9/9] scripts/checkpatch: reject license boilerplate on new files

Daniel P. Berrangé posted 9 patches 7 months ago
There is a newer version of this series
[PATCH v3 9/9] scripts/checkpatch: reject license boilerplate on new files
Posted by Daniel P. Berrangé 7 months ago
The previous commit mandates use of SPDX-License-Identifier on common
source files, and encourages it on all other files.

Some contributors are none the less still also including the license
boilerplate text. This is redundant and will potentially cause
trouble if inconsistent with the SPDX declaration.

Match common boilerplate text blurbs and report them as invalid,
for newly added files.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 87050e6677..cb1942c021 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1496,6 +1496,13 @@ sub process_end_of_file {
 			     "' need 'SPDX-License-Identifier'?");
 		}
 	}
+	if ($fileinfo->{action} eq "new" &&
+	    !exists $fileinfo->{facts}->{sawboilerplate}) {
+		ERROR("New file '" . $fileinfo->{filenew} . "' must " .
+		      "not have license boilerplate header text unless " .
+		      "this file is copied from existing code with such " .
+		      "text already present.");
+	}
 }
 
 sub process {
@@ -1798,6 +1805,15 @@ sub process {
 			&checkspdx($realfile, $1);
 		}
 
+		if ($rawline =~ /licensed under the terms of the GNU GPL/ ||
+		    $rawline =~ /under the terms of the GNU General Public License/ ||
+		    $rawline =~ /under the terms of the GNU Lesser General Public/ ||
+		    $rawline =~ /Permission is hereby granted, free of charge/ ||
+		    $rawline =~ /GNU GPL, version 2 or later/ ||
+		    $rawline =~ /See the COPYING file/) {
+			$fileinfo->{facts}->{sawboilerplate} = 1;
+		}
+
 		if ($rawline =~ m,(SPDX-[a-zA-Z0-9-_]+):,) {
 			my $tag = $1;
 			my @permitted = qw(
-- 
2.49.0


Re: [PATCH v3 9/9] scripts/checkpatch: reject license boilerplate on new files
Posted by Peter Maydell 7 months ago
On Thu, 15 May 2025 at 15:00, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The previous commit mandates use of SPDX-License-Identifier on common
> source files, and encourages it on all other files.
>
> Some contributors are none the less still also including the license
> boilerplate text. This is redundant and will potentially cause
> trouble if inconsistent with the SPDX declaration.
>
> Match common boilerplate text blurbs and report them as invalid,
> for newly added files.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  scripts/checkpatch.pl | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 87050e6677..cb1942c021 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1496,6 +1496,13 @@ sub process_end_of_file {
>                              "' need 'SPDX-License-Identifier'?");
>                 }
>         }
> +       if ($fileinfo->{action} eq "new" &&
> +           !exists $fileinfo->{facts}->{sawboilerplate}) {
> +               ERROR("New file '" . $fileinfo->{filenew} . "' must " .
> +                     "not have license boilerplate header text unless " .
> +                     "this file is copied from existing code with such " .
> +                     "text already present.");

Should we also say here
  The SPDX-License-Identifier line is sufficient.

(just to make it clear why we're not allowing the boilerplate text) ?

> +       }
>  }
>
>  sub process {
> @@ -1798,6 +1805,15 @@ sub process {
>                         &checkspdx($realfile, $1);
>                 }
>
> +               if ($rawline =~ /licensed under the terms of the GNU GPL/ ||
> +                   $rawline =~ /under the terms of the GNU General Public License/ ||
> +                   $rawline =~ /under the terms of the GNU Lesser General Public/ ||
> +                   $rawline =~ /Permission is hereby granted, free of charge/ ||
> +                   $rawline =~ /GNU GPL, version 2 or later/ ||
> +                   $rawline =~ /See the COPYING file/) {
> +                       $fileinfo->{facts}->{sawboilerplate} = 1;
> +               }
> +

We could perhaps pull this out into a top level variable, similar
to how the script pre-defines some other long regexes it wants to
match against (untested code):

# Match text found in common license boilerplate comments:
# for new files the SPDX-License-Identifier line is sufficient.
our $LICENSE_BOILERPLATE = qr{
    licensed under the terms of the GNU GPL|
    under the terms of the GNU General Public License|
    under the terms of the GNU Lesser General Public|
    Permission is hereby granted, free of charge|
    GNU GPL, version 2 or later|
    See the COPYING file
}x;

and then
    if ($rawline =~ /$LICENSE_BOILERPLATE/) {

This seems to me a little better than having it all inline
in the already rather large process() function.

But either way
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Re: [PATCH v3 9/9] scripts/checkpatch: reject license boilerplate on new files
Posted by Daniel P. Berrangé 7 months ago
On Thu, May 15, 2025 at 02:59:36PM +0100, Daniel P. Berrangé wrote:
> The previous commit mandates use of SPDX-License-Identifier on common
> source files, and encourages it on all other files.
> 
> Some contributors are none the less still also including the license
> boilerplate text. This is redundant and will potentially cause
> trouble if inconsistent with the SPDX declaration.
> 
> Match common boilerplate text blurbs and report them as invalid,
> for newly added files.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  scripts/checkpatch.pl | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 87050e6677..cb1942c021 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1496,6 +1496,13 @@ sub process_end_of_file {
>  			     "' need 'SPDX-License-Identifier'?");
>  		}
>  	}
> +	if ($fileinfo->{action} eq "new" &&
> +	    !exists $fileinfo->{facts}->{sawboilerplate}) {

/face-palm  - I forgot to remove the '!' here so the check is
inverted and my test patch had two very similarly named files
so didn't notice it :-(

> +		ERROR("New file '" . $fileinfo->{filenew} . "' must " .
> +		      "not have license boilerplate header text unless " .
> +		      "this file is copied from existing code with such " .
> +		      "text already present.");
> +	}
>  }
>  
>  sub process {
> @@ -1798,6 +1805,15 @@ sub process {
>  			&checkspdx($realfile, $1);
>  		}
>  
> +		if ($rawline =~ /licensed under the terms of the GNU GPL/ ||
> +		    $rawline =~ /under the terms of the GNU General Public License/ ||
> +		    $rawline =~ /under the terms of the GNU Lesser General Public/ ||
> +		    $rawline =~ /Permission is hereby granted, free of charge/ ||
> +		    $rawline =~ /GNU GPL, version 2 or later/ ||
> +		    $rawline =~ /See the COPYING file/) {
> +			$fileinfo->{facts}->{sawboilerplate} = 1;
> +		}
> +
>  		if ($rawline =~ m,(SPDX-[a-zA-Z0-9-_]+):,) {
>  			my $tag = $1;
>  			my @permitted = qw(
> -- 
> 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 v3 9/9] scripts/checkpatch: reject license boilerplate on new files
Posted by Cédric Le Goater 7 months ago
On 5/15/25 18:05, Daniel P. Berrangé wrote:
> On Thu, May 15, 2025 at 02:59:36PM +0100, Daniel P. Berrangé wrote:
>> The previous commit mandates use of SPDX-License-Identifier on common
>> source files, and encourages it on all other files.
>>
>> Some contributors are none the less still also including the license
>> boilerplate text. This is redundant and will potentially cause
>> trouble if inconsistent with the SPDX declaration.
>>
>> Match common boilerplate text blurbs and report them as invalid,
>> for newly added files.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   scripts/checkpatch.pl | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 87050e6677..cb1942c021 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1496,6 +1496,13 @@ sub process_end_of_file {
>>   			     "' need 'SPDX-License-Identifier'?");
>>   		}
>>   	}
>> +	if ($fileinfo->{action} eq "new" &&
>> +	    !exists $fileinfo->{facts}->{sawboilerplate}) {
> 
> /face-palm  - I forgot to remove the '!' here so the check is
> inverted and my test patch had two very similarly named files
> so didn't notice it :-(

Oh. I didn't see. No need to resend. I will fix in my tree.


Thanks,

C.




> 
>> +		ERROR("New file '" . $fileinfo->{filenew} . "' must " .
>> +		      "not have license boilerplate header text unless " .
>> +		      "this file is copied from existing code with such " .
>> +		      "text already present.");
>> +	}
>>   }
>>   
>>   sub process {
>> @@ -1798,6 +1805,15 @@ sub process {
>>   			&checkspdx($realfile, $1);
>>   		}
>>   
>> +		if ($rawline =~ /licensed under the terms of the GNU GPL/ ||
>> +		    $rawline =~ /under the terms of the GNU General Public License/ ||
>> +		    $rawline =~ /under the terms of the GNU Lesser General Public/ ||
>> +		    $rawline =~ /Permission is hereby granted, free of charge/ ||
>> +		    $rawline =~ /GNU GPL, version 2 or later/ ||
>> +		    $rawline =~ /See the COPYING file/) {
>> +			$fileinfo->{facts}->{sawboilerplate} = 1;
>> +		}
>> +
>>   		if ($rawline =~ m,(SPDX-[a-zA-Z0-9-_]+):,) {
>>   			my $tag = $1;
>>   			my @permitted = qw(
>> -- 
>> 2.49.0
>>
> 
> With regards,
> Daniel


Re: [PATCH v3 9/9] scripts/checkpatch: reject license boilerplate on new files
Posted by Cédric Le Goater 7 months ago
On 5/15/25 18:06, Cédric Le Goater wrote:
> On 5/15/25 18:05, Daniel P. Berrangé wrote:
>> On Thu, May 15, 2025 at 02:59:36PM +0100, Daniel P. Berrangé wrote:
>>> The previous commit mandates use of SPDX-License-Identifier on common
>>> source files, and encourages it on all other files.
>>>
>>> Some contributors are none the less still also including the license
>>> boilerplate text. This is redundant and will potentially cause
>>> trouble if inconsistent with the SPDX declaration.
>>>
>>> Match common boilerplate text blurbs and report them as invalid,
>>> for newly added files.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
>>>   scripts/checkpatch.pl | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index 87050e6677..cb1942c021 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -1496,6 +1496,13 @@ sub process_end_of_file {
>>>                    "' need 'SPDX-License-Identifier'?");
>>>           }
>>>       }
>>> +    if ($fileinfo->{action} eq "new" &&
>>> +        !exists $fileinfo->{facts}->{sawboilerplate}) {
>>
>> /face-palm  - I forgot to remove the '!' here so the check is
>> inverted and my test patch had two very similarly named files
>> so didn't notice it :-(
> 
> Oh. I didn't see. No need to resend. I will fix in my tree.

It is now catching valid errors on :


* https://lore.kernel.org/qemu-devel/20250512180230.50129-5-rreyes@linux.ibm.com/
   ERROR: New file 'hw/s390x/ap-stub.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
   total: 1 errors, 0 warnings, 67 lines checked

* https://lore.kernel.org/qemu-devel/1747063973-124548-7-git-send-email-steven.sistare@oracle.com/
   ERROR: New file 'hw/vfio/cpr-legacy.c' requires 'SPDX-License-Identifier'
   ERROR: New file 'hw/vfio/cpr-legacy.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
   WARNING: added, moved or deleted file(s):

* https://lore.kernel.org/qemu-devel/1747063973-124548-36-git-send-email-steven.sistare@oracle.com/
   ERROR: New file 'hw/vfio/cpr-iommufd.c' requires 'SPDX-License-Identifier'
   ERROR: New file 'hw/vfio/cpr-iommufd.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
   WARNING: added, moved or deleted file(s):
   
     hw/vfio/cpr-iommufd.c
   
   Does MAINTAINERS need updating?
   
   total: 2 errors, 1 warnings, 161 lines checked

* https://lore.kernel.org/qemu-devel/20250515154413.210315-1-john.levon@nutanix.com
   ERROR: New file 'hw/vfio-user/container.h' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
   ERROR: New file 'hw/vfio-user/container.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
   ERROR: New file 'hw/vfio-user/pci.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
   WARNING: Does new file 'hw/vfio-user/meson.build' need 'SPDX-License-Identifier'?
   total: 3 errors, 1 warnings, 490 lines checked


and more.


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

Thanks,

C.


Re: [PATCH v3 9/9] scripts/checkpatch: reject license boilerplate on new files
Posted by Daniel P. Berrangé 7 months ago
On Thu, May 15, 2025 at 06:19:45PM +0200, Cédric Le Goater wrote:
> On 5/15/25 18:06, Cédric Le Goater wrote:
> > On 5/15/25 18:05, Daniel P. Berrangé wrote:
> > > On Thu, May 15, 2025 at 02:59:36PM +0100, Daniel P. Berrangé wrote:
> > > > The previous commit mandates use of SPDX-License-Identifier on common
> > > > source files, and encourages it on all other files.
> > > > 
> > > > Some contributors are none the less still also including the license
> > > > boilerplate text. This is redundant and will potentially cause
> > > > trouble if inconsistent with the SPDX declaration.
> > > > 
> > > > Match common boilerplate text blurbs and report them as invalid,
> > > > for newly added files.
> > > > 
> > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > ---
> > > >   scripts/checkpatch.pl | 16 ++++++++++++++++
> > > >   1 file changed, 16 insertions(+)


> It is now catching valid errors on :
> 
> 
> * https://lore.kernel.org/qemu-devel/20250512180230.50129-5-rreyes@linux.ibm.com/
>   ERROR: New file 'hw/s390x/ap-stub.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
>   total: 1 errors, 0 warnings, 67 lines checked
> 
> * https://lore.kernel.org/qemu-devel/1747063973-124548-7-git-send-email-steven.sistare@oracle.com/
>   ERROR: New file 'hw/vfio/cpr-legacy.c' requires 'SPDX-License-Identifier'
>   ERROR: New file 'hw/vfio/cpr-legacy.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
>   WARNING: added, moved or deleted file(s):
> 
> * https://lore.kernel.org/qemu-devel/1747063973-124548-36-git-send-email-steven.sistare@oracle.com/
>   ERROR: New file 'hw/vfio/cpr-iommufd.c' requires 'SPDX-License-Identifier'
>   ERROR: New file 'hw/vfio/cpr-iommufd.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
>   WARNING: added, moved or deleted file(s):
>     hw/vfio/cpr-iommufd.c
>   Does MAINTAINERS need updating?
>   total: 2 errors, 1 warnings, 161 lines checked
> 
> * https://lore.kernel.org/qemu-devel/20250515154413.210315-1-john.levon@nutanix.com
>   ERROR: New file 'hw/vfio-user/container.h' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
>   ERROR: New file 'hw/vfio-user/container.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
>   ERROR: New file 'hw/vfio-user/pci.c' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
>   WARNING: Does new file 'hw/vfio-user/meson.build' need 'SPDX-License-Identifier'?
>   total: 3 errors, 1 warnings, 490 lines checked
> 
> 
> and more.

That nicely demonstrates the value of this extra check :-)

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

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 v3 9/9] scripts/checkpatch: reject license boilerplate on new files
Posted by Cédric Le Goater 7 months ago
On 5/15/25 15:59, Daniel P. Berrangé wrote:
> The previous commit mandates use of SPDX-License-Identifier on common
> source files, and encourages it on all other files.
> 
> Some contributors are none the less still also including the license
> boilerplate text. This is redundant and will potentially cause
> trouble if inconsistent with the SPDX declaration.
> 
> Match common boilerplate text blurbs and report them as invalid,
> for newly added files.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   scripts/checkpatch.pl | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 87050e6677..cb1942c021 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1496,6 +1496,13 @@ sub process_end_of_file {
>   			     "' need 'SPDX-License-Identifier'?");
>   		}
>   	}
> +	if ($fileinfo->{action} eq "new" &&
> +	    !exists $fileinfo->{facts}->{sawboilerplate}) {
> +		ERROR("New file '" . $fileinfo->{filenew} . "' must " .
> +		      "not have license boilerplate header text unless " .
> +		      "this file is copied from existing code with such " .
> +		      "text already present.");
> +	}
>   }
>   
>   sub process {
> @@ -1798,6 +1805,15 @@ sub process {
>   			&checkspdx($realfile, $1);
>   		}
>   
> +		if ($rawline =~ /licensed under the terms of the GNU GPL/ ||
> +		    $rawline =~ /under the terms of the GNU General Public License/ ||
> +		    $rawline =~ /under the terms of the GNU Lesser General Public/ ||
> +		    $rawline =~ /Permission is hereby granted, free of charge/ ||
> +		    $rawline =~ /GNU GPL, version 2 or later/ ||
> +		    $rawline =~ /See the COPYING file/) {
> +			$fileinfo->{facts}->{sawboilerplate} = 1;
> +		}
> +
>   		if ($rawline =~ m,(SPDX-[a-zA-Z0-9-_]+):,) {
>   			my $tag = $1;
>   			my @permitted = qw(

This patch reported this error :

     ERROR: New file 'include/hw/vfio/vfio-cpr.h' must not have license boilerplate header text unless this file is copied from existing code with such text already present.
     total: 1 errors, 0 warnings, 58 lines checked

See patch https://lore.kernel.org/qemu-devel/1747063973-124548-6-git-send-email-steven.sistare@oracle.com/

But this looks wrong. Right ? I don't understand how rawline matched though.


Thanks,

C.