Hi.
2016-02-10 3:18 GMT+09:00 Tom Rini <tr...@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 <swar...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
