If a file imported from Linux is touched, emit a warning and suggest
using scripts/update-linux-headers.sh
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
scripts/checkpatch.pl | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ff373a7083..b0e8266fa2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1374,6 +1374,7 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
my $reported_maintainer_file = 0;
+ my $reported_imported_file = 0;
my $non_utf8_charset = 0;
our @report = ();
@@ -1673,8 +1674,17 @@ sub process {
# ignore non-hunk lines and lines being removed
next if (!$hunk_line || $line =~ /^-/);
-# ignore files that are being periodically imported from Linux
- next if ($realfile =~ /^(linux-headers|include\/standard-headers)\//);
+# ignore files that are being periodically imported from Linux and emit a warning
+ if ($realfile =~ /^(linux-headers|include\/standard-headers)\//) {
+ if (!$reported_imported_file) {
+ $reported_imported_file = 1;
+ WARN("added, moved or deleted file(s) " .
+ "imported from Linux, are you using " .
+ "scripts/update-linux-headers.sh?\n" .
+ $herecurr);
+ }
+ next;
+ }
#trailing whitespace
if ($line =~ /^\+.*\015/) {
--
2.45.2
On Wed, Jul 17 2024, Stefano Garzarella <sgarzare@redhat.com> wrote: > If a file imported from Linux is touched, emit a warning and suggest > using scripts/update-linux-headers.sh > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > scripts/checkpatch.pl | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index ff373a7083..b0e8266fa2 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1374,6 +1374,7 @@ sub process { > my $in_header_lines = $file ? 0 : 1; > my $in_commit_log = 0; #Scanning lines before patch > my $reported_maintainer_file = 0; > + my $reported_imported_file = 0; > my $non_utf8_charset = 0; > > our @report = (); > @@ -1673,8 +1674,17 @@ sub process { > # ignore non-hunk lines and lines being removed > next if (!$hunk_line || $line =~ /^-/); > > -# ignore files that are being periodically imported from Linux > - next if ($realfile =~ /^(linux-headers|include\/standard-headers)\//); > +# ignore files that are being periodically imported from Linux and emit a warning > + if ($realfile =~ /^(linux-headers|include\/standard-headers)\//) { > + if (!$reported_imported_file) { > + $reported_imported_file = 1; > + WARN("added, moved or deleted file(s) " . > + "imported from Linux, are you using " . > + "scripts/update-linux-headers.sh?\n" . > + $herecurr); > + } > + next; > + } Thanks, that looks useful -- just two comments (sorry, my perl-fu is low): - Is there a way to check that this is a proper linux headers update? We'd have to rely on heuristics, but OTOH, we also usually want a headers update to use a certain format ($SUBJECT containing "headers update", patch description pointing to the version this update was done against.) Not sure if it is worth actually trying to figure this out. - A common issue is headers changes mixed in with other code changes, which should not happen -- can we check for that as well and advise to either do a headers update, or use a placeholder patch? > > #trailing whitespace > if ($line =~ /^\+.*\015/) {
On Wed, Jul 17, 2024 at 11:58:46AM GMT, Cornelia Huck wrote: >On Wed, Jul 17 2024, Stefano Garzarella <sgarzare@redhat.com> wrote: > >> If a file imported from Linux is touched, emit a warning and suggest >> using scripts/update-linux-headers.sh >> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> scripts/checkpatch.pl | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >> index ff373a7083..b0e8266fa2 100755 >> --- a/scripts/checkpatch.pl >> +++ b/scripts/checkpatch.pl >> @@ -1374,6 +1374,7 @@ sub process { >> my $in_header_lines = $file ? 0 : 1; >> my $in_commit_log = 0; #Scanning lines before patch >> my $reported_maintainer_file = 0; >> + my $reported_imported_file = 0; >> my $non_utf8_charset = 0; >> >> our @report = (); >> @@ -1673,8 +1674,17 @@ sub process { >> # ignore non-hunk lines and lines being removed >> next if (!$hunk_line || $line =~ /^-/); >> >> -# ignore files that are being periodically imported from Linux >> - next if ($realfile =~ /^(linux-headers|include\/standard-headers)\//); >> +# ignore files that are being periodically imported from Linux and emit a warning >> + if ($realfile =~ /^(linux-headers|include\/standard-headers)\//) { >> + if (!$reported_imported_file) { >> + $reported_imported_file = 1; >> + WARN("added, moved or deleted file(s) " . >> + "imported from Linux, are you using " . >> + "scripts/update-linux-headers.sh?\n" . >> + $herecurr); >> + } >> + next; >> + } > >Thanks, that looks useful -- just two comments (sorry, my perl-fu is >low): Same perl-fu here ;-P >- Is there a way to check that this is a proper linux headers update? > We'd have to rely on heuristics, but OTOH, we also usually want a > headers update to use a certain format ($SUBJECT containing "headers > update", patch description pointing to the version this update was > done against.) Not sure if it is worth actually trying to figure this > out. I think it can be done though I think we should formalize it somewhere first, or integrate the generation of the commit in the scripts/update-linux-headers.sh At that point here we can add a check based on that. >- A common issue is headers changes mixed in with other code changes, > which should not happen -- can we check for that as well and advise > to either do a headers update, or use a placeholder patch? Yeah, Daniel suggested the same, I'll address in v2. Thanks, Stefano
On Wed, Jul 17, 2024 at 11:37:52AM +0200, Stefano Garzarella wrote: > If a file imported from Linux is touched, emit a warning and suggest > using scripts/update-linux-headers.sh > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > scripts/checkpatch.pl | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index ff373a7083..b0e8266fa2 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -1374,6 +1374,7 @@ sub process { > my $in_header_lines = $file ? 0 : 1; > my $in_commit_log = 0; #Scanning lines before patch > my $reported_maintainer_file = 0; > + my $reported_imported_file = 0; > my $non_utf8_charset = 0; > > our @report = (); > @@ -1673,8 +1674,17 @@ sub process { > # ignore non-hunk lines and lines being removed > next if (!$hunk_line || $line =~ /^-/); > > -# ignore files that are being periodically imported from Linux > - next if ($realfile =~ /^(linux-headers|include\/standard-headers)\//); > +# ignore files that are being periodically imported from Linux and emit a warning > + if ($realfile =~ /^(linux-headers|include\/standard-headers)\//) { > + if (!$reported_imported_file) { > + $reported_imported_file = 1; > + WARN("added, moved or deleted file(s) " . > + "imported from Linux, are you using " . > + "scripts/update-linux-headers.sh?\n" . > + $herecurr); This is a good hint, but can we add a further check that is a fatal error, if the headers are changed in the same commit as non-header changes. When importing headers, they should only ever be in a self-contained patch with nothing else touched. > + } > + next; > + } 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 :|
On Wed, Jul 17, 2024 at 10:50:51AM GMT, Daniel P. Berrangé wrote: >On Wed, Jul 17, 2024 at 11:37:52AM +0200, Stefano Garzarella wrote: >> If a file imported from Linux is touched, emit a warning and suggest >> using scripts/update-linux-headers.sh >> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> scripts/checkpatch.pl | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >> index ff373a7083..b0e8266fa2 100755 >> --- a/scripts/checkpatch.pl >> +++ b/scripts/checkpatch.pl >> @@ -1374,6 +1374,7 @@ sub process { >> my $in_header_lines = $file ? 0 : 1; >> my $in_commit_log = 0; #Scanning lines before patch >> my $reported_maintainer_file = 0; >> + my $reported_imported_file = 0; >> my $non_utf8_charset = 0; >> >> our @report = (); >> @@ -1673,8 +1674,17 @@ sub process { >> # ignore non-hunk lines and lines being removed >> next if (!$hunk_line || $line =~ /^-/); >> >> -# ignore files that are being periodically imported from Linux >> - next if ($realfile =~ /^(linux-headers|include\/standard-headers)\//); >> +# ignore files that are being periodically imported from Linux and emit a warning >> + if ($realfile =~ /^(linux-headers|include\/standard-headers)\//) { >> + if (!$reported_imported_file) { >> + $reported_imported_file = 1; >> + WARN("added, moved or deleted file(s) " . >> + "imported from Linux, are you using " . >> + "scripts/update-linux-headers.sh?\n" . >> + $herecurr); > >This is a good hint, but can we add a further check that is a fatal error, >if the headers are changed in the same commit as non-header changes. When >importing headers, they should only ever be in a self-contained patch >with nothing else touched. Yep, good point! I'll add that check in v2. Thanks, Stefano
© 2016 - 2024 Red Hat, Inc.