| Message ID | 20240112221947.1950503-1-willmcvicker@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] checkpatch: allow build files to reference other build files | expand |
On Fri, Jan 12, 2024 at 02:19:46PM -0800, Will McVicker wrote: > Add an exception to the EMBEDDED_FILENAME warning for build files. This As far as I can see, your patch fixes only the checkpatch warnings for top-level Makefile and Kconfig (and leaving out top-level Kbuild). Other build files are not affected, right? Kind regards, Nicolas > fixes the below warnings where the Kconfig and Makefile files reference > other similarly named build files. > > WARNING:EMBEDDED_FILENAME: It's generally not useful to have the filename in the file > #24: FILE: Kconfig:34: > +source "drivers/willmcvicker/Kconfig" > > WARNING:EMBEDDED_FILENAME: It's generally not useful to have the filename in the file > #36: FILE: Makefile:667: > + } > Makefile > > Signed-off-by: Will McVicker <willmcvicker@google.com> > --- > scripts/checkpatch.pl | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > v2: > - Unwrap commit message lines > - Align and update regex > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index f8343b34a28b..c2869803e545 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -3785,7 +3785,8 @@ sub process { > } > > # check for embedded filenames > - if ($rawline =~ /^\+.*\b\Q$realfile\E\b/) { > + if ($rawline =~ /^\+.*\b\Q$realfile\E\b/ && > + $realfile !~ /(?:Kconfig|Makefile)/) { > WARN("EMBEDDED_FILENAME", > "It's generally not useful to have the filename in the file\n" . $herecurr); > } > > base-commit: 70d201a40823acba23899342d62bc2644051ad2e > -- > 2.43.0.275.g3460e3d667-goog >
On 01/29/2024, Nicolas Schier wrote: > On Fri, Jan 12, 2024 at 02:19:46PM -0800, Will McVicker wrote: > > Add an exception to the EMBEDDED_FILENAME warning for build files. This > > As far as I can see, your patch fixes only the checkpatch warnings for > top-level Makefile and Kconfig (and leaving out top-level Kbuild). > Other build files are not affected, right? Since $realfile includes the full path, I wasn't able to find a case where this issue happens outside of the top-level build files. The same goes for Kbuild files -- the top-level Kbuild file doesn't include other Kbuild files and the other Kbuild files don't include other Kbuild files within the same directory. If you prefer to protect against this warning in the future, I can include Kbuild as well if you want. Thanks, Will > > Kind regards, > Nicolas > > > > fixes the below warnings where the Kconfig and Makefile files reference > > other similarly named build files. > > > > WARNING:EMBEDDED_FILENAME: It's generally not useful to have the filename in the file > > #24: FILE: Kconfig:34: > > +source "drivers/willmcvicker/Kconfig" > > > > WARNING:EMBEDDED_FILENAME: It's generally not useful to have the filename in the file > > #36: FILE: Makefile:667: > > + } > Makefile > > > > Signed-off-by: Will McVicker <willmcvicker@google.com> > > --- > > scripts/checkpatch.pl | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > v2: > > - Unwrap commit message lines > > - Align and update regex > > > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index f8343b34a28b..c2869803e545 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -3785,7 +3785,8 @@ sub process { > > } > > > > # check for embedded filenames > > - if ($rawline =~ /^\+.*\b\Q$realfile\E\b/) { > > + if ($rawline =~ /^\+.*\b\Q$realfile\E\b/ && > > + $realfile !~ /(?:Kconfig|Makefile)/) { > > WARN("EMBEDDED_FILENAME", > > "It's generally not useful to have the filename in the file\n" . $herecurr); > > } > > > > base-commit: 70d201a40823acba23899342d62bc2644051ad2e > > -- > > 2.43.0.275.g3460e3d667-goog > >
On Tue, Jan 30, 2024 at 02:19:23PM -0800, William McVicker wrote: > On 01/29/2024, Nicolas Schier wrote: > > On Fri, Jan 12, 2024 at 02:19:46PM -0800, Will McVicker wrote: > > > Add an exception to the EMBEDDED_FILENAME warning for build files. This > > > > As far as I can see, your patch fixes only the checkpatch warnings for > > top-level Makefile and Kconfig (and leaving out top-level Kbuild). > > Other build files are not affected, right? > > Since $realfile includes the full path, I wasn't able to find a case where this > issue happens outside of the top-level build files. The same goes for Kbuild > files -- the top-level Kbuild file doesn't include other Kbuild files and the > other Kbuild files don't include other Kbuild files within the same directory. > If you prefer to protect against this warning in the future, I can include > Kbuild as well if you want. yes, I think it would be more complete if top-level Kbuild is also included. Could you also mention 'top-level' somewhere in the commit message? Thanks and kind regards, Nicolas
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f8343b34a28b..c2869803e545 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3785,7 +3785,8 @@ sub process { } # check for embedded filenames - if ($rawline =~ /^\+.*\b\Q$realfile\E\b/) { + if ($rawline =~ /^\+.*\b\Q$realfile\E\b/ && + $realfile !~ /(?:Kconfig|Makefile)/) { WARN("EMBEDDED_FILENAME", "It's generally not useful to have the filename in the file\n" . $herecurr); }
Add an exception to the EMBEDDED_FILENAME warning for build files. This fixes the below warnings where the Kconfig and Makefile files reference other similarly named build files. WARNING:EMBEDDED_FILENAME: It's generally not useful to have the filename in the file #24: FILE: Kconfig:34: +source "drivers/willmcvicker/Kconfig" WARNING:EMBEDDED_FILENAME: It's generally not useful to have the filename in the file #36: FILE: Makefile:667: + } > Makefile Signed-off-by: Will McVicker <willmcvicker@google.com> --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) v2: - Unwrap commit message lines - Align and update regex base-commit: 70d201a40823acba23899342d62bc2644051ad2e