Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
MIME-Version: 1.0
References: <20230920015559.1877441-1-aik@amd.com> <CAK7LNARrb_NLpBL19LHEdg0WJKsncaUPQLk-wxhR+Rh8sps3jQ@mail.gmail.com>
 <a6a90c61-e884-4b18-83b7-ca3d2e0378e7@amd.com>
In-Reply-To: <a6a90c61-e884-4b18-83b7-ca3d2e0378e7@amd.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Fri, 22 Sep 2023 16:27:48 +0900
Message-ID: <CAK7LNAS+PefrZnUO1R4v8pa6V7Mh_R92dNAR96Wucz0tf1HEpg@mail.gmail.com>
Subject: Re: [PATCH kernel] kbuild: get rid of unwanted "+" when
 CONFIG_LOCALVERSION is set
To:     Alexey Kardashevskiy <aik@amd.com>
Cc:     linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Nicolas Schier <nicolas@fjasle.eu>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Nathan Chancellor <nathan@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Sep 20, 2023 at 10:30=E2=80=AFPM Alexey Kardashevskiy <aik@amd.com>=
 wrote:
>
>
> On 20/9/23 21:59, Masahiro Yamada wrote:
> > On Wed, Sep 20, 2023 at 10:56=E2=80=AFAM Alexey Kardashevskiy <aik@amd.=
com> wrote:
> >>
> >> The scripts/setlocalversion script correctly tries not adding "+" when
> >> CONFIG_LOCALVERSION is defined.
> >
> > I am afraid you are misunderstanding the script.
>
> Possibly :) I should have read of "+set"... sorry :-/ Thanks for
> spending time looking at this.
>
> > CONFIG_LOCALVERSION and the "+" sign is unrelated.
>
> How come?
>
> scripts/setlocalversion -> if [ "${LOCALVERSION+set}" !=3D "set" ] ->
> scm_version --short -> echo "+".
>
> Where is that LOCALVERSION supposed to come from, and when? Is not
> LOCALVERSION related to CONFIG_LOCALVERSION?


1) LOCALVERSION is an environment variable.


  $ LOCALVERSION=3D-foo make
           or
  $ make LOCALVERSION=3D-foo


2) CONFIG_LOCALVERSION is a CONFIG option

  $ make menuconfig

    and set the value.


1) and 2) are orthogonal.
If you set both, you can get both of them.


> >> However, instead of grepping for it
> >> (as it is done for CONFIG_LOCALVERSION_AUTO=3Dy), it relies on LOCALVE=
RSION
> >> set in the shell which is not.
> >>
> >> Export LOCALVERSION so scripts/setlocalversion could see it and not ad=
d
> >> unwanted "+" at the end of the kernelrelease.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
> >
> >
> > So, scripts/setlocalversion will always see
> > defined LOCALVERSION.
> >
> > With your patch, LOCALVERSION would be set to an empty value,
> > which would make the following condition always false.
> >
> > elif [ "${LOCALVERSION+set}" !=3D "set" ]; then
> >
> >
> > Your patch is equivalent to deleting
> > line 175-183 of scripts/setlocalversion.
> >
> > Of course, that is wrong and unacceptable.
>
> Ok. What is the right way of getting rid of the "+"? Thanks,


--
Best Regards
Masahiro Yamada