On Thu, 15 May 2025 at 15:00, Daniel P. Berrangé <berra...@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é <berra...@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.mayd...@linaro.org>

thanks
-- PMM

Reply via email to