Per-package host/target directory support

Hello,

Here is a fifth iteration of the per-package SDK and target directory implementation.

If you're interested in testing it, you can find the patch series at:

http://git.bootlin.com/users/thomas-petazzoni/buildroot/log/?h=ppsh-v5

However, if you want the full set of PPSH related patches, including the package-specific fixes, and work on the Luarocks and Meson infrastructure, you can pick up the following branch:

http://git.bootlin.com/users/thomas-petazzoni/buildroot/log/?h=ppsh-v5-work

I.e the ppsh-v5 branch is more for reviewing, while ppsh-v5-work is more for testing the whole PPSH functionality.

Changes since v5
================

- Add patch "package/pkg-generic: adjust config scripts tweaks for per-package folders" that adjusts how pkg-generic.mk fixes *-config scripts, to use relative paths instead of absolute paths.

- Add patch "package/pkg-generic: make libtool .la files compatible with per-package folders" that fixes libtool.la files for per-package folders.

- Add patch "package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package folders" that properly handles KCONFIG_DEPENDENCIES in the pkg-kconfig infrastructure for per-package folders.

- Added Acked-by from Yann E. Morin on the following patches:

support/scripts/check-host-rpath: split condition on two statements
Makefile: evaluate CCACHE and HOST{CC,CXX} at time of use

- It is not included in this series, but since the v4 was sent, the case of the Luarocks and Meson based packages have been fixed, and I have sent separate series for these, because they can be merged separately. Many thanks to Fran?ois Perrad and Peter Seiderer for providing very useful insights on Luarocks and Meson.

Changes since v4
================

- Dropped patches that have been merged upstream, mainly the "extract dependency" patches, but also a few other preparation patches.

- Dropped the change of the RPATH handling. As discussed during previous Buildroot Developers meeting, it is not a big problem if during the build, the RPATH of a binary points to the library folder of another package per-package folder. Therefore, instead of fixing up RPATH at the end of each package installation, we keep what Buildroot does today: fix them at the very end of the build.

- Made the support for per-package folders optional. Indeed, we realized there was no way to make sure it would be perfect from day
1. Even if the principle works, there are lots of package-specific details to solve, and solving all of them before merging the base per-package folder support is not reasonable. So for now, an option BR2_PER_PACKAGE_FOLDERS is introduced, which defaults to off. When this option is enabled, the .NOTPARALLEL statement of the main Makefile goes away, which allows to do top-level parallel build.

- Addition of a commit that reworks how the top-level directories are created.

- Introduction of a prepare-per-package-folder function in pkg-generic.mk to encapsulate the logic to create the per-package folders from the dependencies of a package.

- Rebased on top of the latest master.

Changes since v3
================

- Dropped patches that have been merged upstream:
pkgconf: use relative path to STAGING_DIR instead of absolute path
toolchain: post-pone evaluation of TOOLCHAIN_EXTERNAL_BIN

- For now, removed the most controversial/complicated patches (changing the rpath strategy, and switching to per-package host/target directories). The goal for now is to merge only the preparation patches.

Changes since v2
================

- Solved the problem of all DEPENDENCIES_HOST_PREREQ targets:
host-ccache, host-tar, host-lzip, host-xz, host-fakedate.

To achieve this, introduced <pkg>_EXTRACT_DEPENDENCIES and used that to handle the dependencies on host-tar, host-lzip and host-xz.

Used regular dependencies for host-ccache and host-fakedate.

See below for more details.

Changes since v1
================

- Rebased on the latest Buildroot next branch

- The pkg-config changes are reworked according to Arnout's comments:
only @STAGING_SUBDIR@ is replaced in pkg-config.in to generate pkg-config, the rest of the logic is internal to the script.

- New patch to move the "host skeleton" logic into a proper host-skeleton package.

- New patch to have the CMake related files generated as part of the toolchain package.

- New patch to add a new "install" step that depends on the different install steps (target, staging, images, host). This is needed to later add some logic that is executed once all install steps of a package have been done.

- Fix the approach to solve the RPATH logic: instead of using LD_LIBRARY_PATH, we actually fix with patchelf the RPATH of host binaries right after their installation.

- Misc other improvements in the per-package SDK implementation.

What is this all about ?
========================

See the cover letter of v3 for details:
http://lists.busybox.net/pipermail/buildroot/2017-December/208384.html

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

Known issues:

- The qt5 stack is entirely broken, because qmake hardcodes the installation path to be $(STAGING_DIR), so all qt5 packages install to the per-package $(STAGING_DIR) of qt5base.

See
http://lists.busybox.net/pipermail/buildroot/2018-November/235942.html

Best regards,

Thomas

Thomas Petazzoni (9):
Makefile: evaluate CCACHE and HOST{CC,CXX} at time of use support/scripts/check-host-rpath: split condition on two statements
Makefile: rework main directory creation logic
Makefile: move .NOTPARALLEL statement after including .config file
Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR package/pkg-generic: adjust config scripts tweaks for per-package
folders
core: implement per-package SDK and target package/pkg-generic: make libtool .la files compatible with
per-package folders
package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package
folders

 Config.in                        | 18 ++++++
 Makefile                         | 94 ++++++++++++++++++++------------
 fs/common.mk                     |  3 +-
 package/pkg-generic.mk           | 41 ++++++++++++--
 package/pkg-kconfig.mk           |  1 +
 support/scripts/check-host-rpath |  8 ++-
 6 files changed, 122 insertions(+), 43 deletions(-)

2.19.1

As we are going to move to per-package SDK, the location of CCACHE and therefore the definitions of HOSTCC and HOSTCXX need to be evaluated at the time of use and not at the time of assignment. Indeed, the value of HOST_DIR changes from one package to the other.

Therefore, we need to change from := to =.

In addition, while doing A := $(something) $(A) is possible, doing A = $(something) $(A) is not legal. So, instead of defining HOSTCC in terms of the current HOSTCC variable, we re-use HOSTCC_NOCCACHE instead.

---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 94adff3406..2819d44124 100644
@@ -471,11 +471,11 @@ BR_PATH = "$(HOST_DIR)/bin:$(HOST_DIR)/sbin:$(PATH)"
 TARGET_DIR_WARNING_FILE = $(BASE_TARGET_DIR)/THIS_IS_NOT_YOUR_ROOT_FILESYSTEM
 
 ifeq ($(BR2_CCACHE),y)
-CCACHE := $(HOST_DIR)/bin/ccache
+CCACHE = $(HOST_DIR)/bin/ccache
 BR_CACHE_DIR ?= $(call qstrip,$(BR2_CCACHE_DIR))
 export BR_CACHE_DIR
-HOSTCC := $(CCACHE) $(HOSTCC)
-HOSTCXX := $(CCACHE) $(HOSTCXX)
+HOSTCC = $(CCACHE) $(HOSTCC_NOCCACHE)
+HOSTCXX = $(CCACHE) $(HOSTCXX_NOCCACHE)
 else
 export BR_NO_CCACHE
 endif

2.19.1

Inside the check_elf_has_rpath(), we check if the host binary has a correct RPATH, which should be either an absolute path to $(HOST_DIR)/lib, or a relative path using $ORIGIN. Those two conditions are checked in a single statements, but as we are going to add a third condition, let's split this up a bit:

- If we have a RPATH to $(HOST_DIR)/lib -> we're good, return 0
- If we have a RPATH to $ORIGIN/../lib -> we're good, return 0
- Otherwise, we will exit the loop, and return 1

---
 support/scripts/check-host-rpath | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 169628decb..6c5767da05 100755
@@ -63,7 +63,8 @@ check_elf_has_rpath() {
         for dir in ${rpath//:/ }; do
             # Remove duplicate and trailing '/' for proper match
             dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
-            [ "${dir}" = "${hostdir}/lib" -o "${dir}" = "\$ORIGIN/../lib" ] && return 0
+            [ "${dir}" = "${hostdir}/lib" ] && return 0
+            [ "${dir}" = "\$ORIGIN/../lib" ] && return 0
         done
     done < <( readelf -d "${file}"                                              \
               |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \

2.19.1

In the current code, the creation of the main output folders (BUILD_DIR, STAGING_DIR, HOST_DIR, TARGET_DIR, etc.) is done by a global "dirs" target. While this works fine in the current situation, it doesn't work well in a context where per-package host and target directories are used.

For example, with the current code and per-package host directories, the output/staging symbolic link ends up being created as a link to the per-package package sysroot directory of the first package being built, instead of the global sysroot.

This commit reworks the creation of those directories by having the package/pkg-generic.mk code ensure that the build directory, target directory, host directory, staging directory and binaries directory exist before they are needed.

Two new targets, host-finalize and staging-finalize are added in the main Makefile to create the compatibility symlinks for host and staging directories. They will be extended later with additional logic for per-package directories.

Thanks to those changes, the global "dirs" target is entirely removed.

---
 Makefile               | 21 +++++++++------------
 fs/common.mk           |  1 +
 package/pkg-generic.mk |  7 ++++++-
 3 files changed, 16 insertions(+), 13 deletions(-)

index 2819d44124..e675ac26aa 100644
@@ -572,10 +572,6 @@ $(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\
 
 endif
 
-.PHONY: dirs
-dirs: $(BUILD_DIR) $(STAGING_DIR) $(BASE_TARGET_DIR) \
-	$(HOST_DIR) $(HOST_DIR_SYMLINK) $(BINARIES_DIR)
-
 $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
 	$(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTCXX="$(HOSTCXX_NOCCACHE)" syncconfig
 

@@ -605,11 +601,6 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY)
 		--transform='s#^\.#$(BR2_SDK_PREFIX)#' \
 		-C $(HOST_DIR) "."
 
-# Populating the staging with the base directories is handled by the skeleton package
-$(STAGING_DIR):
-	@mkdir -p $(STAGING_DIR)
-	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
-
 RSYNC_VCS_EXCLUSIONS = \
 	--exclude .svn --exclude .git --exclude .hg --exclude .bzr \
 	--exclude CVS

@@ -710,8 +701,14 @@ $(TARGETS_ROOTFS): target-finalize
 # Avoid the rootfs name leaking down the dependency chain
 target-finalize: ROOTFS=
 
+host-finalize: $(HOST_DIR_SYMLINK)
+
+.PHONY: staging-finalize
+staging-finalize:
+	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
+
 .PHONY: target-finalize
-target-finalize: $(PACKAGES)
+target-finalize: $(PACKAGES) host-finalize
 	@$(call MESSAGE,"Finalizing target directory")
 	# Check files that are touched by more than one package
 	./support/scripts/check-uniq-files -t target $(BUILD_DIR)/packages-file-list.txt

@@ -782,7 +779,7 @@ endif
 	touch $(TARGET_DIR)/usr
 
 .PHONY: target-post-image
-target-post-image: $(TARGETS_ROOTFS) target-finalize
+target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
 	@rm -f $(ROOTFS_COMMON_TAR)
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
 		$(call MESSAGE,"Executing post-image script $(s)"); \

@@ -811,7 +808,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config
 
 .PHONY: legal-info
-legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \
+legal-info: legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \
 		$(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST)
 	@cat support/legal-info/README.header >>$(LEGAL_REPORT)
 	@if [ -r $(LEGAL_WARNINGS) ]; then \

index 2a5a202a89..96658428ba 100644
@@ -145,6 +145,7 @@ $$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2)
 $$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot
 $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES)
 	@$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)")
+	mkdir -p $$(@D)
 	rm -rf $$(ROOTFS_$(2)_DIR)
 	mkdir -p $$(ROOTFS_$(2)_DIR)
 	echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)

index f34f46afc8..309fd8cd48 100644
@@ -173,6 +173,7 @@ $(BUILD_DIR)/%/.stamp_extracted:
 $(BUILD_DIR)/%/.stamp_rsynced:
 	@$(call step_start,rsync)
 	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
+	@mkdir -p $(@D)
 	$(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep))
 	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
 	rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $($(PKG)_OVERRIDE_SRCDIR_RSYNC_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D)

@@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built::
 $(BUILD_DIR)/%/.stamp_host_installed:
 	@$(call step_start,install-host)
 	@$(call MESSAGE,"Installing to host directory")
+	@mkdir -p $(HOST_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))

@@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed:
 $(BUILD_DIR)/%/.stamp_staging_installed:
 	@$(call step_start,install-staging)
 	@$(call MESSAGE,"Installing to staging directory")
+	@mkdir -p $(STAGING_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_STAGING_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))

@@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 # Install to images dir
 $(BUILD_DIR)/%/.stamp_images_installed:
 	@$(call step_start,install-image)
+	@mkdir -p $(BINARIES_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep))
 	@$(call MESSAGE,"Installing to images directory")
 	+$($(PKG)_INSTALL_IMAGES_CMDS)

@@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed:
 $(BUILD_DIR)/%/.stamp_target_installed:
 	@$(call step_start,install-target)
 	@$(call MESSAGE,"Installing to target")
+	@mkdir -p $(TARGET_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_TARGET_CMDS)
 	$(if $(BR2_INIT_SYSTEMD),\

@@ -735,7 +740,7 @@ $$($(2)_TARGET_BUILD):	$$($(2)_TARGET_CONFIGURE)
 $(1)-configure:			$$($(2)_TARGET_CONFIGURE)
 $$($(2)_TARGET_CONFIGURE):	| $$($(2)_FINAL_DEPENDENCIES)
 
-$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare
+$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | prepare
 $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies
 
 ifeq ($$($(2)_OVERRIDE_SRCDIR),)

2.19.1

In a follow-up commit, we will make the .NOTPARALLEL statement conditional on a Config.in option, so we need to move it further down.

---
 Makefile | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

index e675ac26aa..24c803872d 100644
@@ -105,22 +105,6 @@ ifneq ($(firstword $(sort $(RUNNING_MAKE_VERSION) $(MIN_MAKE_VERSION))),$(MIN_MA
 $(error You have make '$(RUNNING_MAKE_VERSION)' installed. GNU make >= $(MIN_MAKE_VERSION) is required)
 endif
 
-# Parallel execution of this Makefile is disabled because it changes
-# the packages building order, that can be a problem for two reasons:
-# - If a package has an unspecified optional dependency and that
-#   dependency is present when the package is built, it is used,
-#   otherwise it isn't (but compilation happily proceeds) so the end
-#   result will differ if the order is swapped due to parallel
-#   building.
-# - Also changing the building order can be a problem if two packages
-#   manipulate the same file in the target directory.
-#
-# Taking into account the above considerations, if you still want to execute
-# this top-level Makefile in parallel comment the ".NOTPARALLEL" line and
-# use the -j<jobs> option when building, e.g:
-#      make -j$((`getconf _NPROCESSORS_ONLN`+1))
-.NOTPARALLEL:
-
 # absolute path
 TOPDIR := $(CURDIR)
 CONFIG_CONFIG_IN = Config.in

@@ -246,6 +230,22 @@ ifeq ($(filter $(noconfig_targets),$(MAKECMDGOALS)),)
 -include $(BR2_CONFIG)
 endif
 
+# Parallel execution of this Makefile is disabled because it changes
+# the packages building order, that can be a problem for two reasons:
+# - If a package has an unspecified optional dependency and that
+#   dependency is present when the package is built, it is used,
+#   otherwise it isn't (but compilation happily proceeds) so the end
+#   result will differ if the order is swapped due to parallel
+#   building.
+# - Also changing the building order can be a problem if two packages
+#   manipulate the same file in the target directory.
+#
+# Taking into account the above considerations, if you still want to execute
+# this top-level Makefile in parallel comment the ".NOTPARALLEL" line and
+# use the -j<jobs> option when building, e.g:
+#      make -j$((`getconf _NPROCESSORS_ONLN`+1))
+.NOTPARALLEL:
+
 # timezone and locale may affect build output
 ifeq ($(BR2_REPRODUCIBLE),y)
 export TZ = UTC

2.19.1

Thomas, All,

On 2018-11-20 17:35 +0100, Thomas Petazzoni spake thusly:

You forgot to carry my:

(Granted, it was hiding below a long rant/research about the need of a config option ;-] )

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Hello,

Ah right, I'll add it for v6.

And I will update the commit log to include some explanation as to why we need an explicit option and not enable per-package folders automagically when make -jN is passed.

Thanks!

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

In commit 7e9870ce32d6329d9e3d602247fbe1709a2275a4 ("core: introduce intermediate BASE_TARGET_DIR variable"), the definition of TARGET_DIR_WARNING_FILE was changed to use $(BASE_TARGET_DIR) instead of $(TARGET_DIR).

However, this change is incompatible with per-package directories, and is in fact not needed.

With per-package directories, using $(BASE_TARGET_DIR) means that TARGET_DIR_WARNING_FILE is output/target/THIS_IS_NOT_YOUR_ROOT_FILESYSTEM. Due to this, when skeleton-init-common or skeleton-custom attempt to install it, it fails, because it should be installed to their package per-package target directory, and not the global output/target folder that doesn't exist yet. The failure looks like this:

/usr/bin/install -m 0644 support/misc/target-dir-warning.txt /home/thomas/projets/buildroot/output/target/THIS_IS_NOT_YOUR_ROOT_FILESYSTEM /usr/bin/install: cannot create regular file '/home/thomas/projets/buildroot/output/target/THIS_IS_NOT_YOUR_ROOT_FILESYSTEM': No such file or directory make[1]: *** [package/pkg-generic.mk:336: /home/thomas/projets/buildroot/output/build/skeleton-init-common/.stamp_target_installed] Error 1

TARGET_DIR_WARNING_FILE is used in three places:

- In skeleton-custom.mk and skeleton-init-common.mk, where as explained above, using $(TARGET_DIR) fixes the use of $(TARGET_DIR_WARNING_FILE) in the context of per-package target directories.

- In fs/common.mk, where it is used as argument to $(notdir ...) to retrieve just the name of the warning file. So in this case, we really don't care about the path of the file, just its name.

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 24c803872d..23032988a5 100644
@@ -468,7 +468,7 @@ BR_PATH = "$(HOST_DIR)/bin:$(HOST_DIR)/sbin:$(PATH)"
 
 # Location of a file giving a big fat warning that output/target
 # should not be used as the root filesystem.
-TARGET_DIR_WARNING_FILE = $(BASE_TARGET_DIR)/THIS_IS_NOT_YOUR_ROOT_FILESYSTEM
+TARGET_DIR_WARNING_FILE = $(TARGET_DIR)/THIS_IS_NOT_YOUR_ROOT_FILESYSTEM
 
 ifeq ($(BR2_CCACHE),y)
 CCACHE = $(HOST_DIR)/bin/ccache

2.19.1

Thomas, All,

On 2018-11-20 17:35 +0100, Thomas Petazzoni spake thusly:

At the beginning, I was a bit skeptical, and I was also wondering how it would work with my on-going refactoring due to handling the capabilities, but I so far can't see any problem with it. So:

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

This commit adjusts the logic in pkg-generic.mk that tweaks the *-config shell scripts installed by various libraries to make it compatible with per-package folders.

This requires two fixes:

- replacing $(STAGING_DIR) with a relative path from the config script to the staging directory, rather than using an absolute path of the staging directory.

Without this, a *-config script provided by package A, but called from package B per-package folder will return paths from package A per-package folder:

   $ ./output/per-package/mcrypt/host/usr/<tuple>/sysroot/usr/bin/libmcrypt-config --libs
   -L..../output/per-package/libmcrypt/host/usr/<tuple>/sysroot/usr/lib/

The libmcrypt-config script is installed by the libmcrypt package, and mcrypt is a package that depends on libmcrypt. When we call the libmcrypt-config script from the mcrypt per-package folder, it returns a -L flag that points to the libmcrypt per-package folder.

One might say: but this is OK, since the sysroot of the libmcrypt per-package folder also contains the libmcrypt library. This is true, but we encounter a more subtle issue: because -L paths are considered before standard paths, ld ends up finding libc.so in the libmcrypt per-package folder. This libc.so file is a linker script that looks like this:

GROUP ( /lib/libc.so.6 /usr/lib/libc_nonshared.a AS_NEEDED ( /lib/ld-linux.so.3 ) )

Normally, thanks to ld sysroot awareness, /lib/libc.so.6 in this script is re-interpreted according to the sysroot. But in this case, the library is *outside* the compiler sysroot. Remember: we are using the compiler/linker from the "mcrypt" per-package folder, but we found "libc.so.6" in the "libmcrypt" per-package folder.

This causes the linker to really use the /lib/libc.so.6 from the host machine, obvisouly leading to a build failure such as:

output/per-package/libgcrypt/host/opt/ext-toolchain/bin/../lib/gcc/nios2-linux-gnu/7.3.1/../../../../nios2-linux-gnu/bin/ld: cannot find /lib/libc.so.6 output/per-package/libgcrypt/host/opt/ext-toolchain/bin/../lib/gcc/nios2-linux-gnu/7.3.1/../../../../nios2-linux-gnu/bin/ld: cannot find /usr/lib/libc_nonshared.a output/per-package/libgcrypt/host/opt/ext-toolchain/bin/../lib/gcc/nios2-linux-gnu/7.3.1/../../../../nios2-linux-gnu/bin/ld: cannot find /lib/ld-linux-nios2.so.1

- Some *-config scripts, such as the apr-1-config script, contain references to host tools:

CC=".../output/per-package/apr/hosr/bin/arm-linux-gcc" CCP=".../output/per-package/apr/hosr/bin/arm-linux-cpp"

We also want to replace those with proper relative paths. To achieve this, we need to also replace $(HOST_DIR) with a relative path. Since $(STAGING_DIR) is inside $(HOST_DIR), the first replacement of $(STAGING_DIR) by @STAGING_DIR@ is no longer needed:
replacing $(HOST_DIR) by @HOST_DIR@ is sufficient. We still need to replace @STAGING_DIR@ by the proper path though, as we introduce @STAGING_DIR@ references in exec_prefix and prefix variables, as well as -I and -L flags.

---
 package/pkg-generic.mk | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

index 309fd8cd48..9b4db845b6 100644
@@ -275,12 +275,13 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
 	$(Q)if test -n "$($(PKG)_CONFIG_SCRIPTS)" ; then \
 		$(call MESSAGE,"Fixing package configuration files") ;\
-			$(SED)  "s,$(BASE_DIR), at BASE_DIR@,g" \
-				-e "s,$(STAGING_DIR), at STAGING_DIR@,g" \
+			$(SED)  "s,$(HOST_DIR), at HOST_DIR@,g" \
+				-e "s,$(BASE_DIR), at BASE_DIR@,g" \
 				-e "s,^\(exec_\)\?prefix=.*,\1prefix=@STAGING_DIR@/usr,g" \
 				-e "s,-I/usr/,-I at STAGING_DIR@/usr/,g" \
 				-e "s,-L/usr/,-L at STAGING_DIR@/usr/,g" \
-				-e "s, at STAGING_DIR@,$(STAGING_DIR),g" \
+				-e 's, at STAGING_DIR@,$$(dirname $$0)/../..,g' \
+				-e 's, at HOST_DIR@,$$(dirname $$0)/../../../..,g' \
 				-e "s, at BASE_DIR@,$(BASE_DIR),g" \
 				$(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
 	fi

2.19.1

Thomas, All,

On 2018-11-20 17:35 +0100, Thomas Petazzoni spake thusly:

Note [0], see below...

[--SNIP--]

Note [1], see below...

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Hello Yann,

Correct.

We're not creating symlinks here, we replacing absolute paths with relative paths using the $(dirname $0) trick. But I guess that's what you meant.

Exactly: if we replace $(BASE_DIR) by @BASE_DIR@, we can't easily replace $(HOST_DIR) by @HOST_DIR@ anymore and then do the necessary fixup.

The reason why the $(BASE_DIR) -> @BASE_DIR@ and then @BASE_DIR@ -> $(BASE_DIR) replacement was added in commit 7701fc53d19be7cab8bddba72046a39e8421fd9e was to support the case where build takes place in /usr.

The problem was that:

	s,-L/usr,-L at STAGING_DIR@/usr,g

would continuously re-add @STAGING_DIR@ if the build happens in a sub-dir of /usr, because -L/usr will always match, even if the path has already been fixed up.

The replacement of $(HOST_DIR) to @HOST_DIR@ before the $(BASE_DIR) to @BASE_DIR@ replacement doesn't break this logic. If the path is in $(HOST_DIR), it will have been replaced by @HOST_DIR@ and will not match -L/usr, and then if the path is somewhere else in $(BASE_DIR), it will be replaced by @BASE_DIR@, and the existing logic will continue to work.

So, I'll take your Reviewed-by, but let me know if you're still unconvinced for some reason.

Thanks!

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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.

- The global TARGET_DIR and HOST_DIR variable now automatically point to the per-package directory when PKG is defined. So whenever a package references $(HOST_DIR) or $(TARGET_DIR) in its build process, it effectively references the per-package host/target directories. Note that STAGING_DIR is a sub-dir of HOST_DIR, so it is handled as well.

- Of course, packages have dependencies, so those dependencies must be installed in the per-package host and target folders. 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. Note that this only works because we make the assumption that one package does *not* overwrite files installed by another package.

This is done for "extract dependencies" at the beginning of the extract step, and for "normal dependencies" at the beginning of the configure step.

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

---
 Config.in                        | 18 ++++++++++++++++
 Makefile                         | 37 +++++++++++++++++++++++++++-----
 fs/common.mk                     |  2 +-
 package/pkg-generic.mk           | 20 ++++++++++++++++-
 support/scripts/check-host-rpath |  5 ++++-
 5 files changed, 74 insertions(+), 8 deletions(-)

index f965e9d6d8..70eaab900f 100644
@@ -696,6 +696,24 @@ config BR2_REPRODUCIBLE
 	  This is labeled as an experimental feature, as not all
 	  packages behave properly to ensure reproducibility.
 
+config BR2_PER_PACKAGE_FOLDERS
+	bool "Use per-package folders (experimental)"
+	help
+	  This option will change the build process of Buildroot
+	  package to use per-package target and host directories.
+
+	  This is useful for two related purposes:
+
+	    - Cleanly isolate the build of each package, so that a
+	      given package only "sees" the dependencies it has
+	      explicitly expressed, and not other packages that may
+	      have by chance been built before.
+
+	    - Enable top-level parallel build.
+
+	  This is labeled as an experimental feature, as not all
+	  packages behave properly with per-package folders.
+
 endmenu
 
 comment "Security Hardening Options"

index 23032988a5..f3c0e2326e 100644
@@ -207,7 +207,8 @@ BINARIES_DIR := $(BASE_DIR)/images
 # The target directory is common to all packages,
 # but there is one that is specific to each filesystem.
 BASE_TARGET_DIR := $(BASE_DIR)/target
-TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(BASE_TARGET_DIR))
+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

@@ -230,6 +231,7 @@ ifeq ($(filter $(noconfig_targets),$(MAKECMDGOALS)),)
 -include $(BR2_CONFIG)
 endif
 
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),)
 # Parallel execution of this Makefile is disabled because it changes
 # the packages building order, that can be a problem for two reasons:
 # - If a package has an unspecified optional dependency and that

@@ -245,6 +247,13 @@ endif
 # use the -j<jobs> option when building, e.g:
 #      make -j$((`getconf _NPROCESSORS_ONLN`+1))
 .NOTPARALLEL:
+endif
+
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/target,$(BASE_TARGET_DIR)))
+else
+TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(BASE_TARGET_DIR))
+endif
 
 # timezone and locale may affect build output
 ifeq ($(BR2_REPRODUCIBLE),y)

@@ -455,7 +464,11 @@ LZCAT := $(call qstrip,$(BR2_LZCAT))
 TAR_OPTIONS = $(call qstrip,$(BR2_TAR_OPTIONS)) -xf
 
 # packages compiled for the host go here
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+HOST_DIR = $(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/host,$(call qstrip,$(BR2_HOST_DIR)))
+else
 HOST_DIR := $(call qstrip,$(BR2_HOST_DIR))
+endif
 
 ifneq ($(HOST_DIR),$(BASE_DIR)/host)
 HOST_DIR_SYMLINK = $(BASE_DIR)/host

@@ -701,14 +714,28 @@ $(TARGETS_ROOTFS): target-finalize
 # Avoid the rootfs name leaking down the dependency chain
 target-finalize: ROOTFS=
 
-host-finalize: $(HOST_DIR_SYMLINK)
+host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+	@$(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))
+endif
 
 .PHONY: staging-finalize
 staging-finalize:
 	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
 
 .PHONY: target-finalize
-target-finalize: $(PACKAGES) host-finalize
+target-finalize: $(PACKAGES) $(TARGET_DIR) host-finalize
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+	@$(call MESSAGE,"Creating global target directory")
+	$(foreach pkg,$(PACKAGES),\
+		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
+		$(PER_PACKAGE_DIR)/$(pkg)/target/ \
+		$(TARGET_DIR)$(sep))
+endif
 	@$(call MESSAGE,"Finalizing target directory")
 	# Check files that are touched by more than one package
 	./support/scripts/check-uniq-files -t target $(BUILD_DIR)/packages-file-list.txt

@@ -972,7 +999,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 
 # staging and target directories do NOT list these as
 # dependencies anywhere else
-$(BUILD_DIR) $(BASE_TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
+$(BUILD_DIR) $(BASE_TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST) $(PER_PACKAGE_DIR):
 	@mkdir -p $@
 
 # outputmakefile generates a Makefile in the output directory, if using a

@@ -1011,7 +1038,7 @@ printvars:
 clean:
 	rm -rf $(BASE_TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) $(HOST_DIR_SYMLINK) \
 		$(BUILD_DIR) $(BASE_DIR)/staging \
-		$(LEGAL_INFO_DIR) $(GRAPHS_DIR)
+		$(LEGAL_INFO_DIR) $(GRAPHS_DIR) $(PER_PACKAGE_DIR)
 
 .PHONY: distclean
 distclean: clean

index 96658428ba..fc4be3cc05 100644
@@ -172,7 +172,7 @@ rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1)
 
 ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
 TARGETS_ROOTFS += rootfs-$(1)
-PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES))
+PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES)) $(ROOTFS_COMMON_DEPENDENCIES)
 endif
 
 # Check for legacy POST_TARGETS rules

index 9b4db845b6..884b868a9a 100644
@@ -98,7 +98,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += check_bin_arch
 # 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)))
+		$(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR) $(PER_PACKAGE_DIR)))
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath
 

@@ -126,6 +126,21 @@ endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_user
 endif
 
+# $1: deps
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+define prepare-per-package-folder
+	mkdir -p $(HOST_DIR) $(TARGET_DIR)
+	$(foreach pkg,$(1),\
+		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
+		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
+		$(HOST_DIR)$(sep))
+	$(foreach pkg,$(1),\
+		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
+		$(PER_PACKAGE_DIR)/$(pkg)/target/ \
+		$(TARGET_DIR)$(sep))
+endef
+endif
+
 ################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################

@@ -133,6 +148,7 @@ endif
 # Retrieve the archive
 $(BUILD_DIR)/%/.stamp_downloaded:
 	@$(call step_start,download)
+	$(call prepare-per-package-folder,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES))
 	$(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
 # Only show the download message if it isn't already downloaded
 	$(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \

@@ -159,6 +175,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call step_start,extract)
 	@$(call MESSAGE,"Extracting")
+	$(call prepare-per-package-folder,$($(PKG)_FINAL_EXTRACT_DEPENDENCIES))
 	$(foreach hook,$($(PKG)_PRE_EXTRACT_HOOKS),$(call $(hook))$(sep))
 	$(Q)mkdir -p $(@D)
 	$($(PKG)_EXTRACT_CMDS)

@@ -219,6 +236,7 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
 $(BUILD_DIR)/%/.stamp_configured:
 	@$(call step_start,configure)
 	@$(call MESSAGE,"Configuring")
+	$(call prepare-per-package-folder,$($(PKG)_FINAL_DEPENDENCIES))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))

index 6c5767da05..787a1763b0 100755
@@ -11,6 +11,7 @@ export LC_ALL=C
 main() {
     local pkg="${1}"
     local hostdir="${2}"
+    local perpackagedir="${3}"
     local file ret
 
     # Remove duplicate and trailing '/' for proper match

@@ -20,7 +21,7 @@ main() {
     while read file; do
         is_elf "${file}" || continue
         elf_needs_rpath "${file}" "${hostdir}" || continue
-        check_elf_has_rpath "${file}" "${hostdir}" && continue
+        check_elf_has_rpath "${file}" "${hostdir}" "${perpackagedir}" && continue
         if [ ${ret} -eq 0 ]; then
             ret=1
             printf "***\n"

@@ -57,6 +58,7 @@ elf_needs_rpath() {
 check_elf_has_rpath() {
     local file="${1}"
     local hostdir="${2}"
+    local perpackagedir="${3}"
     local rpath dir
 
     while read rpath; do

@@ -65,6 +67,7 @@ check_elf_has_rpath() {
             dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
             [ "${dir}" = "${hostdir}/lib" ] && return 0
             [ "${dir}" = "\$ORIGIN/../lib" ] && return 0
+            [[ ${dir} =~ ${perpackagedir}/[^/]*/host/lib ]] && return 0
         done
     done < <( readelf -d "${file}"                                              \
               |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \

2.19.1

Thomas, All,

On 2018-11-20 17:35 +0100, Thomas Petazzoni spake thusly:

[--SNIP--]

I'm not fond of the naming here...

In the end, can we expect to have a layout that would look like:

output/build/busybox/host/ output/build/busybox/target/
output/build/busybox/busybox-1.2.3/ # Source tree, and
# currently, build dir
And then we can add for OOT:
output/build/busybox/build-dir/

[--SNIP--]

As I said above, I'm not too fond of that naming.... I have nothing better to suggest for now, except that we can change that later (enough bike-shedding).

    ascii order);

    https://git.buildroot.org/buildroot/commit/?id=305e4487e5c18ed89bf2aa106b2068f9dce686fb

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

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Yay bikeshedding!

If we're changing the world anyway, what about:

output/work/busybox-1.2.3/source
output/work/busybox-1.2.3/host
output/work/busybox-1.2.3/target
output/work/busybox-1.2.3/build

Or maybe even:

output/work/busybox-1.2.3/source output/work/busybox-1.2.3/dependencies/host output/work/busybox-1.2.3/dependencies/staging -> host/tuple/sysroot output/work/busybox-1.2.3/dependencies/target output/work/busybox-1.2.3/build

That said, I'm absolutely fine with keeping the current layout. The proposed
layout has the only advantage that you can kill the ppd together with the source/build dir, but the big disadvantage that it breaks everybody's habits...

[snip]

Except that this patch doesn't even touch that line...

[snip]

While we're bikeshedding: we have about 1000 occurences of "directory" and less
than 30 of "folder".

At some point we started collecting this kind of thing in pkg-utils.mk. But I'm
not sure if we're still doing that...

I kind of like how we first build up all of host, and then all of target. But
really, it doesn't make much of a difference.

More importantly, however: should we also sync the target directory for host
packages? We *are* syncing the staging directory, of course...

[snip]

The commit message doesn't explain why this is needed, and it took me some time
to understand...

Regards,
Arnout

Hello Arnout,

Thanks for the review!

:-)

You need two build folders: one for the target variant and one for the host variant.

Also, I don't really like the word "work", that's what is used in OE and I find it very fuzzy.

I renamed to use "directory", both the macro, but also BR2_PER_PACKAGE_FOLDERS is now BR2_PER_PACKAGE_DIRECTORIES.

I moved the macro to pkg-utils.mk. It makes sense, because the macro is also used in pkg-kconfig.mk (in a follow-up commit).

Agreed, I also like the two loops.

Does it really matter ? A host package will only have other host packages as its dependencies, so its per-package target directory will always be empty. I did a test build, and all output/per-package/host-*/target/ folders were empty.

So I am not sure it is really worth adding more conditionals in the code just to avoid those empty target directories. But if you really want that, I'll do it.

I updated the commit log with an explanation about this. It's obviously not trivial, so it definitely deserves an explanation.

Best regards,

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Clearly not a good name then, because with "host" I meant the per-package host
directory, i.e. the target package's host directory. There would also be a

output/work/host-busybox-1.2.3/source
output/work/host-busybox-1.2.3/host
output/work/host-busybox-1.2.3/target
output/work/host-busybox-1.2.3/build

Oh, but that would defeat the point of the source/build directory split...
Never mind then :-)

That was the whole point, we should become OE! :-) BTW, OE also uses "tmp",
that's a lot better of course.

But I see that in the end you kept the per-package directory. Reviewing all the
options, I agree that that is still the best one.

[snip]

No, let's keep it as it is, it's not worth it.

In fact, host packages may install stuff in target (e.g. host-gcc-final
installs libstdc++.so I think). So if a target package depends indirectly on such a package through a different host package, we should still have its target populated.

Regards,
Arnout

[snip]

Hello Yann,

Thanks a lot for your extensive review!

It would be nice indeed, but it's quite a radical change, as it moves the location of the build directory of each package. I am not sure I want to do that as part of this series, which is already complicated by itself.

Also for OOT, your scheme doesn't yet make sense for package that have both a target and a host variant: the goal is to share the source directory between the host and the target variant. Starting from your example, how would it work for libglib2 and host-libglib2 ?

	output/build/libglib2/libglib2-1.2.3/

for the libglib2 source code ? Where would be the build directories for the target and host variants ?

I think a scheme that would maybe make more sense is:

	output/src/libglib2-1.2.3/
	output/build/libglib2/
	output/build/host-libglib2/
	output/per-package/libglib2/{target,host}/
	output/per-package/host-libglib2/{target,host}/

or something like that.

Since for now there is no consensus on what the scheme would be, I'm keeping my proposal as-is, but I can adapt it to whatever we decide. However, what I would like is that we find a path where this series can remain limited to per-package directory support. It's already complicated enough that I don't want to be drowned into reworking other aspects at the same time.

You don't like the PER_PACKAGE_DIR variable name, or the "per-package" directory name ? Or both ?

The variable name is easy to change any time. The directory name can be changed, but it's a bit more annoying: once users are going to become used to such a name/organization, changing it is going to make things harder for users.

However, keep in mind that the BR2_PER_PACKAGE_DIRECTORIES option (formerly BR2_PER_PACKAGE_FOLDERS) is still experimental, so whatever it does can still be changed until we remove the "experimental" mark.

Indeed, I've split that into a separate commit.

As Arnout explained: this definition was already like this. In the !BR2_PER_PACKAGE_FOLDERS case, there is no need for it to be recursively-expanded. However, in the BR2_PER_PACKAGE_FOLDERS case it *must* be recursively expanded.

Done.

Done.

True, done.

Correct, I've dropped this change from my commit, and simply added the existing commit to my branch in the mean time.

I moved it to pkg-utils.mk, which is a better place since another commit in my series uses this macro from pkg-kconfig.mk. I have also added a help comment to it.

As Arnout explained, it looked pretty nice to see the host directory being populated first, then the target one. Having a single loop really doesn't change anything in terms of the operation cost: we still have the same number of rsync's. So I can of side with Arnout here, I like the two loops. But if you disagree strongly, I can change it, it's not a big/strong feeling on my side.

Thomas

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Libtool .la files unfortunately contain a number of absolute paths, which now refer to per-package folders. Due to this, when building package A, .la files may contain absolute paths referring to folders in package B per-package sysroot. This causes some -L flags referring to other sysroot from being added, which doesn't work as the linker no longer realizes that such paths are within its sysroot.

To fix this, we introduce a replacement step of .la files in the configure step, to make sure all paths refer to this package per-package folder.

---
 package/pkg-generic.mk | 7 +++++++
 1 file changed, 7 insertions(+)

index 884b868a9a..b2da97712b 100644
@@ -237,6 +237,12 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call step_start,configure)
 	@$(call MESSAGE,"Configuring")
 	$(call prepare-per-package-folder,$($(PKG)_FINAL_DEPENDENCIES))
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+	# Make sure .la files only reference the current per-package
+	# folder.
+	$(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
+		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]*/:$(PER_PACKAGE_DIR)/$(NAME)/:g"
+endif
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))

@@ -890,6 +896,7 @@ $$($(2)_TARGET_INSTALL_IMAGES):		PKG=$(2)
 $$($(2)_TARGET_INSTALL_HOST):		PKG=$(2)
 $$($(2)_TARGET_BUILD):			PKG=$(2)
 $$($(2)_TARGET_CONFIGURE):		PKG=$(2)
+$$($(2)_TARGET_CONFIGURE):		NAME=$(1)
 $$($(2)_TARGET_RSYNC):			SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
 $$($(2)_TARGET_RSYNC):			PKG=$(2)
 $$($(2)_TARGET_PATCH):			PKG=$(2)

2.19.1

The pkg-kconfig infrastructure hijacks the regular chain of build steps to insert its own step to prepare the configuration of kconfig packages. This additional step may have dependencies of its own, such as host-flex, host-bison or toolchain.

In the context of per-package folder support, those dependencies must be copied to the per-package folder of the current package prior to doing the config preparation. This commit implements this logic by adding a call to prepare-per-package-folder at the right spot.

---
 package/pkg-kconfig.mk | 1 +
 1 file changed, 1 insertion(+)

index d6c95b897e..bf43632adc 100644
@@ -113,6 +113,7 @@ endef
 # Since the file could be a defconfig file it needs to be expanded to a
 # full .config first.
 $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG): $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES)
+	$$(call prepare-per-package-folder,$$($(2)_KCONFIG_DEPENDENCIES))
 	$$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \
 		$$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \
 		$$(INSTALL) -m 0644 -D $$($(2)_KCONFIG_FILE) $$(@))

2.19.1