Patchwork [01/22] Remove the DATE variable

login
register
mail settings
Submitter Thomas Petazzoni
Date April 17, 2012, 2:45 p.m.
Message ID <3a993084fda775e7a2c37371225c124fd620a244.1334673910.git.thomas.petazzoni@free-electrons.com>
Download mbox | patch
Permalink /patch/153198/
State Rejected
Headers show

Comments

Thomas Petazzoni - April 17, 2012, 2:45 p.m.
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(-)
Arnout Vandecappelle - April 21, 2012, 1:05 p.m.
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
Peter Korsgaard - April 22, 2012, 8:23 a.m.
>>>>> "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).
Thomas Petazzoni - April 22, 2012, 3:32 p.m.
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
Peter Korsgaard - April 22, 2012, 7:55 p.m.
>>>>> "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
Thomas De Schampheleire - April 24, 2012, 7:43 a.m.
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
Peter Korsgaard - April 24, 2012, 8:31 a.m.
>>>>> "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

Patch

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))