public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: Simon Glass <sjg@chromium.org>
Cc: linux-kernel@vger.kernel.org, Joe Perches <joe@perches.com>,
	Andy Whitcroft <apw@canonical.com>,
	Dwaipayan Ray <dwaipayanray1@gmail.com>,
	Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	Kory Maincent <kory.maincent@bootlin.com>,
	Tom Rini <trini@konsulko.com>,
	Kuan-Wei Chiu <visitorckw@gmail.com>
Subject: Re: [PATCH v3 1/2] checkpatch: Allow to pass config directory
Date: Mon, 13 Apr 2026 10:22:51 +0200	[thread overview]
Message-ID: <20260413082251.GA117516@pevik> (raw)
In-Reply-To: <CAFLszTiwZdhOjMA6TysSErkJJAb8u0UzEH8qwGNMe8v50m36YA@mail.gmail.com>

> Hi Petr,

> On Wed, 8 Apr 2026 at 06:06, Petr Vorel <pvorel@suse.cz> wrote:

> > checkpatch.pl searches for .checkpatch.conf in $HOME, $CWD/.scripts
> > and $CWD.  Allow to pass 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@suse.cz>
> > ---
> > Changes in v3:
> > * CHECKPATCH_CONFIG_DIR can point to only single directory (':' would
> > make it not finding the directory, Simon).
> > * Avoid join undef string (Simon).
> > * Don't use $ENV{$env_config_dir} twice (Joe).
> > * Pass new variables to which_conf() instead of using global.

> > Link to v2:
> > https://lore.kernel.org/lkml/20260224181623.89904-1-pvorel@suse.cz/

> > Link to v1:
> > https://lore.kernel.org/lkml/20260202144221.76765-2-pvorel@suse.cz/

> >  scripts/checkpatch.pl | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)

> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 0492d6afc9a1f..58f3d5a98204c 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
> > +Script searches for a configuration file $configuration_file in a directory
> > +specified by \$$env_config_dir environment variable, or
> > +(if variable not defined) in path: '$def_configuration_dirs'
> >  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,12 @@ sub which {
> >  }

> >  sub which_conf {
> > -       my ($conf) = @_;
> > +       my ($conf, $env_key, $paths) = @_;
> > +       my $env_dir = $ENV{$env_key};
> > +
> > +       return "$env_dir/$conf" if (defined($env_dir));

> > -       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@chromium.org>

Thank you! I'll probably send v4, but I will not dare to add your RBT due
changes I will add. Thanks for a careful review!

> Some nits:

> When CHECKPATCH_CONFIG_DIR is set but the file doesn't exist there,
> the default paths are silently skipped. A warning would help catch
> typos.

I was thinking about it as well. Probably worth to add it => v4.

> The commit message says the search order is "$HOME, $CWD/.scripts and
> $CWD" but the code has ".:$HOME:.scripts" (CWD first).

+1, thanks!

> 'Allow to pass' -> 'Allow passing'

+1, thanks!

Kind regards,
Petr

> Regards,
> Simon

      reply	other threads:[~2026-04-13  8:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-08 12:06 [PATCH v3 1/2] checkpatch: Allow to pass config directory Petr Vorel
2026-04-08 12:06 ` [PATCH v3 2/2] checkpatch: Add option to not force /* */ for SPDX Petr Vorel
2026-04-11 19:49   ` Simon Glass
2026-04-13  8:24     ` Petr Vorel
2026-04-11 19:49 ` [PATCH v3 1/2] checkpatch: Allow to pass config directory Simon Glass
2026-04-13  8:22   ` Petr Vorel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260413082251.GA117516@pevik \
    --to=pvorel@suse.cz \
    --cc=apw@canonical.com \
    --cc=dwaipayanray1@gmail.com \
    --cc=joe@perches.com \
    --cc=kory.maincent@bootlin.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=visitorckw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox