* [PATCH] scripts/checkpatch: emit a warning if an imported file is touched @ 2024-07-17 9:37 Stefano Garzarella 2024-07-17 9:50 ` Daniel P. Berrangé 2024-07-17 9:58 ` Cornelia Huck 0 siblings, 2 replies; 5+ messages in thread From: Stefano Garzarella @ 2024-07-17 9:37 UTC (permalink / raw) To: qemu-devel; +Cc: cohuck, berrange, Stefano Garzarella 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scripts/checkpatch: emit a warning if an imported file is touched 2024-07-17 9:37 [PATCH] scripts/checkpatch: emit a warning if an imported file is touched Stefano Garzarella @ 2024-07-17 9:50 ` Daniel P. Berrangé 2024-07-17 12:26 ` Stefano Garzarella 2024-07-17 9:58 ` Cornelia Huck 1 sibling, 1 reply; 5+ messages in thread From: Daniel P. Berrangé @ 2024-07-17 9:50 UTC (permalink / raw) To: Stefano Garzarella; +Cc: qemu-devel, cohuck 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 :| ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scripts/checkpatch: emit a warning if an imported file is touched 2024-07-17 9:50 ` Daniel P. Berrangé @ 2024-07-17 12:26 ` Stefano Garzarella 0 siblings, 0 replies; 5+ messages in thread From: Stefano Garzarella @ 2024-07-17 12:26 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel, cohuck 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scripts/checkpatch: emit a warning if an imported file is touched 2024-07-17 9:37 [PATCH] scripts/checkpatch: emit a warning if an imported file is touched Stefano Garzarella 2024-07-17 9:50 ` Daniel P. Berrangé @ 2024-07-17 9:58 ` Cornelia Huck 2024-07-17 12:36 ` Stefano Garzarella 1 sibling, 1 reply; 5+ messages in thread From: Cornelia Huck @ 2024-07-17 9:58 UTC (permalink / raw) To: Stefano Garzarella, qemu-devel; +Cc: berrange, Stefano Garzarella 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/) { ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scripts/checkpatch: emit a warning if an imported file is touched 2024-07-17 9:58 ` Cornelia Huck @ 2024-07-17 12:36 ` Stefano Garzarella 0 siblings, 0 replies; 5+ messages in thread From: Stefano Garzarella @ 2024-07-17 12:36 UTC (permalink / raw) To: Cornelia Huck; +Cc: qemu-devel, berrange 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-07-17 12:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-17 9:37 [PATCH] scripts/checkpatch: emit a warning if an imported file is touched Stefano Garzarella 2024-07-17 9:50 ` Daniel P. Berrangé 2024-07-17 12:26 ` Stefano Garzarella 2024-07-17 9:58 ` Cornelia Huck 2024-07-17 12:36 ` Stefano Garzarella
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).