Re: [PATCH v5 1/2] checkpatch: Allow passing config directory

[Simon Glass]

Hi Petr,

On Tue, 21 Apr 2026 at 01:04, Petr Vorel <pvorel@xxxxxxx> wrote:
>
> > Hi Petr,
>
> > On Sat, 18 Apr 2026 at 05:04, Petr Vorel <pvorel@xxxxxxx> wrote:
>
> > > checkpatch.pl searches for .checkpatch.conf in $CWD, $HOME and
> > > $CWD/.scripts. Allow passing a single directory via
> > > CHECKPATCH_CONFIG_DIR environment variable. This allows to directly use
> > > project configuration file for projects which vendored checkpatch.pl
> > > (e.g. LTP or u-boot).
>
> > > Although it'd be more convenient for user to add --conf-dir option
> > > (instead of using environment variable), code would get ugly because
> > > options from the configuration file needs to be read before processing
> > > command line options with Getopt::Long.
>
> > > While at it, document directories and environment variable in help.
>
> > > Signed-off-by: Petr Vorel <pvorel@xxxxxxx>
> > > ---
> > > Changes in v5 (all by sashiko agent):
> > > https://sashiko.dev/#/patchset/20260415143636.272605-1-pvorel%40suse.cz
> > > * Move warning to don't wrongly warn  when $CHECKPATCH_CONFIG_DIR is not set
> > > (sigh).
>
> > > Link to v4:
> > > https://lore.kernel.org/lkml/20260415143636.272605-1-pvorel@xxxxxxx/
>
> > > Link to v3:
> > > https://lore.kernel.org/lkml/20260408120603.54351-1-pvorel@xxxxxxx/
>
> > > Link to v2:
> > > https://lore.kernel.org/lkml/20260224181623.89904-1-pvorel@xxxxxxx/
>
> > > Link to v1:
> > > https://lore.kernel.org/lkml/20260202144221.76765-2-pvorel@xxxxxxx/
>
> > >  scripts/checkpatch.pl | 19 ++++++++++++++++---
> > >  1 file changed, 16 insertions(+), 3 deletions(-)
>
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 0492d6afc9a1f..8f7a42da12869 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -57,6 +57,8 @@ my %ignore_type = ();
> > >  my @ignore = ();
> > >  my $help = 0;
> > >  my $configuration_file = ".checkpatch.conf";
> > > +my $def_configuration_dirs = ".:$ENV{HOME}:.scripts";
> > > +my $env_config_dir = 'CHECKPATCH_CONFIG_DIR';
> > >  my $max_line_length = 100;
> > >  my $ignore_perl_version = 0;
> > >  my $minimum_perl_version = 5.10.0;
> > > @@ -146,6 +148,11 @@ Options:
> > >    -h, --help, --version      display this help and exit
>
> > >  When FILE is - read standard input.
> > > +
> > > +CONFIGURATION FILE
> > > +Default configuration options can be stored in $configuration_file,
> > > +search path: '$def_configuration_dirs' or in a directory specified by
> > > +\$$env_config_dir environment variable (fallback to the default search path).
> > >  EOM
>
> > >         exit($exitcode);
> > > @@ -237,7 +244,7 @@ sub list_types {
> > >         exit($exitcode);
> > >  }
>
> > > -my $conf = which_conf($configuration_file);
> > > +my $conf = which_conf($configuration_file, $env_config_dir, $def_configuration_dirs);
> > >  if (-f $conf) {
> > >         my @conf_args;
> > >         open(my $conffile, '<', "$conf")
> > > @@ -1531,9 +1538,15 @@ sub which {
> > >  }
>
> > >  sub which_conf {
> > > -       my ($conf) = @_;
> > > +       my ($conf, $env_key, $paths) = @_;
> > > +       my $env_dir = $ENV{$env_key};
> > > +
> > > +       if (defined($env_dir)) {
> > > +               return "$env_dir/$conf" if (-e "$env_dir/$conf");
> > > +               warn "$P: Can't find a readable $conf in '$env_dir', fallback to default locations: $!\n";
>
> > Just some optional nits
>
> > I think you should drop the $! as the errno might be stale here.
>
> > 'falling back to' would read better
>
> +1
>
> > > +       }
>
> > > -       foreach my $path (split(/:/, ".:$ENV{HOME}:.scripts")) {
> > > +       foreach my $path (split(/:/, $paths)) {
> > >                 if (-e "$path/$conf") {
> > >                         return "$path/$conf";
> > >                 }
> > > --
> > > 2.53.0
>
>
> > Reviewed-by: Simon Glass <sjg@xxxxxxxxxxxx>
>
> At this point I dare to add your RBT in next version.

Thanks :-) These are really just nits.

Regards,
Simon