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
prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.