diff mbox

[RFC] pkg-generic: add per-package staging directory

Message ID 1422602264-4725-1-git-send-email-Fabio.Porcedda@gmail.com
State Rejected
Headers show

Commit Message

Fabio Porcedda Jan. 30, 2015, 7:17 a.m. UTC
This commit add a per-package staging directory under output/stagingof/
to improve build reproducibility, because it prevents that a unspecified
dependency affects the build.

It also help to enable default top-level parallel make.

TODO: Do more testing like testing "allyespackageconfig".
---
 Makefile               |  2 +-
 package/Makefile.in    |  1 +
 package/pkg-generic.mk | 16 ++++++++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

Comments

Thomas Petazzoni Jan. 31, 2015, 10:02 p.m. UTC | #1
Dear Fabio Porcedda,

On Fri, 30 Jan 2015 08:17:44 +0100, Fabio Porcedda wrote:
> This commit add a per-package staging directory under output/stagingof/
> to improve build reproducibility, because it prevents that a unspecified
> dependency affects the build.
> 
> It also help to enable default top-level parallel make.

Thanks for attempting to look at this complicated problem. However,
there is a fairly fundamental flaw in your proposal:

> +$(2)_TARGET_CPPFLAGS += $(foreach dep,$($(2)_DEPENDENCIES),\
> +	-I$(STAGINGOF_DIR)/$(dep)/usr/include)

How is this going to account for headers installed in subdirs
of /usr/include ? Those are normally discovered using pkg-config for
example. But with your solution, pkg-config will continue to return
paths in the "common" staging directory.

My perception is that it's not really feasible to have one staging
directory per-package, because it means that pkg-config would have to
know, depending on which library it is asked to provide information
for, should use a different sysroot dir.

I believe the only solution is to create a temporary staging directory
in which you copy/extract only the libraries that are explicitly listed
as the dependency of the package being built.

Best regards,

Thomas
Fabio Porcedda Feb. 2, 2015, 9:20 a.m. UTC | #2
On Sat, Jan 31, 2015 at 11:02 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Fabio Porcedda,
>
> On Fri, 30 Jan 2015 08:17:44 +0100, Fabio Porcedda wrote:
>> This commit add a per-package staging directory under output/stagingof/
>> to improve build reproducibility, because it prevents that a unspecified
>> dependency affects the build.
>>
>> It also help to enable default top-level parallel make.
>
> Thanks for attempting to look at this complicated problem. However,
> there is a fairly fundamental flaw in your proposal:
>
>> +$(2)_TARGET_CPPFLAGS += $(foreach dep,$($(2)_DEPENDENCIES),\
>> +     -I$(STAGINGOF_DIR)/$(dep)/usr/include)
>
> How is this going to account for headers installed in subdirs
> of /usr/include ? Those are normally discovered using pkg-config for
> example. But with your solution, pkg-config will continue to return
> paths in the "common" staging directory.
>
> My perception is that it's not really feasible to have one staging
> directory per-package, because it means that pkg-config would have to
> know, depending on which library it is asked to provide information
> for, should use a different sysroot dir.
>
> I believe the only solution is to create a temporary staging directory
> in which you copy/extract only the libraries that are explicitly listed
> as the dependency of the package being built.

It's just that if there is a way to don't copy files it will be faster.

Now I'm preparing a new version that do as you said.
I will try to use hard links if possible.

BR
Fabio Porcedda Feb. 3, 2015, 11:19 a.m. UTC | #3
I've sent a new version:
http://patchwork.ozlabs.org/patch/435793/

BR
diff mbox

Patch

diff --git a/Makefile b/Makefile
index daf341c..c61e3c3 100644
--- a/Makefile
+++ b/Makefile
@@ -829,7 +829,7 @@  printvars:
 
 clean:
 	rm -rf $(TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) \
-		$(BUILD_DIR) $(BASE_DIR)/staging \
+		$(BUILD_DIR) $(BASE_DIR)/staging $(STAGINGOF_DIR) \
 		$(LEGAL_INFO_DIR)
 
 distclean: clean
diff --git a/package/Makefile.in b/package/Makefile.in
index 2055f00..634dc54 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -108,6 +108,7 @@  endif
 
 STAGING_SUBDIR = usr/$(GNU_TARGET_NAME)/sysroot
 STAGING_DIR    = $(HOST_DIR)/$(STAGING_SUBDIR)
+STAGINGOF_DIR  = $(BASE_DIR)/stagingof
 
 TARGET_OPTIMIZATION := $(call qstrip,$(BR2_TARGET_OPTIMIZATION))
 
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9643a30..e17620e 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -428,6 +428,13 @@  endif
 # Eliminate duplicates in dependencies
 $(2)_FINAL_DEPENDENCIES = $$(sort $$($(2)_DEPENDENCIES))
 
+$(2)_TARGET_CPPFLAGS += $(foreach dep,$($(2)_DEPENDENCIES),\
+	-I$(STAGINGOF_DIR)/$(dep)/usr/include)
+
+$(2)_TARGET_LDFLAGS += $(foreach dep,$($(2)_DEPENDENCIES),\
+	-L$(STAGINGOF_DIR)/$(dep)/usr/lib \
+	$($(call UPPERCASE,$(dep))_TARGET_LDFLAGS))
+
 $(2)_INSTALL_STAGING		?= NO
 $(2)_INSTALL_IMAGES		?= NO
 $(2)_INSTALL_TARGET		?= YES
@@ -598,10 +605,19 @@  $(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)
 # uppercase package variable prefix
 $$($(2)_TARGET_INSTALL_TARGET):		PKG=$(2)
 $$($(2)_TARGET_INSTALL_STAGING):	PKG=$(2)
+ifeq ($$($(2)_ADD_TOOLCHAIN_DEPENDENCY),YES)
+$$($(2)_TARGET_INSTALL_STAGING):	STAGING_DIR=$$(STAGINGOF_DIR)/$(1)
+else
+$$($(2)_TARGET_INSTALL_STAGING):	STAGING_DIR:=$$(STAGING_DIR)
+endif
 $$($(2)_TARGET_INSTALL_IMAGES):		PKG=$(2)
 $$($(2)_TARGET_INSTALL_HOST):           PKG=$(2)
 $$($(2)_TARGET_BUILD):			PKG=$(2)
+$$($(2)_TARGET_BUILD):			TARGET_CPPFLAGS=$$($(2)_TARGET_CPPFLAGS)
+$$($(2)_TARGET_BUILD):			TARGET_LDFLAGS=$$($(2)_TARGET_LDFLAGS)
 $$($(2)_TARGET_CONFIGURE):		PKG=$(2)
+$$($(2)_TARGET_CONFIGURE):		TARGET_CPPFLAGS=$$($(2)_TARGET_CPPFLAGS)
+$$($(2)_TARGET_CONFIGURE):		TARGET_LDFLAGS=$$($(2)_TARGET_LDFLAGS)
 $$($(2)_TARGET_RSYNC):                  SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
 $$($(2)_TARGET_RSYNC):                  PKG=$(2)
 $$($(2)_TARGET_RSYNC_SOURCE):		SRCDIR=$$($(2)_OVERRIDE_SRCDIR)