diff mbox

[12/12] reproducible/syslinux: make syslinux build reproducible

Message ID 1465918337-30732-3-git-send-email-gilles.chanteperdrix@xenomai.org
State Changes Requested
Headers show

Commit Message

Gilles Chanteperdrix June 14, 2016, 3:32 p.m. UTC 
Build with the target toolchain so that the binaries are identical with
different host toolchains.
Sort files lists in order to get deterministic link order.
Build with HEXDATE set to the source date epoch.

Signed-off-by: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
---
 boot/syslinux/0001-fixed-build-order.patch | 42 ++++++++++++++++++++++++++
 boot/syslinux/syslinux.mk                  | 47 +++++++++++++++++++++++++-----
 fs/iso9660/iso9660.mk                      |  4 +--
 3 files changed, 84 insertions(+), 9 deletions(-)
 create mode 100644 boot/syslinux/0001-fixed-build-order.patch

Comments

Yann E. MORIN July 17, 2016, 7:44 p.m. UTC | #1 
Gilles, All,

On 2016-06-14 17:32 +0200, Gilles Chanteperdrix spake thusly:
> Build with the target toolchain so that the binaries are identical with
> different host toolchains.
> Sort files lists in order to get deterministic link order.
> Build with HEXDATE set to the source date epoch.

It looks like those are three different changes, so should have been
three different patches.

Especially the change to use the cross-toolchain should really be
separate (and come first).

Further comments below...

> Signed-off-by: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
[--SNIP--]
> diff --git a/boot/syslinux/0001-fixed-build-order.patch b/boot/syslinux/0001-fixed-build-order.patch
> new file mode 100644
> index 0000000..3697b74
> --- /dev/null
> +++ b/boot/syslinux/0001-fixed-build-order.patch
> @@ -0,0 +1,42 @@
> +Sort source file names in order for the link order not to depend on the order in
> +which find return file names.
> +
> +Signed-off-by: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>

Have you tried submitting this patch upstream?

We do not much like having feature patches in Buildroot, because they
are a pain to maintain when we want to update the package.

Otherwise, this looks pretty simple, I guess upstream will probably like
it. ;-)

[--SNIP--]
> diff --git a/boot/syslinux/syslinux.mk b/boot/syslinux/syslinux.mk
> index 82890c5..cdd5b3c 100644
> --- a/boot/syslinux/syslinux.mk
> +++ b/boot/syslinux/syslinux.mk
> @@ -13,7 +13,7 @@ SYSLINUX_LICENSE_FILES = COPYING
>  
>  SYSLINUX_INSTALL_IMAGES = YES
>  
> -SYSLINUX_DEPENDENCIES = host-nasm host-util-linux host-upx
> +SYSLINUX_DEPENDENCIES = host-nasm host-util-linux host-upx host-perl host-python host-xz

Why are those new host packages needed?

>  ifeq ($(BR2_TARGET_SYSLINUX_LEGACY_BIOS),y)
>  SYSLINUX_TARGET += bios
> @@ -47,12 +47,35 @@ define SYSLINUX_CLEANUP
>  endef
>  SYSLINUX_POST_PATCH_HOOKS += SYSLINUX_CLEANUP
>  
> +ifeq ($(BR2_REPRODUCIBLE),y)
> +define SYSLINUX_REPRODUCIBLE
> +	HEXDATE="`printf "0x%x" $(SOURCE_DATE_EPOCH)`"
> +endef
> +endif
> +
> +define SYSLINUX_MAKE
> +	$(TARGET_MAKE_ENV) $(MAKE1) \
> +		$(SYSLINUX_REPRODUCIBLE) \
> +		NASM=$(HOST_DIR)/usr/bin/nasm \
> +		PERL=$(HOST_DIR)/usr/bin/perl \
> +		PYTHON=$(HOST_DIR)/usr/bin/python \

Why do we need to specify nasm, perl and python? The PATH as set by
Buildroot already has the host dirs early in the PATH, so they should
be found before the system ones.

> +		UPX=$(HOST_DIR)/usr/bin/upx \
> +		CC="$(TARGET_CC)" \
> +		LD="$(TARGET_LD) -m elf_i386" \
> +		OBJDUMP="$(TARGET_OBJDUMP)" \
> +		OBJCOPY="$(TARGET_OBJCOPY)" \
> +		STRIP="$(TARGET_STRIP)" \
> +		AR="$(TARGET_AR)" \
> +		NM="$(TARGET_NM)" \
> +		RANLIB="$(TARGET_RANLIB)" \
> +		XZ=$(HOST_DIR)/usr/bin/xz $(SYSLINUX_EFI_ARGS)

Ditto xz.

You're also adding more variables than were present in the existing
commands; that's why using the cross-toolchain should be a separate
patch: so that we can more easily understand the changes.

> +endef
> +
>  # syslinux build system has no convenient way to pass CFLAGS,
>  # and the internal zlib should take precedence so -I shouldn't
>  # be used.
>  define SYSLINUX_BUILD_CMDS
> -	$(TARGET_MAKE_ENV) $(MAKE1) CC="$(HOSTCC) -idirafter $(HOST_DIR)/usr/include $(HOST_LDFLAGS)" \
> -		AR="$(HOSTAR)" $(SYSLINUX_EFI_ARGS) -C $(@D) $(SYSLINUX_TARGET)
> +	$(SYSLINUX_MAKE) -C $(@D) $(SYSLINUX_TARGET)
>  endef
>  
>  # While the actual bootloader is compiled for the target, several
> @@ -61,8 +84,7 @@ endef
>  # Repeat CC and AR, since syslinux really wants to check them at
>  # install time
>  define SYSLINUX_INSTALL_TARGET_CMDS
> -	$(TARGET_MAKE_ENV) $(MAKE1) CC="$(HOSTCC) -idirafter $(HOST_DIR)/usr/include $(HOST_LDFLAGS)" \
> -		AR="$(HOSTAR)" $(SYSLINUX_EFI_ARGS) INSTALLROOT=$(HOST_DIR) \
> +	$(SYSLINUX_MAKE) INSTALLROOT=$(@D)/inst \
>  		-C $(@D) $(SYSLINUX_TARGET) install
>  endef
>  
> @@ -80,10 +102,21 @@ define SYSLINUX_INSTALL_IMAGES_CMDS
>  	for i in $(SYSLINUX_IMAGES-y); do \
>  		$(INSTALL) -D -m 0755 $(@D)/$$i $(BINARIES_DIR)/syslinux/$${i##*/}; \
>  	done
> -	for i in $(SYSLINUX_C32); do \
> -		$(INSTALL) -D -m 0755 $(HOST_DIR)/usr/share/syslinux/$${i} \
> +	for i in $(SYSLINUX_C32) ldlinux.c32; do \
> +		$(INSTALL) -D -m 0755 $(@D)/inst/usr/share/syslinux/$${i} \
>  			$(BINARIES_DIR)/syslinux/$${i}; \
>  	done
>  endef
>  
> +define HOST_SYSLINUX_BUILD_CMDS
> +       $(HOST_MAKE_ENV) $(MAKE1) CC="$(HOSTCC) -idirafter $(HOST_DIR)/usr/include $(HOST_LDFLAGS)" \
> +-               AR="$(HOSTAR)" -C $(@D) bios
   ^
Leading dash here?...

Also, use TABs for indentation of the *_CMDS defines.

But then, you are building the 'bios' stuff with the host compiler.
Doesn't that defeats the very purpose of that patch, and contradicts the
commit log itself (which states that we are now using the target
toolchain) ?

> +endef
> +
> +define HOST_SYSLINUX_INSTALL_CMDS
> +       $(HOST_MAKE_ENV) $(MAKE1) CC="$(HOSTCC) -idirafter $(HOST_DIR)/usr/include $(HOST_LDFLAGS)" \
> +-               AR="$(HOSTAR)" -C $(@D) INSTALLROOT=$(HOST_DIR) bios install
> +endef
> +
>  $(eval $(generic-package))
> +$(eval $(host-generic-package))

Since you submitted this patch, we've changed the way how dependencies
of host packages are handled: they are no longer automatically inherited
from the dependencies of the target variant; you now have to explicitly
define the dependencies of the host variant.

Regards,
Yann E. MORIN.

> diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
> index f97a9d7..db22ca4 100644
> --- a/fs/iso9660/iso9660.mk
> +++ b/fs/iso9660/iso9660.mk
> @@ -70,8 +70,6 @@ ROOTFS_ISO9660_BOOT_IMAGE = isolinux/isolinux.bin
>  define ROOTFS_ISO9660_INSTALL_BOOTLOADER
>  	$(INSTALL) -D -m 0644 $(BINARIES_DIR)/syslinux/* \
>  		$(ROOTFS_ISO9660_TARGET_DIR)/isolinux/
> -	$(INSTALL) -D -m 0644 $(HOST_DIR)/usr/share/syslinux/ldlinux.c32 \
> -		$(ROOTFS_ISO9660_TARGET_DIR)/isolinux/ldlinux.c32
>  endef
>  endif
>  
> @@ -166,6 +164,8 @@ define ROOTFS_ISO9660_CMD
>  endef
>  
>  ifeq ($(BR2_TARGET_ROOTFS_ISO9660_HYBRID),y)
> +ROOTFS_ISO9660_DEPENDENCIES += host-syslinux
> +
>  define ROOTFS_ISO9660_GEN_HYBRID
>  	$(ROOTFS_ISO9660_ISOHYBRID) -t 0x96 $@
>  endef
> -- 
> 2.8.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
diff mbox

Patch

diff --git a/boot/syslinux/0001-fixed-build-order.patch b/boot/syslinux/0001-fixed-build-order.patch
new file mode 100644
index 0000000..3697b74
--- /dev/null
+++ b/boot/syslinux/0001-fixed-build-order.patch
@@ -0,0 +1,42 @@ 
+Sort source file names in order for the link order not to depend on the order in
+which find return file names.
+
+Signed-off-by: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
+-- 
+--- syslinux-6.03/core/Makefile~	2014-10-06 18:27:44.000000000 +0200
++++ syslinux-6.03/core/Makefile	2016-03-24 14:36:38.207391899 +0100
+@@ -41,9 +41,9 @@
+ # All primary source files for the main syslinux files
+ NASMSRC	 := $(wildcard $(SRC)/*.asm)
+ NASMHDR  := $(wildcard $(SRC)/*.inc)
+-CSRC	 := $(shell find $(SRC) -name '*.c' -print)
+-SSRC	 := $(shell find $(SRC) -name '*.S' -print)
+-CHDR	 := $(shell find $(SRC) -name '*.h' -print)
++CSRC	 := $(shell find $(SRC) -name '*.c' -print | sort)
++SSRC	 := $(shell find $(SRC) -name '*.S' -print | sort)
++CHDR	 := $(shell find $(SRC) -name '*.h' -print | sort)
+ OTHERSRC := keywords
+ ALLSRC    = $(NASMSRC) $(NASMHDR) $(CSRC) $(SSRC) $(CHDR) $(OTHERSRC)
+ 
+@@ -56,18 +56,18 @@
+ 	$(addprefix $(SRC)/fs/pxe/, dhcp_option.c pxe.c tftp.c urlparse.c bios.c)
+ 
+ LPXELINUX_CSRC = $(CORE_PXE_CSRC) \
+-	$(shell find $(SRC)/lwip -name '*.c' -print) \
++	$(shell find $(SRC)/lwip -name '*.c' -print | sort) \
+ 	$(addprefix $(SRC)/fs/pxe/, \
+ 		core.c dnsresolv.c ftp.c ftp_readdir.c gpxeurl.c http.c \
+ 		http_readdir.c idle.c isr.c tcp.c)
+ 
+ PXELINUX_CSRC = $(CORE_PXE_CSRC) \
+-	$(shell find $(SRC)/legacynet -name '*.c' -print)
++	$(shell find $(SRC)/legacynet -name '*.c' -print | sort)
+ 
+ LPXELINUX_OBJS = $(subst $(SRC)/,,$(LPXELINUX_CSRC:%.c=%.o))
+ PXELINUX_OBJS  = $(subst $(SRC)/,,$(PXELINUX_CSRC:%.c=%.o))
+ 
+-UNITTEST_CSRC = $(shell find $(SRC) -path '*/tests/*.c' -print)
++UNITTEST_CSRC = $(shell find $(SRC) -path '*/tests/*.c' -print | sort)
+ UNITTEST_OBJS = $(subst $(SRC)/,,$(UNITTEST_CSRC:%.c=%.o))
+ 
+ # Don't include console and network stack specific objects or unit tests
diff --git a/boot/syslinux/syslinux.mk b/boot/syslinux/syslinux.mk
index 82890c5..cdd5b3c 100644
--- a/boot/syslinux/syslinux.mk
+++ b/boot/syslinux/syslinux.mk
@@ -13,7 +13,7 @@  SYSLINUX_LICENSE_FILES = COPYING
 
 SYSLINUX_INSTALL_IMAGES = YES
 
-SYSLINUX_DEPENDENCIES = host-nasm host-util-linux host-upx
+SYSLINUX_DEPENDENCIES = host-nasm host-util-linux host-upx host-perl host-python host-xz
 
 ifeq ($(BR2_TARGET_SYSLINUX_LEGACY_BIOS),y)
 SYSLINUX_TARGET += bios
@@ -47,12 +47,35 @@  define SYSLINUX_CLEANUP
 endef
 SYSLINUX_POST_PATCH_HOOKS += SYSLINUX_CLEANUP
 
+ifeq ($(BR2_REPRODUCIBLE),y)
+define SYSLINUX_REPRODUCIBLE
+	HEXDATE="`printf "0x%x" $(SOURCE_DATE_EPOCH)`"
+endef
+endif
+
+define SYSLINUX_MAKE
+	$(TARGET_MAKE_ENV) $(MAKE1) \
+		$(SYSLINUX_REPRODUCIBLE) \
+		NASM=$(HOST_DIR)/usr/bin/nasm \
+		PERL=$(HOST_DIR)/usr/bin/perl \
+		PYTHON=$(HOST_DIR)/usr/bin/python \
+		UPX=$(HOST_DIR)/usr/bin/upx \
+		CC="$(TARGET_CC)" \
+		LD="$(TARGET_LD) -m elf_i386" \
+		OBJDUMP="$(TARGET_OBJDUMP)" \
+		OBJCOPY="$(TARGET_OBJCOPY)" \
+		STRIP="$(TARGET_STRIP)" \
+		AR="$(TARGET_AR)" \
+		NM="$(TARGET_NM)" \
+		RANLIB="$(TARGET_RANLIB)" \
+		XZ=$(HOST_DIR)/usr/bin/xz $(SYSLINUX_EFI_ARGS)
+endef
+
 # syslinux build system has no convenient way to pass CFLAGS,
 # and the internal zlib should take precedence so -I shouldn't
 # be used.
 define SYSLINUX_BUILD_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE1) CC="$(HOSTCC) -idirafter $(HOST_DIR)/usr/include $(HOST_LDFLAGS)" \
-		AR="$(HOSTAR)" $(SYSLINUX_EFI_ARGS) -C $(@D) $(SYSLINUX_TARGET)
+	$(SYSLINUX_MAKE) -C $(@D) $(SYSLINUX_TARGET)
 endef
 
 # While the actual bootloader is compiled for the target, several
@@ -61,8 +84,7 @@  endef
 # Repeat CC and AR, since syslinux really wants to check them at
 # install time
 define SYSLINUX_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE1) CC="$(HOSTCC) -idirafter $(HOST_DIR)/usr/include $(HOST_LDFLAGS)" \
-		AR="$(HOSTAR)" $(SYSLINUX_EFI_ARGS) INSTALLROOT=$(HOST_DIR) \
+	$(SYSLINUX_MAKE) INSTALLROOT=$(@D)/inst \
 		-C $(@D) $(SYSLINUX_TARGET) install
 endef
 
@@ -80,10 +102,21 @@  define SYSLINUX_INSTALL_IMAGES_CMDS
 	for i in $(SYSLINUX_IMAGES-y); do \
 		$(INSTALL) -D -m 0755 $(@D)/$$i $(BINARIES_DIR)/syslinux/$${i##*/}; \
 	done
-	for i in $(SYSLINUX_C32); do \
-		$(INSTALL) -D -m 0755 $(HOST_DIR)/usr/share/syslinux/$${i} \
+	for i in $(SYSLINUX_C32) ldlinux.c32; do \
+		$(INSTALL) -D -m 0755 $(@D)/inst/usr/share/syslinux/$${i} \
 			$(BINARIES_DIR)/syslinux/$${i}; \
 	done
 endef
 
+define HOST_SYSLINUX_BUILD_CMDS
+       $(HOST_MAKE_ENV) $(MAKE1) CC="$(HOSTCC) -idirafter $(HOST_DIR)/usr/include $(HOST_LDFLAGS)" \
+-               AR="$(HOSTAR)" -C $(@D) bios
+endef
+
+define HOST_SYSLINUX_INSTALL_CMDS
+       $(HOST_MAKE_ENV) $(MAKE1) CC="$(HOSTCC) -idirafter $(HOST_DIR)/usr/include $(HOST_LDFLAGS)" \
+-               AR="$(HOSTAR)" -C $(@D) INSTALLROOT=$(HOST_DIR) bios install
+endef
+
 $(eval $(generic-package))
+$(eval $(host-generic-package))
diff --git a/fs/iso9660/iso9660.mk b/fs/iso9660/iso9660.mk
index f97a9d7..db22ca4 100644
--- a/fs/iso9660/iso9660.mk
+++ b/fs/iso9660/iso9660.mk
@@ -70,8 +70,6 @@  ROOTFS_ISO9660_BOOT_IMAGE = isolinux/isolinux.bin
 define ROOTFS_ISO9660_INSTALL_BOOTLOADER
 	$(INSTALL) -D -m 0644 $(BINARIES_DIR)/syslinux/* \
 		$(ROOTFS_ISO9660_TARGET_DIR)/isolinux/
-	$(INSTALL) -D -m 0644 $(HOST_DIR)/usr/share/syslinux/ldlinux.c32 \
-		$(ROOTFS_ISO9660_TARGET_DIR)/isolinux/ldlinux.c32
 endef
 endif
 
@@ -166,6 +164,8 @@  define ROOTFS_ISO9660_CMD
 endef
 
 ifeq ($(BR2_TARGET_ROOTFS_ISO9660_HYBRID),y)
+ROOTFS_ISO9660_DEPENDENCIES += host-syslinux
+
 define ROOTFS_ISO9660_GEN_HYBRID
 	$(ROOTFS_ISO9660_ISOHYBRID) -t 0x96 $@
 endef