[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGS

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGS

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
all files get rebuilt. In a continuous integration environment, the value
will change every build. This wastes time assuming that incremental
builds would otherwise occur.

To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
"local version".

This has other advantages too:
- The special case for BUILD_TAG in display_options.c can be removed.
- The version printed by the "version" command exactly matches what is
  printed at boot.

Old sign-on message:
U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas

New sign-on message:
U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 Makefile                | 4 ----
 lib/display_options.c   | 4 ----
 scripts/setlocalversion | 4 ++++
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 42fad45afee1..2265b8995a7b 100644
--- a/Makefile
+++ b/Makefile
@@ -562,10 +562,6 @@ else
 KBUILD_CFLAGS	+= -O2
 endif
 
-ifdef BUILD_TAG
-KBUILD_CFLAGS += -DBUILD_TAG='"$(BUILD_TAG)"'
-endif
-
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
 
diff --git a/lib/display_options.c b/lib/display_options.c
index 29343fc00e3f..5dcdf4e429af 100644
--- a/lib/display_options.c
+++ b/lib/display_options.c
@@ -15,11 +15,7 @@
 
 int display_options (void)
 {
-#if defined(BUILD_TAG)
-	printf ("\n\n%s, Build: %s\n\n", version_string, BUILD_TAG);
-#else
 	printf ("\n\n%s\n\n", version_string);
-#endif
 	return 0;
 }
 
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 63d91e22ed7c..4ef6603b5c27 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -171,4 +171,8 @@ else
 	fi
 fi
 
+if test -n "${BUILD_TAG}"; then
+	res="$res-${BUILD_TAG}"
+fi
+
 echo "$res"
-- 
2.7.0

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGS

[James Chargin]

On 02/08/2016 05:32 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
> all files get rebuilt. In a continuous integration environment, the value
> will change every build. This wastes time assuming that incremental
> builds would otherwise occur.
>
> To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
> "local version".
>
> This has other advantages too:
> - The special case for BUILD_TAG in display_options.c can be removed.
> - The version printed by the "version" command exactly matches what is
>    printed at boot.
>
> Old sign-on message:
> U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas
>
> New sign-on message:
> U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)

I would urge this not be done. The display of the BUILD_TAG on startup 
is pretty useful in my environment. It's been there for a long time and 
some of my users have grown used to it.

Of all the parts of the sign-on message, I'd rather the git hash go away 
than the BUILD_TAG. None of my users really care about the level of 
detail of the git hash and won't spend the time required to use this 
hash to determine if they have the version they want. (Some don't have a 
repo clone, and don't care to, and so can't easily make the 
correspondence even if they wanted to).

Thanks for your consideration.

Jim


>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>   Makefile                | 4 ----
>   lib/display_options.c   | 4 ----
>   scripts/setlocalversion | 4 ++++
>   3 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 42fad45afee1..2265b8995a7b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -562,10 +562,6 @@ else
>   KBUILD_CFLAGS	+= -O2
>   endif
>
> -ifdef BUILD_TAG
> -KBUILD_CFLAGS += -DBUILD_TAG='"$(BUILD_TAG)"'
> -endif
> -
>   KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
>   KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
>
> diff --git a/lib/display_options.c b/lib/display_options.c
> index 29343fc00e3f..5dcdf4e429af 100644
> --- a/lib/display_options.c
> +++ b/lib/display_options.c
> @@ -15,11 +15,7 @@
>
>   int display_options (void)
>   {
> -#if defined(BUILD_TAG)
> -	printf ("\n\n%s, Build: %s\n\n", version_string, BUILD_TAG);
> -#else
>   	printf ("\n\n%s\n\n", version_string);
> -#endif
>   	return 0;
>   }
>
> diff --git a/scripts/setlocalversion b/scripts/setlocalversion
> index 63d91e22ed7c..4ef6603b5c27 100755
> --- a/scripts/setlocalversion
> +++ b/scripts/setlocalversion
> @@ -171,4 +171,8 @@ else
>   	fi
>   fi
>
> +if test -n "${BUILD_TAG}"; then
> +	res="$res-${BUILD_TAG}"
> +fi
> +
>   echo "$res"
>

-- 
Jim Chargin
AJA Video Systems                       jimc at aja.com
(530) 271-3334                          http://www.aja.com

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGS

[Tom Rini]

On Tue, Feb 09, 2016 at 08:11:15AM -0800, James Chargin wrote:
> 
> 
> On 02/08/2016 05:32 PM, Stephen Warren wrote:
> >From: Stephen Warren <swarren@nvidia.com>
> >
> >If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
> >all files get rebuilt. In a continuous integration environment, the value
> >will change every build. This wastes time assuming that incremental
> >builds would otherwise occur.
> >
> >To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
> >"local version".
> >
> >This has other advantages too:
> >- The special case for BUILD_TAG in display_options.c can be removed.
> >- The version printed by the "version" command exactly matches what is
> >   printed at boot.
> >
> >Old sign-on message:
> >U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas
> >
> >New sign-on message:
> >U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)
> 
> I would urge this not be done. The display of the BUILD_TAG on
> startup is pretty useful in my environment. It's been there for a
> long time and some of my users have grown used to it.
> 
> Of all the parts of the sign-on message, I'd rather the git hash go
> away than the BUILD_TAG. None of my users really care about the
> level of detail of the git hash and won't spend the time required to
> use this hash to determine if they have the version they want. (Some
> don't have a repo clone, and don't care to, and so can't easily make
> the correspondence even if they wanted to).

Yeah, I think this is too widely used of a thing to change.  FWIW, I
really like githashes since it means you can see if $random-binary is
something you have in $vendor-tree or not.  So I think in this case, NAK
on the patch and maybe need to poke Travis-CI on how to or not to tag
things?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160209/c1663dca/attachment.sig>

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGS

[Stephen Warren]

On 02/09/2016 10:01 AM, Tom Rini wrote:
> On Tue, Feb 09, 2016 at 08:11:15AM -0800, James Chargin wrote:
>>
>>
>> On 02/08/2016 05:32 PM, Stephen Warren wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
>>> all files get rebuilt. In a continuous integration environment, the value
>>> will change every build. This wastes time assuming that incremental
>>> builds would otherwise occur.
>>>
>>> To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
>>> "local version".
>>>
>>> This has other advantages too:
>>> - The special case for BUILD_TAG in display_options.c can be removed.
>>> - The version printed by the "version" command exactly matches what is
>>>    printed at boot.
>>>
>>> Old sign-on message:
>>> U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas
>>>
>>> New sign-on message:
>>> U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)
>>
>> I would urge this not be done. The display of the BUILD_TAG on
>> startup is pretty useful in my environment. It's been there for a
>> long time and some of my users have grown used to it.
>>
>> Of all the parts of the sign-on message, I'd rather the git hash go
>> away than the BUILD_TAG. None of my users really care about the
>> level of detail of the git hash and won't spend the time required to
>> use this hash to determine if they have the version they want. (Some
>> don't have a repo clone, and don't care to, and so can't easily make
>> the correspondence even if they wanted to).
>
> Yeah, I think this is too widely used of a thing to change.  FWIW, I
> really like githashes since it means you can see if $random-binary is
> something you have in $vendor-tree or not.  So I think in this case, NAK
> on the patch and maybe need to poke Travis-CI on how to or not to tag
> things?

Do you mean you think people currently rely upon the ", Build: xxx" 
format in the sign-on message, and the "version" command /not/ printing 
that information?

If so, I'll go back to trying to work out how to get Kbuild to transfer 
the environment variable into a CONFIG_XXX option so that modifying it 
only causes a rebuild of the one file that uses the value.

I could easily hack my Jenkins build scripts to unset that variable, but 
I do like the test logs showing the Jenkins build ID so I can validate 
the correct binary actually ran. So I'd rather not simply remove the tag 
from the message.

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGSilver pepper blue dog

[Tom Rini]

On Tue, Feb 09, 2016 at 10:26:14AM -0700, Stephen Warren wrote:
> On 02/09/2016 10:01 AM, Tom Rini wrote:
> >On Tue, Feb 09, 2016 at 08:11:15AM -0800, James Chargin wrote:
> >>
> >>
> >>On 02/08/2016 05:32 PM, Stephen Warren wrote:
> >>>From: Stephen Warren <swarren@nvidia.com>
> >>>
> >>>If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
> >>>all files get rebuilt. In a continuous integration environment, the value
> >>>will change every build. This wastes time assuming that incremental
> >>>builds would otherwise occur.
> >>>
> >>>To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
> >>>"local version".
> >>>
> >>>This has other advantages too:
> >>>- The special case for BUILD_TAG in display_options.c can be removed.
> >>>- The version printed by the "version" command exactly matches what is
> >>>   printed at boot.
> >>>
> >>>Old sign-on message:
> >>>U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas
> >>>
> >>>New sign-on message:
> >>>U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)
> >>
> >>I would urge this not be done. The display of the BUILD_TAG on
> >>startup is pretty useful in my environment. It's been there for a
> >>long time and some of my users have grown used to it.
> >>
> >>Of all the parts of the sign-on message, I'd rather the git hash go
> >>away than the BUILD_TAG. None of my users really care about the
> >>level of detail of the git hash and won't spend the time required to
> >>use this hash to determine if they have the version they want. (Some
> >>don't have a repo clone, and don't care to, and so can't easily make
> >>the correspondence even if they wanted to).
> >
> >Yeah, I think this is too widely used of a thing to change.  FWIW, I
> >really like githashes since it means you can see if $random-binary is
> >something you have in $vendor-tree or not.  So I think in this case, NAK
> >on the patch and maybe need to poke Travis-CI on how to or not to tag
> >things?
> 
> Do you mean you think people currently rely upon the ", Build: xxx"
> format in the sign-on message, and the "version" command /not/
> printing that information?

I mean that the places we construct strings like this are likely relied
upon by people, yes.

> If so, I'll go back to trying to work out how to get Kbuild to
> transfer the environment variable into a CONFIG_XXX option so that
> modifying it only causes a rebuild of the one file that uses the
> value.

A quick peak and I think that:
(a) LOCALVERSION needs a little love somewhere to ensure that it in fact
does not exceed 64 characters (and pushing upstream to the kernel
anything relevant here)
(b) Yes, adjusting BUILD_TAG into CONFIG_BUILD_TAG is probably the best
answer and probably not even that bad.  I don't think we need an
automagic conversion, just noting it in the release emails and the
sudden lack of Build: should get people that are using it to update
their scripts.

> I could easily hack my Jenkins build scripts to unset that variable,
> but I do like the test logs showing the Jenkins build ID so I can
> validate the correct binary actually ran. So I'd rather not simply
> remove the tag from the message.

Good point.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160209/d55b9544/attachment.sig>

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGSilver pepper blue dog

[Stephen Warren]

On 02/09/2016 10:43 AM, Tom Rini wrote:
> On Tue, Feb 09, 2016 at 10:26:14AM -0700, Stephen Warren wrote:
>> On 02/09/2016 10:01 AM, Tom Rini wrote:
>>> On Tue, Feb 09, 2016 at 08:11:15AM -0800, James Chargin wrote:
>>>>
>>>>
>>>> On 02/08/2016 05:32 PM, Stephen Warren wrote:
>>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>>
>>>>> If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
>>>>> all files get rebuilt. In a continuous integration environment, the value
>>>>> will change every build. This wastes time assuming that incremental
>>>>> builds would otherwise occur.
>>>>>
>>>>> To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
>>>>> "local version".
>>>>>
>>>>> This has other advantages too:
>>>>> - The special case for BUILD_TAG in display_options.c can be removed.
>>>>> - The version printed by the "version" command exactly matches what is
>>>>>    printed at boot.
>>>>>
>>>>> Old sign-on message:
>>>>> U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas
>>>>>
>>>>> New sign-on message:
>>>>> U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)
>>>>
>>>> I would urge this not be done. The display of the BUILD_TAG on
>>>> startup is pretty useful in my environment. It's been there for a
>>>> long time and some of my users have grown used to it.
>>>>
>>>> Of all the parts of the sign-on message, I'd rather the git hash go
>>>> away than the BUILD_TAG. None of my users really care about the
>>>> level of detail of the git hash and won't spend the time required to
>>>> use this hash to determine if they have the version they want. (Some
>>>> don't have a repo clone, and don't care to, and so can't easily make
>>>> the correspondence even if they wanted to).
>>>
>>> Yeah, I think this is too widely used of a thing to change.  FWIW, I
>>> really like githashes since it means you can see if $random-binary is
>>> something you have in $vendor-tree or not.  So I think in this case, NAK
>>> on the patch and maybe need to poke Travis-CI on how to or not to tag
>>> things?
>>
>> Do you mean you think people currently rely upon the ", Build: xxx"
>> format in the sign-on message, and the "version" command /not/
>> printing that information?
>
> I mean that the places we construct strings like this are likely relied
> upon by people, yes.
>
>> If so, I'll go back to trying to work out how to get Kbuild to
>> transfer the environment variable into a CONFIG_XXX option so that
>> modifying it only causes a rebuild of the one file that uses the
>> value.
>
> A quick peak and I think that:
> (a) LOCALVERSION needs a little love somewhere to ensure that it in fact
> does not exceed 64 characters (and pushing upstream to the kernel
> anything relevant here)
> (b) Yes, adjusting BUILD_TAG into CONFIG_BUILD_TAG is probably the best
> answer and probably not even that bad.  I don't think we need an
> automagic conversion, just noting it in the release emails and the
> sudden lack of Build: should get people that are using it to update
> their scripts.

Unfortunately, simply renaming the variable won't work; Kconfig/Kbuild 
need to know something about it in order to do their 
rebuild-only-on-change magic (and the value must be removed from CFLAGS 
too). I was having a hard time getting that to work, but I'll take 
another look.

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGS

[Tom Rini]

On Tue, Feb 09, 2016 at 10:53:48AM -0700, Stephen Warren wrote:
> On 02/09/2016 10:43 AM, Tom Rini wrote:
> >On Tue, Feb 09, 2016 at 10:26:14AM -0700, Stephen Warren wrote:
> >>On 02/09/2016 10:01 AM, Tom Rini wrote:
> >>>On Tue, Feb 09, 2016 at 08:11:15AM -0800, James Chargin wrote:
> >>>>
> >>>>
> >>>>On 02/08/2016 05:32 PM, Stephen Warren wrote:
> >>>>>From: Stephen Warren <swarren@nvidia.com>
> >>>>>
> >>>>>If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
> >>>>>all files get rebuilt. In a continuous integration environment, the value
> >>>>>will change every build. This wastes time assuming that incremental
> >>>>>builds would otherwise occur.
> >>>>>
> >>>>>To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
> >>>>>"local version".
> >>>>>
> >>>>>This has other advantages too:
> >>>>>- The special case for BUILD_TAG in display_options.c can be removed.
> >>>>>- The version printed by the "version" command exactly matches what is
> >>>>>   printed at boot.
> >>>>>
> >>>>>Old sign-on message:
> >>>>>U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas
> >>>>>
> >>>>>New sign-on message:
> >>>>>U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)
> >>>>
> >>>>I would urge this not be done. The display of the BUILD_TAG on
> >>>>startup is pretty useful in my environment. It's been there for a
> >>>>long time and some of my users have grown used to it.
> >>>>
> >>>>Of all the parts of the sign-on message, I'd rather the git hash go
> >>>>away than the BUILD_TAG. None of my users really care about the
> >>>>level of detail of the git hash and won't spend the time required to
> >>>>use this hash to determine if they have the version they want. (Some
> >>>>don't have a repo clone, and don't care to, and so can't easily make
> >>>>the correspondence even if they wanted to).
> >>>
> >>>Yeah, I think this is too widely used of a thing to change.  FWIW, I
> >>>really like githashes since it means you can see if $random-binary is
> >>>something you have in $vendor-tree or not.  So I think in this case, NAK
> >>>on the patch and maybe need to poke Travis-CI on how to or not to tag
> >>>things?
> >>
> >>Do you mean you think people currently rely upon the ", Build: xxx"
> >>format in the sign-on message, and the "version" command /not/
> >>printing that information?
> >
> >I mean that the places we construct strings like this are likely relied
> >upon by people, yes.
> >
> >>If so, I'll go back to trying to work out how to get Kbuild to
> >>transfer the environment variable into a CONFIG_XXX option so that
> >>modifying it only causes a rebuild of the one file that uses the
> >>value.
> >
> >A quick peak and I think that:
> >(a) LOCALVERSION needs a little love somewhere to ensure that it in fact
> >does not exceed 64 characters (and pushing upstream to the kernel
> >anything relevant here)
> >(b) Yes, adjusting BUILD_TAG into CONFIG_BUILD_TAG is probably the best
> >answer and probably not even that bad.  I don't think we need an
> >automagic conversion, just noting it in the release emails and the
> >sudden lack of Build: should get people that are using it to update
> >their scripts.
> 
> Unfortunately, simply renaming the variable won't work;
> Kconfig/Kbuild need to know something about it in order to do their
> rebuild-only-on-change magic (and the value must be removed from
> CFLAGS too). I was having a hard time getting that to work, but I'll
> take another look.

Right.  Can't we just make it another string Kconfig like LOCALVERSION,
not need to pass -D.... and then it's set via poking the config file by
CI, etc?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160209/81722045/attachment.sig>

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGS

[Masahiro Yamada]

Hi.



2016-02-10 3:18 GMT+09:00 Tom Rini <trini@konsulko.com>:
> On Tue, Feb 09, 2016 at 10:53:48AM -0700, Stephen Warren wrote:
>> On 02/09/2016 10:43 AM, Tom Rini wrote:
>> >On Tue, Feb 09, 2016 at 10:26:14AM -0700, Stephen Warren wrote:
>> >>On 02/09/2016 10:01 AM, Tom Rini wrote:
>> >>>On Tue, Feb 09, 2016 at 08:11:15AM -0800, James Chargin wrote:
>> >>>>
>> >>>>
>> >>>>On 02/08/2016 05:32 PM, Stephen Warren wrote:
>> >>>>>From: Stephen Warren <swarren@nvidia.com>
>> >>>>>
>> >>>>>If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
>> >>>>>all files get rebuilt. In a continuous integration environment, the value
>> >>>>>will change every build. This wastes time assuming that incremental
>> >>>>>builds would otherwise occur.
>> >>>>>
>> >>>>>To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
>> >>>>>"local version".
>> >>>>>
>> >>>>>This has other advantages too:
>> >>>>>- The special case for BUILD_TAG in display_options.c can be removed.
>> >>>>>- The version printed by the "version" command exactly matches what is
>> >>>>>   printed at boot.
>> >>>>>
>> >>>>>Old sign-on message:
>> >>>>>U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas
>> >>>>>
>> >>>>>New sign-on message:
>> >>>>>U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)
>> >>>>
>> >>>>I would urge this not be done. The display of the BUILD_TAG on
>> >>>>startup is pretty useful in my environment. It's been there for a
>> >>>>long time and some of my users have grown used to it.
>> >>>>
>> >>>>Of all the parts of the sign-on message, I'd rather the git hash go
>> >>>>away than the BUILD_TAG. None of my users really care about the
>> >>>>level of detail of the git hash and won't spend the time required to
>> >>>>use this hash to determine if they have the version they want. (Some
>> >>>>don't have a repo clone, and don't care to, and so can't easily make
>> >>>>the correspondence even if they wanted to).
>> >>>
>> >>>Yeah, I think this is too widely used of a thing to change.  FWIW, I
>> >>>really like githashes since it means you can see if $random-binary is
>> >>>something you have in $vendor-tree or not.  So I think in this case, NAK
>> >>>on the patch and maybe need to poke Travis-CI on how to or not to tag
>> >>>things?
>> >>
>> >>Do you mean you think people currently rely upon the ", Build: xxx"
>> >>format in the sign-on message, and the "version" command /not/
>> >>printing that information?
>> >
>> >I mean that the places we construct strings like this are likely relied
>> >upon by people, yes.
>> >
>> >>If so, I'll go back to trying to work out how to get Kbuild to
>> >>transfer the environment variable into a CONFIG_XXX option so that
>> >>modifying it only causes a rebuild of the one file that uses the
>> >>value.
>> >
>> >A quick peak and I think that:
>> >(a) LOCALVERSION needs a little love somewhere to ensure that it in fact
>> >does not exceed 64 characters (and pushing upstream to the kernel
>> >anything relevant here)
>> >(b) Yes, adjusting BUILD_TAG into CONFIG_BUILD_TAG is probably the best
>> >answer and probably not even that bad.  I don't think we need an
>> >automagic conversion, just noting it in the release emails and the
>> >sudden lack of Build: should get people that are using it to update
>> >their scripts.
>>
>> Unfortunately, simply renaming the variable won't work;
>> Kconfig/Kbuild need to know something about it in order to do their
>> rebuild-only-on-change magic (and the value must be removed from
>> CFLAGS too). I was having a hard time getting that to work, but I'll
>> take another look.
>
> Right.  Can't we just make it another string Kconfig like LOCALVERSION,
> not need to pass -D.... and then it's set via poking the config file by
> CI, etc?
>



We are discussing two things together.

[1] avoid full build
[2] change of the version string format and build-flow



Nobody would complain about fixing [1] only.
(Feel free to continue discussion if you want [2].)


As Stephen pointed out, KBUILD_CFLAGS is passed to all the C files.


The environment "BUILD_TAG" is only referenced from lib/display_option.c,
so how about passing the flag to the file only?


Try the following patch:


diff --git a/Makefile b/Makefile
index a46c1ae..77cb8fa 100644
--- a/Makefile
+++ b/Makefile
@@ -562,10 +562,6 @@ else
 KBUILD_CFLAGS  += -O2
 endif

-ifdef BUILD_TAG
-KBUILD_CFLAGS += -DBUILD_TAG='"$(BUILD_TAG)"'
-endif
-
 KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
 KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)

diff --git a/lib/Makefile b/lib/Makefile
index dd36f25..9325faa 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -67,6 +67,9 @@ obj-$(CONFIG_ADDR_MAP) += addr_map.o
 obj-y += hashtable.o
 obj-y += errno.o
 obj-y += display_options.o
+
+CFLAGS_display_options.o := $(if $(BUILD_TAG),-DBUILD_TAG='"$(BUILD_TAG)"')
+
 obj-$(CONFIG_BCH) += bch.o
 obj-y += crc32.o
 obj-y += ctype.o





-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH] Makefile: remove BUILD_TAG from KBUILD_CFLAGS

[Stephen Warren]

On 02/09/2016 10:58 PM, Masahiro Yamada wrote:
> 2016-02-10 3:18 GMT+09:00 Tom Rini <trini@konsulko.com>:
>> On Tue, Feb 09, 2016 at 10:53:48AM -0700, Stephen Warren wrote:
>>> On 02/09/2016 10:43 AM, Tom Rini wrote:
>>>> On Tue, Feb 09, 2016 at 10:26:14AM -0700, Stephen Warren wrote:
>>>>> On 02/09/2016 10:01 AM, Tom Rini wrote:
>>>>>> On Tue, Feb 09, 2016 at 08:11:15AM -0800, James Chargin wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 02/08/2016 05:32 PM, Stephen Warren wrote:
>>>>>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>>>>>
>>>>>>>> If BUILD_TAG is part of KBUILD_CFLAGS, then any time the value changes,
>>>>>>>> all files get rebuilt. In a continuous integration environment, the value
>>>>>>>> will change every build. This wastes time assuming that incremental
>>>>>>>> builds would otherwise occur.
>>>>>>>>
>>>>>>>> To solve this, remove BUILD_TAG from KBUILD_FLAGS and add it to the end of
>>>>>>>> "local version".
>>>>>>>>
>>>>>>>> This has other advantages too:
>>>>>>>> - The special case for BUILD_TAG in display_options.c can be removed.
>>>>>>>> - The version printed by the "version" command exactly matches what is
>>>>>>>>    printed at boot.
>>>>>>>>
>>>>>>>> Old sign-on message:
>>>>>>>> U-Boot 2016.03-rc1-00044-g4085db5e767b (Feb ...), Build: bar-bas
>>>>>>>>
>>>>>>>> New sign-on message:
>>>>>>>> U-Boot 2016.03-rc1-00044-g4085db5e767b-bar-baz (Feb ...)
>>>>>>>
>>>>>>> I would urge this not be done. The display of the BUILD_TAG on
>>>>>>> startup is pretty useful in my environment. It's been there for a
>>>>>>> long time and some of my users have grown used to it.
>>>>>>>
>>>>>>> Of all the parts of the sign-on message, I'd rather the git hash go
>>>>>>> away than the BUILD_TAG. None of my users really care about the
>>>>>>> level of detail of the git hash and won't spend the time required to
>>>>>>> use this hash to determine if they have the version they want. (Some
>>>>>>> don't have a repo clone, and don't care to, and so can't easily make
>>>>>>> the correspondence even if they wanted to).
>>>>>>
>>>>>> Yeah, I think this is too widely used of a thing to change.  FWIW, I
>>>>>> really like githashes since it means you can see if $random-binary is
>>>>>> something you have in $vendor-tree or not.  So I think in this case, NAK
>>>>>> on the patch and maybe need to poke Travis-CI on how to or not to tag
>>>>>> things?
>>>>>
>>>>> Do you mean you think people currently rely upon the ", Build: xxx"
>>>>> format in the sign-on message, and the "version" command /not/
>>>>> printing that information?
>>>>
>>>> I mean that the places we construct strings like this are likely relied
>>>> upon by people, yes.
>>>>
>>>>> If so, I'll go back to trying to work out how to get Kbuild to
>>>>> transfer the environment variable into a CONFIG_XXX option so that
>>>>> modifying it only causes a rebuild of the one file that uses the
>>>>> value.
>>>>
>>>> A quick peak and I think that:
>>>> (a) LOCALVERSION needs a little love somewhere to ensure that it in fact
>>>> does not exceed 64 characters (and pushing upstream to the kernel
>>>> anything relevant here)
>>>> (b) Yes, adjusting BUILD_TAG into CONFIG_BUILD_TAG is probably the best
>>>> answer and probably not even that bad.  I don't think we need an
>>>> automagic conversion, just noting it in the release emails and the
>>>> sudden lack of Build: should get people that are using it to update
>>>> their scripts.
>>>
>>> Unfortunately, simply renaming the variable won't work;
>>> Kconfig/Kbuild need to know something about it in order to do their
>>> rebuild-only-on-change magic (and the value must be removed from
>>> CFLAGS too). I was having a hard time getting that to work, but I'll
>>> take another look.
>>
>> Right.  Can't we just make it another string Kconfig like LOCALVERSION,
>> not need to pass -D.... and then it's set via poking the config file by
>> CI, etc?
>
> We are discussing two things together.
>
> [1] avoid full build
> [2] change of the version string format and build-flow
>
> Nobody would complain about fixing [1] only.
> (Feel free to continue discussion if you want [2].)
>
> As Stephen pointed out, KBUILD_CFLAGS is passed to all the C files.
>
> The environment "BUILD_TAG" is only referenced from lib/display_option.c,
> so how about passing the flag to the file only?
>
> Try the following patch:
>
> diff --git a/lib/Makefile b/lib/Makefile

> +CFLAGS_display_options.o := $(if $(BUILD_TAG),-DBUILD_TAG='"$(BUILD_TAG)"')

That's a nice simple idea, and should work well. I'll go validate it.

The only disadvantage is that if someone wants to start using BUILD_TAG 
elsewhere, they'd have to manually add it to CFLAGS for that file too. 
If we could import the environment variable into Kconfig automatically 
somehow as CONFIG_BUILD_TAG, that would not be needed; using 
CONFIG_BUILD_TAG would "just work". However, if you think we can live 
with that, I'm all for it.