Per-package SDK and target directories

The pkg-config wrapper script is currently generated with absolute paths to $(STAGING_DIR). However, this will not work properly with per-package SDK, and each package will be built with a different STAGING_DIR value.

In order to fix this, we adjust how the pkg-config wrapper script is generated, so that it uses a relative path to itself: the sysroot (i.e STAGING_DIR) is always located in $(path of pkg-config)/../$(STAGING_SUBDIR).

This change is independent from the per-package SDK work, and could be applied independently from it.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 package/pkgconf/pkgconf.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

package/pkgconf/pkgconf.mk +0 -0

index cc190d26da..d11ce485f7 100644

package/pkgconf/pkgconf.mk +0 -0

 		$(HOST_DIR)/bin/pkg-config
 endef
 

Currently, our strategy for the RPATH of host binaries is as follows:

- During the build, we force an absolute RPATH to be encoded into binaries, by passing -Wl,-rpath,$(HOST_DIR)/lib. This ensures that host binaries will find their libraries.

- In the "make sdk" target, when preparing the SDK to be relocatable, we use patchelf to replace those absolute RPATHs by relative RPATHs that use $ORIGIN.

Unfortunately, the use of absolute RPATH during the build is not compatible with the move to per-package SDK, where a different host directory is used for the build of each package. Therefore, we need to move to a different strategy for RPATH handling.

The new strategy is as follows:

- During the build, we do not encode any RPATH into the host binaries. Instead, we specify LD_LIBRARY_PATH environment to point to the current HOST_DIR/lib directory (for the package being currently built).

- At the end of the build, in order to make sure that binaries are usable without LD_LIBRARY_PATH, we fixup the host binaries to use $ORIGIN/../lib. This is more-or-less what was done previously in the "make sdk" target, except that instead of turning absolute paths into relative paths in the RPATH, we are simply setting the RPATH to $ORIGIN/../lib.

In order to implement this, the fix-rpath script logic is adjusted for the "host" case.

An alternative strategy would have been to keep the -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the host binaries, and fix such RPATH at the end of the build of every package. However, that would require calling fix-rpath after the installation of every package, which is a bit expensive.

This change is independent from the per-package SDK functionality, and could be applied separately.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile                  |  4 +++-
 package/Makefile.in       |  2 +-
 package/pkg-generic.mk    |  8 --------
 support/scripts/fix-rpath | 15 ++++++++++-----
 4 files changed, 14 insertions(+), 15 deletions(-)

Makefile +3 -1

index 79db7fe48a..5496273329 100644
@@ -231,6 +231,8 @@ LEGAL_MANIFEST_CSV_HOST = $(LEGAL_INFO_DIR)/host-manifest.csv
 LEGAL_WARNINGS = $(LEGAL_INFO_DIR)/.warnings
 LEGAL_REPORT = $(LEGAL_INFO_DIR)/README
 
+export LD_LIBRARY_PATH = $(if $(PKG),$(HOST_DIR)/lib)
+
 ################################################################################
 #
 # staging and target directories do NOT list these as

@@ -558,7 +560,6 @@ world: target-post-image
 .PHONY: sdk
 sdk: world
 	@$(call MESSAGE,"Rendering the SDK relocatable")
-	$(TOPDIR)/support/scripts/fix-rpath host
 	$(TOPDIR)/support/scripts/fix-rpath staging
 	$(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)/relocate-sdk.sh
 	echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location

@@ -679,6 +680,7 @@ $(TARGETS_ROOTFS): target-finalize
 .PHONY: target-finalize
 target-finalize: $(PACKAGES)
 	@$(call MESSAGE,"Finalizing target directory")
+	$(TOPDIR)/support/scripts/fix-rpath host
 	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
 	rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \
 		$(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \

package/Makefile.in +1 -1

index a1a5316051..e94a75c230 100644
@@ -220,7 +220,7 @@ HOST_CPPFLAGS  = -I$(HOST_DIR)/include
 HOST_CFLAGS   ?= -O2
 HOST_CFLAGS   += $(HOST_CPPFLAGS)
 HOST_CXXFLAGS += $(HOST_CFLAGS)
-HOST_LDFLAGS  += -L$(HOST_DIR)/lib -Wl,-rpath,$(HOST_DIR)/lib
+HOST_LDFLAGS  += -L$(HOST_DIR)/lib
 
 # The macros below are taken from linux 4.11 and adapted slightly.
 # Copy more when needed.

package/pkg-generic.mk +0 -8

index cca94ba338..82f8c06821 100644
@@ -105,14 +105,6 @@ endef
 
 GLOBAL_INSTRUMENTATION_HOOKS += check_bin_arch
 
-# This hook checks that host packages that need libraries that we build
-# have a proper DT_RPATH or DT_RUNPATH tag
-define check_host_rpath
-	$(if $(filter install-host,$(2)),\
-		$(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR)))
-endef
-GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath
-
 define step_check_build_dir_one
 	if [ -d $(2) ]; then \
 		printf "%s: installs files in %s\n" $(1) $(2) >&2; \

support/scripts/fix-rpath +10 -5

index 15705a3b0d..eed751f5da 100755
@@ -61,7 +61,7 @@ main() {
     local rootdir
     local tree="${1}"
     local find_args=( )
-    local sanitize_extra_args=( )
+    local sanitize_args=( )
 
     if ! "${PATCHELF}" --version > /dev/null 2>&1; then
 	echo "Error: can't execute patchelf utility '${PATCHELF}'"

@@ -89,7 +89,8 @@ main() {
             cp "${PATCHELF}" "${PATCHELF}.__to_be_patched"
 
             # we always want $ORIGIN-based rpaths to make it relocatable.
-            sanitize_extra_args+=( "--relative-to-file" )
+            sanitize_args+=( "--set-rpath" )
+            sanitize_args+=( "\$ORIGIN/../lib" )
             ;;
 
         staging)

@@ -101,14 +102,18 @@ main() {
             done
 
             # should be like for the target tree below
-            sanitize_extra_args+=( "--no-standard-lib-dirs" )
+            sanitize_args+=( "--no-standard-lib-dirs" )
+            sanitize_args+=( "--make-rpath-relative" )
+            sanitize_args+=( "${rootdir}" )
             ;;
 
         target)
             rootdir="${TARGET_DIR}"
             # we don't want $ORIGIN-based rpaths but absolute paths without rootdir.
             # we also want to remove rpaths pointing to /lib or /usr/lib.
-            sanitize_extra_args+=( "--no-standard-lib-dirs" )
+            sanitize_args+=( "--no-standard-lib-dirs" )
+            sanitize_args+=( "--make-rpath-relative" )
+            sanitize_args+=( "${rootdir}" )
             ;;
 
         *)

@@ -125,7 +130,7 @@ main() {
             # make files writable if necessary
             changed=$(chmod -c u+w "${file}")
             # call patchelf to sanitize the rpath
-            ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}"
+            ${PATCHELF} ${sanitize_args[@]} "${file}"
             # restore the original permission
             test "${changed}" != "" && chmod u-w "${file}"
         fi

Signature · 1 line

2.13.6

In the past, we explicitly got rid of LD_LIBRARY_PATH because it does not work. See 34d081674a (core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment).

Briefly, it does not work when we build a library that is also present on the distro, which is used by a tool from the distro, and we build it in an incompatibl way.

So, we'll have to come up with another solution, like your suggested alternative.

Quoted 7 lines

- At the end of the build, in order to make sure that binaries are usable without LD_LIBRARY_PATH, we fixup the host binaries to use $ORIGIN/../lib. This is more-or-less what was done previously in the "make sdk" target, except that instead of turning absolute paths into relative paths in the RPATH, we are simply setting the RPATH to $ORIGIN/../lib.

Quoted 8 lines

In order to implement this, the fix-rpath script logic is adjusted for the "host" case.

An alternative strategy would have been to keep the -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the host binaries, and fix such RPATH at the end of the build of every package. However, that would require calling fix-rpath after the installation of every package, which is a bit expensive.

I wonder how expensive that would be. It would be nice to time this.

Also note that, even if the overhead is noticeable, this would be compensated by the mere fact that we are now doing a parallel build, so we would end up winning.

Hello,

Quoted 29 lines · Yann E. MORIN

In the past, we explicitly got rid of LD_LIBRARY_PATH because it does not work. See 34d081674a (core/pkg-infrastructures: remove LD_LIBRARY_PATH from the environment).

Briefly, it does not work when we build a library that is also present on the distro, which is used by a tool from the distro, and we build it in an incompatibl way.

So, we'll have to come up with another solution, like your suggested alternative.

Quoted 7 lines

- At the end of the build, in order to make sure that binaries are usable without LD_LIBRARY_PATH, we fixup the host binaries to use $ORIGIN/../lib. This is more-or-less what was done previously in the "make sdk" target, except that instead of turning absolute paths into relative paths in the RPATH, we are simply setting the RPATH to $ORIGIN/../lib.

Quoted 8 lines

In order to implement this, the fix-rpath script logic is adjusted for the "host" case.

An alternative strategy would have been to keep the -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the host binaries, and fix such RPATH at the end of the build of every package. However, that would require calling fix-rpath after the installation of every package, which is a bit expensive.

I wonder how expensive that would be. It would be nice to time this.

Quoted 3 lines

Also note that, even if the overhead is noticeable, this would be compensated by the mere fact that we are now doing a parallel build, so we would end up winning.

Thanks for the feedback. We already discussed this on IRC, and I agree. I'll have a look at implementing the solution of doing the patchelf adding $ORIGIN/../lib as an RPATH directly after the installation of each package.

Quoted 5 lines

An alternative strategy would have been to keep the -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the host binaries, and fix such RPATH at the end of the build of every package. However, that would require calling fix-rpath after the installation of every package, which is a bit expensive.

I wonder how expensive that would be. It would be nice to time this.

Hello,

Quoted 7 lines · Yann E. MORIN

Quoted 5 lines

An alternative strategy would have been to keep the -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the host binaries, and fix such RPATH at the end of the build of every package. However, that would require calling fix-rpath after the installation of every package, which is a bit expensive.

I wonder how expensive that would be. It would be nice to time this.

OK, to time this, I did a quick hacky shell script
(http://code.bulix.org/uboevd-223829). What it does is:

- Generate a mix of ELF files and random data files (half of the files are ELF files, half are random data files). This is the "generate" target of the script.

- Fixup rpath to $ORIGIN/../lib if not already fixed. We don't want to fixup files repeatedly, as we don't want to duplicate files over and over by breaking hard links. So we really want to set the RPATH only once per binary.

Results are as follows:

$ ./rpath-evaluation.sh generate 0 500
... generate 250 random data files, 250 ELF programs with a crappy
rpath ...

$ time ./rpath-evaluation.sh fixup

real 0m0.976s
user 0m0.546s
sys 0m0.457s

$ time ./rpath-evaluation.sh fixup

real 0m0.610s
user 0m0.393s
sys 0m0.234s

So the first fixup pass, which actually fixes the RPATH of 250 binaries takes about one second. The next fixup pass doesn't do anything, except checking that all binaries already have a fixed RPATH.

So we see that it will take ~600ms to scan 500 files, provided half of them are ELF files, verify that they already have the correct RPATH.

To me, this sounds very reasonable. What do you think?

Quoted 1 line

To me, this sounds very reasonable. What do you think?

500 files is not a lot if you include staging as well

so I've repeated the experiment with 50000 files and on a HDD. First pass takes 3m34, second pass 3m43. Hm, perhaps I should have left my computer alone for a while during this experiment :-).

Anyway, I think the bottom line is that the fixup script should
avoid going over staging.

But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway.

Hello,

Quoted 3 lines · Arnout Vandecappelle

Quoted 1 line

To me, this sounds very reasonable. What do you think?

500 files is not a lot if you include staging as well

Why would I include staging? I don't see at all why fixing up RPATH in staging after each package build would be necessary.

Quoted 4 lines

so I've repeated the experiment with 50000 files and on a HDD. First pass takes 3m34, second pass 3m43. Hm, perhaps I should have left my computer alone for a while during this experiment :-).

It's not very logical that the second pass is longer than the first pass, since the first pass does do patchelf invocations per file, while the second pass does only one patchelf invocation per file.

Quoted 2 lines

Anyway, I think the bottom line is that the fixup script should
avoid going over staging.

Why would it need to go over staging?

Quoted 1 line

But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway.

Yes, that was my intent. If needed, we could optimize it by only doing the fixup at the end of a host package installation. But since we have a few target packages installing host stuff (toolchain, qt, etc.), it would require them to explicitly state that they need host rpath fixups. Perhaps it's easiest for now to just do the rpath fixup after every package installation.

Best regards,

Thomas

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

To me, this sounds very reasonable. What do you think?

500 files is not a lot if you include staging as well

Why would I include staging? I don't see at all why fixing up RPATH in staging after each package build would be necessary.

The simplest way would be to do find $(HOST_DIR) -type f but that includes staging.

Quoted 1 line

Quoted 1 line

But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway.

So this is not true, we also need $(HOST_DIR)/$(GNU_TARGET_NAME)/{bin,lib}.
So then it's perhaps easier to just -prune staging.

Quoted 6 lines

Yes, that was my intent. If needed, we could optimize it by only doing the fixup at the end of a host package installation. But since we have a few target packages installing host stuff (toolchain, qt, etc.), it would require them to explicitly state that they need host rpath fixups. Perhaps it's easiest for now to just do the rpath fixup after every package installation.

After every package installation would be better, yes.

Hello,

Quoted 4 lines · Arnout Vandecappelle

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

To me, this sounds very reasonable. What do you think?

500 files is not a lot if you include staging as well

Quoted 4 lines

Quoted 2 lines

Why would I include staging? I don't see at all why fixing up RPATH in staging after each package build would be necessary.

The simplest way would be to do find $(HOST_DIR) -type f but that includes staging.

Hum, you didn't reply to my question, which was: "Why would we need to fixup RPATHs in STAGING_DIR" ?

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

But $(HOST_DIR)/{bin,sbin,lib} should be enough anyway.

So this is not true, we also need $(HOST_DIR)/$(GNU_TARGET_NAME)/{bin,lib}.
So then it's perhaps easier to just -prune staging.

Absolutely.

Quoted 8 lines

Quoted 6 lines

Yes, that was my intent. If needed, we could optimize it by only doing the fixup at the end of a host package installation. But since we have a few target packages installing host stuff (toolchain, qt, etc.), it would require them to explicitly state that they need host rpath fixups. Perhaps it's easiest for now to just do the rpath fixup after every package installation.

After every package installation would be better, yes.

ACK.

Thomas

Currently, our strategy for the RPATH of host binaries is as follows:

- During the build, we force an absolute RPATH to be encoded into binaries, by passing -Wl,-rpath,$(HOST_DIR)/lib. This ensures that host binaries will find their libraries.

- In the "make sdk" target, when preparing the SDK to be relocatable, we use patchelf to replace those absolute RPATHs by relative RPATHs that use $ORIGIN.

Unfortunately, the use of absolute RPATH during the build is not compatible with the move to per-package SDK, where a different host directory is used for the build of each package. Therefore, we need to move to a different strategy for RPATH handling.

The new strategy is as follows:

- During the build, we do not encode any RPATH into the host binaries. Instead, we specify LD_LIBRARY_PATH environment to point to the current HOST_DIR/lib directory (for the package being currently built).

- At the end of the build, in order to make sure that binaries are usable without LD_LIBRARY_PATH, we fixup the host binaries to use $ORIGIN/../lib. This is more-or-less what was done previously in the "make sdk" target, except that instead of turning absolute paths into relative paths in the RPATH, we are simply setting the RPATH to $ORIGIN/../lib.

In order to implement this, the fix-rpath script logic is adjusted for the "host" case.

An alternative strategy would have been to keep the -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the host binaries, and fix such RPATH at the end of the build of every package. However, that would require calling fix-rpath after the installation of every package, which is a bit expensive.

This change is independent from the per-package SDK functionality, and could be applied separately.

Signed-off-by Thomas Petazzoni <[email protected]>

---

Quoted 5 lines

An alternative strategy would have been to keep the -Wl,-rpath,$(HOST_DIR) flag, and therefore the absolute RPATH in the host binaries, and fix such RPATH at the end of the build of every package. However, that would require calling fix-rpath after the installation of every package, which is a bit expensive.

As discussed, this would be the preferred approach after all.

Quoted 2 lines

This change is independent from the per-package SDK functionality, and could be applied separately.

Shouldn't this type of comment be below the --- ?

support/scripts/fix-rpath main() { · line 93

Quoted 7 lines

support/scripts/fix-rpath +2 -1

@@ -89,7 +89,8 @@ main() {
             cp "${PATCHELF}" "${PATCHELF}.__to_be_patched"
 
             # we always want $ORIGIN-based rpaths to make it relocatable.
-            sanitize_extra_args+=( "--relative-to-file" )
+            sanitize_args+=( "--set-rpath" )
+            sanitize_args+=( "\$ORIGIN/../lib" )  

One advantage of --make-rpath-relative over --set-rpath is that the former will
also remove rpaths that are not needed (which I guess is often the case for host tools).

Hello,

On Tue, 7 Nov 2017 22:15:38 +0100, Arnout Vandecappelle wrote:

Quoted 3 lines

One advantage of --make-rpath-relative over --set-rpath is that the former will
also remove rpaths that are not needed (which I guess is often the case for host tools).

Well, --set-rpath sets the RPATH, so I believe it entirely removes any existing RPATH and replaces them all by just $ORIGIN/../lib. So I don't see how it could be more "efficient", since we're down to a single RPATH, which is the only rpath that is needed.

Perhaps the only problem that I can think of is if some host binaries do have libraries installed in non-standard locations, and really need a custom absolute RPATH such
as /home/foo/buildroot/output/per-package/host/lib/baz/ to be turned into $ORIGIN/../lib/baz/. However, I don't think we have such non-standard library locations for host packages, so I simply ignored this possible problem for now. It is a problem that can be fixed later on if needed by adjusting the host rpath mangling logic.

The upcoming per-package SDK functionality is heavily based on the fact that HOST_DIR, STAGING_DIR and TARGET_DIR are evaluated during the configure/build/install steps of the packages. Therefore, any evaluation-during-assignment using := is going to cause problems, and need to be turned into evaluation-during-use using =.

This patch fix up one such instance in the external toolchain code.

This change is independent from the per-package SDK functionality, and could be applied separately.

Signed-off-by Thomas Petazzoni <[email protected]>

---
 toolchain/toolchain-external/pkg-toolchain-external.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

toolchain/toolchain-external/pkg-toolchain-external.mk +0 -0

index dc0588c536..b9ad1720a1 100644

toolchain/toolchain-external/pkg-toolchain-external.mk +0 -0

 endif
 
 # If this is a buildroot toolchain, it already has a wrapper which we want to

toolchain/toolchain-external/pkg-toolchain-external.mk ifneq ($(TOOLCHAIN_EXTERNAL_PREFIX),) · line 80

Quoted 10 lines

toolchain/toolchain-external/pkg-toolchain-external.mk +1 -1

index dc0588c536..b9ad1720a1 100644
@@ -77,7 +77,7 @@ ifneq ($(TOOLCHAIN_EXTERNAL_PREFIX),)
 TOOLCHAIN_EXTERNAL_BIN := $(dir $(shell which $(TOOLCHAIN_EXTERNAL_PREFIX)-gcc))
 endif
 else
-TOOLCHAIN_EXTERNAL_BIN := $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/bin
+TOOLCHAIN_EXTERNAL_BIN = $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/bin  

It gives me the shivers to see the same variable sometimes defined recursive
and sometimes defined immediate... But I can't find an elegant solution (other than introducing an artificial extra variable, which is also ugly), so

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Hello,

Quoted 15 lines · Arnout Vandecappelle

toolchain/toolchain-external/pkg-toolchain-external.mk ifneq ($(TOOLCHAIN_EXTERNAL_PREFIX),) · line 80

Quoted 10 lines

toolchain/toolchain-external/pkg-toolchain-external.mk +1 -1

index dc0588c536..b9ad1720a1 100644
@@ -77,7 +77,7 @@ ifneq ($(TOOLCHAIN_EXTERNAL_PREFIX),)
 TOOLCHAIN_EXTERNAL_BIN := $(dir $(shell which $(TOOLCHAIN_EXTERNAL_PREFIX)-gcc))
 endif
 else
-TOOLCHAIN_EXTERNAL_BIN := $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/bin
+TOOLCHAIN_EXTERNAL_BIN = $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/bin  

It gives me the shivers to see the same variable sometimes defined recursive
and sometimes defined immediate... But I can't find an elegant solution (other than introducing an artificial extra variable, which is also ugly), so

Quoted 1 line

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

I also saw the other TOOLCHAIN_EXTERNAL_BIN assignment using := above, and didn't change it for now because that was not the case I was interested in testing/fixing. But my plan was to get back to this := assignment at some point. However, it seems like you have concluded that we have to keep := here. Could you explain why?

Note: I think keeping the := is OK, because TOOLCHAIN_EXTERNAL_PREFIX doesn't reference anything like HOST_DIR/STAGING_DIR/TARGET_DIR, so it's fine to have evaluation at the time of assignment for this case.

Perhaps a comment above would help clarify that it's intentional?

This commit implemnts the core of the move to per-package SDK and target directories. The main idea is that instead of having a global output/host and output/target in which all packages install files, we switch to per-package host and target folders, that only contain their explicit dependencies.

There are two main benefits:

- Packages will no longer discover dependencies that they do not explicitly indicate in their <pkg>_DEPENDENCIES variable.

- We can support top-level parallel build properly, because a package only "sees" its own host directory and target directory, isolated from the build of other packages that can happen in parallel.

It works as follows:

- A new output/per-package/ folder is created, which will contain one sub-folder per package, and inside it, a "host" folder and a "target" folder:

output/per-package/busybox/target
output/per-package/busybox/host
output/per-package/host-fakeroot/target
output/per-package/host-fakeroot/host

This output/per-package/ folder is PER_PACKAGE_DIR, and each package defines <pkg>_TARGET_DIR, <pkg>_HOST_DIR and <pkg>_STAGING_DIR pointing to its own target, host and staging directories.

- The <pkg>_TARGET_DIR, <pkg>_HOST_DIR and <pkg>_STAGING_DIR variable are passed as TARGET_DIR, HOST_DIR and STAGING_DIR to the various make targets used for the different build steps of the packages. Effectively this means that when a package references $(HOST_DIR), the value is in fact $(<pkg>_HOST_DIR).

- Of course, packages have dependencies, so those dependencies must be installed in the per-package host and target folders before configuring the package. To do so, we simply rsync (using hard links to save space and time) the host and target folders of the direct dependencies of the package to the current package host and target folders. We only need to take care of direct dependencies (and not recursively all dependencies), because we accumulate into those per-package host and target folders the files installed by the dependencies.

- We fix LD_LIBRARY_PATH to make it point to the current package host directory.

This is basically enough to make per-package SDK and target work. The only gotcha is that at the end of the build, output/target and output/host are empty, which means that:

- The filesystem image creation code cannot work.

- We don't have a SDK to build code outside of Buildroot.

In order to fix this, this commit extends the target-finalize step so that it starts by populating output/target and output/host by rsync-ing into them the target and host directories of all packages listed in the $(PACKAGES) variable.

[Note: in fact, output/host is not completely empty, even though they should be. It contains the following files:
output/host/
output/host/lib64
output/host/usr
output/host/share
output/host/share/buildroot output/host/share/buildroot/Platform output/host/share/buildroot/Platform/Buildroot.cmake output/host/share/buildroot/toolchainfile.cmake output/host/lib
Perhaps those files should be installed by some package instead, perhaps the 'toolchain' package.]

Signed-off-by Thomas Petazzoni <[email protected]>

---
 Makefile               | 17 ++++++++++++++---
 package/pkg-generic.mk | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

Makefile +4 -2

index 5496273329..4e1ba62569 100644
@@ -216,6 +216,8 @@ BR_GRAPH_OUT := $(or $(BR2_GRAPH_OUT),pdf)
 BUILD_DIR := $(BASE_DIR)/build
 BINARIES_DIR := $(BASE_DIR)/images
 TARGET_DIR := $(BASE_DIR)/target
+PER_PACKAGE_DIR := $(BASE_DIR)/per-package
+
 # initial definition so that 'make clean' works for most users, even without
 # .config. HOST_DIR will be overwritten later when .config is included.
 HOST_DIR := $(BASE_DIR)/host

@@ -231,7 +233,7 @@ LEGAL_MANIFEST_CSV_HOST = $(LEGAL_INFO_DIR)/host-manifest.csv
 LEGAL_WARNINGS = $(LEGAL_INFO_DIR)/.warnings
 LEGAL_REPORT = $(LEGAL_INFO_DIR)/README
 
-export LD_LIBRARY_PATH = $(if $(PKG),$(HOST_DIR)/lib)
+export LD_LIBRARY_PATH = $(if $(PKG),$($(PKG)_HOST_DIR)/lib)
 
 ################################################################################
 #

@@ -239,7 +241,7 @@ export LD_LIBRARY_PATH = $(if $(PKG),$(HOST_DIR)/lib)
 # dependencies anywhere else
 #
 ################################################################################
-$(BUILD_DIR) $(TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
+$(BUILD_DIR) $(TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST) $(PER_PACKAGE_DIR):
 	@mkdir -p $@
 
 BR2_CONFIG = $(CONFIG_DIR)/.config

@@ -675,10 +677,19 @@ endef
 TARGET_FINALIZE_HOOKS += PURGE_LOCALES
 endif
 

Makefile +4 -1

+	@$(call MESSAGE,"Creating global host directory")
+	$(foreach dir,$(ALL_PACKAGES_HOST_DIRS),\
+		rsync -au --link-dest=$(dir) $(dir) $(HOST_DIR)$(sep))
 	@$(call MESSAGE,"Finalizing target directory")
 	$(TOPDIR)/support/scripts/fix-rpath host
 	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))

@@ -972,7 +983,7 @@ printvars:
 clean:
 	rm -rf $(TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) \
 		$(BUILD_DIR) $(BASE_DIR)/staging \
-		$(LEGAL_INFO_DIR) $(GRAPHS_DIR)
+		$(LEGAL_INFO_DIR) $(GRAPHS_DIR) $(PER_PACKAGE_DIR)
 
 .PHONY: distclean
 distclean: clean

package/pkg-generic.mk +0 -0

index 82f8c06821..aee4011f63 100644

package/pkg-generic.mk +1 -0

+
 # Keep the package version that may contain forward slashes in the _DL_VERSION
 # variable, then replace all forward slashes ('/') by underscores ('_') to
 # sanitize the package version that is used in paths, directory and file names.

package/pkg-generic.mk +22 -0

+
 $(2)_INSTALL_STAGING		?= NO
 $(2)_INSTALL_IMAGES		?= NO
 $(2)_INSTALL_TARGET		?= YES

@@ -789,17 +805,38 @@ $(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):		HOST_DIR=$$($(2)_HOST_DIR)
+$$($(2)_TARGET_INSTALL_TARGET):		STAGING_DIR=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_INSTALL_TARGET):		TARGET_DIR=$$($(2)_TARGET_DIR)
 $$($(2)_TARGET_INSTALL_STAGING):	PKG=$(2)
+$$($(2)_TARGET_INSTALL_STAGING):	HOST_DIR=$$($(2)_HOST_DIR)
+$$($(2)_TARGET_INSTALL_STAGING):	STAGING_DIR=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_INSTALL_STAGING):	TARGET_DIR=$$($(2)_TARGET_DIR)
 $$($(2)_TARGET_INSTALL_IMAGES):		PKG=$(2)
+$$($(2)_TARGET_INSTALL_IMAGES):		HOST_DIR=$$($(2)_HOST_DIR)
+$$($(2)_TARGET_INSTALL_IMAGES):		STAGING_DIR=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_INSTALL_IMAGES):		TARGET_DIR=$$($(2)_TARGET_DIR)
 $$($(2)_TARGET_INSTALL_HOST):		PKG=$(2)
+$$($(2)_TARGET_INSTALL_HOST):		HOST_DIR=$$($(2)_HOST_DIR)
+$$($(2)_TARGET_INSTALL_HOST):		STAGING_DIR=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_INSTALL_HOST):		TARGET_DIR=$$($(2)_TARGET_DIR)
 $$($(2)_TARGET_BUILD):			PKG=$(2)
+$$($(2)_TARGET_BUILD):			HOST_DIR=$$($(2)_HOST_DIR)
+$$($(2)_TARGET_BUILD):			STAGING_DIR=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_BUILD):			TARGET_DIR=$$($(2)_TARGET_DIR)
 $$($(2)_TARGET_CONFIGURE):		PKG=$(2)
+$$($(2)_TARGET_CONFIGURE):		HOST_DIR=$$($(2)_HOST_DIR)
+$$($(2)_TARGET_CONFIGURE):		STAGING_DIR=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_CONFIGURE):		TARGET_DIR=$$($(2)_TARGET_DIR)
 $$($(2)_TARGET_RSYNC):			SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
 $$($(2)_TARGET_RSYNC):			PKG=$(2)
 $$($(2)_TARGET_PATCH):			PKG=$(2)
 $$($(2)_TARGET_PATCH):			RAWNAME=$$(patsubst host-%,%,$(1))
 $$($(2)_TARGET_PATCH):			PKGDIR=$(pkgdir)
 $$($(2)_TARGET_EXTRACT):		PKG=$(2)
+$$($(2)_TARGET_EXTRACT):		HOST_DIR=$$($(2)_HOST_DIR)
+$$($(2)_TARGET_EXTRACT):		STAGING_DIR=$$($(2)_STAGING_DIR)
+$$($(2)_TARGET_EXTRACT):		TARGET_DIR=$$($(2)_TARGET_DIR)
 $$($(2)_TARGET_SOURCE):			PKG=$(2)
 $$($(2)_TARGET_SOURCE):			PKGDIR=$(pkgdir)
 $$($(2)_TARGET_ACTUAL_SOURCE):		PKG=$(2)

Quoted 4 lines

In order to fix this, this commit extends the target-finalize step so that it starts by populating output/target and output/host by rsync-ing into them the target and host directories of all packages listed in the $(PACKAGES) variable.

It would have been good to mention somewhere in the commit log the problem of 2
packages writing the same file...

Also, is there a good reason not to do the rsync at the end of package
installation? I don't know how robust rsync is against race conditions in directory or hardlink creation... OK, so I did a test and it races pretty badly :-) So, could you add this to the commit log? Like:

It is necessary to do this sequentially in the target-finalize step and not in each package. Doing it in package installation means that it can be done in parallel. In that case, there is a chance that two rsyncs are creating the same hardlink or directory at the same time, which makes one of them fail.

Quoted 5 lines

[Note: in fact, output/host is not completely empty, even though they should be. It contains the following files:
output/host/
output/host/lib64
output/host/usr

Quoted 8 lines

output/host/share
output/host/share/buildroot output/host/share/buildroot/Platform output/host/share/buildroot/Platform/Buildroot.cmake output/host/share/buildroot/toolchainfile.cmake output/host/lib
Perhaps those files should be installed by some package instead, perhaps the 'toolchain' package.]

If we go that way: perhaps the .cmake files should be installed by some package
like 'host-cmake-infra' that is added to the dependencies from cmake-package.

Makefile endef · line 689

Quoted 5 lines

.PHONY: target-finalize
target-finalize: $(PACKAGES)

Makefile +3 -0

+	@$(call MESSAGE,"Creating global target directory")
+	$(foreach dir,$(ALL_PACKAGES_TARGET_DIRS),\
+		rsync -au --link-dest=$(dir) $(dir) $(TARGET_DIR)$(sep))  

Why -u? Since we're not actually copying anything, there is no reason to not
overwrite files that already exist, right?

Also, shouldn't there be a / behind $(dir)? Otherwise, if TARGET_DIR exists
already, rsync will create $(dir) as a subdirectory of $(TARGET_DIR), no?

Quoted 4 lines

package/pkg-generic.mk +0 -0

index 82f8c06821..aee4011f63 100644

package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_actual_downloaded: · line 161

Quoted 5 lines

package/pkg-generic.mk +1 -0

@@ -158,6 +158,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call step_start,extract)
 	@$(call MESSAGE,"Extracting")
+	@mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR)  

Why do you create them here? It would make more sense to do it just before the
rsync, no?

Given the possible host-tar and host-lzip dependencies, the rsync should be
done here already, not in the configure step. Ouch, that's not good, because the dependencies are only evaluated for the configure step... There are PATCH_DEPENDENCIES, but those are only before patch. Well, I guess that's a reason to keep host-tar and host-lzip as DEPENDENCIES_HOST_PREREQ :-)

package/pkg-generic.mk $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ · line 220

Quoted 2 lines

package/pkg-generic.mk +2 -0

+	$(foreach dir,$($(PKG)_FINAL_DEPENDENCIES_HOST_DIRS),\
+		rsync -au --link-dest=$(dir) $(dir) $(HOST_DIR)$(sep))  

Shouldn't there be a $Q somewhere?

package/pkg-generic.mk $(2)_NAME = $(1) · line 417

Quoted 1 line

package/pkg-generic.mk +1 -0

+$(2)_TARGET_DIR		= $$(PER_PACKAGE_DIR)/$(1)/target/  

Ah, here is the trailing slash, that explains why it worked. I don't really
like it when you have to rely on a trailing slash in the variable definition...

Quoted 3 lines

+$(2)_HOST_DIR = $$(PER_PACKAGE_DIR)/$(1)/host/
+$(2)_STAGING_DIR = $$(PER_PACKAGE_DIR)/$(1)/host/$(STAGING_SUBDIR)

I hate adding more per-package variables. Can't you use the expanded values,
substituting $($(PKG)_NAME) for $(1)? Or maybe even better, change the definition of TARGET_DIR etc:

TARGET_DIR = $(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_RAWNAME)/target,$(BASE_DIR)/target)

That way you also don't need to pass those variables to each individual stamp rule.

package/pkg-generic.mk $(2)_FINAL_DEPENDENCIES = $$(sort $$($(2)_DEPENDENCIES)) · line 578

Quoted 13 lines

# Keep the package version that may contain forward slashes in the _DL_VERSION # variable, then replace all forward slashes ('/') by underscores ('_') to

 # sanitize the package version that is used in paths, directory and file names.

package/pkg-generic.mk +6 -0

@@ -561,6 +570,13 @@ $(2)_FINAL_DEPENDENCIES = $$(sort $$($(2)_DEPENDENCIES))
 $(2)_FINAL_PATCH_DEPENDENCIES = $$(sort $$($(2)_PATCH_DEPENDENCIES))
 $(2)_FINAL_ALL_DEPENDENCIES = $$(sort $$($(2)_FINAL_DEPENDENCIES) $$($(2)_FINAL_PATCH_DEPENDENCIES))
 
+$(2)_FINAL_DEPENDENCIES_HOST_DIRS = \
+	$$(foreach dep,$$($(2)_FINAL_DEPENDENCIES),\
+		$$($$(call UPPERCASE,$$(dep))_HOST_DIR))
+$(2)_FINAL_DEPENDENCIES_TARGET_DIRS = \
+	$$(foreach dep,$$($(2)_FINAL_DEPENDENCIES),\
+		$$($$(call UPPERCASE,$$(dep))_TARGET_DIR))  

I may be overengineering things, but perhaps things can be simplified as
follows (with the definition of TARGET_DIR etc as above):

define RSYNC_HOST_DIRS

	$(foreach pkg,$(1),\
		$(Q)dir=$(PER_PACKAGE_DIR)/$(pkg)/host;\
		rsync -au --link-dest=$$dir $$dir/ $(HOST_DIR)$(sep))

endef

define RSYNC_TARGET_DIRS

	$(foreach pkg,$(1),\
		$(Q)dir=$(PER_PACKAGE_DIR)/$(pkg)/target;\
		rsync -au --link-dest=$$dir $$dir/ $(TARGET_DIR)$(sep))

endef

(as always, completely untested). These macros can be used both in the config stamp rule and in the finalize rule. I even considered leaving out the argument and instead defaulting $(1) to
$(if $(PKG),$($(PKG)_FINAL_DEPENDENCIES),$(PACKAGES))
but that probably just makes things harder to understand.

Hello,

Thanks a lot for the suggestions, I'll take them into account for the next version. Just a few comments/questions below.

On Tue, 7 Nov 2017 23:50:52 +0100, Arnout Vandecappelle wrote:

Quoted 9 lines

Also, is there a good reason not to do the rsync at the end of package
installation? I don't know how robust rsync is against race conditions in directory or hardlink creation... OK, so I did a test and it races pretty badly :-) So, could you add this to the commit log? Like:

It is necessary to do this sequentially in the target-finalize step and not in each package. Doing it in package installation means that it can be done in parallel. In that case, there is a chance that two rsyncs are creating the same hardlink or directory at the same time, which makes one of them fail.

I didn't think at all about doing this in parallel.

To me, the creation
of the final/common HOST_DIR/TARGET_DIR/STAGING_DIR is not at all something that needs to be done while the build is on-going. And I could go even further: unless you call "make sdk", we technically don't really need to create the final/common HOST_DIR/STAGING_DIR.

Only a
TARGET_DIR is needed, and perhaps even we could avoid a global one, and instead have a per-filesystem image one, in order to avoid the parallel filesystem image creation problem.

Quoted 4 lines

Quoted 4 lines

package/pkg-generic.mk +0 -0

index 82f8c06821..aee4011f63 100644

Quoted 8 lines

package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_actual_downloaded: · line 161

Quoted 5 lines

package/pkg-generic.mk +1 -0

@@ -158,6 +158,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call step_start,extract)
 	@$(call MESSAGE,"Extracting")
+	@mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR)  

Why do you create them here? It would make more sense to do it just before the
rsync, no?

I don't remember, I'll have to check again. I don't immediately see a reason why they would be needed before the rsync.

Quoted 5 lines

Given the possible host-tar and host-lzip dependencies, the rsync should be
done here already, not in the configure step. Ouch, that's not good, because the dependencies are only evaluated for the configure step... There are PATCH_DEPENDENCIES, but those are only before patch. Well, I guess that's a reason to keep host-tar and host-lzip as DEPENDENCIES_HOST_PREREQ :-)

These are all topics that I haven't looked at yet. So thanks for pointing them out. I should probably collect a TODO list on per-package SDK to not forget about all the things people have mentioned as things to look at.

Quoted 8 lines

I hate adding more per-package variables. Can't you use the expanded values,
substituting $($(PKG)_NAME) for $(1)? Or maybe even better, change the definition of TARGET_DIR etc:

TARGET_DIR = $(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_RAWNAME)/target,$(BASE_DIR)/target)

That way you also don't need to pass those variables to each individual stamp rule.

I didn't think about this, sounds like a good idea, I'll try that.

However, I'm actually planning to drop entirely having TARGET_DIR defined outside of package rules. What I want to have ultimately is:

- TARGET_DIR points to the current package per-package target directory. If we're outside of a package rule, TARGET_DIR has no value (or possibly a bogus value such as /this/is/a/bogus/thing/). Indeed, we really don't want things to directly install/mess with the global/common target directory.

- Have something like COMMON_TARGET_DIR that points to $(BASE_DIR)/target, and we carefully change the places that should access the COMMON_TARGET_DIR (i.e mainly target-finalize and some filesystem generation stuff).

Quoted 5 lines

To me, the creation
of the final/common HOST_DIR/TARGET_DIR/STAGING_DIR is not at all something that needs to be done while the build is on-going. And I could go even further: unless you call "make sdk", we technically don't really need to create the final/common HOST_DIR/STAGING_DIR.

Absolutely true, it would make sense to do that in 'make sdk'.

Quoted 4 lines

Only a
TARGET_DIR is needed, and perhaps even we could avoid a global one, and instead have a per-filesystem image one, in order to avoid the parallel filesystem image creation problem.

That's a great idea!

Quoted 13 lines

Quoted 12 lines

package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_actual_downloaded: · line 161

Quoted 9 lines

package/pkg-generic.mk +1 -0

index 82f8c06821..aee4011f63 100644
@@ -158,6 +158,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call step_start,extract)
 	@$(call MESSAGE,"Extracting")
+	@mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR)    

Why do you create them here? It would make more sense to do it just before the
rsync, no?

Quoted 2 lines

I don't remember, I'll have to check again. I don't immediately see a reason why they would be needed before the rsync.

A package without any dependencies will not do the rsync, and I do think you
want the directories to exist. Or maybe not, I'm not sure... Actually, shouldn't a skeleton be installed there as well?

Quoted 6 lines

Quoted 5 lines

Given the possible host-tar and host-lzip dependencies, the rsync should be
done here already, not in the configure step. Ouch, that's not good, because the dependencies are only evaluated for the configure step... There are PATCH_DEPENDENCIES, but those are only before patch. Well, I guess that's a reason to keep host-tar and host-lzip as DEPENDENCIES_HOST_PREREQ :-)

Quoted 4 lines

These are all topics that I haven't looked at yet. So thanks for pointing them out. I should probably collect a TODO list on per-package SDK to not forget about all the things people have mentioned as things to look at.

Yeah, a skeleton HOST_DIR that every package depends on would simplify that,
perhaps.

Given the possible host-tar and host-lzip dependencies, the rsync
should be done here already, not in the configure step. Ouch, that's not good, because the dependencies are only evaluated for the configure step... There are PATCH_DEPENDENCIES, but those are only before patch. Well, I guess that's a reason to keep host-tar and host-lzip as DEPENDENCIES_HOST_PREREQ :-)

Quoted 2 lines

+$(2)_HOST_DIR = $$(PER_PACKAGE_DIR)/$(1)/host/
+$(2)_STAGING_DIR = $$(PER_PACKAGE_DIR)/$(1)/host/$(STAGING_SUBDIR)

I hate adding more per-package variables. Can't you use the expanded values,
substituting $($(PKG)_NAME) for $(1)? Or maybe even better, change the definition of TARGET_DIR etc:

TARGET_DIR = $(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_RAWNAME)/target,$(BASE_DIR)/target)

That way you also don't need to pass those variables to each individual stamp rule.

Hello,

A few more answers/topics I wanted to discuss.

Quoted 6 lines · Arnout Vandecappelle

Given the possible host-tar and host-lzip dependencies, the rsync
should be done here already, not in the configure step. Ouch, that's not good, because the dependencies are only evaluated for the configure step... There are PATCH_DEPENDENCIES, but those are only before patch. Well, I guess that's a reason to keep host-tar and host-lzip as DEPENDENCIES_HOST_PREREQ :-)

Here is how I'm thinking of solving the problem:

- Next to <pkg>_PATCH_DEPENDENCIES, introduce the concept of <pkg>_EXTRACT_DEPENDENCIES.

- If there is no suitable tar in the system, then host-tar would be added to <pkg>_EXTRACT_DEPENDENCIES of all packages, except host-tar itself.

- If the package needs lzip and there is no lzip available on the system, host-lzip is added to <pkg>_EXTRACT_DEPENDENCIES.

- If ccache support is enabled, host-ccache is added to <pkg>_DEPENDENCIES of all packages, except host-tar, which is built with HOSTCC_NOCCACHE.

- At the beginning of the extract step of a package, we rsync to its per-package target/host directories the per-package target/host dirs of the packages listed in its <pkg>_EXTRACT_DEPENDENCIES variable.

- At the beginning of the patch step of a package, we rsync to its per-package target/host directories the per-package target/host dirs of the packages listed in its <pkg>_PATCH_DEPENDENCIES variable.

Thoughts?

Quoted 4 lines

Quoted 3 lines

+$(2)_HOST_DIR = $$(PER_PACKAGE_DIR)/$(1)/host/
+$(2)_STAGING_DIR = $$(PER_PACKAGE_DIR)/$(1)/host/$(STAGING_SUBDIR)

Quoted 8 lines

I hate adding more per-package variables. Can't you use the expanded values,
substituting $($(PKG)_NAME) for $(1)? Or maybe even better, change the definition of TARGET_DIR etc:

TARGET_DIR = $(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_RAWNAME)/target,$(BASE_DIR)/target)

That way you also don't need to pass those variables to each individual stamp rule.

I've changed this in my new iteration. The only drawback is that since we no longer have those shortcut variables, the foreach loops building the per-package directories, and building the final global directories are a bit less readable:

Per-package host/target creation:

	$(foreach pkg,$($(PKG)_FINAL_DEPENDENCIES),\
		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
		$(HOST_DIR)$(sep))
	$(foreach pkg,$($(PKG)_FINAL_DEPENDENCIES),\
		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
		$(PER_PACKAGE_DIR)/$(pkg)/target/ \
		$(TARGET_DIR)$(sep))

Global host/target creation in target-finalize:

	$(foreach pkg,$(PACKAGES),\
		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
		$(PER_PACKAGE_DIR)/$(pkg)/target/ \
		$(TARGET_DIR)$(sep))
	@$(call MESSAGE,"Creating global host directory")
	$(foreach pkg,$(PACKAGES),\
		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
		$(HOST_DIR)$(sep))

But I guess that's OK. If we find that too weird, I can always introduce some some make helpers:

per-package-target-dir = $(PER_PACKAGE_DIR)/$(1)/target per-package-host-dir = $(PER_PACKAGE_DIR)/$(1)/host

But:

  $(call per-package-target-dir,$(pkg))

is not really shorter than

  $(PER_PACKAGE_DIR)/$(1)/target

It is actually longer :-)

Thomas

Quoted 4 lines

Here is how I'm thinking of solving the problem:

- Next to <pkg>_PATCH_DEPENDENCIES, introduce the concept of <pkg>_EXTRACT_DEPENDENCIES.

_PATCH_DEPENDENCIES is different: it introduces a dependency between foo-patch
and bar-patch. Here you want a dependency between foo-extract and tar.

So this bit doesn't make sense: _PATCH_DEPENDENCIES are not (necessarily) built
before foo-patch. So to be reproducible, it should *not* be rsynced yet.

Quoted 2 lines

Thoughts?

I'm not at all happy with this approach. It adds generic-package stuff that is
used for only one package, and it spreads the logic out over different places.

I'd rather make the logic explicit in dependencies.mk. Something like

ifneq ($(filter host-tar,$(DEPENDENCIES_HOST_PREREQ)),) $(filter-out host-tar,$(DEPENDENCIES_HOST_PREREQ)): host-tar endif

ifneq ($(filter host-ccache,$(DEPENDENCIES_HOST_PREREQ)),) $(filter-out host-tar host-ccache,$(DEPENDENCIES_HOST_PREREQ)): host-ccache endif

Quoted 8 lines

Quoted 5 lines

Here is how I'm thinking of solving the problem:

- Next to <pkg>_PATCH_DEPENDENCIES, introduce the concept of <pkg>_EXTRACT_DEPENDENCIES.

_PATCH_DEPENDENCIES is different: it introduces a dependency between foo-patch
and bar-patch. Here you want a dependency between foo-extract and tar.

Quoted 8 lines

So this bit doesn't make sense: _PATCH_DEPENDENCIES are not (necessarily) built
before foo-patch. So to be reproducible, it should *not* be rsynced yet.

Quoted 2 lines

Thoughts?

I'm not at all happy with this approach. It adds generic-package stuff that is
used for only one package, and it spreads the logic out over different places.

Quoted 9 lines

I'd rather make the logic explicit in dependencies.mk. Something like

ifneq ($(filter host-tar,$(DEPENDENCIES_HOST_PREREQ)),) $(filter-out host-tar,$(DEPENDENCIES_HOST_PREREQ)): host-tar endif

ifneq ($(filter host-ccache,$(DEPENDENCIES_HOST_PREREQ)),) $(filter-out host-tar host-ccache,$(DEPENDENCIES_HOST_PREREQ)): host-ccache endif

I don't understand how this can work.

The big thing to remember is that when I'm building a package, it only sees in its per-package host/staging directory the dependencies explicitly listed in this package <pkg>_DEPENDENCIES variable.

With your approach, neither host-tar nor host-ccache are listed in any package <pkg>_DEPENDENCIES variable, so the per-package host directory of host-ccache and host-tar will never be rsync'ed into the per-package host directories of other packages.

Due to this, the package infrastructure _will_ have to know about the fact that all packages depend on host-tar/host-ccache, for the simple reason that we need to know that we have to rsync host-tar/host-ccache when populating the per-package host directory in the configure step.

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

What remains to be done?
========================

- Completely get rid of the global HOST_DIR, TARGET_DIR and STAGING_DIR definitions, so that nothing uses them, and they really become per-package variables.

- While the toolchain sysroot, pkg-config paths and host RPATHs are properly handled, there are quite certainly a lot of other places that have absolute paths that won't work well with per-package SDK, and that need to be adjusted.

- Perhaps use a temporary per-package SDK and target directory when building a package, and rename it to the final name once the package has finished building. This would allow to detect situations where the per-package SDK directory of package A contains absolute paths referring to the per-package SDK directory of package B. Such absolute paths are wrong, but we currently don't see them because the per-package SDK directory for package B still exists.

- Many other issues that I'm sure people will report.

Comparison with Gustavo's patch series
======================================

Gustavo Zacarias did a previous implementation of this, available in
http://repo.or.cz/buildroot-gz.git/shortlog/refs/heads/pps3-next. Compared to Gustavo's patch series, this patch series:

- I haven't integrated Gustavo patches that move from $(shell ...) to using backticks to delay the evaluation of STAGING_DIR/HOST_DIR/TARGET_DIR:

   http://repo.or.cz/buildroot-gz.git/commitdiff/0c11c60865f1445830643bb404f92c20d563260c
   http://repo.or.cz/buildroot-gz.git/commitdiff/207d2248ec8542293b6201b5c86f971021a3a591
   http://repo.or.cz/buildroot-gz.git/commitdiff/7f6a69819fbb176e29a1c3a53b4a76efe87dad15
   http://repo.or.cz/buildroot-gz.git/commitdiff/3328a9745eee555f5e5ef7df835afc26612ecd7b
   http://repo.or.cz/buildroot-gz.git/commitdiff/45a9d8c2aa6f3c0d2d8ed989d843890025faa3e8

- I haven't looked at Qt specific issues, such as the ones fixed by Gustavo in:

   http://repo.or.cz/buildroot-gz.git/commitdiff/643e982e635f4386ccefcda9da4b84536af84d04

- I am not doing all the .la files, *-config, *.prl, etc. tweaks that Gustavo is doing in:

http://repo.or.cz/buildroot-gz.git/commitdiff/3f7b227b4c8e4514df6e5d54f88fcfb3a3a4bae0

It is part of the "remaining absolute paths that need to be fixed" item that I mentionned above.