[PATCH v2] checkpatch: allow build files to reference other build files

[PATCH v2] checkpatch: allow build files to reference other build files

[Will McVicker]

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


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

Re: [PATCH v2] checkpatch: allow build files to reference other build files

[Nicolas Schier]

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
> 

Re: [PATCH v2] checkpatch: allow build files to reference other build files

[William McVicker]

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
> > 

Re: [PATCH v2] checkpatch: allow build files to reference other build files

[Nicolas Schier]

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