* [PATCH kernel] kbuild: get rid of unwanted "+" when CONFIG_LOCALVERSION is set
@ 2023-09-20 1:55 Alexey Kardashevskiy
2023-09-20 11:59 ` Masahiro Yamada
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Kardashevskiy @ 2023-09-20 1:55 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Nicolas Schier, Nick Desaulniers,
Nathan Chancellor, Masahiro Yamada, Alexey Kardashevskiy
The scripts/setlocalversion script correctly tries not adding "+" when
CONFIG_LOCALVERSION is defined. 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@amd.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 57698d048e2c..fc45bed69790 100644
--- a/Makefile
+++ b/Makefile
@@ -368,7 +368,7 @@ include $(srctree)/scripts/Kbuild.include
# Read KERNELRELEASE from include/config/kernel.release (if it exists)
KERNELRELEASE = $(call read-file, include/config/kernel.release)
KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(SUBLEVEL)))$(EXTRAVERSION)
-export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
+export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION LOCALVERSION
include $(srctree)/scripts/subarch.include
--
2.41.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH kernel] kbuild: get rid of unwanted "+" when CONFIG_LOCALVERSION is set
2023-09-20 1:55 [PATCH kernel] kbuild: get rid of unwanted "+" when CONFIG_LOCALVERSION is set Alexey Kardashevskiy
@ 2023-09-20 11:59 ` Masahiro Yamada
2023-09-20 13:30 ` Alexey Kardashevskiy
0 siblings, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2023-09-20 11:59 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: linux-kbuild, linux-kernel, Nicolas Schier, Nick Desaulniers,
Nathan Chancellor
On Wed, Sep 20, 2023 at 10:56 AM 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.
CONFIG_LOCALVERSION and the "+" sign is unrelated.
> 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@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}" != "set" ]; then
Your patch is equivalent to deleting
line 175-183 of scripts/setlocalversion.
Of course, that is wrong and unacceptable.
> ---
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 57698d048e2c..fc45bed69790 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -368,7 +368,7 @@ include $(srctree)/scripts/Kbuild.include
> # Read KERNELRELEASE from include/config/kernel.release (if it exists)
> KERNELRELEASE = $(call read-file, include/config/kernel.release)
> KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(SUBLEVEL)))$(EXTRAVERSION)
> -export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
> +export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION LOCALVERSION
>
> include $(srctree)/scripts/subarch.include
>
> --
> 2.41.0
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kernel] kbuild: get rid of unwanted "+" when CONFIG_LOCALVERSION is set
2023-09-20 11:59 ` Masahiro Yamada
@ 2023-09-20 13:30 ` Alexey Kardashevskiy
2023-09-22 0:59 ` Alexey Kardashevskiy
2023-09-22 7:27 ` Masahiro Yamada
0 siblings, 2 replies; 6+ messages in thread
From: Alexey Kardashevskiy @ 2023-09-20 13:30 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Nicolas Schier, Nick Desaulniers,
Nathan Chancellor
On 20/9/23 21:59, Masahiro Yamada wrote:
> On Wed, Sep 20, 2023 at 10:56 AM 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}" != "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@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}" != "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,
>
>
>
>
>
>
>
>> ---
>> Makefile | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 57698d048e2c..fc45bed69790 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -368,7 +368,7 @@ include $(srctree)/scripts/Kbuild.include
>> # Read KERNELRELEASE from include/config/kernel.release (if it exists)
>> KERNELRELEASE = $(call read-file, include/config/kernel.release)
>> KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(SUBLEVEL)))$(EXTRAVERSION)
>> -export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
>> +export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION LOCALVERSION
>>
>> include $(srctree)/scripts/subarch.include
>>
>> --
>> 2.41.0
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kernel] kbuild: get rid of unwanted "+" when CONFIG_LOCALVERSION is set
2023-09-20 13:30 ` Alexey Kardashevskiy
@ 2023-09-22 0:59 ` Alexey Kardashevskiy
2023-09-22 7:31 ` Masahiro Yamada
2023-09-22 7:27 ` Masahiro Yamada
1 sibling, 1 reply; 6+ messages in thread
From: Alexey Kardashevskiy @ 2023-09-22 0:59 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Nicolas Schier, Nick Desaulniers,
Nathan Chancellor
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@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}" != "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@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}" != "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.
--
Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kernel] kbuild: get rid of unwanted "+" when CONFIG_LOCALVERSION is set
2023-09-20 13:30 ` Alexey Kardashevskiy
2023-09-22 0:59 ` Alexey Kardashevskiy
@ 2023-09-22 7:27 ` Masahiro Yamada
1 sibling, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2023-09-22 7:27 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: linux-kbuild, linux-kernel, Nicolas Schier, Nick Desaulniers,
Nathan Chancellor
On Wed, Sep 20, 2023 at 10:30 PM Alexey Kardashevskiy <aik@amd.com> wrote:
>
>
> On 20/9/23 21:59, Masahiro Yamada wrote:
> > On Wed, Sep 20, 2023 at 10:56 AM 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}" != "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=-foo make
or
$ make LOCALVERSION=-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=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@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}" != "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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kernel] kbuild: get rid of unwanted "+" when CONFIG_LOCALVERSION is set
2023-09-22 0:59 ` Alexey Kardashevskiy
@ 2023-09-22 7:31 ` Masahiro Yamada
0 siblings, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2023-09-22 7:31 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: linux-kbuild, linux-kernel, Nicolas Schier, Nick Desaulniers,
Nathan Chancellor
On Fri, Sep 22, 2023 at 9:59 AM Alexey Kardashevskiy <aik@amd.com> 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@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}" != "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@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}" != "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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-22 7:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-20 1:55 [PATCH kernel] kbuild: get rid of unwanted "+" when CONFIG_LOCALVERSION is set Alexey Kardashevskiy
2023-09-20 11:59 ` Masahiro Yamada
2023-09-20 13:30 ` Alexey Kardashevskiy
2023-09-22 0:59 ` Alexey Kardashevskiy
2023-09-22 7:31 ` Masahiro Yamada
2023-09-22 7:27 ` Masahiro Yamada
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.