On Fri, Sep 22, 2023 at 9:59 AM Alexey Kardashevskiy <aik@xxxxxxx> wrote:
>
>
> On 20/9/23 23:30, Alexey Kardashevskiy wrote:
> >
> > On 20/9/23 21:59, Masahiro Yamada wrote:
> >> On Wed, Sep 20, 2023 at 10:56 AM Alexey Kardashevskiy <aik@xxxxxxx>
> >> 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}" != "set" ] ->
> > scm_version --short -> echo "+".
> >
> > Where is that LOCALVERSION supposed to come from, and when? Is not
> > LOCALVERSION related to CONFIG_LOCALVERSION?
> >
> >>> However, instead of grepping for it
> >>> (as it is done for CONFIG_LOCALVERSION_AUTO=y), it relies on
> >>> LOCALVERSION
> >>> set in the shell which is not.
> >>>
> >>> Export LOCALVERSION so scripts/setlocalversion could see it and not add
> >>> unwanted "+" at the end of the kernelrelease.
> >>>
> >>> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx>
> >>
> >>
> >> 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}" != "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,
>
>
> make LOCALVERSION="" ...
> seems to be the only way, is that right? I thought CONFIG_LOCALVERSION
> is good enough for not having "+", hmm.
Correct.
Reading the commit description of
5df99bec210a2cf89dd91e52f0d0a714bf4cd96a
makes it clearer.
Best Regards
Masahiro Yamada
