[Buildroot] [PATCH next v6 00/10] Per-package host/target directory support

[Buildroot] [PATCH next v6 00/10] Per-package host/target directory support

[Thomas Petazzoni]

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

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-v6-work

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

Changes since v6
================

 - Add Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> on patch
   "Makefile: move .NOTPARALLEL statement after including .config
   file"

 - Add Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr> on patch
   Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR

 - Add Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> on patch
   "package/pkg-generic: adjust config scripts tweaks for per-package
   directories"

 - Move the change that really allows top-level parallel build (by
   making .NOTPARALLEL conditional) into a separate commit, as
   suggested by Yann E. Morin.

 - Sort $(PACKAGES) in host-finalize and target-finalize when creating
   the global HOST_DIR and global TARGET_DIR. Suggested by Yann
   E. Morin.

 - Split too long line in the main Makefile listing all the top-level
   directories as targets. Suggested by Yann E. Morin.

 - Drop change in fs/common.mk adding ROOTFS_COMMON_DEPENDENCIES to
   PACKAGES, since a similar change has already been merged in
   master. Noticed by Yann E. Morin.

 - Use "directory/directories" instead of "folder/folders", as
   suggested by Arnout Vandecappelle.

 - Move prepare-per-package-folder (now named
   prepare-per-package-directory) to pkg-utils.mk. Suggested by both
   Yann E. Morin and Arnout Vandecappelle.

 - Add a comment above prepare-per-package-directory, as suggested by
   Yann E. Morin.

 - Update the commit log of "core: implement per-package SDK and
   target" to explain why we are modifying the check-host-rpath
   script. Suggested by Arnout Vandecappelle.

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

 - The Python external modules that need native libraries currently do
   not build. This is due to the Python module building logic reading
   a Makefile that hardcodes the path to the compiler. Investigation
   is underway.

 - The host-cargo package doesn't build, it fails to find the
   http_parser library at link time.

Best regards,

Thomas


Thomas Petazzoni (10):
  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
    directories
  core: implement per-package SDK and target
  Makefile: allow top-level parallel build with
    BR2_PER_PACKAGE_DIRECTORIES=y
  package/pkg-generic: make libtool .la files compatible with
    per-package directories
  package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package
    directories

 Config.in                        | 18 ++++++
 Makefile                         | 95 ++++++++++++++++++++------------
 fs/common.mk                     |  1 +
 package/pkg-generic.mk           | 26 +++++++--
 package/pkg-kconfig.mk           |  1 +
 package/pkg-utils.mk             | 21 +++++++
 support/scripts/check-host-rpath |  8 ++-
 7 files changed, 128 insertions(+), 42 deletions(-)

-- 
2.19.1

[Buildroot] [PATCH next v6 01/10] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use

[Thomas Petazzoni]

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.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 94adff3406..2819d44124 100644
--- a/Makefile
+++ b/Makefile
@@ -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

[Buildroot] [PATCH next v6 02/10] support/scripts/check-host-rpath: split condition on two statements

[Thomas Petazzoni]

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

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 support/scripts/check-host-rpath | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index 169628decb..6c5767da05 100755
--- a/support/scripts/check-host-rpath
+++ b/support/scripts/check-host-rpath
@@ -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

[Buildroot] [PATCH next v6 03/10] Makefile: rework main directory creation logic

[Thomas Petazzoni]

In the current code, the creation of the main output directories
(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.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 Makefile               | 21 +++++++++------------
 fs/common.mk           |  1 +
 package/pkg-generic.mk |  7 ++++++-
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 2819d44124..e675ac26aa 100644
--- a/Makefile
+++ b/Makefile
@@ -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 \
diff --git a/fs/common.mk b/fs/common.mk
index 2a5a202a89..96658428ba 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -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)
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f34f46afc8..309fd8cd48 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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

[Buildroot] [PATCH next v6 04/10] Makefile: move .NOTPARALLEL statement after including .config file

[Thomas Petazzoni]

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.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 Makefile | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/Makefile b/Makefile
index e675ac26aa..24c803872d 100644
--- a/Makefile
+++ b/Makefile
@@ -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

[Buildroot] [PATCH next v6 05/10] Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR

[Thomas Petazzoni]

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

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 24c803872d..23032988a5 100644
--- a/Makefile
+++ b/Makefile
@@ -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

[Buildroot] [PATCH next v6 06/10] package/pkg-generic: adjust config scripts tweaks for per-package directories

[Thomas Petazzoni]

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

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 directory will return paths from package A
   per-package directory:

   $ ./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 directory, it
   returns a -L flag that points to the libmcrypt per-package
   directory.

   One might say: but this is OK, since the sysroot of the libmcrypt
   per-package directory 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 directory. 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
   directory, but we found "libc.so.6" in the "libmcrypt" per-package
   directory.

   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.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-generic.mk | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 309fd8cd48..9b4db845b6 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

This commit implements 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 directories, 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/ directory is created, which will contain
   one sub-directory per package, and inside it, a "host" directory
   and a "target" directory:

   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/ directory 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 directories. To do
   so, we simply rsync (using hard links to save space and time) the
   host and target directories of the direct dependencies of the
   package to the current package host and target directories.

   We only need to take care of direct dependencies (and not
   recursively all dependencies), because we accumulate into those
   per-package host and target directories 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.

Also, with this change to per-package directories, binaries built for
the host may contain RPATH pointing to per-package directories. This
commit therefore teaches the check-host-rpath script with the fact
that RPATH pointing to a per-package directory are OK.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 Config.in                        | 18 ++++++++++++++++
 Makefile                         | 36 +++++++++++++++++++++++++++-----
 package/pkg-generic.mk           |  5 ++++-
 package/pkg-utils.mk             | 21 +++++++++++++++++++
 support/scripts/check-host-rpath |  5 ++++-
 5 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/Config.in b/Config.in
index f965e9d6d8..2d8a07fda7 100644
--- a/Config.in
+++ b/Config.in
@@ -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_DIRECTORIES
+	bool "Use per-package directories (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 directories.
+
 endmenu
 
 comment "Security Hardening Options"
diff --git a/Makefile b/Makefile
index 23032988a5..35fe1b3644 100644
--- a/Makefile
+++ b/Makefile
@@ -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
@@ -246,6 +247,12 @@ endif
 #      make -j$((`getconf _NPROCESSORS_ONLN`+1))
 .NOTPARALLEL:
 
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),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)
 export TZ = UTC
@@ -455,7 +462,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_DIRECTORIES),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 +712,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_DIRECTORIES),y)
+	@$(call MESSAGE,"Creating global host directory")
+	$(foreach pkg,$(sort $(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_DIRECTORIES),y)
+	@$(call MESSAGE,"Creating global target directory")
+	$(foreach pkg,$(sort $(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 +997,8 @@ 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 +1037,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
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9b4db845b6..b1f0cdf34a 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
 
@@ -133,6 +133,7 @@ endif
 # Retrieve the archive
 $(BUILD_DIR)/%/.stamp_downloaded:
 	@$(call step_start,download)
+	$(call prepare-per-package-directory,$($(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 +160,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call step_start,extract)
 	@$(call MESSAGE,"Extracting")
+	$(call prepare-per-package-directory,$($(PKG)_FINAL_EXTRACT_DEPENDENCIES))
 	$(foreach hook,$($(PKG)_PRE_EXTRACT_HOOKS),$(call $(hook))$(sep))
 	$(Q)mkdir -p $(@D)
 	$($(PKG)_EXTRACT_CMDS)
@@ -219,6 +221,7 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
 $(BUILD_DIR)/%/.stamp_configured:
 	@$(call step_start,configure)
 	@$(call MESSAGE,"Configuring")
+	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index bffd79dfb0..4ca2eebc95 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -62,6 +62,27 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
 endif
 endef
 
+# This function prepares the per-package HOST_DIR and TARGET_DIR of
+# the current package, by rsync the host and target directories of the
+# dependencies of this package. The list of dependencies is passed as
+# argument, so that this function can be used to prepare with
+# different set of dependencies (download, extract, configure, etc.)
+#
+# $1: space-separated list of dependencies
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+define prepare-per-package-directory
+	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
+
 #
 # legal-info helper functions
 #
diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index 6c5767da05..787a1763b0 100755
--- a/support/scripts/check-host-rpath
+++ b/support/scripts/check-host-rpath
@@ -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

[Buildroot] [PATCH next v6 08/10] Makefile: allow top-level parallel build with BR2_PER_PACKAGE_DIRECTORIES=y

[Thomas Petazzoni]

With per-package folder support, top-level parallel build becomes
safe, so we can enclose the .NOTPARALLEL statement in a
!BR2_PER_PACKAGE_DIRECTORIES condition.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 35fe1b3644..7bd7291cfd 100644
--- a/Makefile
+++ b/Makefile
@@ -231,6 +231,7 @@ ifeq ($(filter $(noconfig_targets),$(MAKECMDGOALS)),)
 -include $(BR2_CONFIG)
 endif
 
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),)
 # 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
@@ -246,6 +247,7 @@ endif
 # use the -j<jobs> option when building, e.g:
 #      make -j$((`getconf _NPROCESSORS_ONLN`+1))
 .NOTPARALLEL:
+endif
 
 ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
 TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/target,$(BASE_TARGET_DIR)))
-- 
2.19.1

[Buildroot] [PATCH next v6 09/10] package/pkg-generic: make libtool .la files compatible with per-package directories

[Thomas Petazzoni]

Libtool .la files unfortunately contain a number of absolute paths,
which now refer to per-package directories. Due to this, when building
package A, .la files may contain absolute paths referring to
directories 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 directory.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/pkg-generic.mk | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index b1f0cdf34a..17909f4a61 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -222,6 +222,12 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call step_start,configure)
 	@$(call MESSAGE,"Configuring")
 	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+	# Make sure .la files only reference the current per-package
+	# directory
+	$(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))
@@ -875,6 +881,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

[Buildroot] [PATCH next v6 10/10] package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package directories

[Thomas Petazzoni]

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 directory support, those dependencies
must be copied to the per-package directory of the current package
prior to doing the config preparation. This commit implements this
logic by adding a call to prepare-per-package-directory at the right
spot.

Reported-by: Andreas Naumann <anaumann@ultratronik.de>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/pkg-kconfig.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index d6c95b897e..bf43632adc 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -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

[Buildroot] [PATCH next v6 03/10] Makefile: rework main directory creation logic

[Yann E. MORIN]

Thomas, All,

On 2018-11-23 15:58 +0100, Thomas Petazzoni spake thusly:
> In the current code, the creation of the main output directories
> (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.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Since I don't have strong arguments to make for the suggestions in my
previous review, after your reply, that's OK for me:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Thanks! :-)

Regards,
Yann E. MORIN.

> ---
>  Makefile               | 21 +++++++++------------
>  fs/common.mk           |  1 +
>  package/pkg-generic.mk |  7 ++++++-
>  3 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 2819d44124..e675ac26aa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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 \
> diff --git a/fs/common.mk b/fs/common.mk
> index 2a5a202a89..96658428ba 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -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)
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f34f46afc8..309fd8cd48 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
> 

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

[Buildroot] [PATCH next v6 06/10] package/pkg-generic: adjust config scripts tweaks for per-package directories

[Yann E. MORIN]

Thomas, All,

On 2018-11-23 15:58 +0100, Thomas Petazzoni spake thusly:
> 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 directories.
[--SNIP--]
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

This reviewed tag does stand after your explanations on your previous
iteration. Thanks! :-)

Regards,
Yann E. MORIN.

> ---
>  package/pkg-generic.mk | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 309fd8cd48..9b4db845b6 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
> 

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

[Buildroot] [PATCH next v6 08/10] Makefile: allow top-level parallel build with BR2_PER_PACKAGE_DIRECTORIES=y

[Yann E. MORIN]

Thomas, All,

On 2018-11-23 15:58 +0100, Thomas Petazzoni spake thusly:
> With per-package folder support, top-level parallel build becomes
> safe, so we can enclose the .NOTPARALLEL statement in a
> !BR2_PER_PACKAGE_DIRECTORIES condition.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 35fe1b3644..7bd7291cfd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -231,6 +231,7 @@ ifeq ($(filter $(noconfig_targets),$(MAKECMDGOALS)),)
>  -include $(BR2_CONFIG)
>  endif
>  
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),)
>  # 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
> @@ -246,6 +247,7 @@ endif
>  # use the -j<jobs> option when building, e.g:
>  #      make -j$((`getconf _NPROCESSORS_ONLN`+1))

The biggish comment is now completely wrong. The correct solution is not
to remove the line, but to enable the BR2_PER_PACKAGE_DIRECTORIES option.

Regards,
Yann E. MORIN.

>  .NOTPARALLEL:
> +endif
>  
>  ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
>  TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/target,$(BASE_TARGET_DIR)))
> -- 
> 2.19.1
> 

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Yann E. MORIN]

Thomas, All,

On 2018-11-23 15:58 +0100, Thomas Petazzoni spake thusly:
> This commit implements 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 directories, that only contain
> their explicit dependencies.
[--SNIP--]
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
[--SNIP--]
> diff --git a/Makefile b/Makefile
> index 23032988a5..35fe1b3644 100644
> --- a/Makefile
> +++ b/Makefile
[--SNIP--]
> @@ -701,14 +712,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_DIRECTORIES),y)
> +	@$(call MESSAGE,"Creating global host directory")
> +	$(foreach pkg,$(sort $(PACKAGES)),\
> +		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> +		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> +		$(HOST_DIR)$(sep))
> +endif

Up until now, host-finalize had no command, so it was de facto a phony
target. But with this patch, it gains commands in its recipe.o

So it now has to be explicitly marke .PHONY, if at least for consistency
with target-finalize and staging-finalize.

Also, as you noticed on IRC, and so you don't forget: foo-dirclean
should also remove the PPD for that package, while foo-reconfigure
should not remove it.

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

[Buildroot] [PATCH next v6 09/10] package/pkg-generic: make libtool .la files compatible with per-package directories

[Yann E. MORIN]

Thomas, All,

On 2018-11-23 15:58 +0100, Thomas Petazzoni spake thusly:
> Libtool .la files unfortunately contain a number of absolute paths,
> which now refer to per-package directories. Due to this, when building
> package A, .la files may contain absolute paths referring to
> directories 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 directory.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Since I really want this topic to come to a conclusion sooner than
later, and since the solution is good-enough for now:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Howerver, see below...

> ---
>  package/pkg-generic.mk | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index b1f0cdf34a..17909f4a61 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -222,6 +222,12 @@ $(BUILD_DIR)/%/.stamp_configured:
>  	@$(call step_start,configure)
>  	@$(call MESSAGE,"Configuring")
>  	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> +	# Make sure .la files only reference the current per-package
> +	# directory
> +	$(Q)find $(STAGING_DIR)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
> +		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]*/:$(PER_PACKAGE_DIR)/$(NAME)/:g"
> +endif

I don't find it very clean that we shoe-horn such preparation in the
generic rules; I'd rather we use a hook mechanism, which we can define
for the prepare-per-package-directory macro to call.

For example:

    define PER_PKG_CONFIGURE_FIXUP_LA_FILES
        Blabla your big find command
    endef
    PER_PACKAGE_CONFIGURE_PREPARE_HOOKS += PER_PKG_CONFIGURE_FIXUP_LA_FILES

And then:

    # $(1): step to prepare for
    # $(2): space-separated list of dependencies to grab
    define prepare-per-package-directory
        mkdir -p $(HOST_DIR) $(TARGET_DIR)
        $(foreach pkg,$(2),\
            rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
                $(PER_PACKAGE_DIR)/$(pkg)/host/ \
                $(HOST_DIR)$(sep))
        $(foreach pkg,$(2),\
            rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
            $(PER_PACKAGE_DIR)/$(pkg)/target/ \
            $(TARGET_DIR)$(sep))
        $(foreach hook,$(PER_PACKAGE_$(call UPPER,$(1))_PREPARE_HOOKS),$(call $(hook))$(sep))
    endef

And then you'd just have to call prepare-per-package-directory with one
of:

    $(call prepare-per-package-directory,source,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES))
    $(call prepare-per-package-directory,extract,$($(PKG)_FINAL_EXTRACT_DEPENDENCIES))
    $(call prepare-per-package-directory,patch,$($(PKG)_FINAL_PATCH_DEPENDENCIES))
    $(call prepare-per-package-directory,configure,$($(PKG)_FINAL_DEPENDENCIES))

>  	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))

ternatively, we could change this line to be able to run hooks before
the package-registered hooks:

    $(foreach hook,\
        $($(PKG)_EARLY_PRE_CONFIGURE_HOOKS) $($(PKG)_PRE_CONFIGURE_HOOKS),\
        $(call $(hook))$(sep))

and earlier:

    $(2)_EARLY_PRE_CONFIGURE_HOOKS += \
        PER_PKG_PREPARE_DIRECTORY_FOR_CONFIGURE \
        PER_PKG_CONFIGURE_FIXUP_LA_FILES

And so on for each steps, no?

But again: I'm pretty much OK with the patch as is, because it is better
than nothing, and it would be awesome to have that in early December, so
it is part of the next LTS.

(Unrelated note: why do we $(call $(hook)) instead of just $($(hook)) ?)

Regards,
Yann E. MORIN.

>  	$($(PKG)_CONFIGURE_CMDS)
>  	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
> @@ -875,6 +881,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
> 

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

[Buildroot] [PATCH next v6 10/10] package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package directories

[Yann E. MORIN]

Thomas, All,

On 2018-11-23 15:58 +0100, Thomas Petazzoni spake thusly:
> 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 directory support, those dependencies
> must be copied to the per-package directory of the current package
> prior to doing the config preparation. This commit implements this
> logic by adding a call to prepare-per-package-directory at the right
> spot.
> 
> Reported-by: Andreas Naumann <anaumann@ultratronik.de>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  package/pkg-kconfig.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index d6c95b897e..bf43632adc 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -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
> 

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

[Buildroot] [PATCH next v6 01/10] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

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

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Committed to next, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH next v6 02/10] support/scripts/check-host-rpath: split condition on two statements

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > 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

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Committed to next, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH next v6 03/10] Makefile: rework main directory creation logic

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > In the current code, the creation of the main output directories
 > (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.

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Committed to next, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH next v6 04/10] Makefile: move .NOTPARALLEL statement after including .config file

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

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

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Committed to next, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH next v6 05/10] Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

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

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Committed to next, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH next v6 06/10] package/pkg-generic: adjust config scripts tweaks for per-package directories

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

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

 > 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 directory will return paths from package A
 >    per-package directory:

 >    $ ./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 directory, it
 >    returns a -L flag that points to the libmcrypt per-package
 >    directory.

 >    One might say: but this is OK, since the sysroot of the libmcrypt
 >    per-package directory 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 directory. 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
 >    directory, but we found "libc.so.6" in the "libmcrypt" per-package
 >    directory.

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

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Committed to next, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH next v6 09/10] package/pkg-generic: make libtool .la files compatible with per-package directories

[Thomas Petazzoni]

Hello,

On Sun, 25 Nov 2018 18:57:02 +0100, Yann E. MORIN wrote:

> Since I really want this topic to come to a conclusion sooner than
> later, and since the solution is good-enough for now:
> 
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Thanks. However, since I have changed a bit this patch for v7, I will
not include your Reviewed-by.

> I don't find it very clean that we shoe-horn such preparation in the
> generic rules; I'd rather we use a hook mechanism, which we can define
> for the prepare-per-package-directory macro to call.

I understand your proposal. However, I think that it's premature to
introduce such a hook mechanism: for now there's only one such fixup to
do. Let's wait until we have a few.

I have moved the code outside of the package step, into a helper
function. It looks a bit nicer, and perhaps if we need to do other
fixups this helper function can be extended to be the one doing all the
per-package related fixups.

What do you think ?

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

[Buildroot] [PATCH next v6 09/10] package/pkg-generic: make libtool .la files compatible with per-package directories

[Yann E. MORIN]

Thomas, All,

On 2018-11-30 11:20 +0100, Thomas Petazzoni spake thusly:
> On Sun, 25 Nov 2018 18:57:02 +0100, Yann E. MORIN wrote:
> > Since I really want this topic to come to a conclusion sooner than
> > later, and since the solution is good-enough for now:
> > 
> > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Thanks. However, since I have changed a bit this patch for v7, I will
> not include your Reviewed-by.
> 
> > I don't find it very clean that we shoe-horn such preparation in the
> > generic rules; I'd rather we use a hook mechanism, which we can define
> > for the prepare-per-package-directory macro to call.
> 
> I understand your proposal. However, I think that it's premature to
> introduce such a hook mechanism: for now there's only one such fixup to
> do. Let's wait until we have a few.
> 
> I have moved the code outside of the package step, into a helper
> function. It looks a bit nicer, and perhaps if we need to do other
> fixups this helper function can be extended to be the one doing all the
> per-package related fixups.
> 
> What do you think ?

If you were speaking of:
    https://git.bootlin.com/users/thomas-petazzoni/buildroot/commit/?h=ppsh-v7-work&id=5fd4ce2bbed14478d49afcd775f37eb8c4e487a4

... then you can keep my reviewed-by tag when you submit that new
iteration.

Thanks! :-)

Regards,
Yann E. MORIN.

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

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Arnout Vandecappelle]

 Hi Thomas,

 A few random remarks.

On 23/11/2018 15:58, Thomas Petazzoni wrote:
> This commit implements 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 directories, 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/ directory is created, which will contain
>    one sub-directory per package, and inside it, a "host" directory
>    and a "target" directory:
> 
>    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/ directory 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 directories. To do
>    so, we simply rsync (using hard links to save space and time) the
>    host and target directories of the direct dependencies of the
>    package to the current package host and target directories.
> 
>    We only need to take care of direct dependencies (and not
>    recursively all dependencies), because we accumulate into those
>    per-package host and target directories 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.

 So that means that if BR2_PER_PACKAGE_DIRECTORIES=y, check-uniq-files should
give errors instead of warnings, no?

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

 Great that this is explained in the commit log.

> 
> Also, with this change to per-package directories, binaries built for
> the host may contain RPATH pointing to per-package directories. This
> commit therefore teaches the check-host-rpath script with the fact
> that RPATH pointing to a per-package directory are OK.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  Config.in                        | 18 ++++++++++++++++
>  Makefile                         | 36 +++++++++++++++++++++++++++-----
>  package/pkg-generic.mk           |  5 ++++-
>  package/pkg-utils.mk             | 21 +++++++++++++++++++
>  support/scripts/check-host-rpath |  5 ++++-
>  5 files changed, 78 insertions(+), 7 deletions(-)
> 
> diff --git a/Config.in b/Config.in
> index f965e9d6d8..2d8a07fda7 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -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_DIRECTORIES
> +	bool "Use per-package directories (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 directories.
> +
>  endmenu
>  
>  comment "Security Hardening Options"
> diff --git a/Makefile b/Makefile
> index 23032988a5..35fe1b3644 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -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

 Perhaps it would be nice to add a preparatory patch that reorders the
definitions of BASE_TARGET_DIR, TARGET_DIR, STAGING_DIR and HOST_DIR so they are
all together, and can be in a single BR2_PER_PACKAGE_DIRECTORIES condition.

> +
>  # 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
> @@ -246,6 +247,12 @@ endif
>  #      make -j$((`getconf _NPROCESSORS_ONLN`+1))
>  .NOTPARALLEL:
>  
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),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

 Was there any reason to move the definition of TARGET_DIR?

> +
>  # timezone and locale may affect build output
>  ifeq ($(BR2_REPRODUCIBLE),y)
>  export TZ = UTC
> @@ -455,7 +462,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_DIRECTORIES),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 +712,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_DIRECTORIES),y)	
> +	@$(call MESSAGE,"Creating global host directory")
> +	$(foreach pkg,$(sort $(PACKAGES)),\
> +		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> +		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> +		$(HOST_DIR)$(sep))

 Don't you think it's worth factoring this into a pkg-utils macro? I'm thinking:

# rsync-per-package(packages,type,destdir)
# copy every per-package host or target dir (as determined by type) for packages
# (a list of packages) into destdir.
# copying is done by making hardlinks using rsync
define rsync-per-package
	$(foreach pkg,$(sort $(1)),\
		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
		$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
		$(3)$(sep))
endef

rsync-per-package-host = $(call rsync-per-package,$(1),host,$(HOST_DIR))
rsync-per-package-target = $(call rsync-per-package,$(1),target,$(TARGET_DIR))

The latter two are maybe not needed.

> +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_DIRECTORIES),y)
> +	@$(call MESSAGE,"Creating global target directory")
> +	$(foreach pkg,$(sort $(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

 Side note: if we make check-uniq-files return errors, we should probably also
move it to a per package step so the autobuilders can identify the failing
package. But that can be done later.

[snip]
> diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> index 6c5767da05..787a1763b0 100755
> --- a/support/scripts/check-host-rpath
> +++ b/support/scripts/check-host-rpath
> @@ -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() {

 I think there should be some explanation why elf_needs_rpath doesn't need to
check all per-package directories. Basically it's because any library that the
executable may depend on must already have been rsynced into ${host_dir} which
is the per-package host directory.

 Regards,
 Arnout


>  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' \
> 

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello Arnout,

Thanks for the review.

On Tue, 4 Dec 2018 23:24:58 +0100, Arnout Vandecappelle wrote:

> >    We only need to take care of direct dependencies (and not
> >    recursively all dependencies), because we accumulate into those
> >    per-package host and target directories 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.  
> 
>  So that means that if BR2_PER_PACKAGE_DIRECTORIES=y, check-uniq-files should
> give errors instead of warnings, no?

Possibly yes. But even as of today, check-uniq-files shows warnings for
legitimate things that have been overwritten, such as .la files. So
even if having check-uniq-files error out if files are overwritten is a
desirable long-term goal, I'm not sure it's going to be reasonable to
achieve this goal as part of this per-package SDK/target series.

> >  comment "Security Hardening Options"
> > diff --git a/Makefile b/Makefile
> > index 23032988a5..35fe1b3644 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -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  
> 
>  Perhaps it would be nice to add a preparatory patch that reorders the
> definitions of BASE_TARGET_DIR, TARGET_DIR, STAGING_DIR and HOST_DIR so they are
> all together, and can be in a single BR2_PER_PACKAGE_DIRECTORIES condition.

I'll have a look at what can I do, but the ordering of those variables
is a bit messy/complicated.

> >  # 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
> > @@ -246,6 +247,12 @@ endif
> >  #      make -j$((`getconf _NPROCESSORS_ONLN`+1))
> >  .NOTPARALLEL:
> >  
> > +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),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  
> 
>  Was there any reason to move the definition of TARGET_DIR?

Yes: TARGET_DIR was defined prior to the .config file being included.
Now that its definition depends on BR2_PER_PACKAGE_DIRECTORIES, we need
to move the TARGET_DIR definition after the .config file is included.

> > -host-finalize: $(HOST_DIR_SYMLINK)
> > +host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
> > +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)	
> > +	@$(call MESSAGE,"Creating global host directory")
> > +	$(foreach pkg,$(sort $(PACKAGES)),\
> > +		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> > +		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> > +		$(HOST_DIR)$(sep))  
> 
>  Don't you think it's worth factoring this into a pkg-utils macro? I'm thinking:
> 
> # rsync-per-package(packages,type,destdir)
> # copy every per-package host or target dir (as determined by type) for packages
> # (a list of packages) into destdir.
> # copying is done by making hardlinks using rsync
> define rsync-per-package
> 	$(foreach pkg,$(sort $(1)),\
> 		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
> 		$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
> 		$(3)$(sep))
> endef

Perhaps I could re-use the prepare-per-package-directory macro already
in package/pkg-utils.mk, which is currently defined as:

ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
define prepare-per-package-directory
        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

This macro is for now used to prepare the per-package directories during
the build, but it could also be used to prepare the global directories
at the end of the build. The only thing that we need is to separate it
into two macros that do the host and target separately, since for the
global directories, these are done in host-finalize and target-finalize
respectively.

> >  	@$(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  
> 
>  Side note: if we make check-uniq-files return errors, we should probably also
> move it to a per package step so the autobuilders can identify the failing
> package. But that can be done later.

Yes, let's not do everything in this series please :)

> >      # 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() {  
> 
>  I think there should be some explanation why elf_needs_rpath doesn't need to
> check all per-package directories. Basically it's because any library that the
> executable may depend on must already have been rsynced into ${host_dir} which
> is the per-package host directory.

It seems pretty obvious to me, no? I mean the per-package host
directory contains all the stuff installed by the dependencies of the
package, so it obviously contains all the necessary libraries.

But fair enough: where do you want to see this explanation ? In the
commit log ? As a comment in the script itself ?

Once again, thanks for the review!

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Arnout Vandecappelle]

On 05/12/2018 09:04, Thomas Petazzoni wrote:
> Hello Arnout,
> 
> Thanks for the review.
> 
> On Tue, 4 Dec 2018 23:24:58 +0100, Arnout Vandecappelle wrote:
> 
>>>    We only need to take care of direct dependencies (and not
>>>    recursively all dependencies), because we accumulate into those
>>>    per-package host and target directories 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.  
>>
>>  So that means that if BR2_PER_PACKAGE_DIRECTORIES=y, check-uniq-files should
>> give errors instead of warnings, no?
> 
> Possibly yes. But even as of today, check-uniq-files shows warnings for
> legitimate things that have been overwritten, such as .la files. So
> even if having check-uniq-files error out if files are overwritten is a
> desirable long-term goal, I'm not sure it's going to be reasonable to
> achieve this goal as part of this per-package SDK/target series.

 True. But on the other hand, the idea of this experimental, user-configurable
feature is that we get some time to debug it. That means we have to detect bugs,
so we need to do this.

 So, mark it as a side note as one of those things that still have to be done.

> 
>>>  comment "Security Hardening Options"
>>> diff --git a/Makefile b/Makefile
>>> index 23032988a5..35fe1b3644 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -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  
>>
>>  Perhaps it would be nice to add a preparatory patch that reorders the
>> definitions of BASE_TARGET_DIR, TARGET_DIR, STAGING_DIR and HOST_DIR so they are
>> all together, and can be in a single BR2_PER_PACKAGE_DIRECTORIES condition.
> 
> I'll have a look at what can I do, but the ordering of those variables
> is a bit messy/complicated.

 I think the only ordering constraint before this patch is HOST_DIR, which gets
defined twice (once before and once after including .config). The other ones
don't have any complication, do they?


>>>  # 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
>>> @@ -246,6 +247,12 @@ endif
>>>  #      make -j$((`getconf _NPROCESSORS_ONLN`+1))
>>>  .NOTPARALLEL:
>>>  
>>> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),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  
>>
>>  Was there any reason to move the definition of TARGET_DIR?
> 
> Yes: TARGET_DIR was defined prior to the .config file being included.
> Now that its definition depends on BR2_PER_PACKAGE_DIRECTORIES, we need
> to move the TARGET_DIR definition after the .config file is included.

 All the more reason to move it in a separate patch :-)

 I think the place where now HOST_DIR is defined a second time would be a good
location for doing this. TARGET_DIR should not be used anywhere outside of the
BR2_HAVE_DOT_CONFIG condition (to be double-checked) - it should be
BASE_TARGET_DIR instead, like is the case in the clean target.


>>> -host-finalize: $(HOST_DIR_SYMLINK)
>>> +host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
>>> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)	
>>> +	@$(call MESSAGE,"Creating global host directory")
>>> +	$(foreach pkg,$(sort $(PACKAGES)),\
>>> +		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
>>> +		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
>>> +		$(HOST_DIR)$(sep))  
>>
>>  Don't you think it's worth factoring this into a pkg-utils macro? I'm thinking:
>>
>> # rsync-per-package(packages,type,destdir)
>> # copy every per-package host or target dir (as determined by type) for packages
>> # (a list of packages) into destdir.
>> # copying is done by making hardlinks using rsync
>> define rsync-per-package
>> 	$(foreach pkg,$(sort $(1)),\
>> 		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
>> 		$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
>> 		$(3)$(sep))
>> endef
> 
> Perhaps I could re-use the prepare-per-package-directory macro already
> in package/pkg-utils.mk, which is currently defined as:
> 
> ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> define prepare-per-package-directory
>         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
> 
> This macro is for now used to prepare the per-package directories during
> the build, but it could also be used to prepare the global directories
> at the end of the build. The only thing that we need is to separate it
> into two macros that do the host and target separately, since for the
> global directories, these are done in host-finalize and target-finalize
> respectively.

 Well, yes, that's the same indeed... But you'll still want the
prepare-per-package-directory macro, I think, otherwise you'd have to call the
inner macro twice everywhere (admittedly, "everywhere" is just in 2 places, or 3
if we add it to kconfig as well).

> 
>>>  	@$(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  
>>
>>  Side note: if we make check-uniq-files return errors, we should probably also
>> move it to a per package step so the autobuilders can identify the failing
>> package. But that can be done later.
> 
> Yes, let's not do everything in this series please :)

 Absolutely, but I guess you have a todo list somewhere, no?

> 
>>>      # 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() {  
>>
>>  I think there should be some explanation why elf_needs_rpath doesn't need to
>> check all per-package directories. Basically it's because any library that the
>> executable may depend on must already have been rsynced into ${host_dir} which
>> is the per-package host directory.
> 
> It seems pretty obvious to me, no? 

 I admit it was pretty late already, but still, it took me more than 10 minutes
to realize that.

> I mean the per-package host directory

 That's the first problem already: it's hard to realize that HOST_DIR/host_dir
really is the per-package host directory. But I guess that that's a matter of
being used to it.

 That reminds me: perhaps a paragraph should be added to
docs/manual/adding-packages-generic.txt to explain that HOST_DIR, STAGING_DIR
and TARGET_DIR point to the per-package directory when used within a package.

> contains all the stuff installed by the dependencies of the
> package, so it obviously contains all the necessary libraries.

 That's the second reason why it's not so obvious. When executable A links with
library B, it will typically have an rpath to the per-package directory of B.
It's not directly obvious that you can just as well check for the presence of
the library in the per-package of A to know that it needs an rpath to the
per-package of B.

> 
> But fair enough: where do you want to see this explanation ? In the
> commit log ? As a comment in the script itself ?

 I think in the script itself. It could use a little bit more explanation in its
overall comment at the beginning of the file even without this patch.

 Regards,
 Arnout


> 
> Once again, thanks for the review!
> 
> Thomas
> 

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello,

On Wed, 5 Dec 2018 10:18:07 +0100, Arnout Vandecappelle wrote:

> > Possibly yes. But even as of today, check-uniq-files shows warnings for
> > legitimate things that have been overwritten, such as .la files. So
> > even if having check-uniq-files error out if files are overwritten is a
> > desirable long-term goal, I'm not sure it's going to be reasonable to
> > achieve this goal as part of this per-package SDK/target series.  
> 
>  True. But on the other hand, the idea of this experimental, user-configurable
> feature is that we get some time to debug it. That means we have to detect bugs,
> so we need to do this.

Agreed. But at this point, I'm already trying to fix all the
per-package directory issues, and they are still a number of them.

>  So, mark it as a side note as one of those things that still have to be done.

Right.

> >>  Perhaps it would be nice to add a preparatory patch that reorders the
> >> definitions of BASE_TARGET_DIR, TARGET_DIR, STAGING_DIR and HOST_DIR so they are
> >> all together, and can be in a single BR2_PER_PACKAGE_DIRECTORIES condition.  
> > 
> > I'll have a look at what can I do, but the ordering of those variables
> > is a bit messy/complicated.  
> 
>  I think the only ordering constraint before this patch is HOST_DIR, which gets
> defined twice (once before and once after including .config). The other ones
> don't have any complication, do they?

I haven't looked in details, perhaps it's not that messy, I'll have a
look.

> > Yes: TARGET_DIR was defined prior to the .config file being included.
> > Now that its definition depends on BR2_PER_PACKAGE_DIRECTORIES, we need
> > to move the TARGET_DIR definition after the .config file is included.  
> 
>  All the more reason to move it in a separate patch :-)
> 
>  I think the place where now HOST_DIR is defined a second time would be a good
> location for doing this. TARGET_DIR should not be used anywhere outside of the
> BR2_HAVE_DOT_CONFIG condition (to be double-checked) - it should be
> BASE_TARGET_DIR instead, like is the case in the clean target.

Right. I'll try to cook a preparatory patch.

> >>> -host-finalize: $(HOST_DIR_SYMLINK)
> >>> +host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
> >>> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)	
> >>> +	@$(call MESSAGE,"Creating global host directory")
> >>> +	$(foreach pkg,$(sort $(PACKAGES)),\
> >>> +		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> >>> +		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> >>> +		$(HOST_DIR)$(sep))    
> >>
> >>  Don't you think it's worth factoring this into a pkg-utils macro? I'm thinking:
> >>
> >> # rsync-per-package(packages,type,destdir)
> >> # copy every per-package host or target dir (as determined by type) for packages
> >> # (a list of packages) into destdir.
> >> # copying is done by making hardlinks using rsync
> >> define rsync-per-package
> >> 	$(foreach pkg,$(sort $(1)),\
> >> 		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
> >> 		$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
> >> 		$(3)$(sep))
> >> endef  
> > 
> > Perhaps I could re-use the prepare-per-package-directory macro already
> > in package/pkg-utils.mk, which is currently defined as:
> > 
> > ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> > define prepare-per-package-directory
> >         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
> > 
> > This macro is for now used to prepare the per-package directories during
> > the build, but it could also be used to prepare the global directories
> > at the end of the build. The only thing that we need is to separate it
> > into two macros that do the host and target separately, since for the
> > global directories, these are done in host-finalize and target-finalize
> > respectively.  
> 
>  Well, yes, that's the same indeed... But you'll still want the
> prepare-per-package-directory macro, I think, otherwise you'd have to call the
> inner macro twice everywhere (admittedly, "everywhere" is just in 2 places, or 3
> if we add it to kconfig as well).

I can always have an intermediate macro that calls this macro twice.

> >>  Side note: if we make check-uniq-files return errors, we should probably also
> >> move it to a per package step so the autobuilders can identify the failing
> >> package. But that can be done later.  
> > 
> > Yes, let's not do everything in this series please :)  
> 
>  Absolutely, but I guess you have a todo list somewhere, no?

You probably over-estimate my level of organization and sanity :-)

Such a todo list would be some massive that it would discouraging to
look at, and I solve this problem by not having a todo list!

> >>  I think there should be some explanation why elf_needs_rpath doesn't need to
> >> check all per-package directories. Basically it's because any library that the
> >> executable may depend on must already have been rsynced into ${host_dir} which
> >> is the per-package host directory.  
> > 
> > It seems pretty obvious to me, no?   
> 
>  I admit it was pretty late already, but still, it took me more than 10 minutes
> to realize that.
> 
> > I mean the per-package host directory  
> 
>  That's the first problem already: it's hard to realize that HOST_DIR/host_dir
> really is the per-package host directory. But I guess that that's a matter of
> being used to it.
> 
>  That reminds me: perhaps a paragraph should be added to
> docs/manual/adding-packages-generic.txt to explain that HOST_DIR, STAGING_DIR
> and TARGET_DIR point to the per-package directory when used within a package.

OK. But I'll have to say "if BR2_PER_PACKAGE_DIRECTORIES" is enabled :-)

> > contains all the stuff installed by the dependencies of the
> > package, so it obviously contains all the necessary libraries.  
> 
>  That's the second reason why it's not so obvious. When executable A links with
> library B, it will typically have an rpath to the per-package directory of B.

Yes, because we decided to not rewrite the rpath right at the end of
the installation of each package (this was done in the earlier versions
of this series, which I posted end of 2017).

> It's not directly obvious that you can just as well check for the presence of
> the library in the per-package of A to know that it needs an rpath to the
> per-package of B.

The check-host-rpath script is not checking for the presence of
libraries. Nowhere it verifies that the NEEDED fields in host binaries
match libraries found in the RPATH listed in the binaries.

All it does is look at the RPATH in the binaries, and validate that
they have a RPATH pointing to the host directory. So all what my change
is doing is check that it has at least one RPATH that looks like
blablabla/per-package/<somepackage>/host/lib. It doesn't verify at all
the <somepackage> matches the package being built currently or not.

> > But fair enough: where do you want to see this explanation ? In the
> > commit log ? As a comment in the script itself ?  
> 
>  I think in the script itself. It could use a little bit more explanation in its
> overall comment at the beginning of the file even without this patch.

OK.

Thanks,

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Arnout Vandecappelle]

On 05/12/2018 10:44, Thomas Petazzoni wrote:
> The check-host-rpath script is not checking for the presence of
> libraries. 

 Line 48 (before this patch):

	[ -e "${hostdir}/lib/${lib}" ] && return 0

 It *does* check for the presence of libraries, because rpath is only needed
when it uses a library from host_dir. So, the confusing thing is that we check
here that the library is present in per-package/A/host/lib while the rpath is
per-package/B/host/lib. That's what makes it so unintuitive.

 Regards,
 Arnout


> Nowhere it verifies that the NEEDED fields in host binaries
> match libraries found in the RPATH listed in the binaries.

[Buildroot] [PATCH next v6 10/10] package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package directories

[Andreas Naumann]

Hi Thomas,

Am 23.11.18 um 15:58 schrieb Thomas Petazzoni:
> 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 directory support, those dependencies
> must be copied to the per-package directory of the current package
> prior to doing the config preparation. This commit implements this
> logic by adding a call to prepare-per-package-directory at the right
> spot.
> 
> Reported-by: Andreas Naumann <anaumann@ultratronik.de>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>   package/pkg-kconfig.mk | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index d6c95b897e..bf43632adc 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -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) $$(@))
> 

Finally I could test this and found an issue. Because I use ccache, the 
linux/uboot kconfig steps fail, because ccache apparently is not part of 
UBOOT/LINUX_KCONFIG_DEPENDENCIES.
Instead of conditionally adding it, my simple proposal would be to not 
use it, e.g.:

  # $(HOST_DIR)/lib/libncurses.so (which is not).  We don't actually
  # need any host-package for kconfig, so remove the HOSTCC/HOSTLDFLAGS
  # override again.
-UBOOT_KCONFIG_OPTS = $(UBOOT_MAKE_OPTS) HOSTCC="$(HOSTCC)" HOSTLDFLAGS=""
+UBOOT_KCONFIG_OPTS = $(UBOOT_MAKE_OPTS) HOSTCC="$(HOSTCC_NOCCACHE)" 
HOSTLDFLAGS=""

So after this, I'm not sure if there really was a need for above fix 
since the fixup/.config creation may not even need the only kconfig 
dependencies set so far: host-flex and host-bison..


regards,
Andreas

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Andreas Naumann]

Hi Thomas,

Am 23.11.18 um 15:58 schrieb Thomas Petazzoni:
> This commit implements the core of the move to per-package SDK and
<SNIP>
>   
> -host-finalize: $(HOST_DIR_SYMLINK)
> +host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> +	@$(call MESSAGE,"Creating global host directory")
> +	$(foreach pkg,$(sort $(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_DIRECTORIES),y)
> +	@$(call MESSAGE,"Creating global target directory")
> +	$(foreach pkg,$(sort $(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

In an answer to v4 of this series I mentioned the issue I had with empty 
RPATH in some host-lib. After digging some more I found this happens 
during the prepare-sdk step (which I call in my wrapper script). Before 
this step, your suggested readelf output reports the correct pathes, 
however after, RPATH is empty. I compared this to a "standard" SDK, 
there RPATH is set to $ORIGIN/../lib or something similar.

Actually, an unrelated question arises: Why does the SDK need to build 
world, which includes all filesystem images, first? Should it not be 
sufficient to depend on $(PACKAGES) or now host-finalize only?


regards,
Andreas


> @@ -972,7 +997,8 @@ 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 +1037,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
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 9b4db845b6..b1f0cdf34a 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
>   
> @@ -133,6 +133,7 @@ endif
>   # Retrieve the archive
>   $(BUILD_DIR)/%/.stamp_downloaded:
>   	@$(call step_start,download)
> +	$(call prepare-per-package-directory,$($(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 +160,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
>   $(BUILD_DIR)/%/.stamp_extracted:
>   	@$(call step_start,extract)
>   	@$(call MESSAGE,"Extracting")
> +	$(call prepare-per-package-directory,$($(PKG)_FINAL_EXTRACT_DEPENDENCIES))
>   	$(foreach hook,$($(PKG)_PRE_EXTRACT_HOOKS),$(call $(hook))$(sep))
>   	$(Q)mkdir -p $(@D)
>   	$($(PKG)_EXTRACT_CMDS)
> @@ -219,6 +221,7 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
>   $(BUILD_DIR)/%/.stamp_configured:
>   	@$(call step_start,configure)
>   	@$(call MESSAGE,"Configuring")
> +	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
>   	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
>   	$($(PKG)_CONFIGURE_CMDS)
>   	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index bffd79dfb0..4ca2eebc95 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -62,6 +62,27 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
>   endif
>   endef
>   
> +# This function prepares the per-package HOST_DIR and TARGET_DIR of
> +# the current package, by rsync the host and target directories of the
> +# dependencies of this package. The list of dependencies is passed as
> +# argument, so that this function can be used to prepare with
> +# different set of dependencies (download, extract, configure, etc.)
> +#
> +# $1: space-separated list of dependencies
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> +define prepare-per-package-directory
> +	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
> +
>   #
>   # legal-info helper functions
>   #
> diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> index 6c5767da05..787a1763b0 100755
> --- a/support/scripts/check-host-rpath
> +++ b/support/scripts/check-host-rpath
> @@ -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' \
> 

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello Andreas,

On Wed, 5 Dec 2018 17:08:20 +0100, Andreas Naumann wrote:

> In an answer to v4 of this series I mentioned the issue I had with empty 
> RPATH in some host-lib. After digging some more I found this happens 
> during the prepare-sdk step (which I call in my wrapper script). Before 
> this step, your suggested readelf output reports the correct pathes, 
> however after, RPATH is empty. I compared this to a "standard" SDK, 
> there RPATH is set to $ORIGIN/../lib or something similar.

So are you saying the issue still exists, and to reproduce, I simply
need to do:

 $ make
 $ make prepare-sdk

of a build with BR2_PER_PACKAGE_DIRECTORIES=y ?

> Actually, an unrelated question arises: Why does the SDK need to build 
> world, which includes all filesystem images, first? Should it not be 
> sufficient to depend on $(PACKAGES) or now host-finalize only?

Yes, it would be sufficient to build packages + host-finalization I
believe.

Best regards,

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Andreas Naumann]

Hi,

Am 05.12.18 um 17:31 schrieb Thomas Petazzoni:
> Hello Andreas,
> 
> On Wed, 5 Dec 2018 17:08:20 +0100, Andreas Naumann wrote:
> 
>> In an answer to v4 of this series I mentioned the issue I had with empty
>> RPATH in some host-lib. After digging some more I found this happens
>> during the prepare-sdk step (which I call in my wrapper script). Before
>> this step, your suggested readelf output reports the correct pathes,
>> however after, RPATH is empty. I compared this to a "standard" SDK,
>> there RPATH is set to $ORIGIN/../lib or something similar.
> 
> So are you saying the issue still exists, and to reproduce, I simply
> need to do:
> 
>   $ make
>   $ make prepare-sdk
> 
> of a build with BR2_PER_PACKAGE_DIRECTORIES=y ?

in an essence that's what I did. My setup has two externals and some 
custom defconfig, but that shouldnt matter since without the ppsh 
patches things work fine.

> 
>> Actually, an unrelated question arises: Why does the SDK need to build
>> world, which includes all filesystem images, first? Should it not be
>> sufficient to depend on $(PACKAGES) or now host-finalize only?
> 
> Yes, it would be sufficient to build packages + host-finalization I
> believe.

Ok, I might submit a patch in this direction then..


regards,
Andreas

> 
> Best regards,
> 
> Thomas
> 

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Andreas Naumann]

Hi Thomas,

Am 05.12.18 um 17:52 schrieb Andreas Naumann:
> Hi,
> 
> Am 05.12.18 um 17:31 schrieb Thomas Petazzoni:
>> Hello Andreas,
>>
>> On Wed, 5 Dec 2018 17:08:20 +0100, Andreas Naumann wrote:
>>
>>> In an answer to v4 of this series I mentioned the issue I had with empty
>>> RPATH in some host-lib. After digging some more I found this happens
>>> during the prepare-sdk step (which I call in my wrapper script). Before
>>> this step, your suggested readelf output reports the correct pathes,
>>> however after, RPATH is empty. I compared this to a "standard" SDK,
>>> there RPATH is set to $ORIGIN/../lib or something similar.
>>
>> So are you saying the issue still exists, and to reproduce, I simply
>> need to do:
>>
>> ? $ make
>> ? $ make prepare-sdk
>>
>> of a build with BR2_PER_PACKAGE_DIRECTORIES=y ?
> 
> in an essence that's what I did. My setup has two externals and some 
> custom defconfig, but that shouldnt matter since without the ppsh 
> patches things work fine.

Just to make sure I just did the above using your ppsh-v6 branch with 
the wandboard_defconfig. The only additional change was to use the 
external linaro toolchain. After 'make'
   $ readelf -d output/host/sbin/mkfs.ext3
shows
   0x000000000000001d (RUNPATH)            Bibliothek runpath: 
[/local/gsrc/buildroot.upstream/output/per-package/host-e2fsprogs/host/lib]

which is already unexpected, shouldn't it show .../output/host/lib?
After 'make prepare-sdk'
   $ readelf -d output/host/sbin/mkfs.ext3
shows
   0x000000000000001d (RUNPATH)            Bibliothek runpath: []


regards,
Andreas

> 
>>
>>> Actually, an unrelated question arises: Why does the SDK need to build
>>> world, which includes all filesystem images, first? Should it not be
>>> sufficient to depend on $(PACKAGES) or now host-finalize only?
>>
>> Yes, it would be sufficient to build packages + host-finalization I
>> believe.
> 
> Ok, I might submit a patch in this direction then..
> 
> 
> regards,
> Andreas
> 
>>
>> Best regards,
>>
>> Thomas
>>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello,

On Thu, 6 Dec 2018 11:21:25 +0100, Andreas Naumann wrote:

> Just to make sure I just did the above using your ppsh-v6 branch with 
> the wandboard_defconfig. The only additional change was to use the 
> external linaro toolchain. After 'make'

Thanks!

>    $ readelf -d output/host/sbin/mkfs.ext3
> shows
>    0x000000000000001d (RUNPATH)            Bibliothek runpath: 
> [/local/gsrc/buildroot.upstream/output/per-package/host-e2fsprogs/host/lib]
> 
> which is already unexpected, shouldn't it show .../output/host/lib?

Nope, this is expected. During the build output/host/lib doesn't exist,
it's only populated at the very end of the build in host-finalize. So
indeed the RPATH of binaries point to various per-package host/lib
folders. This is totally expected.

> After 'make prepare-sdk'
>    $ readelf -d output/host/sbin/mkfs.ext3
> shows
>    0x000000000000001d (RUNPATH)            Bibliothek runpath: []

This is obviously not good :-)

Thanks for reporting that, I'll have a look. I guess it's simply the
fixup of RPATH from absolute to relative that goes wrong.

Best regards,

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Andreas Naumann]

Hi,

Am 06.12.18 um 11:28 schrieb Thomas Petazzoni:
> Hello,
> 
> On Thu, 6 Dec 2018 11:21:25 +0100, Andreas Naumann wrote:
> 
>> Just to make sure I just did the above using your ppsh-v6 branch with
>> the wandboard_defconfig. The only additional change was to use the
>> external linaro toolchain. After 'make'
> 
> Thanks!
> 
>>     $ readelf -d output/host/sbin/mkfs.ext3
>> shows
>>     0x000000000000001d (RUNPATH)            Bibliothek runpath:
>> [/local/gsrc/buildroot.upstream/output/per-package/host-e2fsprogs/host/lib]
>>
>> which is already unexpected, shouldn't it show .../output/host/lib?
> 
> Nope, this is expected. During the build output/host/lib doesn't exist,

of course

> it's only populated at the very end of the build in host-finalize. So
> indeed the RPATH of binaries point to various per-package host/lib
> folders. This is totally expected.

Hmm, at this stage the host-finalize step was already run, so now 
output/host does exist.

> 
>> After 'make prepare-sdk'
>>     $ readelf -d output/host/sbin/mkfs.ext3
>> shows
>>     0x000000000000001d (RUNPATH)            Bibliothek runpath: []
> 
> This is obviously not good :-)
> 
> Thanks for reporting that, I'll have a look. I guess it's simply the
> fixup of RPATH from absolute to relative that goes wrong.

Would moving the fix-rpath step from prepare-sdk to host-finalize hurt?


regards,
Andi

> 
> Best regards,
> 
> Thomas
> 

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello Andreas,

On Thu, 6 Dec 2018 11:42:37 +0100, Andreas Naumann wrote:

> > Nope, this is expected. During the build output/host/lib doesn't exist,  
> 
> of course
> 
> > it's only populated at the very end of the build in host-finalize. So
> > indeed the RPATH of binaries point to various per-package host/lib
> > folders. This is totally expected.  
> 
> Hmm, at this stage the host-finalize step was already run, so now 
> output/host does exist.

Yes, but the RPATH of host binaries are not fixed up in host-finalize.
They are fixed up as part of prepare-sdk:

prepare-sdk: world
        @$(call MESSAGE,"Rendering the SDK relocatable")
        $(TOPDIR)/support/scripts/fix-rpath host
        $(TOPDIR)/support/scripts/fix-rpath staging

so it is totally expected that at the end of the build the host
binaries carry those RPATH pointing to per-package host/lib folders.
And it is not a problem: those folders exist, they contain the right
libraries, so it's perfectly fine.

> >> After 'make prepare-sdk'
> >>     $ readelf -d output/host/sbin/mkfs.ext3
> >> shows
> >>     0x000000000000001d (RUNPATH)            Bibliothek runpath: []  
> > 
> > This is obviously not good :-)
> > 
> > Thanks for reporting that, I'll have a look. I guess it's simply the
> > fixup of RPATH from absolute to relative that goes wrong.  
> 
> Would moving the fix-rpath step from prepare-sdk to host-finalize hurt?

It would not fix your problem: this fix-rpath step is currently broken
as it doesn't understand how to rewrite those RPATHs that point to
per-package host/lib.

So there are really two separate things:

 - Fixing the fix-rpath logic so that it works with per-package
   host/lib RPATHs. This is mandatory. After doing that, at the end of
   the build, the RPATH would still point to per-package host/lib, but
   after prepare-sdk you will have again the correct relative RPATH you
   used to have before the per-package stuff.

 - Moving fix-rpath at the end of the build so that instead of those
   weird per-package host/lib RPATH, you have nice relative RPATHs,
   without the need to run prepare-sdk. This is not mandatory at all,
   and would be just for convenience/beauty.

Best regards,

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Andreas Naumann]

Hi Thomas,

Am 06.12.18 um 11:58 schrieb Thomas Petazzoni:
> Hello Andreas,
> 
> On Thu, 6 Dec 2018 11:42:37 +0100, Andreas Naumann wrote:
> 
>>> Nope, this is expected. During the build output/host/lib doesn't exist,
>>
>> of course
>>
>>> it's only populated at the very end of the build in host-finalize. So
>>> indeed the RPATH of binaries point to various per-package host/lib
>>> folders. This is totally expected.
>>
>> Hmm, at this stage the host-finalize step was already run, so now
>> output/host does exist.
> 
> Yes, but the RPATH of host binaries are not fixed up in host-finalize.
> They are fixed up as part of prepare-sdk:
> 
> prepare-sdk: world
>          @$(call MESSAGE,"Rendering the SDK relocatable")
>          $(TOPDIR)/support/scripts/fix-rpath host
>          $(TOPDIR)/support/scripts/fix-rpath staging
> 
> so it is totally expected that at the end of the build the host
> binaries carry those RPATH pointing to per-package host/lib folders.
> And it is not a problem: those folders exist, they contain the right
> libraries, so it's perfectly fine.
> 
>>>> After 'make prepare-sdk'
>>>>      $ readelf -d output/host/sbin/mkfs.ext3
>>>> shows
>>>>      0x000000000000001d (RUNPATH)            Bibliothek runpath: []
>>>
>>> This is obviously not good :-)
>>>
>>> Thanks for reporting that, I'll have a look. I guess it's simply the
>>> fixup of RPATH from absolute to relative that goes wrong.
>>
>> Would moving the fix-rpath step from prepare-sdk to host-finalize hurt?
> 
> It would not fix your problem: this fix-rpath step is currently broken
> as it doesn't understand how to rewrite those RPATHs that point to
> per-package host/lib.

Yes, I didnt mean this as remedy, but as addon to a fix exactly as you 
explained below.

> 
> So there are really two separate things:
> 
>   - Fixing the fix-rpath logic so that it works with per-package
>     host/lib RPATHs. This is mandatory. After doing that, at the end of
>     the build, the RPATH would still point to per-package host/lib, but
>     after prepare-sdk you will have again the correct relative RPATH you
>     used to have before the per-package stuff.
> 
>   - Moving fix-rpath at the end of the build so that instead of those
>     weird per-package host/lib RPATH, you have nice relative RPATHs,
>     without the need to run prepare-sdk. This is not mandatory at all,
>     and would be just for convenience/beauty.

One additional thought: Regardless of when fix-rpath runs, once it ran, 
the content of host, and because of all the hard-links, per-package are 
changed. I understand that in theory this should have no impact (to 
further compiling other packages for example). But in case of 
bugs/corner-cases whatever, it might be more difficult to debug the two 
different states. So I guess this is more an argument against earlier 
execution of fix-rpath.


best regards,
Andreas


> 
> Best regards,
> 
> Thomas
> 

[Buildroot] [PATCH next v6 10/10] package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package directories

[Andreas Naumann]

Hi Thomas,

Am 05.12.18 um 16:23 schrieb Andreas Naumann:
> Hi Thomas,
> 
> Am 23.11.18 um 15:58 schrieb Thomas Petazzoni:
>> 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 directory support, those dependencies
>> must be copied to the per-package directory of the current package
>> prior to doing the config preparation. This commit implements this
>> logic by adding a call to prepare-per-package-directory at the right
>> spot.
>>
>> Reported-by: Andreas Naumann <anaumann@ultratronik.de>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>> ---
>> ? package/pkg-kconfig.mk | 1 +
>> ? 1 file changed, 1 insertion(+)
>>
>> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
>> index d6c95b897e..bf43632adc 100644
>> --- a/package/pkg-kconfig.mk
>> +++ b/package/pkg-kconfig.mk
>> @@ -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))

just realized that this doesn't match the function definition in 
pkg-utils.mk in this series (prepare-per-package-directories), thus is 
not executed at all. With that fixed ccache is, as part of toolchain, 
properly prepared and I no longer see the issue below.

regards,
Andreas



>> ????? $$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \
>> ????????? $$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \
>> ????????? $$(INSTALL) -m 0644 -D $$($(2)_KCONFIG_FILE) $$(@))
>>
> 
> Finally I could test this and found an issue. Because I use ccache, the 
> linux/uboot kconfig steps fail, because ccache apparently is not part of 
> UBOOT/LINUX_KCONFIG_DEPENDENCIES.
> Instead of conditionally adding it, my simple proposal would be to not 
> use it, e.g.:
> 
>  ?# $(HOST_DIR)/lib/libncurses.so (which is not).? We don't actually
>  ?# need any host-package for kconfig, so remove the HOSTCC/HOSTLDFLAGS
>  ?# override again.
> -UBOOT_KCONFIG_OPTS = $(UBOOT_MAKE_OPTS) HOSTCC="$(HOSTCC)" HOSTLDFLAGS=""
> +UBOOT_KCONFIG_OPTS = $(UBOOT_MAKE_OPTS) HOSTCC="$(HOSTCC_NOCCACHE)" 
> HOSTLDFLAGS=""
> 
> So after this, I'm not sure if there really was a need for above fix 
> since the fixup/.config creation may not even need the only kconfig 
> dependencies set so far: host-flex and host-bison..
> 
> 
> regards,
> Andreas
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 

[Buildroot] [PATCH next v6 10/10] package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package directories

[Thomas Petazzoni]

Hello,

On Thu, 6 Dec 2018 15:07:53 +0100, Andreas Naumann wrote:

> >> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> >> index d6c95b897e..bf43632adc 100644
> >> --- a/package/pkg-kconfig.mk
> >> +++ b/package/pkg-kconfig.mk
> >> @@ -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))  
> 
> just realized that this doesn't match the function definition in 
> pkg-utils.mk in this series (prepare-per-package-directories), thus is 
> not executed at all. With that fixed ccache is, as part of toolchain, 
> properly prepared and I no longer see the issue below.

Gaah, indeed. Well-spotted. Will fix. Thanks!

Best regards,

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

[Buildroot] [PATCH next v6 10/10] package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with per-package directories

[Thomas Petazzoni]

Hello,

On Thu, 6 Dec 2018 15:07:53 +0100, Andreas Naumann wrote:

> >> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> >> index d6c95b897e..bf43632adc 100644
> >> --- a/package/pkg-kconfig.mk
> >> +++ b/package/pkg-kconfig.mk
> >> @@ -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))  
> 
> just realized that this doesn't match the function definition in 
> pkg-utils.mk in this series (prepare-per-package-directories), thus is 
> not executed at all. With that fixed ccache is, as part of toolchain, 
> properly prepared and I no longer see the issue below.

This will certainly not fix the U-Boot issue you reported. It will fix
the issue for linux because LINUX_KCONFIG_DEPENDENCIES contains
"toolchain", and toolchain depends on host-ccache.

However, UBOOT_KCONFIG_DEPENDENCIES does not contain toolchain, and
therefore does not depend on host-ccache.

The issue you're seeing already exists today, without per-package
directory support. Try the current master, with a completely clean
build tree. Use the following defconfig:

BR2_arm=y
BR2_CCACHE=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2018.05.tar.bz2"
BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_1=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
# BR2_TARGET_ROOTFS_TAR is not set
BR2_TARGET_UBOOT=y
BR2_TARGET_UBOOT_BOARD_DEFCONFIG="mvebu_mcbin-88f8040"

And run just "make uboot-menuconfig". It will fail due to ccache being
missing. This bug is not related to per-package directory support.

The fix you proposed in your previous e-mail, i.e using HOSTCC_NOCCACHE
looks good, but I believe we need a more generic solution to this
problem, because all other kconfig packages will be affected as well
(except linux, because it has "toolchain" in its
<pkg>_KCONFIG_DEPENDENCIES) variable.

Best regards,

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello,

Arnout, Yann, see below some discussion where your input might be
useful.

On Thu, 6 Dec 2018 11:21:25 +0100, Andreas Naumann wrote:

> Just to make sure I just did the above using your ppsh-v6 branch with 
> the wandboard_defconfig. The only additional change was to use the 
> external linaro toolchain. After 'make'
>    $ readelf -d output/host/sbin/mkfs.ext3
> shows
>    0x000000000000001d (RUNPATH)            Bibliothek runpath: 
> [/local/gsrc/buildroot.upstream/output/per-package/host-e2fsprogs/host/lib]
> 
> which is already unexpected, shouldn't it show .../output/host/lib?
> After 'make prepare-sdk'
>    $ readelf -d output/host/sbin/mkfs.ext3
> shows
>    0x000000000000001d (RUNPATH)            Bibliothek runpath: []

OK, so I investigated this issue, and now I understand where it comes
from, but I'm not yet sure how to fix it properly.

Basically, *without* per-package directory enabled, the RPATH of
binaries looks like this:

  Library rpath: [/home/thomas/projets/buildroot/output/host/lib]

then, support/scripts/fix-rpath calls patchelf with the following
invocation:

  /home/thomas/projets/buildroot/output/host/bin/patchelf --make-rpath-relative /home/thomas/projets/buildroot/output/host --relative-to-file /home/thomas/projets/buildroot/output/host/sbin/mke2fs

This works fine, and causes the resulting binary to have the following
RPATH:

  Library runpath: [$ORIGIN/../lib]

You can see that the original "Library rpath", which
is /home/thomas/projets/buildroot/output/host/lib is located *inside*
the ROOTDIR passed as the --make-rpath-relative
option: /home/thomas/projets/buildroot/output/host.

Now, with per-package directory enabled, the RPATH of binaries when
building looks like this:

  Library rpath: [/home/thomas/projets/buildroot/output/per-package/host-e2fsprogs/host/lib]

So it is no longer within the ROOTDIR passed as patchelf's
--make-rpath-relative argument.

The logic used by patchelf is explained in
package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch:

====

RPATHDIR starts with "$ORIGIN":
    The original build-system already took care of setting a relative
    RPATH, resolve it and test if it's valid (does exist)

RPATHDIR starts with ROOTDIR:
    The original build-system added some absolute RPATH (absolute on
    the build machine). Test if it's valid (does exist).

ROOTDIR/RPATHDIR exists:
    The original build-system already took care of setting an absolute
    RPATH (absolute in the final rootfs), resolve it and test if it's
    valid (does exist).

RPATHDIR points somewhere else:
    (can be anywhere: build trees, staging tree, host location,
    non-existing location, etc.). Just discard such a path.

====

So, without per-package directory, we fall in case (2) above.

With per-package directory, we fall in case (4) above, and therefore
the RPATH is discarded.

At this point, I am not sure at which level and how the issue should be
fixed. Needs some thought. Input/ideas welcome.

Best regards,

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas De Schampheleire]

Hello,

El mi?., 26 dic. 2018 a las 18:34, Thomas Petazzoni
(<thomas.petazzoni@bootlin.com>) escribi?:
>
> Hello,
>
> Arnout, Yann, see below some discussion where your input might be
> useful.
>
> On Thu, 6 Dec 2018 11:21:25 +0100, Andreas Naumann wrote:
>
> > Just to make sure I just did the above using your ppsh-v6 branch with
> > the wandboard_defconfig. The only additional change was to use the
> > external linaro toolchain. After 'make'
> >    $ readelf -d output/host/sbin/mkfs.ext3
> > shows
> >    0x000000000000001d (RUNPATH)            Bibliothek runpath:
> > [/local/gsrc/buildroot.upstream/output/per-package/host-e2fsprogs/host/lib]
> >
> > which is already unexpected, shouldn't it show .../output/host/lib?
> > After 'make prepare-sdk'
> >    $ readelf -d output/host/sbin/mkfs.ext3
> > shows
> >    0x000000000000001d (RUNPATH)            Bibliothek runpath: []
>
> OK, so I investigated this issue, and now I understand where it comes
> from, but I'm not yet sure how to fix it properly.
>
> Basically, *without* per-package directory enabled, the RPATH of
> binaries looks like this:
>
>   Library rpath: [/home/thomas/projets/buildroot/output/host/lib]
>
> then, support/scripts/fix-rpath calls patchelf with the following
> invocation:
>
>   /home/thomas/projets/buildroot/output/host/bin/patchelf --make-rpath-relative /home/thomas/projets/buildroot/output/host --relative-to-file /home/thomas/projets/buildroot/output/host/sbin/mke2fs
>
> This works fine, and causes the resulting binary to have the following
> RPATH:
>
>   Library runpath: [$ORIGIN/../lib]
>
> You can see that the original "Library rpath", which
> is /home/thomas/projets/buildroot/output/host/lib is located *inside*
> the ROOTDIR passed as the --make-rpath-relative
> option: /home/thomas/projets/buildroot/output/host.
>
> Now, with per-package directory enabled, the RPATH of binaries when
> building looks like this:
>
>   Library rpath: [/home/thomas/projets/buildroot/output/per-package/host-e2fsprogs/host/lib]
>
> So it is no longer within the ROOTDIR passed as patchelf's
> --make-rpath-relative argument.
>
> The logic used by patchelf is explained in
> package/patchelf/0003-Add-option-to-make-the-rpath-relative-under-a-specif.patch:
>
> ====
>
> RPATHDIR starts with "$ORIGIN":
>     The original build-system already took care of setting a relative
>     RPATH, resolve it and test if it's valid (does exist)
>
> RPATHDIR starts with ROOTDIR:
>     The original build-system added some absolute RPATH (absolute on
>     the build machine). Test if it's valid (does exist).
>
> ROOTDIR/RPATHDIR exists:
>     The original build-system already took care of setting an absolute
>     RPATH (absolute in the final rootfs), resolve it and test if it's
>     valid (does exist).
>
> RPATHDIR points somewhere else:
>     (can be anywhere: build trees, staging tree, host location,
>     non-existing location, etc.). Just discard such a path.
>
> ====
>
> So, without per-package directory, we fall in case (2) above.
>
> With per-package directory, we fall in case (4) above, and therefore
> the RPATH is discarded.
>
> At this point, I am not sure at which level and how the issue should be
> fixed. Needs some thought. Input/ideas welcome.
>

In a similar context I once had following problem:
http://lists.busybox.net/pipermail/buildroot/2018-August/226955.html
This was for target binaries, rather than host, but for the below
discussion it does not matter.

Looking back at this problem, taking into account the above comment
from the patchelf patch, I would say that my problem would have been
fixed if case (4) above would not discard the path.

If that change would be adopted, then it would also preserve the rpath
'/home/thomas/projets/buildroot/output/per-package/host-e2fsprogs/host/lib'.

Of course, in your case you might actually want a different final
rpath, i.e. rewrite it to the consolidated host directory. I think
that in this case it would be better to read the rpath via patchelf,
calculate the transformed rpath from our own script, then writing it
back via patchelf,  rather than adding more options into patchelf with
e.g. transformation rules.

Best regards,
Thomas

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Yann E. MORIN]

Thomas, All,

On 2018-12-26 18:34 +0100, Thomas Petazzoni spake thusly:
> On Thu, 6 Dec 2018 11:21:25 +0100, Andreas Naumann wrote:
> > Just to make sure I just did the above using your ppsh-v6 branch with 
> > the wandboard_defconfig. The only additional change was to use the 
> > external linaro toolchain. After 'make'
> >    $ readelf -d output/host/sbin/mkfs.ext3
> > shows
> >    0x000000000000001d (RUNPATH)            Bibliothek runpath: 
> > [/local/gsrc/buildroot.upstream/output/per-package/host-e2fsprogs/host/lib]
[--SNIP--]
> OK, so I investigated this issue, and now I understand where it comes
> from, but I'm not yet sure how to fix it properly.
[--SNIP--]
> Now, with per-package directory enabled, the RPATH of binaries when
> building looks like this:
>   Library rpath: [/home/thomas/projets/buildroot/output/per-package/host-e2fsprogs/host/lib]
> 
> So it is no longer within the ROOTDIR passed as patchelf's
> --make-rpath-relative argument.
[--SNIP--]
> At this point, I am not sure at which level and how the issue should be
> fixed. Needs some thought. Input/ideas welcome.

Well, one obvious suggestion would be to run patchelf after the
installation of each package, and scan the package-file-0list to limit
ourselves to the files the current package just installed (to avoid
re-patchelf-ing what has already been patchelf-ed).

That way, we know at that time what the host dir is, and we can tweak it
appropriately.

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello,

On Wed, 26 Dec 2018 18:34:22 +0100, Thomas Petazzoni wrote:

> So, without per-package directory, we fall in case (2) above.
> 
> With per-package directory, we fall in case (4) above, and therefore
> the RPATH is discarded.
> 
> At this point, I am not sure at which level and how the issue should be
> fixed. Needs some thought. Input/ideas welcome.

So for now, the way I fixed this is by rewriting the RPATH in fix-rpath
so that they don't point to per-package host directories. It is a bit
annoying that we end up rewriting RPATHs twice, but it is the easiest /
most straightforward solution to be able to move forward with the whole
per-package directory stuff.

Here is what I have locally:

diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
index fa138ca15a..67bf1eb1b3 100755
--- a/support/scripts/fix-rpath
+++ b/support/scripts/fix-rpath
@@ -127,14 +127,24 @@ main() {
 
     while read file ; do
         # check if it's an ELF file
-        if ${PATCHELF} --print-rpath "${file}" > /dev/null 2>&1; then
-            # 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}"
-            # restore the original permission
-            test "${changed}" != "" && chmod u-w "${file}"
+        rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1)
+        if test $? -ne 0 ; then
+            continue
         fi
+
+        # make files writable if necessary
+        changed=$(chmod -c u+w "${file}")
+
+        # rewrite the per-package rpaths
+        changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]*/host@${HOST_DIR}@")
+        if test "${rpath}" != "${changed_rpath}" ; then
+            ${PATCHELF} --set-rpath ${changed_rpath} "${file}"
+        fi
+
+        # call patchelf to sanitize the rpath
+        ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}"
+        # restore the original permission
+        test "${changed}" != "" && chmod u-w "${file}"
     done < <(find "${rootdir}" ${find_args[@]})
 
     # Restore patched patchelf utility

Best regards,

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Arnout Vandecappelle]

 Hi,

 I realize v7 is already out, but it is probably easier to reply here.


On 27/12/2018 17:17, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 26 Dec 2018 18:34:22 +0100, Thomas Petazzoni wrote:
> 
>> So, without per-package directory, we fall in case (2) above.
>>
>> With per-package directory, we fall in case (4) above, and therefore
>> the RPATH is discarded.
>>
>> At this point, I am not sure at which level and how the issue should be
>> fixed. Needs some thought. Input/ideas welcome.

 Actually, I think we have been conflating the --relative-to-file case with the
--make-rpath-relative case when we developed the patchelf modifications. For the
--relative-to-file case (i.e. for host dirs), we really don't need the rootDir
at all, we can just make all rpaths relative. Or maybe we should still pass the
per-package base directory, just to determine whether it's a system path or not.

 But of course, the staging and target directories have exactly the same
problem. And there, we really do need the correct rootDir, because it is also
prepended to the existing rpath to determine the full path for case (3). So we
still need a solution...



> So for now, the way I fixed this is by rewriting the RPATH in fix-rpath
> so that they don't point to per-package host directories.

 Why did you prefer this over Yann's proposal to do the fix-rpath after each
package installation (and making use of packages-files-list)?

> It is a bit
> annoying that we end up rewriting RPATHs twice, but it is the easiest /
> most straightforward solution to be able to move forward with the whole
> per-package directory stuff.

 To avoid rewriting it twice, we could move the logic into patchelf itself.
I.e., in addition to rootDir (which is the thing that will be used for
prepending), pass a match regex that is used for stripping off a pre-existing
rootDir.

> 
> Here is what I have locally:
> 
> diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
> index fa138ca15a..67bf1eb1b3 100755
> --- a/support/scripts/fix-rpath
> +++ b/support/scripts/fix-rpath
> @@ -127,14 +127,24 @@ main() {
>  
>      while read file ; do
>          # check if it's an ELF file
> -        if ${PATCHELF} --print-rpath "${file}" > /dev/null 2>&1; then
> -            # 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}"
> -            # restore the original permission
> -            test "${changed}" != "" && chmod u-w "${file}"
> +        rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1)
> +        if test $? -ne 0 ; then
> +            continue
>          fi
> +
> +        # make files writable if necessary
> +        changed=$(chmod -c u+w "${file}")
> +
> +        # rewrite the per-package rpaths
> +        changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]*/host@${HOST_DIR}@")

 Not just host, also staging and target! Staging is already covered by host
because it's a subdirectory, but target is still needed. That said, it's
unlikely that target ever ends up in one of the rpaths, because we normally only
pass it as the DESTDIR in the installation step. Still, you never know, so I'd
include target for completeness.

 Regards,
 Arnout


> +        if test "${rpath}" != "${changed_rpath}" ; then
> +            ${PATCHELF} --set-rpath ${changed_rpath} "${file}"
> +        fi
> +
> +        # call patchelf to sanitize the rpath
> +        ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}"
> +        # restore the original permission
> +        test "${changed}" != "" && chmod u-w "${file}"
>      done < <(find "${rootdir}" ${find_args[@]})
>  
>      # Restore patched patchelf utility
> 
> Best regards,
> 
> Thomas
> 

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Arnout Vandecappelle]

On 26/12/2018 19:33, Thomas De Schampheleire wrote:
> In a similar context I once had following problem:
> http://lists.busybox.net/pipermail/buildroot/2018-August/226955.html
> This was for target binaries, rather than host, but for the below
> discussion it does not matter.

 I don't think you ever replied to my question in that thread:

 Let's first find out why it is cleared. patchelf should check for each
directory in rpath if it is actually needed, and only remove it if it is not
needed. So, what library do you have in /opt/foobar/lib, and is it really linked
with the program?

 Hm, I realize that we never thought about dlopen()ed libraries. Could that be
the cause? I guess that hasn't been a problem up to now because usually programs
using dlopen() will use an absolute path (to a build-time or run-time configured
plugin directory) rather than relying on RPATH.



> Looking back at this problem, taking into account the above comment
> from the patchelf patch, I would say that my problem would have been
> fixed if case (4) above would not discard the path.

 I think one motivation for removing redundant rpaths is to avoid having build
directories leaking into target binaries. Especially for reproducible builds
this is important.


> If that change would be adopted, then it would also preserve the rpath
> '/home/thomas/projets/buildroot/output/per-package/host-e2fsprogs/host/lib'.
> 
> Of course, in your case you might actually want a different final
> rpath, i.e. rewrite it to the consolidated host directory. I think
> that in this case it would be better to read the rpath via patchelf,
> calculate the transformed rpath from our own script, then writing it
> back via patchelf,  rather than adding more options into patchelf with
> e.g. transformation rules.

 The problem is that patchelf is rather slow, so running it twice is even slower...

 Regards,
 Arnout

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas De Schampheleire]

El jue., 10 ene. 2019 a las 22:25, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 26/12/2018 19:33, Thomas De Schampheleire wrote:
> > In a similar context I once had following problem:
> > http://lists.busybox.net/pipermail/buildroot/2018-August/226955.html
> > This was for target binaries, rather than host, but for the below
> > discussion it does not matter.
>
>  I don't think you ever replied to my question in that thread:
>
>  Let's first find out why it is cleared. patchelf should check for each
> directory in rpath if it is actually needed, and only remove it if it is not
> needed. So, what library do you have in /opt/foobar/lib, and is it really linked
> with the program?
>
>  Hm, I realize that we never thought about dlopen()ed libraries. Could that be
> the cause? I guess that hasn't been a problem up to now because usually programs
> using dlopen() will use an absolute path (to a build-time or run-time configured
> plugin directory) rather than relying on RPATH.

Sorry that I did not seem to have replied on that question.

The binary in question did in fact link with the library, there is no
dlopen used.
Some redacted output showing the initial rpath, which disappears after
fix-rpath.

$ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
...
 0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
...
 0x0000000f (RPATH)                      Library rpath:
[/../lib:/opt/foobar/lib:/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib]

$ find output/target/ -name libfoo.so.1
output/target/opt/foobar/lib/libfoo.so.1

$ env HOST_DIR=output/host STAGING_DIR=output/staging
TARGET_DIR=output/target support/scripts/fix-rpath target

$ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
...
 0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
...


>
>
>
> > Looking back at this problem, taking into account the above comment
> > from the patchelf patch, I would say that my problem would have been
> > fixed if case (4) above would not discard the path.
>
>  I think one motivation for removing redundant rpaths is to avoid having build
> directories leaking into target binaries. Especially for reproducible builds
> this is important.

Ok, I can understand that.
But it should only remove directories which are not present in the
target directory, e.g. if rpath contains /opt/foobar/lib then the
script should check if output/target/opt/foobar/lib exists. If it
does, then the rpath can remain, if it does not then it could be
removed.

>
>
> > If that change would be adopted, then it would also preserve the rpath
> > '/home/thomas/projets/buildroot/output/per-package/host-e2fsprogs/host/lib'.
> >
> > Of course, in your case you might actually want a different final
> > rpath, i.e. rewrite it to the consolidated host directory. I think
> > that in this case it would be better to read the rpath via patchelf,
> > calculate the transformed rpath from our own script, then writing it
> > back via patchelf,  rather than adding more options into patchelf with
> > e.g. transformation rules.
>
>  The problem is that patchelf is rather slow, so running it twice is even slower...

How so is it slow, can you clarify? Which times are you talking about?

Thanks,
Thomas

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Arnout Vandecappelle]

On 11/01/2019 21:09, Thomas De Schampheleire wrote:
> El jue., 10 ene. 2019 a las 22:25, Arnout Vandecappelle
> (<arnout@mind.be>) escribi?:
>>
>>
>>
>> On 26/12/2018 19:33, Thomas De Schampheleire wrote:
>>> In a similar context I once had following problem:
>>> http://lists.busybox.net/pipermail/buildroot/2018-August/226955.html
>>> This was for target binaries, rather than host, but for the below
>>> discussion it does not matter.
>>
>>  I don't think you ever replied to my question in that thread:
>>
>>  Let's first find out why it is cleared. patchelf should check for each
>> directory in rpath if it is actually needed, and only remove it if it is not
>> needed. So, what library do you have in /opt/foobar/lib, and is it really linked
>> with the program?
>>
>>  Hm, I realize that we never thought about dlopen()ed libraries. Could that be
>> the cause? I guess that hasn't been a problem up to now because usually programs
>> using dlopen() will use an absolute path (to a build-time or run-time configured
>> plugin directory) rather than relying on RPATH.
> 
> Sorry that I did not seem to have replied on that question.
> 
> The binary in question did in fact link with the library, there is no
> dlopen used.
> Some redacted output showing the initial rpath, which disappears after
> fix-rpath.
> 
> $ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
> ...
>  0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
> ...
>  0x0000000f (RPATH)                      Library rpath:
> [/../lib:/opt/foobar/lib:/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib]
> 
> $ find output/target/ -name libfoo.so.1
> output/target/opt/foobar/lib/libfoo.so.1
> 
> $ env HOST_DIR=output/host STAGING_DIR=output/staging
> TARGET_DIR=output/target support/scripts/fix-rpath target
> 
> $ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
> ...
>  0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
> ...

 OK, so that's a bug in our patchelf modification... At least, if libfoo.so.1
doesn't exist in output/target/lib or output/target/usr/lib.

 If you run patchelf with --debug, it should tell you why the rpath got removed.


>>> Looking back at this problem, taking into account the above comment
>>> from the patchelf patch, I would say that my problem would have been
>>> fixed if case (4) above would not discard the path.
>>
>>  I think one motivation for removing redundant rpaths is to avoid having build
>> directories leaking into target binaries. Especially for reproducible builds
>> this is important.
> 
> Ok, I can understand that.
> But it should only remove directories which are not present in the
> target directory, e.g. if rpath contains /opt/foobar/lib then the
> script should check if output/target/opt/foobar/lib exists. If it
> does, then the rpath can remain, if it does not then it could be
> removed.

 Indeed, and that's what patchelf is supposed to do. Well, it also checks that
the library exists.


>>> If that change would be adopted, then it would also preserve the rpath
>>> '/home/thomas/projets/buildroot/output/per-package/host-e2fsprogs/host/lib'.
>>>
>>> Of course, in your case you might actually want a different final
>>> rpath, i.e. rewrite it to the consolidated host directory. I think
>>> that in this case it would be better to read the rpath via patchelf,
>>> calculate the transformed rpath from our own script, then writing it
>>> back via patchelf,  rather than adding more options into patchelf with
>>> e.g. transformation rules.
>>
>>  The problem is that patchelf is rather slow, so running it twice is even slower...
> 
> How so is it slow, can you clarify? Which times are you talking about?

 I don't remember, but someone did measurements in 2017 and it was slow.

 Regards,
 Arnout

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas De Schampheleire]

El dom., 13 ene. 2019 a las 23:10, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 11/01/2019 21:09, Thomas De Schampheleire wrote:
> > El jue., 10 ene. 2019 a las 22:25, Arnout Vandecappelle
> > (<arnout@mind.be>) escribi?:
> >>
> >>
> >>
> >> On 26/12/2018 19:33, Thomas De Schampheleire wrote:
> >>> In a similar context I once had following problem:
> >>> http://lists.busybox.net/pipermail/buildroot/2018-August/226955.html
> >>> This was for target binaries, rather than host, but for the below
> >>> discussion it does not matter.
> >>
> >>  I don't think you ever replied to my question in that thread:
> >>
> >>  Let's first find out why it is cleared. patchelf should check for each
> >> directory in rpath if it is actually needed, and only remove it if it is not
> >> needed. So, what library do you have in /opt/foobar/lib, and is it really linked
> >> with the program?
> >>
> >>  Hm, I realize that we never thought about dlopen()ed libraries. Could that be
> >> the cause? I guess that hasn't been a problem up to now because usually programs
> >> using dlopen() will use an absolute path (to a build-time or run-time configured
> >> plugin directory) rather than relying on RPATH.
> >
> > Sorry that I did not seem to have replied on that question.
> >
> > The binary in question did in fact link with the library, there is no
> > dlopen used.
> > Some redacted output showing the initial rpath, which disappears after
> > fix-rpath.
> >
> > $ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
> > ...
> >  0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
> > ...
> >  0x0000000f (RPATH)                      Library rpath:
> > [/../lib:/opt/foobar/lib:/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib]
> >
> > $ find output/target/ -name libfoo.so.1
> > output/target/opt/foobar/lib/libfoo.so.1
> >
> > $ env HOST_DIR=output/host STAGING_DIR=output/staging
> > TARGET_DIR=output/target support/scripts/fix-rpath target
> >
> > $ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
> > ...
> >  0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
> > ...
>
>  OK, so that's a bug in our patchelf modification... At least, if libfoo.so.1
> doesn't exist in output/target/lib or output/target/usr/lib.
>
>  If you run patchelf with --debug, it should tell you why the rpath got removed.

Part of the debug output for fooprogram:

patching ELF file `output/target/opt/foobar/bin/fooprogram'
Kernel page size is 4096 bytes
removing directory '/../lib' from RPATH because it's not in rootdir
keeping relative path of
/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib
removing directory
'/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib' from
RPATH because it's not in rootdir
new rpath is `m/repo/isam/buildroot/output/target/opt/foobar/lib'

The last line seems odd with the leading 'm'. I don't know if that is
the problem, I will check it further.
Thanks for pointing to the --debug option.


>
>
> >>> Looking back at this problem, taking into account the above comment
> >>> from the patchelf patch, I would say that my problem would have been
> >>> fixed if case (4) above would not discard the path.
> >>
> >>  I think one motivation for removing redundant rpaths is to avoid having build
> >> directories leaking into target binaries. Especially for reproducible builds
> >> this is important.
> >
> > Ok, I can understand that.
> > But it should only remove directories which are not present in the
> > target directory, e.g. if rpath contains /opt/foobar/lib then the
> > script should check if output/target/opt/foobar/lib exists. If it
> > does, then the rpath can remain, if it does not then it could be
> > removed.
>
>  Indeed, and that's what patchelf is supposed to do. Well, it also checks that
> the library exists.
>
>
> >>> If that change would be adopted, then it would also preserve the rpath
> >>> '/home/thomas/projets/buildroot/output/per-package/host-e2fsprogs/host/lib'.
> >>>
> >>> Of course, in your case you might actually want a different final
> >>> rpath, i.e. rewrite it to the consolidated host directory. I think
> >>> that in this case it would be better to read the rpath via patchelf,
> >>> calculate the transformed rpath from our own script, then writing it
> >>> back via patchelf,  rather than adding more options into patchelf with
> >>> e.g. transformation rules.
> >>
> >>  The problem is that patchelf is rather slow, so running it twice is even slower...
> >
> > How so is it slow, can you clarify? Which times are you talking about?
>
>  I don't remember, but someone did measurements in 2017 and it was slow.

It seems the code already calls patchelf twice:

        if ${PATCHELF} --print-rpath "${file}" > /dev/null 2>&1; then
            # 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}"

so by saving the string returned in the first call, we can actually do
a better transformation ourselves.

/Thomas

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello,

On Mon, 14 Jan 2019 17:01:44 +0100, Thomas De Schampheleire wrote:

> It seems the code already calls patchelf twice:
> 
>         if ${PATCHELF} --print-rpath "${file}" > /dev/null 2>&1; then
>             # 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}"
> 
> so by saving the string returned in the first call, we can actually do
> a better transformation ourselves.

My latest iteration (v7) is already saving the string to do some
transformation:

  http://patchwork.ozlabs.org/patch/1019149/

Best regards,

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

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas De Schampheleire]

El lun., 14 ene. 2019 a las 17:01, Thomas De Schampheleire
(<patrickdepinguin@gmail.com>) escribi?:
>
> El dom., 13 ene. 2019 a las 23:10, Arnout Vandecappelle
> (<arnout@mind.be>) escribi?:
> >
> >
> >
> > On 11/01/2019 21:09, Thomas De Schampheleire wrote:
> > > El jue., 10 ene. 2019 a las 22:25, Arnout Vandecappelle
> > > (<arnout@mind.be>) escribi?:
> > >>
> > >>
> > >>
> > >> On 26/12/2018 19:33, Thomas De Schampheleire wrote:
> > >>> In a similar context I once had following problem:
> > >>> http://lists.busybox.net/pipermail/buildroot/2018-August/226955.html
> > >>> This was for target binaries, rather than host, but for the below
> > >>> discussion it does not matter.
> > >>
> > >>  I don't think you ever replied to my question in that thread:
> > >>
> > >>  Let's first find out why it is cleared. patchelf should check for each
> > >> directory in rpath if it is actually needed, and only remove it if it is not
> > >> needed. So, what library do you have in /opt/foobar/lib, and is it really linked
> > >> with the program?
> > >>
> > >>  Hm, I realize that we never thought about dlopen()ed libraries. Could that be
> > >> the cause? I guess that hasn't been a problem up to now because usually programs
> > >> using dlopen() will use an absolute path (to a build-time or run-time configured
> > >> plugin directory) rather than relying on RPATH.
> > >
> > > Sorry that I did not seem to have replied on that question.
> > >
> > > The binary in question did in fact link with the library, there is no
> > > dlopen used.
> > > Some redacted output showing the initial rpath, which disappears after
> > > fix-rpath.
> > >
> > > $ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
> > > ...
> > >  0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
> > > ...
> > >  0x0000000f (RPATH)                      Library rpath:
> > > [/../lib:/opt/foobar/lib:/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib]
> > >
> > > $ find output/target/ -name libfoo.so.1
> > > output/target/opt/foobar/lib/libfoo.so.1
> > >
> > > $ env HOST_DIR=output/host STAGING_DIR=output/staging
> > > TARGET_DIR=output/target support/scripts/fix-rpath target
> > >
> > > $ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
> > > ...
> > >  0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
> > > ...
> >
> >  OK, so that's a bug in our patchelf modification... At least, if libfoo.so.1
> > doesn't exist in output/target/lib or output/target/usr/lib.
> >
> >  If you run patchelf with --debug, it should tell you why the rpath got removed.
>
> Part of the debug output for fooprogram:
>
> patching ELF file `output/target/opt/foobar/bin/fooprogram'
> Kernel page size is 4096 bytes
> removing directory '/../lib' from RPATH because it's not in rootdir
> keeping relative path of
> /home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib
> removing directory
> '/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib' from
> RPATH because it's not in rootdir
> new rpath is `m/repo/isam/buildroot/output/target/opt/foobar/lib'
>
> The last line seems odd with the leading 'm'. I don't know if that is
> the problem, I will check it further.
> Thanks for pointing to the --debug option.

Sorry for the confusion, this specific problem is caused by my calling
fix-rpath with relative reference to the target directory.
(the code in patchelf could perhaps be made more robust by resolving
the input 'rootdir' with realpath as well.)

The actual output after fixing that parameter problem is:

output/host/bin/patchelf --debug --make-rpath-relative
/home/tdescham/repo/isam/buildroot/output/target
--no-standard-lib-dirs
/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/bin/fooprogram

patching ELF file
`/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/bin/fooprogram'
Kernel page size is 4096 bytes
removing directory '/../lib' from RPATH because it's not in rootdir
keeping relative path of
/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib
removing directory
'/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib' from
RPATH because it does not contain needed libs
new rpath is `/opt/foobar/lib'

So the logic above seems correct, but it is not correctly stored in
the binary. After running patchelf, the rpath is cleared. I will
investigate that further.

/Thomas

[Buildroot] [PATCH next v6 07/10] core: implement per-package SDK and target

[Thomas De Schampheleire]

El lun., 14 ene. 2019 a las 21:19, Thomas De Schampheleire
(<patrickdepinguin@gmail.com>) escribi?:
>
> El lun., 14 ene. 2019 a las 17:01, Thomas De Schampheleire
> (<patrickdepinguin@gmail.com>) escribi?:
> >
> > El dom., 13 ene. 2019 a las 23:10, Arnout Vandecappelle
> > (<arnout@mind.be>) escribi?:
> > >
> > >
> > >
> > > On 11/01/2019 21:09, Thomas De Schampheleire wrote:
> > > > El jue., 10 ene. 2019 a las 22:25, Arnout Vandecappelle
> > > > (<arnout@mind.be>) escribi?:
> > > >>
> > > >>
> > > >>
> > > >> On 26/12/2018 19:33, Thomas De Schampheleire wrote:
> > > >>> In a similar context I once had following problem:
> > > >>> http://lists.busybox.net/pipermail/buildroot/2018-August/226955.html
> > > >>> This was for target binaries, rather than host, but for the below
> > > >>> discussion it does not matter.
> > > >>
> > > >>  I don't think you ever replied to my question in that thread:
> > > >>
> > > >>  Let's first find out why it is cleared. patchelf should check for each
> > > >> directory in rpath if it is actually needed, and only remove it if it is not
> > > >> needed. So, what library do you have in /opt/foobar/lib, and is it really linked
> > > >> with the program?
> > > >>
> > > >>  Hm, I realize that we never thought about dlopen()ed libraries. Could that be
> > > >> the cause? I guess that hasn't been a problem up to now because usually programs
> > > >> using dlopen() will use an absolute path (to a build-time or run-time configured
> > > >> plugin directory) rather than relying on RPATH.
> > > >
> > > > Sorry that I did not seem to have replied on that question.
> > > >
> > > > The binary in question did in fact link with the library, there is no
> > > > dlopen used.
> > > > Some redacted output showing the initial rpath, which disappears after
> > > > fix-rpath.
> > > >
> > > > $ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
> > > > ...
> > > >  0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
> > > > ...
> > > >  0x0000000f (RPATH)                      Library rpath:
> > > > [/../lib:/opt/foobar/lib:/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib]
> > > >
> > > > $ find output/target/ -name libfoo.so.1
> > > > output/target/opt/foobar/lib/libfoo.so.1
> > > >
> > > > $ env HOST_DIR=output/host STAGING_DIR=output/staging
> > > > TARGET_DIR=output/target support/scripts/fix-rpath target
> > > >
> > > > $ readelf -d output/target/opt/foobar/bin/fooprogram | rg 'NEEDED|RPATH'
> > > > ...
> > > >  0x00000001 (NEEDED)                     Shared library: [libfoo.so.1]
> > > > ...
> > >
> > >  OK, so that's a bug in our patchelf modification... At least, if libfoo.so.1
> > > doesn't exist in output/target/lib or output/target/usr/lib.
> > >
> > >  If you run patchelf with --debug, it should tell you why the rpath got removed.
> >
> > Part of the debug output for fooprogram:
> >
> > patching ELF file `output/target/opt/foobar/bin/fooprogram'
> > Kernel page size is 4096 bytes
> > removing directory '/../lib' from RPATH because it's not in rootdir
> > keeping relative path of
> > /home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib
> > removing directory
> > '/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib' from
> > RPATH because it's not in rootdir
> > new rpath is `m/repo/isam/buildroot/output/target/opt/foobar/lib'
> >
> > The last line seems odd with the leading 'm'. I don't know if that is
> > the problem, I will check it further.
> > Thanks for pointing to the --debug option.
>
> Sorry for the confusion, this specific problem is caused by my calling
> fix-rpath with relative reference to the target directory.
> (the code in patchelf could perhaps be made more robust by resolving
> the input 'rootdir' with realpath as well.)
>
> The actual output after fixing that parameter problem is:
>
> output/host/bin/patchelf --debug --make-rpath-relative
> /home/tdescham/repo/isam/buildroot/output/target
> --no-standard-lib-dirs
> /home/tdescham/repo/isam/buildroot/output/target/opt/foobar/bin/fooprogram
>
> patching ELF file
> `/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/bin/fooprogram'
> Kernel page size is 4096 bytes
> removing directory '/../lib' from RPATH because it's not in rootdir
> keeping relative path of
> /home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib
> removing directory
> '/home/tdescham/repo/isam/buildroot/output/target/opt/foobar/lib' from
> RPATH because it does not contain needed libs
> new rpath is `/opt/foobar/lib'
>
> So the logic above seems correct, but it is not correctly stored in
> the binary. After running patchelf, the rpath is cleared. I will
> investigate that further.

and now I see that patchelf does not fill in the RPATH field but
RUNPATH, which my grep did not catch :-S

So conclusion seems to be that fix-rpath does work correctly for
absolute intentional paths.
In the original issue there _was_ a problem, but likely it was only
the endianness problem fixed by Bryce. And during my analysis I
probably did not correctly verify the RUNPATH field.

Sorry for the confusion :-)

/Thomas