Re: [PATCH v2] setlocalversion: use ${objtree}/include/config/auto.conf

[Hong, Yifan]

On Sat, Mar 22, 2025 at 8:15 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> On Tue, Mar 18, 2025 at 9:59 AM HONG Yifan <elsk@xxxxxxxxxx> wrote:
> >
> > setlocalversion reads include/config/auto.conf, which is located below
> > $(objtree) with commit 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to
> > some in-kernel build artifacts").
> >
> > To be consistent, the setlocalversion script should use
>
> "To be consistent" is too weak because we do not add
> $(objtree)/ to include/config/auto.conf
>
> Just run "git grep include/config/auto.conf"
>
> You will see more include/config/auto.conf instances
> that lack $(objtree)/ prefix.
>
> So, "To be consistent" is not a reason.
>
> You described why Google needs to have this
> specifically scripts/setlocalversion.
>
> Without that explained, I do not understand _why_.
>

Without the context of Google's out-of-tree patch, I can't really
think of any good reason other than "To be consistent". Would it be
better if I quote
https://lore.kernel.org/all/20210121213641.3477522-1-willmcvicker@xxxxxxxxxx/
("[PATCH v6] modules: introduce the MODULE_SCMVERSION config") in the
commit message, explaining that "prepending $(objtree)/ is required
for this other patch to work"?

>
>
> > ${objtree}/include/config/auto.conf as well.
> >
> > Signed-off-by: HONG Yifan <elsk@xxxxxxxxxx>
> > ---
> > v1: https://lore.kernel.org/lkml/20250312021154.102262-2-elsk@xxxxxxxxxx/
> > v1 -> v2: fixed the other two locations of include/config/auto.conf in
> > setlocalversion script; also removed incorrect claim in commit message.
> >
> >  scripts/setlocalversion | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/setlocalversion b/scripts/setlocalversion
> > index 28169d7e143b..c13fe6e585e9 100755
> > --- a/scripts/setlocalversion
> > +++ b/scripts/setlocalversion
> > @@ -186,16 +186,16 @@ if ${no_local}; then
> >         exit 0
> >  fi
> >
> > -if ! test -e include/config/auto.conf; then
> > +if ! test -e ${objtree}/include/config/auto.conf; then
>
>
> Please quote
>
> "${objtree}/include/config/auto.conf"
>
> to avoid a shellcheck warning.
>
>
>
>
> >         echo "Error: kernelrelease not valid - run 'make prepare' to update it" >&2
> >         exit 1
> >  fi
> >
> >  # version string from CONFIG_LOCALVERSION
> > -config_localversion=$(sed -n 's/^CONFIG_LOCALVERSION=\(.*\)$/\1/p' include/config/auto.conf)
> > +config_localversion=$(sed -n 's/^CONFIG_LOCALVERSION=\(.*\)$/\1/p' ${objtree}/include/config/auto.conf)
> >
> >  # scm version string if not at the kernel version tag or at the file_localversion
> > -if grep -q "^CONFIG_LOCALVERSION_AUTO=y$" include/config/auto.conf; then
> > +if grep -q "^CONFIG_LOCALVERSION_AUTO=y$" ${objtree}/include/config/auto.conf; then
> >         # full scm version string
> >         scm_version="$(scm_version)"
> >  elif [ "${LOCALVERSION+set}" != "set" ]; then
> > --
> > 2.49.0.rc1.451.g8f38331e32-goog
> >
>
>
> --
> Best Regards
> Masahiro Yamada