Add per-package staging feature

To do that add a patch already sent to upstream:
https://github.com/ffainelli/faifa/pull/9

This is in order to support the per-package staging directory.

Signed-off-by Fabio Porcedda <[email protected]>

---
 ...-handle-LDFLAGS-passed-to-the-configure-s.patch | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch

package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch new file +24 -0

new file mode 100644
index 0000000..5d54374
@@ -0,0 +1,40 @@
+From 62cafe305f4b9f5fb4305dcfb68ae3144efc3353 Mon Sep 17 00:00:00 2001
+From: Fabio Porcedda <Fabio Porcedda fabio.porcedda@gmail>
+Date: Wed, 18 Feb 2015 17:58:23 +0100
+Subject: [PATCH] Makefile.in: handle LDFLAGS passed to the configure script
+
+Taking in account CFLAGS and LDFLAGS passed to the configure script is a
+common behaviour.
+
+This fix is also useful for supporting the per-package staging directory
+within Buildroot.
+
+Signed-off-by: Fabio Porcedda <Fabio Porcedda fabio.porcedda@gmail>
+---
+ Makefile.in | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/Makefile.in b/Makefile.in
+index b1b7c58..792ac82 100644
+--- a/Makefile.in
++++ b/Makefile.in
+@@ -14,6 +14,7 @@
+ CC	= @CC@
+ STRIP	?= $(CROSS)strip
+ CFLAGS	= @CFLAGS@

package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch new file +3 -0

+ endif
+ 
+ LIBS:=$(LDFLAGS) $(LIBS)

Hi Fabio,

Quoted 45 lines · Fabio Porcedda

To do that add a patch already sent to upstream:
https://github.com/ffainelli/faifa/pull/9

This is in order to support the per-package staging directory.

Signed-off-by Fabio Porcedda <[email protected]>

---
 ...-handle-LDFLAGS-passed-to-the-configure-s.patch | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch

package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch new file +28 -0

new file mode 100644
index 0000000..5d54374
@@ -0,0 +1,40 @@
+From 62cafe305f4b9f5fb4305dcfb68ae3144efc3353 Mon Sep 17 00:00:00 2001
+From: Fabio Porcedda <Fabio Porcedda fabio.porcedda@gmail>
+Date: Wed, 18 Feb 2015 17:58:23 +0100
+Subject: [PATCH] Makefile.in: handle LDFLAGS passed to the configure script
+
+Taking in account CFLAGS and LDFLAGS passed to the configure script is a
+common behaviour.
+
+This fix is also useful for supporting the per-package staging directory
+within Buildroot.
+
+Signed-off-by: Fabio Porcedda <Fabio Porcedda fabio.porcedda@gmail>
+---
+ Makefile.in | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/Makefile.in b/Makefile.in
+index b1b7c58..792ac82 100644
+--- a/Makefile.in
++++ b/Makefile.in
+@@ -14,6 +14,7 @@
+ CC  = @CC@
+ STRIP       ?= $(CROSS)strip
+ CFLAGS      = @CFLAGS@
++LDFLAGS     = @LDFLAGS@
+ INSTALL     = @INSTALL@
+ LIBS        = @LIBS@
+

package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch line 34

Quoted 6 lines

package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch new file +6 -0

+@@ -50,7 +51,7 @@ SIM_LIBS:=-levent
+ SIM_CFLAGS:=-Wno-unused
+
+ ifeq ($(OS),DARWIN)
+-LDFLAGS:=-dynamiclib -Wl,-dylib_install_name -Wl,$(LIB_SONAME)
++LDFLAGS+=-dynamiclib -Wl,-dylib_install_name -Wl,$(LIB_SONAME)

Why do you need this hunk?

Quoted 1 line

Quoted 1 line

faifa v0.1 Building

Quoted 12 lines

Quoted 12 lines

package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch new file +12 -0

+diff --git a/Makefile.in b/Makefile.in
+index b1b7c58..792ac82 100644
+--- a/Makefile.in
++++ b/Makefile.in
+@@ -14,6 +14,7 @@
+ CC  = @CC@
+ STRIP       ?= $(CROSS)strip
+ CFLAGS      = @CFLAGS@
++LDFLAGS     = @LDFLAGS@
+ INSTALL     = @INSTALL@
+ LIBS        = @LIBS@
+

Quoted 8 lines

Quoted 6 lines

+@@ -50,7 +51,7 @@ SIM_LIBS:=-levent
+ SIM_CFLAGS:=-Wno-unused
+
+ ifeq ($(OS),DARWIN)
+-LDFLAGS:=-dynamiclib -Wl,-dylib_install_name -Wl,$(LIB_SONAME) ++LDFLAGS+=-dynamiclib -Wl,-dylib_install_name -Wl,$(LIB_SONAME)

Why do you need this hunk?

Because without that the Makefile doesn't use the TARGET_LDFLAGS passed by buildroot to the configure script, so it doesn't find the libraries in the per-package staging directory, and i get this build failure:

Quoted 1 line

Quoted 1 line

Quoted 1 line

faifa v0.1 Building

sha2.c: In function 'SHA256_Last':
sha2.c:492:2: warning: dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing] /home/tetsuya/buildroot/br2/output/host/opt/ext-toolchain/bin/../lib/gcc/i686-pc-linux-gnu/4.7.2/../../../../i686-pc-linux-gnu/bin/ld:
cannot find -lpcap
collect2: error: ld returned 1 exit status
Makefile:78: recipe for target 'libfaifa.so.0' failed
make[1]: *** [libfaifa.so.0] Error 1
package/pkg-generic.mk:182: recipe for target
'/home/tetsuya/buildroot/br2/output/build/faifa-v0.1/.stamp_built'
failed

Hi Fabio,

Quoted 27 lines · Fabio Porcedda

Quoted 12 lines

Quoted 12 lines

package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch new file +12 -0

+diff --git a/Makefile.in b/Makefile.in
+index b1b7c58..792ac82 100644
+--- a/Makefile.in
++++ b/Makefile.in
+@@ -14,6 +14,7 @@
+ CC  = @CC@
+ STRIP       ?= $(CROSS)strip
+ CFLAGS      = @CFLAGS@
++LDFLAGS     = @LDFLAGS@
+ INSTALL     = @INSTALL@
+ LIBS        = @LIBS@
+

Quoted 8 lines

Quoted 6 lines

+@@ -50,7 +51,7 @@ SIM_LIBS:=-levent
+ SIM_CFLAGS:=-Wno-unused
+
+ ifeq ($(OS),DARWIN)
+-LDFLAGS:=-dynamiclib -Wl,-dylib_install_name -Wl,$(LIB_SONAME) ++LDFLAGS+=-dynamiclib -Wl,-dylib_install_name -Wl,$(LIB_SONAME)

Why do you need this hunk?

Because without that the Makefile doesn't use the TARGET_LDFLAGS passed by buildroot to the configure script, so it doesn't find the libraries in the per-package staging directory, and i get this build failure:

Quoted 1 line

Quoted 1 line

Quoted 1 line

faifa v0.1 Building

Quoted 11 lines

sha2.c: In function 'SHA256_Last':
sha2.c:492:2: warning: dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing] /home/tetsuya/buildroot/br2/output/host/opt/ext-toolchain/bin/../lib/gcc/i686-pc-linux-gnu/4.7.2/../../../../i686-pc-linux-gnu/bin/ld:
cannot find -lpcap
collect2: error: ld returned 1 exit status
Makefile:78: recipe for target 'libfaifa.so.0' failed
make[1]: *** [libfaifa.so.0] Error 1
package/pkg-generic.mk:182: recipe for target
'/home/tetsuya/buildroot/br2/output/build/faifa-v0.1/.stamp_built'
failed

Does $(OS) really evaluates to "DARWIN" on your system? This looks quite weird.

Quoted 40 lines · Baruch Siach

Quoted 38 lines · Fabio Porcedda

Quoted 12 lines

Quoted 12 lines

package/faifa/0001-Makefile.in-handle-LDFLAGS-passed-to-the-configure-s.patch new file +12 -0

+diff --git a/Makefile.in b/Makefile.in
+index b1b7c58..792ac82 100644
+--- a/Makefile.in
++++ b/Makefile.in
+@@ -14,6 +14,7 @@
+ CC  = @CC@
+ STRIP       ?= $(CROSS)strip
+ CFLAGS      = @CFLAGS@
++LDFLAGS     = @LDFLAGS@
+ INSTALL     = @INSTALL@
+ LIBS        = @LIBS@
+

Quoted 8 lines

Quoted 6 lines

+@@ -50,7 +51,7 @@ SIM_LIBS:=-levent
+ SIM_CFLAGS:=-Wno-unused
+
+ ifeq ($(OS),DARWIN)
+-LDFLAGS:=-dynamiclib -Wl,-dylib_install_name -Wl,$(LIB_SONAME) ++LDFLAGS+=-dynamiclib -Wl,-dylib_install_name -Wl,$(LIB_SONAME)

Why do you need this hunk?

Because without that the Makefile doesn't use the TARGET_LDFLAGS passed by buildroot to the configure script, so it doesn't find the libraries in the per-package staging directory, and i get this build failure:

Quoted 1 line

Quoted 1 line

Quoted 1 line

faifa v0.1 Building

sha2.c: In function 'SHA256_Last':
sha2.c:492:2: warning: dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing] /home/tetsuya/buildroot/br2/output/host/opt/ext-toolchain/bin/../lib/gcc/i686-pc-linux-gnu/4.7.2/../../../../i686-pc-linux-gnu/bin/ld:
cannot find -lpcap
collect2: error: ld returned 1 exit status
Makefile:78: recipe for target 'libfaifa.so.0' failed
make[1]: *** [libfaifa.so.0] Error 1
package/pkg-generic.mk:182: recipe for target
'/home/tetsuya/buildroot/br2/output/build/faifa-v0.1/.stamp_built'
failed

Quoted 2 lines

Does $(OS) really evaluates to "DARWIN" on your system? This looks quite weird.

So are you just speaking about the last line, not the whole patch?

That line it's not useful on my system, but because the fafia project support the darwin os, I've added the change anyway, the change prevents that the LDFLAGS passed to the configure script will be overwritten.
I've done that because I've sent the patch upstream.

---
 package/lmbench/lmbench.mk | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

package/lmbench/lmbench.mk +0 -0

index 63a83fd..954f7eb 100644

package/lmbench/lmbench.mk +1 -1

 ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
 LMBENCH_DEPENDENCIES += libtirpc
 LMBENCH_CFLAGS += -I$(STAGING_DIR)/usr/include/tirpc/

@@ -28,7 +25,7 @@ define LMBENCH_CONFIGURE_CMDS
 endef
 
 define LMBENCH_BUILD_CMDS
-	$(MAKE) CFLAGS="$(LMBENCH_CFLAGS)" LDFLAGS="$(LMBENCH_LDFLAGS)" OS=$(ARCH) CC="$(TARGET_CC)" -C $(@D)/src
+	$(MAKE) CFLAGS="$(TARGET_CFLAGS) $(LMBENCH_CFLAGS)" LDFLAGS="$(TARGET_LDFLAGS) $(LMBENCH_LDFLAGS)" OS=$(ARCH) CC="$(TARGET_CC)" -C $(@D)/src
 endef
 
 define LMBENCH_INSTALL_TARGET_CMDS

Quoted 12 lines · Fabio Porcedda

---
 package/lmbench/lmbench.mk | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

package/lmbench/lmbench.mk +0 -0

index 63a83fd..954f7eb 100644
@@ -10,9 +10,6 @@ LMBENCH_SITE = http://downloads.sourceforge.net/project/lmbench/development/lmbe
 LMBENCH_LICENSE = lmbench license (based on GPLv2)
 LMBENCH_LICENSE_FILES = COPYING COPYING-2
 

package/lmbench/lmbench.mk LMBENCH_SITE = http://downloads.sourceforge.net/project/lmbench/development/lmbe · line 15

Quoted 3 lines

package/lmbench/lmbench.mk +0 -3

-LMBENCH_CFLAGS = $(TARGET_CFLAGS)
-LMBENCH_LDLIBS = $(TARGET_LDFLAGS)
-

Please see http://patchwork.ozlabs.org/patch/442832/.

Signed-off-by Fabio Porcedda <[email protected]>

---
 Makefile               |  2 +-
 package/Makefile.in    |  1 +
 package/pkg-generic.mk | 54 ++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 54 insertions(+), 3 deletions(-)

Makefile +1 -1

index 3723abc..6619bad 100644
@@ -832,7 +832,7 @@ printvars:
 
 clean:
 	rm -rf $(TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) \
-		$(BUILD_DIR) $(BASE_DIR)/staging \
+		$(BUILD_DIR) $(BASE_DIR)/staging $(STAGINGPKG_DIR) \
 		$(LEGAL_INFO_DIR)
 
 distclean: clean

package/Makefile.in +0 -0

index 6cfe3ed..2a21c9d 100644

package/pkg-generic.mk +1 -1

@@ -247,7 +247,7 @@ $(BUILD_DIR)/%/.stamp_target_installed:
 
 # Remove package sources
 $(BUILD_DIR)/%/.stamp_dircleaned:
-	rm -Rf $(@D)
+	rm -Rf $(@D) $(STAGINGPKG_DIR)/$(call LOWERCASE,$(PKG))
 
 ################################################################################
 # virt-provides-single -- check that provider-pkg is the declared provider for

package/pkg-generic.mk +15 -0

+ $(2)_TARGET_LDFLAGS = $$(TARGET_LDFLAGS) -L$$($(2)_STAGING_DIR)/usr/lib -Wl,-rpath,$$($(2)_STAGING_DIR)/usr/lib
+ $(2)_TARGET_CONFIGURE_OPTS = \
+	$$(TARGET_CONFIGURE_OPTS) \
+	PKG_CONFIG_SYSROOT_DIR="$$($(2)_STAGING_DIR)" \
+	PKG_CONFIG_PATH="$$($(2)_STAGING_DIR)/usr/lib/pkgconfig"
+ $(2)_TARGET_MAKE_ENV = \
+	$$(TARGET_MAKE_ENV) \
+	PKG_CONFIG_SYSROOT_DIR="$$($(2)_STAGING_DIR)" \
+	PKG_CONFIG_PATH="$$($(2)_STAGING_DIR)/usr/lib/pkgconfig"
+else
+ $(2)_STAGING_DIR = $$(STAGING_DIR)
+ $(2)_TARGET_CONFIGURE_OPTS = $$(TARGET_CONFIGURE_OPTS)
+ $(2)_TARGET_MAKE_ENV = $$(TARGET_MAKE_ENV)
+endif
+

package/pkg-generic.mk +22 -0

+ endef
+ $(2)_PRE_CONFIGURE_HOOKS := $(2)_PREPARE_STAGING_DIR $$($(2)_PRE_CONFIGURE_HOOKS)
+endif
+
 $(2)_INSTALL_STAGING		?= NO
 $(2)_INSTALL_IMAGES		?= NO
 $(2)_INSTALL_TARGET		?= YES

@@ -604,11 +636,29 @@ $(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)
 # define the PKG variable for all targets, containing the
 # uppercase package variable prefix
 $$($(2)_TARGET_INSTALL_TARGET):		PKG=$(2)
+$$($(2)_TARGET_INSTALL_TARGET):		STAGING_DIR:=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_INSTALL_TARGET):		TARGET_CPPFLAGS:=$$($(2)_TARGET_CPPFLAGS)
+$$($(2)_TARGET_INSTALL_TARGET):		TARGET_LDFLAGS:=$$($(2)_TARGET_LDFLAGS)
+$$($(2)_TARGET_INSTALL_TARGET):		TARGET_CONFIGURE_OPTS:=$$($(2)_TARGET_CONFIGURE_OPTS)
 $$($(2)_TARGET_INSTALL_STAGING):	PKG=$(2)
+$$($(2)_TARGET_INSTALL_STAGING):	STAGING_DIR:=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_INSTALL_STAGING):	TARGET_CPPFLAGS:=$$($(2)_TARGET_CPPFLAGS)
+$$($(2)_TARGET_INSTALL_STAGING):	TARGET_LDFLAGS:=$$($(2)_TARGET_LDFLAGS)
+$$($(2)_TARGET_INSTALL_STAGING):	TARGET_CONFIGURE_OPTS:=$$($(2)_TARGET_CONFIGURE_OPTS)
 $$($(2)_TARGET_INSTALL_IMAGES):		PKG=$(2)
 $$($(2)_TARGET_INSTALL_HOST):           PKG=$(2)
+$$($(2)_TARGET_INSTALL_HOST):		STAGING_DIR:=$$($(2)_STAGING_DIR)
 $$($(2)_TARGET_BUILD):			PKG=$(2)
+$$($(2)_TARGET_BUILD):			STAGING_DIR:=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_BUILD):			TARGET_CPPFLAGS:=$$($(2)_TARGET_CPPFLAGS)
+$$($(2)_TARGET_BUILD):			TARGET_LDFLAGS:=$$($(2)_TARGET_LDFLAGS)
+$$($(2)_TARGET_BUILD):			TARGET_CONFIGURE_OPTS:=$$($(2)_TARGET_CONFIGURE_OPTS)
+$$($(2)_TARGET_BUILD):			TARGET_MAKE_ENV:=$$($(2)_TARGET_MAKE_ENV)
 $$($(2)_TARGET_CONFIGURE):		PKG=$(2)
+$$($(2)_TARGET_CONFIGURE):		STAGING_DIR:=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_CONFIGURE):		TARGET_CPPFLAGS:=$$($(2)_TARGET_CPPFLAGS)
+$$($(2)_TARGET_CONFIGURE):		TARGET_LDFLAGS:=$$($(2)_TARGET_LDFLAGS)
+$$($(2)_TARGET_CONFIGURE):		TARGET_CONFIGURE_OPTS:=$$($(2)_TARGET_CONFIGURE_OPTS)
 $$($(2)_TARGET_RSYNC):                  SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
 $$($(2)_TARGET_RSYNC):                  PKG=$(2)
 $$($(2)_TARGET_RSYNC_SOURCE):		SRCDIR=$$($(2)_OVERRIDE_SRCDIR)

Quoted 1 line · Fabio Porcedda

Signed-off-by Fabio Porcedda <[email protected]>

Quoted 15 lines

+ $(2)_TARGET_LDFLAGS = $$(TARGET_LDFLAGS) -L$$($(2)_STAGING_DIR)/usr/lib -Wl,-rpath,$$($(2)_STAGING_DIR)/usr/lib
+ $(2)_TARGET_CONFIGURE_OPTS = \

package/pkg-generic.mk +13 -0

+     $$(TARGET_CONFIGURE_OPTS) \
+     PKG_CONFIG_SYSROOT_DIR="$$($(2)_STAGING_DIR)" \
+     PKG_CONFIG_PATH="$$($(2)_STAGING_DIR)/usr/lib/pkgconfig"
+ $(2)_TARGET_MAKE_ENV = \
+     $$(TARGET_MAKE_ENV) \
+     PKG_CONFIG_SYSROOT_DIR="$$($(2)_STAGING_DIR)" \
+     PKG_CONFIG_PATH="$$($(2)_STAGING_DIR)/usr/lib/pkgconfig"
+else
+ $(2)_STAGING_DIR = $$(STAGING_DIR)
+ $(2)_TARGET_CONFIGURE_OPTS = $$(TARGET_CONFIGURE_OPTS)
+ $(2)_TARGET_MAKE_ENV = $$(TARGET_MAKE_ENV)
+endif
+

package/pkg-generic.mk endif · line 461

Quoted 10 lines

package/pkg-generic.mk +10 -0

+ifeq ($$($(2)_ADD_TOOLCHAIN_DEPENDENCY),YES)
+ $(2)_STAGING_DIRS = $$(wildcard $$(foreach dep,\
+     $$(filter-out host-% toolchain,$$($(2)_FINAL_DEPENDENCIES)),\
+     $$($$(call UPPERCASE,$$(dep))_STAGING_DIR)))
+
+ define $(2)_PREPARE_STAGING_DIR
+     mkdir -p $$(STAGING_DIR)/usr/include $$(STAGING_DIR)/usr/lib
+     $$(if $$($(2)_STAGING_DIRS),
+             cp -rdpf $$(addsuffix /*,$$($(2)_STAGING_DIRS)) \
+                     $$($(2)_STAGING_DIR))

Why not just

                cp -rdpf -t $$($(2)_STAGING_DIR) $$($(2)_STAGING_DIRS)

?

this patch set aims to improve build reproducibility by using a per-package staging directory.
Also this feature aims to enable as default the top-level parallel make.

Dear Fabio Porcedda,

Quoted 3 lines · Fabio Porcedda

this patch set aims to improve build reproducibility by using a per-package staging directory.
Also this feature aims to enable as default the top-level parallel make.

Interesting! I must say I'm a bit worried about the implications of having to pass custom -I and -L flags all the time: we did arrange the compiler do not require those (by configuration for the internal toolchain case, and using the wrapper for the external toolchain case).

Could you give some hints about the performance impact of those copies of the staging directory? Does it continue to make the top-level parallel build thing useful?

I.e can you compare in terms of build time the following cases:

* Normal non-top-level parallel build.
* A top-level parallel build without this PPS feature
* A top-level parallel build with this PPS feature

Preferably something with a fairly deep dependency tree, i.e lots of small packages that depend on each other. A X.org stack + gtk2 would be a good example.

Quoted 3 lines

Could you give some hints about the performance impact of those copies of the staging directory? Does it continue to make the top-level parallel build thing useful?

IMHO the per-package staging feature is useful even without taking in consideration the top-level parallel build, the unspecified optional dependency issue are not common, they can generate build faulres or just binary with unexpected behavior.

Quoted 3 lines

Preferably something with a fairly deep dependency tree, i.e lots of small packages that depend on each other. A X.org stack + gtk2 would be a good example.

The most complete selection of package that i've tested is the defconfig + allyespacakgeconfig (693 targets), i've tried to enable a wider range of packages, defconfig + glibc + allyespackageconfig but even without using the pps they fail to build.
So i will provide build times of the configuration defconfig + allyespackageconfig.

If you provide a defconfig on the release 2015.02, i can try to build that.

Dear Fabio Porcedda,

Quoted 8 lines · Fabio Porcedda

Quoted 3 lines

Could you give some hints about the performance impact of those copies of the staging directory? Does it continue to make the top-level parallel build thing useful?

IMHO the per-package staging feature is useful even without taking in consideration the top-level parallel build, the unspecified optional dependency issue are not common, they can generate build faulres or just binary with unexpected behavior.

Yes, correct, this is indeed another benefit of this approach.

Quoted 10 lines

Quoted 3 lines

Preferably something with a fairly deep dependency tree, i.e lots of small packages that depend on each other. A X.org stack + gtk2 would be a good example.

The most complete selection of package that i've tested is the defconfig + allyespacakgeconfig (693 targets), i've tried to enable a wider range of packages, defconfig + glibc + allyespackageconfig but even without using the pps they fail to build.
So i will provide build times of the configuration defconfig + allyespackageconfig.

Ok.

Quoted 1 line

If you provide a defconfig on the release 2015.02, i can try to build that.

Something like:

BR2_arm=y
BR2_TOOLCHAIN_BUILDROOT_LARGEFILE=y
BR2_TOOLCHAIN_BUILDROOT_INET_IPV6=y
BR2_TOOLCHAIN_BUILDROOT_INET_RPC=y
BR2_TOOLCHAIN_BUILDROOT_LOCALE=y
BR2_TOOLCHAIN_BUILDROOT_CXX=y
BR2_PACKAGE_XORG7=y
BR2_PACKAGE_XSERVER_XORG_SERVER=y
BR2_PACKAGE_LIBGTK3=y

Could you give some hints about the performance impact of those copies of the staging directory? Does it continue to make the top-level parallel build thing useful?

Quoted 3 lines · Thomas Petazzoni

Could you give some hints about the performance impact of those copies of the staging directory? Does it continue to make the top-level parallel build thing useful?

In this case, it may make sense to use file linking (cp -al) instead of file copy, assuming the staging spaces are on the same filesystem ?

Dear Fabio Porcedda,

Quoted 3 lines · Fabio Porcedda

Quoted 3 lines

this patch set aims to improve build reproducibility by using a per-package staging directory.
Also this feature aims to enable as default the top-level parallel make.

Interesting! I must say I'm a bit worried about the implications of having to pass custom -I and -L flags all the time: we did arrange the compiler do not require those (by configuration for the internal toolchain case, and using the wrapper for the external toolchain case).

Quoted 11 lines · Thomas Petazzoni

Dear Fabio Porcedda,

Quoted 3 lines · Fabio Porcedda

Quoted 3 lines

this patch set aims to improve build reproducibility by using a per-package staging directory.
Also this feature aims to enable as default the top-level parallel make.

Interesting! I must say I'm a bit worried about the implications of having to pass custom -I and -L flags all the time: we did arrange the compiler do not require those (by configuration for the internal toolchain case, and using the wrapper for the external toolchain case).

I also like this feature very much!

However, I don't like at all the approach of passing the custom -I and -L
flags. I think it's much better to start with something that was suggested before in the BR developer meeting: use the toolchain wrapper also for the internal toolchain. Then we can pass the per-package staging dir through the environment, and use it in the --sysroot argument of the compiler.

We'll also have to do something about ccache then - that could be a problem.
Perhaps ccache should be called from within the wrapper as well, that may solve a bunch of other ccache problems.

On Thu, Mar 5, 2015 at 11:48 PM, Arnout Vandecappelle <[email protected]> wrote:

Quoted 6 lines

However, I don't like at all the approach of passing the custom -I and -L
flags. I think it's much better to start with something that was suggested before in the BR developer meeting: use the toolchain wrapper also for the internal toolchain. Then we can pass the per-package staging dir through the environment, and use it in the --sysroot argument of the compiler.

Quoted 3 lines

We'll also have to do something about ccache then - that could be a problem.
Perhaps ccache should be called from within the wrapper as well, that may solve a bunch of other ccache problems.

Using an internal wrapper we can also pass pass the -I and -L automatically like the --sysroot option?

If we use a per-package sysroot we need to copy the sysroot directory too, because the size of a uclibc sysroot is at least 11MB (if i remember correctly the size of a glibc sysroot is something like 40MB), so if we copy without using hard links, it will take at least 10MB up to 40MB for every package.

The size of the output directory of a defconfig + allyespackageconfig is 11GB. IMHO we have the following choices:
a) common sysroot & per-package staging & without hard links
- the -I -L options are passed trough the toolchain wrapper
- output size is 16GB (11GB+5GB)

- no side effects to fear, because hard links are not used
b) per-package sysroot & without hard links
- sysroot are passed trough the toolchain wrapper
- output size will be 16GB plus 11MB * 694 = 16GB + 7GB = 23 GB
- no side effects to fear, because hard links are not used
c) per-package sysroot & with hard links for toolchain

- sysroot are passed trough the toolchain wrapper
- output size will be 16GB
- side effects to fear for the toolchain files because hard links
are used for them
d) per-package sysroot & with hard links for all files
- sysroot are passed trough the toolchain wrapper
- output size will be 11GB, so no increased size
- side effects to fear because hard links are used

The cheaper one with almost no size and speed penalization is d) but if some file is overwritten other package sysroot could be affected. Maybe we can prevent the overwriting by removing the write permission on staging files?

Which solution is most suited for buildroot?

Thanks and my BR

Quoted 1 line · Fabio Porcedda

On Thu, Mar 5, 2015 at 11:48 PM, Arnout Vandecappelle <[email protected]> wrote:

[snip]

Quoted 6 lines

Quoted 6 lines

However, I don't like at all the approach of passing the custom -I and -L
flags. I think it's much better to start with something that was suggested before in the BR developer meeting: use the toolchain wrapper also for the internal toolchain. Then we can pass the per-package staging dir through the environment, and use it in the --sysroot argument of the compiler.

Quoted 6 lines

Quoted 3 lines

We'll also have to do something about ccache then - that could be a problem.
Perhaps ccache should be called from within the wrapper as well, that may solve a bunch of other ccache problems.

Using an internal wrapper we can also pass pass the -I and -L automatically like the --sysroot option?

Yes we can. Of course, the path to the package sysroot has to be passed through
the environment.

Quoted 7 lines

If we use a per-package sysroot we need to copy the sysroot directory too, because the size of a uclibc sysroot is at least 11MB (if i remember correctly the size of a glibc sysroot is something like 40MB), so if we copy without using hard links, it will take at least 10MB up to 40MB for every package.

Quoted 5 lines

The size of the output directory of a defconfig + allyespackageconfig is 11GB. IMHO we have the following choices:
a) common sysroot & per-package staging & without hard links
- the -I -L options are passed trough the toolchain wrapper
- output size is 16GB (11GB+5GB)

Where does the 5GB come from? Experimental evidence from running this series?

Quoted 6 lines

- no side effects to fear, because hard links are not used
b) per-package sysroot & without hard links
- sysroot are passed trough the toolchain wrapper
- output size will be 16GB plus 11MB * 694 = 16GB + 7GB = 23 GB
- no side effects to fear, because hard links are not used
c) per-package sysroot & with hard links for toolchain

Quoted 8 lines

- sysroot are passed trough the toolchain wrapper
- output size will be 16GB
- side effects to fear for the toolchain files because hard links
are used for them
d) per-package sysroot & with hard links for all files
- sysroot are passed trough the toolchain wrapper
- output size will be 11GB, so no increased size
- side effects to fear because hard links are used

Actually I'm not really concerned about the size, even if it's more than
doubled by this feature. We use factors less space than yocto so we can afford to double it :-)

I'm more concerned about the time penalty of copying things around all the
time. So in that respect, option a) or d) are more attractive. But even creating all the hardlinks takes a lot of time (probably not that much faster than copying).

A completely wild alternative is to build and use unionfs-fuse, but that
requires fusermount to be properly installed on the host. Can we assume that?

Quoted 5 lines

The cheaper one with almost no size and speed penalization is d) but if some file is overwritten other package sysroot could be affected. Maybe we can prevent the overwriting by removing the write permission on staging files?

Yep, that's an option. It can actually be done while rsyncing.

this patch set aims to improve build reproducibility by using a per-package staging directory.
Also this feature aims to enable as default the top-level parallel make.

Dear Fabio Porcedda,

Quoted 3 lines · Fabio Porcedda

this patch set aims to improve build reproducibility by using a per-package staging directory.
Also this feature aims to enable as default the top-level parallel make.

Here are some comments I wrote down a long time ago. It isn't a full review, but it should give some initial thoughts:

If I understood correctly, the strategy used by your patches consists in storing only the bare toolchain sysroot in output/staging/, and using a per-package output/stagingpkg/<pkg>/ directory to store as an input to the build the staging files needed to build the package, and also as output the result of the build of the package.

I am not entirely happy with this solution, for the following reasons:

* We no longer have a single staging directory that has all the libraries installed. This is needed for the toolchain to be a proper SDK usable by application developers outside of Buildroot.

To solve this, either we install everything to both the real STAGING_DIR and a per-package staging directory, or we create a final build step that consolidates all the per-package staging directories into a global one. The advantage of this second option is that we don't have any parallel installation going on in the global staging directory.

* I am not super happy with the idea of having the toolchain sysroot left in the global staging directory and referenced by the compiler --sysroot option on one side, and all other libraries found by using -L, -I and pkg-config tricks.

I would actually prefer if a real complete sysroot was used when building each package, and the compiler --sysroot option used to adjust the compiler sysroot. This has however one significant
drawback: the toolchain sysroot must be copied for each and every package, which can become quite time and space consuming. So on this aspect, I'd like to have some input from other Buildroot developers.

* Your patch uses $($(2)_ADD_TOOLCHAIN_DEPENDENCY) to decide whether the per-package sysroot mechanism must be used or not. Which means it will only be used for target packages, and not for host packages. However, I'm wondering if we should not also apply the principle to host packages: to get reproducible builds, I believe we should also have separate sysroots when building host packages. Opinions from other Buildroot developers?

* Your approach only takes care of make the sysroot handled in a per-package fashion. But what about HOST_DIR ? We could have the same inconsistencies as the ones we discussed about STAGING_DIR, but this time caused by the presence/absence of host utilities. One build may give a given result because host tool "foo" is present (it happened to be built before), and the next build may give a different result because host tool "foo" is absent (it's going to be build after).

Now some more specific comments:

- The "enable parallel" patch should come last in the series.

- The patches "popt: add to the "popt.pc" file the libintl library" and "logrotate: use pkg-config for the opt library" have been merged in master, so you could get rid of them from your series.

- In patch "xinetd: use TARGET_LDFLAGS in order to support per-package staging", why don't we simply use the LDFLAGS environment variable, which seems to be understood by xinetd Makefile? Also, missing SoB line.

- Patch "iproute2: use the TARGET_LDFLAGS variable" seems good to go, if the commit log doesn't mention per-package sysroot, so that we can merge it independently.

- Patch "opentyrian: use TARGET_LDFLAGS", having -lm before $(TARGET_LDFLAGS) is probably more logical.

I didn't finished reviewing all of the package specific patches.

Quoted 6 lines

I am not entirely happy with this solution, for the following reasons:

* We no longer have a single staging directory that has all the libraries installed. This is needed for the toolchain to be a proper SDK usable by application developers outside of Buildroot.

Quoted 6 lines

To solve this, either we install everything to both the real STAGING_DIR and a per-package staging directory, or we create a final build step that consolidates all the per-package staging directories into a global one. The advantage of this second option is that we don't have any parallel installation going on in the global staging directory.

I personally prefer too the second solution.

Quoted 5 lines

* I am not super happy with the idea of having the toolchain sysroot left in the global staging directory and referenced by the compiler --sysroot option on one side, and all other libraries found by using -L, -I and pkg-config tricks.

Quoted 7 lines

I would actually prefer if a real complete sysroot was used when building each package, and the compiler --sysroot option used to adjust the compiler sysroot. This has however one significant
drawback: the toolchain sysroot must be copied for each and every package, which can become quite time and space consuming. So on this aspect, I'd like to have some input from other Buildroot developers.

I've already rewritten the patch set using the --sysroot option in the toolchain wrapper. I just need to clean it up and send it, i hope to be able to send it tomorrow so we can choose the best solution for buildroot.

Quoted 8 lines

* Your patch uses $($(2)_ADD_TOOLCHAIN_DEPENDENCY) to decide whether the per-package sysroot mechanism must be used or not. Which means it will only be used for target packages, and not for host packages. However, I'm wondering if we should not also apply the principle to host packages: to get reproducible builds, I believe we should also have separate sysroots when building host packages. Opinions from other Buildroot developers?

Quoted 8 lines

* Your approach only takes care of make the sysroot handled in a per-package fashion. But what about HOST_DIR ? We could have the same inconsistencies as the ones we discussed about STAGING_DIR, but this time caused by the presence/absence of host utilities. One build may give a given result because host tool "foo" is present (it happened to be built before), and the next build may give a different result because host tool "foo" is absent (it's going to be build after).

You are right, but if possible i just want to handle the host utilities matter as a next step after this step is done.

Also, and while this is definitely reaching much further than doing top-level parallel build, I'm wondering if we shouldn't do the Buildroot builds within a well-controlled chroot. This would allow us to control the host environment, and ultimately cache the build results and re-use them across builds. This is clearly not simple, as we need to capture all the dependencies that have been built, their version, their configuration, etc. I have no idea if it is reasonable to do that in Buildroot, or if it is really too complicated to be compatible with the "keep it simple" principle of Buildroot.

On 06/15/15 11:17, Thomas Petazzoni wrote:
[snip]

Quoted 9 lines

Also, and while this is definitely reaching much further than doing top-level parallel build, I'm wondering if we shouldn't do the Buildroot builds within a well-controlled chroot. This would allow us to control the host environment, and ultimately cache the build results and re-use them across builds. This is clearly not simple, as we need to capture all the dependencies that have been built, their version, their configuration, etc. I have no idea if it is reasonable to do that in Buildroot, or if it is really too complicated to be compatible with the "keep it simple" principle of Buildroot.

That starts to smell more and more like NixOS... I.e., too complicated for
buildroot. But if someone proposes a patch and its diffstat looks acceptable, the option is of course still open.

Quoted 3 lines

That starts to smell more and more like NixOS... I.e., too complicated for
buildroot. But if someone proposes a patch and its diffstat looks acceptable, the option is of course still open.

Maybe at least initially can be solved using an external script that prepare the system and launch buildroot inside the chroot.

Dear Fabio Porcedda,

On Mon, 15 Jun 2015 11:06:40 +0200, Fabio Porcedda wrote:

Quoted 13 lines

Quoted 12 lines

* I am not super happy with the idea of having the toolchain sysroot left in the global staging directory and referenced by the compiler --sysroot option on one side, and all other libraries found by using -L, -I and pkg-config tricks.

I would actually prefer if a real complete sysroot was used when building each package, and the compiler --sysroot option used to adjust the compiler sysroot. This has however one significant
drawback: the toolchain sysroot must be copied for each and every package, which can become quite time and space consuming. So on this aspect, I'd like to have some input from other Buildroot developers.

Quoted 4 lines

I've already rewritten the patch set using the --sysroot option in the toolchain wrapper. I just need to clean it up and send it, i hope to be able to send it tomorrow so we can choose the best solution for buildroot.

Good. We'll see how it looks. So now the per-package sysroot also contains the C library and kernel headers as well? It's really a complete sysroot?

Quoted 17 lines

Quoted 16 lines

* Your patch uses $($(2)_ADD_TOOLCHAIN_DEPENDENCY) to decide whether the per-package sysroot mechanism must be used or not. Which means it will only be used for target packages, and not for host packages. However, I'm wondering if we should not also apply the principle to host packages: to get reproducible builds, I believe we should also have separate sysroots when building host packages. Opinions from other Buildroot developers?

* Your approach only takes care of make the sysroot handled in a per-package fashion. But what about HOST_DIR ? We could have the same inconsistencies as the ones we discussed about STAGING_DIR, but this time caused by the presence/absence of host utilities. One build may give a given result because host tool "foo" is present (it happened to be built before), and the next build may give a different result because host tool "foo" is absent (it's going to be build after).

Quoted 2 lines

You are right, but if possible i just want to handle the host utilities matter as a next step after this step is done.

I'm indeed fine with solving this part as a next step, however, I'd like to at least have some basic ideas of how it will be solved, to see the big picture and see if solving the big picture is doable.

Quoted 3 lines

Good. We'll see how it looks. So now the per-package sysroot also contains the C library and kernel headers as well? It's really a complete sysroot?

Yes, i copy the whole directory ${STAGING_DIR}, e.g. output/host/usr/arm-buildroot-linux-gnueabi/sysroot

Quoted 3 lines

I'm indeed fine with solving this part as a next step, however, I'd like to at least have some basic ideas of how it will be solved, to see the big picture and see if solving the big picture is doable.

My idea is to solve it in the same way as the staging directory, copying the whole "output/host" directory expect for the sysroot part if it is a host package.
Or maybe there is better way to do it?

Hello,

Quoted 4 lines · Fabio Porcedda

Quoted 3 lines

Good. We'll see how it looks. So now the per-package sysroot also contains the C library and kernel headers as well? It's really a complete sysroot?

Quoted 2 lines

Yes, i copy the whole directory ${STAGING_DIR}, e.g. output/host/usr/arm-buildroot-linux-gnueabi/sysroot

Ok. When you say "whole", you say that you also "package" what the toolchain installs, and create a complete sysroot from scratch for each package being built, including for the toolchain libs/headers ?

Quoted 8 lines

Quoted 3 lines

I'm indeed fine with solving this part as a next step, however, I'd like to at least have some basic ideas of how it will be solved, to see the big picture and see if solving the big picture is doable.

My idea is to solve it in the same way as the staging directory, copying the whole "output/host" directory expect for the sysroot part if it is a host package.
Or maybe there is better way to do it?

Again, when you say "whole", you also separate in multiple host dirs whatever is installed by each package, and then create a custom host dir when each package is being built, containing only the dependencies that it needs?