Message ID | 3a993084fda775e7a2c37371225c124fd620a244.1334673910.git.thomas.petazzoni@free-electrons.com |
---|---|
State | Rejected |
Headers | show |
On Tuesday 17 April 2012 16:45:13 Thomas Petazzoni wrote: > This variable was only used by uboot.mk to add a #define DATE into the > U-Boot .h configuration file. But such a definition does not seem to > be used by the mainline U-Boot, so there's no reason to keep it. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> This patch could be considered risky, because the DATE thing may be used by some custom U-Boot source. But then, the only way to find out if it is is to commit it and see if anyone complains :-) Regards, Arnout
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes: Arnout> On Tuesday 17 April 2012 16:45:13 Thomas Petazzoni wrote: >> This variable was only used by uboot.mk to add a #define DATE into the >> U-Boot .h configuration file. But such a definition does not seem to >> be used by the mainline U-Boot, so there's no reason to keep it. >> >> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Arnout> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Arnout> This patch could be considered risky, because the DATE thing Arnout> may be used by some custom U-Boot source. But then, the only Arnout> way to find out if it is is to commit it and see if anyone Arnout> complains :-) I know we have been (are?) using it at work to add some info about the build date and name some files with the date. Chances are others might have been doing the same, so I'm hessitating about removing it (which is why I didn't commit it right away). It doesn't really cost us much to keep it (single line).
Le Sun, 22 Apr 2012 10:23:15 +0200, Peter Korsgaard <jacmet@uclibc.org> a écrit : > I know we have been (are?) using it at work to add some info about the > build date and name some files with the date. > > Chances are others might have been doing the same, so I'm hessitating > about removing it (which is why I didn't commit it right away). It > doesn't really cost us much to keep it (single line). Well, it's just that I don't very much like to have unused variables here and there. One way to make it useful is for example to store the build date into some file in the target filesystem. Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
Hi,
Thomas> Well, it's just that I don't very much like to have unused
Thomas> variables here and there. One way to make it useful is for
Thomas> example to store the build date into some file in the target
Thomas> filesystem.
True. We could perhaps add it to /etc/os-release?
http://www.freedesktop.org/software/systemd/man/os-release.html
On Sun, Apr 22, 2012 at 9:55 PM, Peter Korsgaard <jacmet@uclibc.org> wrote: >>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > > Hi, > > Thomas> Well, it's just that I don't very much like to have unused > Thomas> variables here and there. One way to make it useful is for > Thomas> example to store the build date into some file in the target > Thomas> filesystem. > > True. We could perhaps add it to /etc/os-release? > > http://www.freedesktop.org/software/systemd/man/os-release.html > A disadvantage of adding changing variables like DATE is that two builds that are built from the exact same set of sources, are not binary equal. You cannot simply compare the rootfs image and expect no differences. You'd have to unpack the rootfs and compare file by file, leaving out files like /etc/os-release that would contain a date. I haven't yet looked at which binaries inside a typical rootfs do contain such changing variables, but I will do that for my configuration in the near future. My intention is to ensure binary equality for the entire rootfs. With this in mind, I would not favor adding a DATE of some kind to a file in buildroot just to mark a variable as used, unless we provide a way to override DATE in the config file (in the Linux kernel, the 'user' and 'machine' etc. build variables are also overridable. Best regards, Thomas
>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> writes:
Hi,
Thomas> A disadvantage of adding changing variables like DATE is that
Thomas> two builds that are built from the exact same set of sources,
Thomas> are not binary equal. You cannot simply compare the rootfs
Thomas> image and expect no differences. You'd have to unpack the
Thomas> rootfs and compare file by file, leaving out files like
Thomas> /etc/os-release that would contain a date.
Thomas> I haven't yet looked at which binaries inside a typical rootfs do
Thomas> contain such changing variables, but I will do that for my
Thomas> configuration in the near future. My intention is to ensure binary
Thomas> equality for the entire rootfs.
I think you'll find that this is quite common. From the top of my head I
know that atleast the Linux kernel and busybox adds the build time to
the binary, so I doubt this will really work.
Thomas> With this in mind, I would not favor adding a DATE of some kind
Thomas> to a file in buildroot just to mark a variable as used, unless
Thomas> we provide a way to override DATE in the config file (in the
Thomas> Linux kernel, the 'user' and 'machine' etc. build variables are
Thomas> also overridable.
You can always build with make DATE=foo
diff --git a/Makefile b/Makefile index 3a09417..f56c8ee 100644 --- a/Makefile +++ b/Makefile @@ -39,7 +39,6 @@ endif TOPDIR:=$(shell pwd) CONFIG_CONFIG_IN=Config.in CONFIG=support/kconfig -DATE:=$(shell date +%Y%m%d) # Compute the full local version string so packages can use it as-is # Need to export it, so it can be got from environment in children (eg. mconf) diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk index db9de8d..83696be 100644 --- a/boot/uboot/uboot.mk +++ b/boot/uboot/uboot.mk @@ -69,7 +69,6 @@ define UBOOT_CONFIGURE_CMDS @echo "/* Add a wrapper around the values Buildroot sets. */" >> $(@D)/include/config.h @echo "#ifndef __BR2_ADDED_CONFIG_H" >> $(@D)/include/config.h @echo "#define __BR2_ADDED_CONFIG_H" >> $(@D)/include/config.h - $(call insert_define,DATE,$(DATE)) $(call insert_define,CONFIG_LOAD_SCRIPTS,1) $(call insert_define,CONFIG_IPADDR,$(BR2_TARGET_UBOOT_IPADDR)) $(call insert_define,CONFIG_GATEWAYIP,$(BR2_TARGET_UBOOT_GATEWAY))
This variable was only used by uboot.mk to add a #define DATE into the U-Boot .h configuration file. But such a definition does not seem to be used by the mainline U-Boot, so there's no reason to keep it. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- Makefile | 1 - boot/uboot/uboot.mk | 1 - 2 files changed, 0 insertions(+), 2 deletions(-)