All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Schier <nsc@kernel.org>
To: Dmitry Voytik <voytikd@gmail.com>
Cc: Nathan Chancellor <nathan@kernel.org>, Rong Zhang <i@rong.moe>,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org
Subject: Re: [PATCH v3] scripts/config: honor $KBUILD_OUTPUT by default
Date: Thu, 1 Oct 2026 17:06:14 +0200	[thread overview]
Message-ID: <ar525jXEMt3ZecH_@levanger> (raw)
In-Reply-To: <CAAX90H1Ma5TkiSSg0CB-Tb__G++h-_P=+kiJbaQ4DtS0c+kVRw@mail.gmail.com>

On Tue, Sep 29, 2026 at 09:15:28PM +0200, Dmitry Voytik wrote:
> Hi Nicolas,
> 
> Thank you for taking your time and reviewing the patch.
> 
> On Tue, Sep 29, 2026 at 2:05 PM Nicolas Schier <nsc@kernel.org> wrote:
> >
> > On Sun, Sep 27, 2026 at 05:41:01PM +0200, Dmitry Voytik wrote:
> > > If $KBUILD_OUTPUT is set, use it by default if --file is not passed.
> > >
> > > Fix the issue when the script silently updates a stale .config in
> > > the root directory, while the user expects ${KBUILD_OUTPUT}/.config
> > > to be updated.
> > > The problem can be worked around with --file, but this is easy to miss,
> > > and is inconsistent with how the Makefile and other tools work
> > > (e.g., scripts/diffconfig).
> > >
> > > How to reproduce the issue:
> > >       rm .config
> > >       export KBUILD_OUTPUT=.out
> > >       make defconfig
> > >       ./scripts/config \
> > >              -e CONFIG_UBSAN
> > >       grep: .config: No such file or directory
> > >
> > > Signed-off-by: Dmitry Voytik <voytikd@gmail.com>
> > > ---
> > > v3:
> > >  - Added an example in the commit message how to reproduce the issue
> > > v2:
> > >  - Addressed comments by sashiko:
> > >    - https://sashiko.dev/#/patchset/20260920064350.17999-1-voytikd%40gmail.com
> > >    - use basename when --refresh is used
> > >    - remove echo which can be misleading when --file is passed
> > > v1:
> > >  - https://lore.kernel.org/all/20260920064350.17999-1-voytikd@gmail.com/
> > > ---
> > >  scripts/config | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/config b/scripts/config
> > > index 32428ea909c2..cdee67157fe1 100755
> > > --- a/scripts/config
> > > +++ b/scripts/config
> > > @@ -130,6 +130,10 @@ on_exit() {
> > >  trap on_exit EXIT
> > >
> > >  FN=.config
> > > +if [ -n "${KBUILD_OUTPUT}" ]; then
> > > +     FN=${KBUILD_OUTPUT}/${FN}
> > > +fi
> > > +
> > >  CMDS=()
> > >  while [[ $# -gt 0 ]]; do
> > >       if [ "$1" = "--file" ]; then
> > > @@ -229,7 +233,7 @@ while [ "$1" != "" ] ; do
> > >               ;;
> > >
> > >       --refresh)
> > > -             yes "" | make oldconfig KCONFIG_CONFIG=$FN
> > > +             yes "" | make oldconfig KCONFIG_CONFIG=$(basename $FN)
> >
> > Thanks for attempting to fix the '--refresh' option.  But this way,
> > this may lead to very unexpected behaviour, as it regardlessly modifies
> > the path to the config file, even if it was specified via --file.
> >
> > If --file is given, it should take precedence over any KBUILD_OUTPUT
> > (or KCONFIG_CONFIG) from the environment and not be changed unrequested.
> 
> Thanks. I misunderstood what KBUILD_OUTPUT really means. For some reason,
> I thought it should be a filename and not a full path. The
> documentation entry is a bit vague on it.
> 
> > It would be a tiny bit nicer to just strip the KBUILD_OUTPUT path from
> > FN, e.g. (untested):
> >
> >                 yes "" | make oldconfig KCONFIG_CONFIG="${FN#${KBUILD_OUTPUT}/}"
> >
> > But I am still not feeling happy with that.
> 
> Thanks for the suggestion! So, what I did - I just completely dropped
> the second change hunk, i.e. kept it as is:
>       yes "" | make oldconfig KCONFIG_CONFIG=$FN
> 
> I tested the following three scenarios and it looks like it works as expected:
> 
> make mrproper
> export KBUILD_OUTPUT=.out
> make defconfig
> grep "CONFIG_UBSAN=y" $KBUILD_OUTPUT/.config
> 
> # test basic use case:
> ./scripts/config \
>        -e CONFIG_UBSAN
> grep "CONFIG_UBSAN=y" $KBUILD_OUTPUT/.config
> ./scripts/config -d CONFIG_UBSAN
> 
> # test --refresh picks up correct config file by using $KBUILD_OUTPUT
> ./scripts/config --refresh \
>        -e CONFIG_UBSAN
> grep "CONFIG_UBSAN=y" $KBUILD_OUTPUT/.config
> ./scripts/config -d CONFIG_UBSAN
> 
> # test --file takes precedence over $KBUILD_OUTPUT
> cp .out/.config .my_config
> grep "CONFIG_UBSAN=y" .my_config
> ./scripts/config --file $(pwd)/.my_config --refresh \
>        -e CONFIG_UBSAN
> grep "CONFIG_UBSAN=y" .my_config

This one shows one of the problems with scripts/config: while the script
itself handles relative paths to --file FILE from the current working
directory, a call of Kbuild changes the working directory to
$(KBUILD_OUTOUT) and realtive paths in KCONFIG_CONFIG are resolved from
there.

I don't have a solution in mind, that fits it all.  But I think it's
important to not make things worse (or more unexpected) than before, by
accident.

> If you are fine with the above, I can send v4 with adding the above
> testing steps into the commit message.

yeah, please send a v4 and then let's see what we'll do with it, ok?

Thanks and kind regards,
Nicolas

  reply	other threads:[~2026-10-01 15:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27 15:41 [PATCH v3] scripts/config: honor $KBUILD_OUTPUT by default Dmitry Voytik
2026-09-29  8:45 ` Nicolas Schier
2026-09-29 19:15   ` Dmitry Voytik
2026-10-01 15:06     ` Nicolas Schier [this message]
2026-10-01 21:40       ` Dmitry Voytik

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=ar525jXEMt3ZecH_@levanger \
    --to=nsc@kernel.org \
    --cc=i@rong.moe \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=voytikd@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.