[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Thomas Petazzoni]

Hello,

Here is a seventh iteration of the per-package SDK and target
directory implementation, offering top-level parallel build
capability.

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

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

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

Changes v6 -> v7
================

 - Make host-finalize a PHONY target, as suggested by Yann
   E. Morin. Change done in "core: implement per-package SDK and
   target"

 - Adjust <pkg>-dirclean so that it also removes the per-package
   directory for <pkg>. Change done in "core: implement per-package
   SDK and target"

 - Completely rework the comment that explains the .NOTPARALLEL
   statement. Indeed, the existing explanation no longer made
   sense. The new comment explains why .NOTPARALLEL is added when
   !BR2_PER_PACKAGE_DIRECTORIES. Suggested by Yann E. Morin. Change
   done in "Makefile: allow top-level parallel build with
   BR2_PER_PACKAGE_DIRECTORIES=y".

 - Move the libtool .la files fixup logic into a separate function,
   just because it is a bit nicer. Change done in
   "package/pkg-generic: make libtool .la files compatible with
   per-package directories".

 - Added Acked-by from Yann E. Morin on patch "package/pkg-kconfig:
   handle KCONFIG_DEPENDENCIES with per-package directories".

 - Fix "package/pkg-kconfig: handle KCONFIG_DEPENDENCIES with
   per-package directories" to use the proper per-package-directory
   macro instead of prepare-per-package-folder. This was missed during
   the rename of the macro that happened in the v6 of this series.

 - Drop the following patches that have been merged in master since v6
   was posted:

   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

 - Add a preparatory patch that moves the definition of TARGET_DIR
   inside the BR2_HAVE_DOT_CONFIG condition, next to the existing
   HOST_DIR definition. This allows the HOST_DIR and TARGET_DIR
   definition to be next to each other when introducing the
   per-package directory support. Suggested by Arnout.

 - Add a preparatory patch that documents the functions in
   check-host-rpath.

 - Add more comments in check-host-rpath to explain how it works in
   relation with per-package directory support. Suggested by Arnout.

 - Modify the fix-rpath script to rewrite RPATH referencing
   per-package host directories so that they point to the global host
   directory. This is needed for the patchelf --make-rpath-relative
   logic to work, and rewrite all RPATHs as relative RPATHs. Reported
   by Andreas Naumann.

 - Add documentation in the Buildroot manual about top-level parallel
   build support.

Changes v5 -> 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
   folders"

 - 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
   folders 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 v4 -> 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 v3 -> 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 v2 -> 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 v1 -> 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 RFC -> 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

Related work
============

In order for per-package directory support to work with a number of
pakages, we have several separate patches or patch series:

 - To fix per-package directory support for Meson packages

 - To fix per-package directory support for Python packages

 - To fix per-package directory support in apr-util, apache, cargo,
   owfs, etc.

 - Generally, a number of fixes that are not directly related to
   per-package directory, but have been detected thanks to this
   work. These fixes have already been submitted.

We are handling them through separate series to not make this series
longer than it already is.

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

Known issues:

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

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

Best regards,

Thomas


Thomas Petazzoni (8):
  support/scripts/check-host-rpath: document existing functions
  Makefile: move definition of TARGET_DIR inside .config condition
  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
  docs/manual: add details about top-level parallel build support
  docs/manual: document the effect of per-package directory on variables

 Config.in                               | 18 +++++++++
 Makefile                                | 49 ++++++++++++-------------
 docs/manual/adding-packages-generic.txt |  9 ++++-
 docs/manual/common-usage.txt            | 44 ++++++++++++++++++++++
 docs/manual/faq-troubleshooting.txt     |  3 ++
 docs/manual/quickstart.txt              |  8 ++--
 package/pkg-generic.mk                  | 24 +++++++++++-
 package/pkg-kconfig.mk                  |  1 +
 package/pkg-utils.mk                    | 26 +++++++++++++
 support/scripts/check-host-rpath        | 31 +++++++++++++++-
 support/scripts/fix-rpath               | 29 +++++++++++----
 11 files changed, 203 insertions(+), 39 deletions(-)

-- 
2.20.1

[Buildroot] [PATCH v7 1/8] support/scripts/check-host-rpath: document existing functions

[Thomas Petazzoni]

As suggested by Arnout Vandecappelle, let's document the
elf_needs_rpath() and check_elf_has_rpath() functions, before we make
them a bit more complicated with per-package directory support.

Suggested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 support/scripts/check-host-rpath | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index 6c5767da05..c8939569e2 100755
--- a/support/scripts/check-host-rpath
+++ b/support/scripts/check-host-rpath
@@ -39,6 +39,11 @@ is_elf() {
     |grep -E 'Requesting program interpreter:' >/dev/null 2>&1
 }
 
+# This function tells whether a given ELF executable (first argument)
+# needs a RPATH pointing to the host library directory or not. It
+# needs such an RPATH if at least of the libraries used by the ELF
+# executable is available in the host library directory. This function
+# returns 0 when a RPATH is needed, 1 otherwise.
 elf_needs_rpath() {
     local file="${1}"
     local hostdir="${2}"
@@ -54,6 +59,13 @@ elf_needs_rpath() {
     return 1
 }
 
+# This function checks whether at least one of the RPATH of the given
+# ELF executable (first argument) properly points to the host library
+# directory (second argument), either through an absolute RPATH or a
+# relative RPATH. Having such a RPATH will make sure the ELF
+# executable will find at runtime the shared libraries it depends
+# on. This function returns 0 when a proper RPATH was found, or 1
+# otherwise.
 check_elf_has_rpath() {
     local file="${1}"
     local hostdir="${2}"
-- 
2.20.1

[Buildroot] [PATCH v7 2/8] Makefile: move definition of TARGET_DIR inside .config condition

[Thomas Petazzoni]

In a follow-up commit introducing per-package directory support, we
will need to define TARGET_DIR in a different way depending on the
value of a Config.in option. To make this possible, the definition of
TARGET_DIR should be moved inside the BR2_HAVE_DOT_CONFIG condition.

We have verified that $(TARGET_DIR) is only used within the
BR2_HAVE_DOT_CONFIG condition. Outside of this condition, such as in
the "clean" target, $(BASE_TARGET_DIR) is used.

Suggested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 Makefile | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index c5b78b3274..9de8e0c725 100644
--- a/Makefile
+++ b/Makefile
@@ -204,10 +204,7 @@ BR_GRAPH_OUT := $(or $(BR2_GRAPH_OUT),pdf)
 
 BUILD_DIR := $(BASE_DIR)/build
 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))
 # 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
@@ -457,6 +454,10 @@ TAR_OPTIONS = $(call qstrip,$(BR2_TAR_OPTIONS)) -xf
 # packages compiled for the host go here
 HOST_DIR := $(call qstrip,$(BR2_HOST_DIR))
 
+# The target directory is common to all packages,
+# but there is one that is specific to each filesystem.
+TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(BASE_TARGET_DIR))
+
 ifneq ($(HOST_DIR),$(BASE_DIR)/host)
 HOST_DIR_SYMLINK = $(BASE_DIR)/host
 $(HOST_DIR_SYMLINK): $(BASE_DIR)
-- 
2.20.1

[Buildroot] [PATCH v7 3/8] 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.

This change to per-package directories has an impact on the RPATH
built into the host binaries, as those RPATH now point to various
per-package host directories, and no longer to the global host
directory. We do not try to rewrite such RPATHs during the build as
having such RPATHs is perfectly fine, but we still need to handle two
fallouts from this change:

 - The check-host-rpath script, which verifies at the end of each
   package installation that it has the appropriate RPATH, is modified
   to understand that a RPATH to $(PER_PACKAGE_DIR)/<pkg>/host/lib is
   a correct RPAT.

 - The fix-rpath script, which mungles the RPATH mainly for the SDK
   preparation, is modified to rewrite the RPATH to not point to
   per-package directories. Indeed the patchelf --make-rpath-relative
   call only works if the RPATH points to the ROOTDIR passed as
   argument, and this ROOTDIR is the global host directory. Rewriting
   the RPATH to not point to per-package host directories prior to
   this is an easy solution to this issue.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 Config.in                        | 18 ++++++++++++++++++
 Makefile                         | 29 ++++++++++++++++++-----------
 package/pkg-generic.mk           |  7 ++++++-
 package/pkg-utils.mk             | 26 ++++++++++++++++++++++++++
 support/scripts/check-host-rpath | 27 ++++++++++++++++++++++-----
 support/scripts/fix-rpath        | 29 ++++++++++++++++++++++-------
 6 files changed, 112 insertions(+), 24 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 9de8e0c725..e01ec4c963 100644
--- a/Makefile
+++ b/Makefile
@@ -205,6 +205,7 @@ BR_GRAPH_OUT := $(or $(BR2_GRAPH_OUT),pdf)
 BUILD_DIR := $(BASE_DIR)/build
 BINARIES_DIR := $(BASE_DIR)/images
 BASE_TARGET_DIR := $(BASE_DIR)/target
+PER_PACKAGE_DIR := $(BASE_DIR)/per-package
 # initial definition so that 'make clean' works for most users, even without
 # .config. HOST_DIR will be overwritten later when .config is included.
 HOST_DIR := $(BASE_DIR)/host
@@ -451,12 +452,13 @@ XZCAT := $(call qstrip,$(BR2_XZCAT))
 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)))
+TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/target,$(BASE_TARGET_DIR)))
+else
 HOST_DIR := $(call qstrip,$(BR2_HOST_DIR))
-
-# The target directory is common to all packages,
-# but there is one that is specific to each filesystem.
 TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(BASE_TARGET_DIR))
+endif
 
 ifneq ($(HOST_DIR),$(BASE_DIR)/host)
 HOST_DIR_SYMLINK = $(BASE_DIR)/host
@@ -585,8 +587,8 @@ world: target-post-image
 .PHONY: prepare-sdk
 prepare-sdk: world
 	@$(call MESSAGE,"Rendering the SDK relocatable")
-	$(TOPDIR)/support/scripts/fix-rpath host
-	$(TOPDIR)/support/scripts/fix-rpath staging
+	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath host
+	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath staging
 	$(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh $(HOST_DIR)/relocate-sdk.sh
 	mkdir -p $(HOST_DIR)/share/buildroot
 	echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location
@@ -702,15 +704,19 @@ $(TARGETS_ROOTFS): target-finalize
 # Avoid the rootfs name leaking down the dependency chain
 target-finalize: ROOTFS=
 
-host-finalize: $(HOST_DIR_SYMLINK)
+.PHONY: host-finalize
+host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
+	@$(call MESSAGE,"Finalizing host directory")
+	$(call per-package-rsync,$(sort $(PACKAGES)),host,$(HOST_DIR))
 
 .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
 	@$(call MESSAGE,"Finalizing target directory")
+	$(call per-package-rsync,$(sort $(PACKAGES)),target,$(TARGET_DIR))
 	# Check files that are touched by more than one package
 	./support/scripts/check-uniq-files -t target $(BUILD_DIR)/packages-file-list.txt
 	./support/scripts/check-uniq-files -t staging $(BUILD_DIR)/packages-file-list-staging.txt
@@ -765,7 +771,7 @@ endif
 	ln -sf ../usr/lib/os-release $(TARGET_DIR)/etc
 
 	@$(call MESSAGE,"Sanitizing RPATH in target tree")
-	$(TOPDIR)/support/scripts/fix-rpath target
+	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath target
 
 	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
 		$(call MESSAGE,"Copying overlay $(d)"); \
@@ -973,7 +979,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
@@ -1012,7 +1019,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 f5cab2b9c2..8ea86514d7 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))
@@ -330,6 +333,7 @@ $(BUILD_DIR)/%/.stamp_target_installed:
 
 # Remove package sources
 $(BUILD_DIR)/%/.stamp_dircleaned:
+	$(if $(BR2_PER_PACKAGE_DIRECTORIES),rm -Rf $(PER_PACKAGE_DIR)/$(NAME))
 	rm -Rf $(@D)
 
 ################################################################################
@@ -886,6 +890,7 @@ $$($(2)_TARGET_SOURCE):			PKGDIR=$(pkgdir)
 $$($(2)_TARGET_ACTUAL_SOURCE):		PKG=$(2)
 $$($(2)_TARGET_ACTUAL_SOURCE):		PKGDIR=$(pkgdir)
 $$($(2)_TARGET_DIRCLEAN):		PKG=$(2)
+$$($(2)_TARGET_DIRCLEAN):		NAME=$(1)
 
 # Compute the name of the Kconfig option that correspond to the
 # package being enabled. We handle three cases: the special Linux
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index bffd79dfb0..34345a0d65 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -62,6 +62,32 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
 endif
 endef
 
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+# rsync the contents of per-package directories
+# $1: space-separated list of packages to rsync from
+# $2: 'host' or 'target'
+# $3: destination directory
+define per-package-rsync
+	mkdir -p $(3)
+	$(foreach pkg,$(1),\
+		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
+		$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
+		$(3)$(sep))
+endef
+
+# 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 packages to rsync from
+define prepare-per-package-directory
+	$(call per-package-rsync,$(1),host,$(HOST_DIR))
+	$(call per-package-rsync,$(1),target,$(TARGET_DIR))
+endef
+endif
+
 #
 # legal-info helper functions
 #
diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index c8939569e2..d903a82958 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"
@@ -44,6 +45,15 @@ is_elf() {
 # needs such an RPATH if at least of the libraries used by the ELF
 # executable is available in the host library directory. This function
 # returns 0 when a RPATH is needed, 1 otherwise.
+#
+# With per-package directory support, ${hostdir} will point to the
+# current package per-package host directory, and this is where this
+# function will check if the libraries needed by the executable are
+# located (or not). In practice, the ELF executable RPATH may point to
+# another package per-package host directory, but that is fine because
+# if such an executable is within the current package per-package host
+# directory, its libraries will also have been copied into the current
+# package per-package host directory.
 elf_needs_rpath() {
     local file="${1}"
     local hostdir="${2}"
@@ -62,13 +72,19 @@ elf_needs_rpath() {
 # This function checks whether at least one of the RPATH of the given
 # ELF executable (first argument) properly points to the host library
 # directory (second argument), either through an absolute RPATH or a
-# relative RPATH. Having such a RPATH will make sure the ELF
-# executable will find at runtime the shared libraries it depends
-# on. This function returns 0 when a proper RPATH was found, or 1
-# otherwise.
+# relative RPATH. In the context of per-package directory support,
+# ${hostdir} (second argument) points to the current package host
+# directory. However, it is perfectly valid for an ELF binary to have
+# a RPATH pointing to another package per-package host directory,
+# which is why such RPATH is also accepted (the per-package directory
+# gets passed as third argument). Having a RPATH pointing to the host
+# directory will make sure the ELF executable will find at runtime the
+# shared libraries it depends on. This function returns 0 when a
+# proper RPATH was found, or 1 otherwise.
 check_elf_has_rpath() {
     local file="${1}"
     local hostdir="${2}"
+    local perpackagedir="${3}"
     local rpath dir
 
     while read rpath; do
@@ -77,6 +93,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' \
diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
index fa138ca15a..926767a54f 100755
--- a/support/scripts/fix-rpath
+++ b/support/scripts/fix-rpath
@@ -127,14 +127,29 @@ 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}")
+
+        # With per-package directory support, most RPATH of host
+        # binaries will point to per-package directories. This won't
+        # work with the --make-rpath-relative ${rootdir} invocation as
+        # the per-package host directory is not within ${rootdir}. So,
+        # we rewrite all RPATHs pointing to per-package directories so
+        # that they point to the global host directry.
+        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
-- 
2.20.1

[Buildroot] [PATCH v7 4/8] 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 | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index e01ec4c963..74c0757fd8 100644
--- a/Makefile
+++ b/Makefile
@@ -228,21 +228,12 @@ 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))
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),)
+# Disable top-level parallel build if per-package directories is not
+# used. Indeed, per-package directories is necessary to guarantee
+# determinism and reproducibility with top-level parallel build.
 .NOTPARALLEL:
+endif
 
 # timezone and locale may affect build output
 ifeq ($(BR2_REPRODUCIBLE),y)
-- 
2.20.1

[Buildroot] [PATCH v7 5/8] 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 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 8ea86514d7..9575639b43 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -126,6 +126,21 @@ endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_user
 endif
 
+#######################################
+# Helper functions
+
+# Make sure .la files only reference the current per-package
+# directory.
+
+# $1: package name (lower case)
+# $2: staging directory of the package
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+define fixup-libtool-files
+	$(Q)find $(2)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
+		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]*/:$(PER_PACKAGE_DIR)/$(1)/:g"
+endef
+endif
+
 ################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
@@ -222,6 +237,7 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call step_start,configure)
 	@$(call MESSAGE,"Configuring")
 	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
+	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
@@ -879,6 +895,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.20.1

[Buildroot] [PATCH v7 6/8] 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>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-kconfig.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index d6c95b897e..281d02de98 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-directory,$$($(2)_KCONFIG_DEPENDENCIES))
 	$$(Q)$$(if $$($(2)_KCONFIG_DEFCONFIG), \
 		$$($(2)_KCONFIG_MAKE) $$($(2)_KCONFIG_DEFCONFIG), \
 		$$(INSTALL) -m 0644 -D $$($(2)_KCONFIG_FILE) $$(@))
-- 
2.20.1

[Buildroot] [PATCH v7 7/8] docs/manual: add details about top-level parallel build support

[Thomas Petazzoni]

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 docs/manual/common-usage.txt        | 44 +++++++++++++++++++++++++++++
 docs/manual/faq-troubleshooting.txt |  3 ++
 docs/manual/quickstart.txt          |  8 +++---
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/docs/manual/common-usage.txt b/docs/manual/common-usage.txt
index e3d7578c85..ebe3238e38 100644
--- a/docs/manual/common-usage.txt
+++ b/docs/manual/common-usage.txt
@@ -329,6 +329,50 @@ Refer to the help text of this script for more details:
 utils/size-stats-compare -h
 ----------------
 
+[[top-level-parallel-build]]
+=== Top-level parallel build
+
+Buildroot has always been capable of using parallel build on a per
+package basis: each package is built by Buildroot using +make -jN+ (or
+the equivalent invocation for non-make-based build systems). The level
+of parallelism is by default number of CPUs + 1, but it can be
+adjusted using the +BR2_JLEVEL+ configuration option.
+
+Until 2019.02, Buildroot was however building packages in a serial
+fashion: each package was built one after the other, without
+parallelization of the build between packages. As of 2019.02,
+Buildroot has experimental support for *top-level parallel build*,
+which allows some signicant build time savings by building packages
+that have no dependency relationship in parallel. This feature is
+however marked as experimental and is known to not work in all
+situations.
+
+In order to use top-level parallel build, one must:
+
+. Enable the option +BR2_PER_PACKAGE_DIRECTORIES+ in the Buildroot
+configuration
+
+. Use +make -jN+ when starting the Buildroot build
+
+Internally, the +BR2_PER_PACKAGE_DIRECTORIES+ will enable a mechanism
+called *per-package directories*, which will have the following
+effects:
+
+* Instead of a global _target_ directory and a global _host_ directory
+  common to all packages, per-package _target_ and _host_ directories
+  will be used, in +$(O)/per-package/<pkg>/target/+ and
+  +$(O)/per-package/<pkg>/host/+ respectively. Those folders will be
+  populated from the corresponding folders of the package dependencies
+  at the beginning of +<pkg>+ build. The compiler and all other tools
+  will therefore only be able to see and access files installed by
+  dependencies explicitly listed by +<pkg>+.
+
+* At the end of the build, the global _target_ and _host_ directories
+  will be populated, located in +$(O)/target+ and +$(O)/host+
+  respectively. This means that during the build, those folders will
+  be empty and it's only at the very end of the build that they will
+  be populated.
+
 include::eclipse-integration.txt[]
 
 include::advanced.txt[]
diff --git a/docs/manual/faq-troubleshooting.txt b/docs/manual/faq-troubleshooting.txt
index b144c9e7f0..5adf3fa6ce 100644
--- a/docs/manual/faq-troubleshooting.txt
+++ b/docs/manual/faq-troubleshooting.txt
@@ -239,3 +239,6 @@ help reduce the build time:
 
  * Buy new hardware. SSDs and lots of RAM are key to speeding up the
    builds.
+
+ * Experiment with top-level parallel build, see
+   xref:top-level-parallel-build[].
diff --git a/docs/manual/quickstart.txt b/docs/manual/quickstart.txt
index 74158ae249..77b73ef116 100644
--- a/docs/manual/quickstart.txt
+++ b/docs/manual/quickstart.txt
@@ -60,10 +60,10 @@ To start the build process, simply run:
  $ make
 --------------------
 
-You *should never* use +make -jN+ with Buildroot: top-level parallel
-make is currently not supported. Instead, use the +BR2_JLEVEL+ option
-to tell Buildroot to run the compilation of each individual package
-with +make -jN+.
+By default, Buildroot does not support top-level parallel build, so
+running +make -jN+ is not necessary. There is however experimental
+support for top-level parallel build, see
+xref:top-level-parallel-build[].
 
 The `make` command will generally perform the following steps:
 
-- 
2.20.1

[Buildroot] [PATCH v7 8/8] docs/manual: document the effect of per-package directory on variables

[Thomas Petazzoni]

As suggested by Arnout Vandecappelle, this commit adjusts the
generic-package documentation to document the effect of per-package
directory support on HOST_DIR, STAGING_DIR and TARGET_DIR.

Suggested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 docs/manual/adding-packages-generic.txt | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 7be1754f54..4a991140a8 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -578,6 +578,13 @@ In the action definitions, you can use the following variables:
 * +$(TARGET_CROSS)+ to get the cross-compilation toolchain prefix
 
 * Of course the +$(HOST_DIR)+, +$(STAGING_DIR)+ and +$(TARGET_DIR)+
-  variables to install the packages properly.
+  variables to install the packages properly. Those variables point to
+  the global _host_, _staging_ and _target_ directories, unless
+  _per-package directory_ support is used, in which case they point to
+  the current package _host_, _staging_ and _target_ directories. In
+  both cases, it doesn't make any difference from the package point of
+  view: it should simply use +HOST_DIR+, +STAGING_DIR+ and
+  +TARGET_DIR+. See xref:top-level-parallel-build[] for more details
+  about _per-package directory_ support.
 
 Finally, you can also use hooks. See xref:hooks[] for more information.
-- 
2.20.1

[Buildroot] [PATCH v7 1/8] support/scripts/check-host-rpath: document existing functions

[Yann E. MORIN]

Thomas, All,

On 2018-12-28 11:43 +0100, Thomas Petazzoni spake thusly:
> As suggested by Arnout Vandecappelle, let's document the
> elf_needs_rpath() and check_elf_has_rpath() functions, before we make
> them a bit more complicated with per-package directory support.
> 
> Suggested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  support/scripts/check-host-rpath | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> index 6c5767da05..c8939569e2 100755
> --- a/support/scripts/check-host-rpath
> +++ b/support/scripts/check-host-rpath
> @@ -39,6 +39,11 @@ is_elf() {
>      |grep -E 'Requesting program interpreter:' >/dev/null 2>&1
>  }
>  
> +# This function tells whether a given ELF executable (first argument)
> +# needs a RPATH pointing to the host library directory or not. It
> +# needs such an RPATH if at least of the libraries used by the ELF
                                   ^^^
Missing 'one', i.e.:  ... at least one of the libraries...

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

Regards,
Yann E. MORIN.

> +# executable is available in the host library directory. This function
> +# returns 0 when a RPATH is needed, 1 otherwise.
>  elf_needs_rpath() {
>      local file="${1}"
>      local hostdir="${2}"
> @@ -54,6 +59,13 @@ elf_needs_rpath() {
>      return 1
>  }
>  
> +# This function checks whether at least one of the RPATH of the given
> +# ELF executable (first argument) properly points to the host library
> +# directory (second argument), either through an absolute RPATH or a
> +# relative RPATH. Having such a RPATH will make sure the ELF
> +# executable will find at runtime the shared libraries it depends
> +# on. This function returns 0 when a proper RPATH was found, or 1
> +# otherwise.
>  check_elf_has_rpath() {
>      local file="${1}"
>      local hostdir="${2}"
> -- 
> 2.20.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 v7 2/8] Makefile: move definition of TARGET_DIR inside .config condition

[Yann E. MORIN]

Thomas, All,

On 2018-12-28 11:43 +0100, Thomas Petazzoni spake thusly:
> In a follow-up commit introducing per-package directory support, we
> will need to define TARGET_DIR in a different way depending on the
> value of a Config.in option. To make this possible, the definition of
> TARGET_DIR should be moved inside the BR2_HAVE_DOT_CONFIG condition.
> 
> We have verified that $(TARGET_DIR) is only used within the
> BR2_HAVE_DOT_CONFIG condition. Outside of this condition, such as in
> the "clean" target, $(BASE_TARGET_DIR) is used.
> 
> Suggested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

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

Regards,
Yann E. MORIN.

> ---
>  Makefile | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c5b78b3274..9de8e0c725 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -204,10 +204,7 @@ BR_GRAPH_OUT := $(or $(BR2_GRAPH_OUT),pdf)
>  
>  BUILD_DIR := $(BASE_DIR)/build
>  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))
>  # 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
> @@ -457,6 +454,10 @@ TAR_OPTIONS = $(call qstrip,$(BR2_TAR_OPTIONS)) -xf
>  # packages compiled for the host go here
>  HOST_DIR := $(call qstrip,$(BR2_HOST_DIR))
>  
> +# The target directory is common to all packages,
> +# but there is one that is specific to each filesystem.
> +TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(BASE_TARGET_DIR))
> +
>  ifneq ($(HOST_DIR),$(BASE_DIR)/host)
>  HOST_DIR_SYMLINK = $(BASE_DIR)/host
>  $(HOST_DIR_SYMLINK): $(BASE_DIR)
> -- 
> 2.20.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 v7 4/8] Makefile: allow top-level parallel build with BR2_PER_PACKAGE_DIRECTORIES=y

[Yann E. MORIN]

Thomas, All,

On 2018-12-28 11:43 +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>

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

Regards,
Yann E. MORIN.

> ---
>  Makefile | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index e01ec4c963..74c0757fd8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -228,21 +228,12 @@ 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))
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),)
> +# Disable top-level parallel build if per-package directories is not
> +# used. Indeed, per-package directories is necessary to guarantee
> +# determinism and reproducibility with top-level parallel build.
>  .NOTPARALLEL:
> +endif
>  
>  # timezone and locale may affect build output
>  ifeq ($(BR2_REPRODUCIBLE),y)
> -- 
> 2.20.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 v7 7/8] docs/manual: add details about top-level parallel build support

[Yann E. MORIN]

Thomas, All,

On 2018-12-28 11:43 +0100, Thomas Petazzoni spake thusly:
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  docs/manual/common-usage.txt        | 44 +++++++++++++++++++++++++++++
>  docs/manual/faq-troubleshooting.txt |  3 ++
>  docs/manual/quickstart.txt          |  8 +++---
>  3 files changed, 51 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/manual/common-usage.txt b/docs/manual/common-usage.txt
> index e3d7578c85..ebe3238e38 100644
> --- a/docs/manual/common-usage.txt
> +++ b/docs/manual/common-usage.txt
> @@ -329,6 +329,50 @@ Refer to the help text of this script for more details:
>  utils/size-stats-compare -h
>  ----------------
>  
> +[[top-level-parallel-build]]
> +=== Top-level parallel build

I would really add a big-fat warning that this is still experimental:

    .Note:
    This section deals with a very experimental feature, which is known
    to break even in some non-unusual situations. Use at your own risk.

> +Buildroot has always been capable of using parallel build on a per
> +package basis: each package is built by Buildroot using +make -jN+ (or
> +the equivalent invocation for non-make-based build systems). The level
> +of parallelism is by default number of CPUs + 1, but it can be
> +adjusted using the +BR2_JLEVEL+ configuration option.
> +
> +Until 2019.02, Buildroot was however building packages in a serial
> +fashion: each package was built one after the other, without
> +parallelization of the build between packages. As of 2019.02,
> +Buildroot has experimental support for *top-level parallel build*,
> +which allows some signicant build time savings by building packages
> +that have no dependency relationship in parallel. This feature is
> +however marked as experimental and is known to not work in all
> +situations.

... is know not to work in some cases.

(otherwise, the way you wrote it means it never works).

Besides, it would be interesting to list at least a few cases where it
is known to break, e.g.: qt5 packages...

> +In order to use top-level parallel build, one must:
> +
> +. Enable the option +BR2_PER_PACKAGE_DIRECTORIES+ in the Buildroot
> +configuration
> +
> +. Use +make -jN+ when starting the Buildroot build
> +
> +Internally, the +BR2_PER_PACKAGE_DIRECTORIES+ will enable a mechanism
> +called *per-package directories*, which will have the following
> +effects:
> +
> +* Instead of a global _target_ directory and a global _host_ directory
> +  common to all packages, per-package _target_ and _host_ directories
> +  will be used, in +$(O)/per-package/<pkg>/target/+ and
> +  +$(O)/per-package/<pkg>/host/+ respectively. Those folders will be

s/folders/directories/  and elsewhere, too...

> +  populated from the corresponding folders of the package dependencies
> +  at the beginning of +<pkg>+ build. The compiler and all other tools
> +  will therefore only be able to see and access files installed by
> +  dependencies explicitly listed by +<pkg>+.
> +
> +* At the end of the build, the global _target_ and _host_ directories
> +  will be populated, located in +$(O)/target+ and +$(O)/host+
> +  respectively. This means that during the build, those folders will
> +  be empty and it's only at the very end of the build that they will
> +  be populated.
> +
>  include::eclipse-integration.txt[]
>  
>  include::advanced.txt[]
> diff --git a/docs/manual/faq-troubleshooting.txt b/docs/manual/faq-troubleshooting.txt
> index b144c9e7f0..5adf3fa6ce 100644
> --- a/docs/manual/faq-troubleshooting.txt
> +++ b/docs/manual/faq-troubleshooting.txt
> @@ -239,3 +239,6 @@ help reduce the build time:
>  
>   * Buy new hardware. SSDs and lots of RAM are key to speeding up the
>     builds.
> +
> + * Experiment with top-level parallel build, see
> +   xref:top-level-parallel-build[].
> diff --git a/docs/manual/quickstart.txt b/docs/manual/quickstart.txt
> index 74158ae249..77b73ef116 100644
> --- a/docs/manual/quickstart.txt
> +++ b/docs/manual/quickstart.txt
> @@ -60,10 +60,10 @@ To start the build process, simply run:
>   $ make
>  --------------------
>  
> -You *should never* use +make -jN+ with Buildroot: top-level parallel
> -make is currently not supported. Instead, use the +BR2_JLEVEL+ option
> -to tell Buildroot to run the compilation of each individual package
> -with +make -jN+.
> +By default, Buildroot does not support top-level parallel build, so
> +running +make -jN+ is not necessary. There is however experimental
> +support for top-level parallel build, see
> +xref:top-level-parallel-build[].

What about:

    Buildroot does not officially support top-level parallel build,
    so you should not use +make -jN+ when calling Buildroot. However,
    there is *experimental* support for top-level parallel build, see
    xref:top-level-parallel-build[].

Regards,
Yann E. MORIN.

>  The `make` command will generally perform the following steps:
>  
> -- 
> 2.20.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 v7 7/8] docs/manual: add details about top-level parallel build support

[Thomas Petazzoni]

Hello,

Thanks for the review. I obviously agree on all points, except one,
which I'm discussing below.

On Fri, 28 Dec 2018 14:03:27 +0100, Yann E. MORIN wrote:

> Besides, it would be interesting to list at least a few cases where it
> is known to break, e.g.: qt5 packages...

I'm not sure this is really useful to do, because the list of things
that are known to not work is going to vary a lot over time.

With just the base patch series I posted, a number of things is
breaking, but I will be submitting separately fixes for Meson packages,
for Python packages, and for various random other packages that need
some minor fixups. Hence keeping a list of what works/breaks in the
manual is going to be really annoying to maintain, and I prefer to keep
things simple and say "the feature is experimental, some things may be
broken" until the point where we are confident enough with the feature
to drop the "experimental" mark.

What do you think?

Best regards,

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

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Thomas Petazzoni]

Hello,

On Fri, 28 Dec 2018 11:43:27 +0100, Thomas Petazzoni wrote:

> Here is a seventh iteration of the per-package SDK and target
> directory implementation, offering top-level parallel build
> capability.

Regarding top-level parallel build, I prepared this Wiki page that
documents the progress of the effort:

  https://elinux.org/Buildroot:Top_Level_Parallel_Build

Especially, I'd like to make it clear: this patch series is only the
*core* support for per-package directories. There are a number of fixes
needed for Meson, Python and more to make it usable with some packages.
I wanted to keep this patch series reasonable in size by only
submitting the core support.

If you want to do some testing, please use
https://git.bootlin.com/users/thomas-petazzoni/buildroot/log/?h=ppsh-v7-work
which has all the fixes I currently have.

Best regards,

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

[Buildroot] [PATCH v7 3/8] core: implement per-package SDK and target

[Yann E. MORIN]

Thomas, All,

I don;t have much to propose asa review, except very-minor issues.

On 2018-12-28 11:43 +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.
> 
> There are two main benefits:
> 
>  - Packages will no longer discover dependencies that they do not
>    explicitly indicate in their <pkg>_DEPENDENCIES variable.

Note that non-expressed dependencies may still be gathered, if they are
transitive dependencies.

[--SNIP--]
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
[--SNIP--]
> diff --git a/Makefile b/Makefile
> index 9de8e0c725..e01ec4c963 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -205,6 +205,7 @@ BR_GRAPH_OUT := $(or $(BR2_GRAPH_OUT),pdf)
>  BUILD_DIR := $(BASE_DIR)/build
>  BINARIES_DIR := $(BASE_DIR)/images
>  BASE_TARGET_DIR := $(BASE_DIR)/target
> +PER_PACKAGE_DIR := $(BASE_DIR)/per-package

Why don't you simply export this variable, like HOST_DIR and TARGET_DIR?
This would simplify calls to fix-rpath:

[--SNIP--]
> @@ -585,8 +587,8 @@ world: target-post-image
>  .PHONY: prepare-sdk
>  prepare-sdk: world
>  	@$(call MESSAGE,"Rendering the SDK relocatable")
> -	$(TOPDIR)/support/scripts/fix-rpath host
> -	$(TOPDIR)/support/scripts/fix-rpath staging
> +	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath host
> +	PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath staging

... here.

[--SNIP--]
> @@ -973,7 +979,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):

Don't ident this continuation line, it is misleading: the target of
the rule appear to be part of the commands.

[--SNIP--]
> diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> index c8939569e2..d903a82958 100755
> --- a/support/scripts/check-host-rpath
> +++ b/support/scripts/check-host-rpath

As I was saying on IRC: when we are not using per-package directories,
then I was a bit surprised to see that this script (and fix_rpath,
below) still worked, when the support for PPD is not optional in it.

What confused me, was that PER_PACKAGE_DIR is always used, even when
BR2_PER_PACKAGE_DIRECTORIES is unset.

But as you pointed to on IRC, PER_PACKAGE_DIR is also always defined to
an non-empty string, that either points to a valid location when
BR2_PER_PACKAGE_DIRECTORIES=y, or points to a non-existent location
otherwise.

> @@ -77,6 +93,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
                                            ^^^^^^^
That would also match the string '//' so maybe we want:

    ${perpackagedir}/([^/]+/)?host/lib

[--SNIP--]
> diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
> index fa138ca15a..926767a54f 100755
> --- a/support/scripts/fix-rpath
> +++ b/support/scripts/fix-rpath
> @@ -127,14 +127,29 @@ 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}")
> +
> +        # With per-package directory support, most RPATH of host
> +        # binaries will point to per-package directories. This won't
> +        # work with the --make-rpath-relative ${rootdir} invocation as
> +        # the per-package host directory is not within ${rootdir}. So,
> +        # we rewrite all RPATHs pointing to per-package directories so
> +        # that they point to the global host directry.
> +        changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]*/host@${HOST_DIR}@")

Ditto?

> +        if test "${rpath}" != "${changed_rpath}" ; then
> +            ${PATCHELF} --set-rpath ${changed_rpath} "${file}"

Can't you do that unconditioanlly? If it's changed, we need to set it;
if it's not changed, that set it to the initial value anyway...

> +
> +        # 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
> -- 
> 2.20.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 v7 5/8] package/pkg-generic: make libtool .la files compatible with per-package directories

[Yann E. MORIN]

Thomas, All,

On 2018-12-28 11:43 +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>
> ---
>  package/pkg-generic.mk | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 8ea86514d7..9575639b43 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -126,6 +126,21 @@ endef
>  GLOBAL_INSTRUMENTATION_HOOKS += step_user
>  endif
>  
> +#######################################
> +# Helper functions
> +
> +# Make sure .la files only reference the current per-package
> +# directory.
> +
> +# $1: package name (lower case)
> +# $2: staging directory of the package
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> +define fixup-libtool-files
> +	$(Q)find $(2)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
> +		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]*/:$(PER_PACKAGE_DIR)/$(1)/:g"
                                    ^^^^^^^
This can match the string '//'. Don't you in fact want to match at least
one char? I.e.:  $(PER_PACKAGE_DIR)/[^/]+/

> +endef
> +endif
> +
>  ################################################################################
>  # Implicit targets -- produce a stamp file for each step of a package build
>  ################################################################################
> @@ -222,6 +237,7 @@ $(BUILD_DIR)/%/.stamp_configured:
>  	@$(call step_start,configure)
>  	@$(call MESSAGE,"Configuring")
>  	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
> +	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))

At some point, we may have to turn that into a generic hook-registration
mechanism.

For example, in inner-generic-package:

    $(2)_PRE_CONFIGURE_HOOKS_FINAL = \
        PREPARE_PER_PACKAGE_DIRECTORY \
        FIXUP_LIBTOOL_FILES \
        $($(2)_PRECONFIGURE_HOOKS)

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

... and here, we'd use $($(PKG)_PRE_CONFIGURE_HOOKS_FINAL).

Similarly for the other hooks, of course...

But that's for the future! ;-)

Regards,
Yann E. MORIN.

>  	$($(PKG)_CONFIGURE_CMDS)
>  	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
> @@ -879,6 +895,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.20.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 v7 7/8] docs/manual: add details about top-level parallel build support

[Yann E. MORIN]

Thomas, All,

On 2018-12-28 14:08 +0100, Thomas Petazzoni spake thusly:
> On Fri, 28 Dec 2018 14:03:27 +0100, Yann E. MORIN wrote:
> > Besides, it would be interesting to list at least a few cases where it
> > is known to break, e.g.: qt5 packages...
> I'm not sure this is really useful to do, because the list of things
> that are known to not work is going to vary a lot over time.
[--SNIP--]
> What do you think?

OK for me! :-)

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 v7 3/8] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello,

Thanks for the review!

On Sun, 30 Dec 2018 22:52:12 +0100, Yann E. MORIN wrote:

> I don;t have much to propose asa review, except very-minor issues.

Which is good :-) Hopefully I can get some Reviewed-by soon, and merge
this series.

> > There are two main benefits:
> > 
> >  - Packages will no longer discover dependencies that they do not
> >    explicitly indicate in their <pkg>_DEPENDENCIES variable.  
> 
> Note that non-expressed dependencies may still be gathered, if they are
> transitive dependencies.

Yes, of course, but what can we do about this ? It's pretty logical and
obvious that transitive dependencies will be copied, no ?

> > +PER_PACKAGE_DIR := $(BASE_DIR)/per-package  
> 
> Why don't you simply export this variable, like HOST_DIR and TARGET_DIR?
> This would simplify calls to fix-rpath:

Because I generally don't like those global exports. It pollutes the
namespace with some random variable that is really internal to
Buildroot. There is no reason for the build system of all packages to
even see this variable. In fact, I am personally not a big fan of
exporting TARGET_DIR, STAGING_DIR, etc. since they become visible in
packages, and people tend to use them in their package build system,
which is very wrong.

Of course, if the overall consensus is that PER_PACKAGE_DIR should be
exported, I'll do so because I don't want to hold this series just for
this detail.


> > -$(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):  
> 
> Don't ident this continuation line, it is misleading: the target of
> the rule appear to be part of the commands.

ACK.

> [--SNIP--]
> > diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> > index c8939569e2..d903a82958 100755
> > --- a/support/scripts/check-host-rpath
> > +++ b/support/scripts/check-host-rpath  
> 
> As I was saying on IRC: when we are not using per-package directories,
> then I was a bit surprised to see that this script (and fix_rpath,
> below) still worked, when the support for PPD is not optional in it.
> 
> What confused me, was that PER_PACKAGE_DIR is always used, even when
> BR2_PER_PACKAGE_DIRECTORIES is unset.
> 
> But as you pointed to on IRC, PER_PACKAGE_DIR is also always defined to
> an non-empty string, that either points to a valid location when
> BR2_PER_PACKAGE_DIRECTORIES=y, or points to a non-existent location
> otherwise.

Yes, I'll add some comments about this.

> 
> > @@ -77,6 +93,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  
>                                             ^^^^^^^
> That would also match the string '//' so maybe we want:
> 
>     ${perpackagedir}/([^/]+/)?host/lib

I'm not sure to understand the ? in ([^/]+/)?. We definitely want one
path component between $(PER_PACKAGE_DIR) and host/lib. So what about:

	${perpackagedir}/[^/]+/host/lib

> > +        if test "${rpath}" != "${changed_rpath}" ; then
> > +            ${PATCHELF} --set-rpath ${changed_rpath} "${file}"  
> 
> Can't you do that unconditioanlly? If it's changed, we need to set it;
> if it's not changed, that set it to the initial value anyway...

There is a reason to do it conditionally: patchelf is slow, you don't
want to run patchelf when you don't need to, and here it's trivial to
know whether it is useful or not to run it. For example, this condition
ensures that people not using per-package directory don't pay the cost
of running all those additional patchelf invocations.

Best regards,

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

[Buildroot] [PATCH v7 3/8] core: implement per-package SDK and target

[Yann E. MORIN]

Thomas, All,

On 2018-12-31 15:31 +0100, Thomas Petazzoni spake thusly:
> On Sun, 30 Dec 2018 22:52:12 +0100, Yann E. MORIN wrote:
> > > There are two main benefits:
> > >  - Packages will no longer discover dependencies that they do not
> > >    explicitly indicate in their <pkg>_DEPENDENCIES variable.  
> > Note that non-expressed dependencies may still be gathered, if they are
> > transitive dependencies.
> Yes, of course, but what can we do about this ?

What I mean is, if

  - A needs B and C, but has a dependency only on B,
  - B depends on C,

then the dependency of A to C is fullfilled, even though it is missing
in the dependencies. I.e, it is a _hidden_ dependency. It is not
"explicitly indicate[d] in A_DEPENDENCIES" but still gathered.

And no, there is nothing we can do about it.

> It's pretty logical and
> obvious that transitive dependencies will be copied, no ?

Not as you wrote it.

So, you could rephrase as:

    Packages will now see only the dependencies they explicitly list in
    their <pkg>_DEPENDENCIES variable, and the recursive dependencies
    thereof.

> > > +PER_PACKAGE_DIR := $(BASE_DIR)/per-package  
> > 
> > Why don't you simply export this variable, like HOST_DIR and TARGET_DIR?
> > This would simplify calls to fix-rpath:
> 
> Because I generally don't like those global exports. It pollutes the
> namespace with some random variable that is really internal to
> Buildroot. There is no reason for the build system of all packages to
> even see this variable. In fact, I am personally not a big fan of
> exporting TARGET_DIR, STAGING_DIR, etc. since they become visible in
> packages, and people tend to use them in their package build system,
> which is very wrong.
> 
> Of course, if the overall consensus is that PER_PACKAGE_DIR should be
> exported, I'll do so because I don't want to hold this series just for
> this detail.

Oh, I would tend to agree with you.

I'm just pointing a discrepancy in the way those variables are handled,
and I think it is good to have some consistency, especially in this
difficult topic, even though said consistency's not very nice...

[--SNIP--]
> > > @@ -77,6 +93,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  
> >                                             ^^^^^^^
> > That would also match the string '//' so maybe we want:
> > 
> >     ${perpackagedir}/([^/]+/)?host/lib
> 
> I'm not sure to understand the ? in ([^/]+/)?. We definitely want one
> path component between $(PER_PACKAGE_DIR) and host/lib. So what about:
> 
> 	${perpackagedir}/[^/]+/host/lib

Yes, that is it (and is what I eventually suggested in my review of
patch 5 (about fixing libtool.la files).

> > > +        if test "${rpath}" != "${changed_rpath}" ; then
> > > +            ${PATCHELF} --set-rpath ${changed_rpath} "${file}"  
> > 
> > Can't you do that unconditioanlly? If it's changed, we need to set it;
> > if it's not changed, that set it to the initial value anyway...
> 
> There is a reason to do it conditionally: patchelf is slow, you don't
> want to run patchelf when you don't need to, and here it's trivial to
> know whether it is useful or not to run it. For example, this condition
> ensures that people not using per-package directory don't pay the cost
> of running all those additional patchelf invocations.

OK.

Thanks! :-)

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 v7 3/8] core: implement per-package SDK and target

[Jan Kundrát]

On p?tek 28. prosince 2018 11:43:30 CET, Thomas Petazzoni wrote:
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> +# rsync the contents of per-package directories
> +# $1: space-separated list of packages to rsync from
> +# $2: 'host' or 'target'
> +# $3: destination directory
> +define per-package-rsync
> +	mkdir -p $(3)
> +	$(foreach pkg,$(1),\
> +		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
> +		$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
> +		$(3)$(sep))
> +endef

Hi Thomas,
I gave this series (the ppsh-v7-work merged into master, actually, with a 
revert of 51395b14ed1a23858eef5d7f2bcf3a03cca6dfb3). The only immediate 
breakage I see so far on my config (ARM, systemd, glibc, linaro toolchain) 
is related to systemd-networkd and its /etc/resolv.conf symlink handling, 
but I am afraid that I see a bigger problem.

The basic skeleton defines a symlink (it's in 
system/skeleton/etc/resolv.conf) pointing to ../tmp/resolv.conf . This is 
overwritten in package/systemd/systemd.mk through 
SYSTEMD_INSTALL_RESOLVCONF_HOOK, and indeed it results in a correct symlink 
in the systemd's per-package target dir:

  per-package/systemd/target/etc/resolv.conf -> 
../run/systemd/resolve/resolv.conf

The problem is that at the rsync time, packages are processed in 
alphabetical order. If the very last package to be rsynced (in my case, 
this is zlib) does not transitively depend on systemd, then rsync will 
update the /etc/resolv.conf symlink back to one obtained from the default 
skeleton.

I think that this is -- potentially -- also a problem for any package "P2" 
which calls `ln -sf` from its *_INSTALL_TARGET_HOOKS to overwrite stuff 
which belongs to another package "P1". If any other package "P3" depends on 
"P1" and not on "P2", *and* if P3's name sorts after P2, then the P1's 
symlink gets preserved via P3.

This will not necessary be fixed by changing to do the rsync in a 
dependency order because "P3" can still be rsynced after "P2".

What is the cleanest fix here? Should this symlink overriding go to 
*_TARGET_FINALIZE_HOOKS? If the `ln -sf` was just in TARGET_FINALIZE_HOOKS, 
that would mean that the corresponding per-package/*/target would *not* 
contain these fixes which would be quite confusing, IMHO... OTOH, two hooks 
for overwriting would be ugly.

With kind regards,
Jan

[Buildroot] [PATCH v7 1/8] support/scripts/check-host-rpath: document existing functions

[Peter Korsgaard]

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

 > As suggested by Arnout Vandecappelle, let's document the
 > elf_needs_rpath() and check_elf_has_rpath() functions, before we make
 > them a bit more complicated with per-package directory support.

 > Suggested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Committed after fixing the typo in the comment as pointed out by Yann,
thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v7 2/8] Makefile: move definition of TARGET_DIR inside .config condition

[Peter Korsgaard]

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

 > In a follow-up commit introducing per-package directory support, we
 > will need to define TARGET_DIR in a different way depending on the
 > value of a Config.in option. To make this possible, the definition of
 > TARGET_DIR should be moved inside the BR2_HAVE_DOT_CONFIG condition.

 > We have verified that $(TARGET_DIR) is only used within the
 > BR2_HAVE_DOT_CONFIG condition. Outside of this condition, such as in
 > the "clean" target, $(BASE_TARGET_DIR) is used.

 > Suggested-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Andreas Naumann]

Hi Thomas,

finally I found some time to try this v7 on top of 2019.02-rc1.

Am 28.12.18 um 18:21 schrieb Thomas Petazzoni:
> Hello,
> 
> On Fri, 28 Dec 2018 11:43:27 +0100, Thomas Petazzoni wrote:
> 
>> Here is a seventh iteration of the per-package SDK and target
>> directory implementation, offering top-level parallel build
>> capability.
> 
> Regarding top-level parallel build, I prepared this Wiki page that
> documents the progress of the effort:
> 
>    https://elinux.org/Buildroot:Top_Level_Parallel_Build
> 

My update to the known remaining issues is:
1. I no longer see the ccache problem for the kconfig packages
(uboot+linux)

2. the package-file-list problem has become a blocker since commit
3c8f0d9 core/pkg-infra: restore completeness of packages files lists
with failures like

comm: /home/jenkins/.../build/.files-list-staging.new: No such file or 
directory

Would it be an idea to write the statistics per package :-) inside the 
package build folder and assemble them into one file only once all 
packages are built?

best regards,
Andreas




PS. Actually I just tried flock, which looks promising. If it works I'll 
send a patch next week.

> Especially, I'd like to make it clear: this patch series is only the
> *core* support for per-package directories. There are a number of fixes
> needed for Meson, Python and more to make it usable with some packages.
> I wanted to keep this patch series reasonable in size by only
> submitting the core support.
> 
> If you want to do some testing, please use
> https://git.bootlin.com/users/thomas-petazzoni/buildroot/log/?h=ppsh-v7-work
> which has all the fixes I currently have.
> 
> Best regards,
> 
> Thomas
> 

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Vadim Kochan]

Hi All,

On Fri, Feb 22, 2019 at 6:19 PM Andreas Naumann <dev@andin.de> wrote:
>
> Hi Thomas,
>
> finally I found some time to try this v7 on top of 2019.02-rc1.
>
> Am 28.12.18 um 18:21 schrieb Thomas Petazzoni:
> > Hello,
> >
> > On Fri, 28 Dec 2018 11:43:27 +0100, Thomas Petazzoni wrote:
> >
> >> Here is a seventh iteration of the per-package SDK and target
> >> directory implementation, offering top-level parallel build
> >> capability.
> >
> > Regarding top-level parallel build, I prepared this Wiki page that
> > documents the progress of the effort:
> >
> >    https://elinux.org/Buildroot:Top_Level_Parallel_Build
> >
>
> My update to the known remaining issues is:
> 1. I no longer see the ccache problem for the kconfig packages
> (uboot+linux)
>
> 2. the package-file-list problem has become a blocker since commit
> 3c8f0d9 core/pkg-infra: restore completeness of packages files lists
> with failures like
>
> comm: /home/jenkins/.../build/.files-list-staging.new: No such file or
> directory
>
> Would it be an idea to write the statistics per package :-) inside the
> package build folder and assemble them into one file only once all
> packages are built?
>
> best regards,
> Andreas
>
>
>
>
> PS. Actually I just tried flock, which looks promising. If it works I'll
> send a patch next week.
>

I 'd try it too, but the provided URL does not work :-(

Regards,
Vadim Kochan

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Thomas Petazzoni]

Vadim,

On Fri, 22 Feb 2019 20:07:37 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:

> I 'd try it too, but the provided URL does not work :-(

I updated the Wiki page with URLs on Github.com. We just shut down the
git.bootlin.com service a few days ago, so now my personal Buildroot
Git repo is at https://github.com/tpetazzoni/buildroot/.

I'm looking forward to your feedback!

Best regards,

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

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Vadim Kochan]

Hi Thomas,

On Fri, Feb 22, 2019 at 09:29:12PM +0100, Thomas Petazzoni wrote:
> Vadim,
> 
> On Fri, 22 Feb 2019 20:07:37 +0200
> Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > I 'd try it too, but the provided URL does not work :-(
> 
> I updated the Wiki page with URLs on Github.com. We just shut down the
> git.bootlin.com service a few days ago, so now my personal Buildroot
> Git repo is at https://github.com/tpetazzoni/buildroot/.
> 
> I'm looking forward to your feedback!
> 

I think that in case of per-packages parallel build the logs became
messy and meaningless so I send a patch which allows to do per-package
logging too:

	https://patchwork.ozlabs.org/patch/1047549/

this is a POC so it just shows the idea.

Regards,
Vadim Kochan

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Thomas Petazzoni]

Hello Vadim,

On Mon, 25 Feb 2019 03:10:03 +0200
Vadim Kochan <vadim4j@gmail.com> wrote:

> I think that in case of per-packages parallel build the logs became
> messy and meaningless so I send a patch which allows to do per-package
> logging too:
> 
> 	https://patchwork.ozlabs.org/patch/1047549/
> 
> this is a POC so it just shows the idea.

Thanks for proposing this!

The problem of logging was discussed quite a lot back when I started to
work on per-package directories. I think we investigated overriding the
SHELL variable, but couldn't find an approach that worked properly, but
maybe the approach was different than yours, I need to get back to the
original discussion.

Back then, what we concluded is that people could use the "make
--output-sync=target" option, or "make -Otarget" in short. This option
buffers the output of make on a per-target basis, and displays this
output at once when the target has completed. The advantage is that the
output is no longer garbled, but the drawback is that you don't see
"live" what is happening: if a package takes 20 minutes to build,
during 20 minutes you see nothing, and at the end of the 20 minutes,
you get in one go the full build output of that package.

Best regards,

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

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Vadim Kochan]

Hi Thomas,

On Mon, Feb 25, 2019 at 09:05:43AM +0100, Thomas Petazzoni wrote:
> Hello Vadim,
> 
> On Mon, 25 Feb 2019 03:10:03 +0200
> Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > I think that in case of per-packages parallel build the logs became
> > messy and meaningless so I send a patch which allows to do per-package
> > logging too:
> > 
> > 	https://patchwork.ozlabs.org/patch/1047549/
> > 
> > this is a POC so it just shows the idea.
> 
> Thanks for proposing this!
> 
> The problem of logging was discussed quite a lot back when I started to
> work on per-package directories. I think we investigated overriding the
> SHELL variable, but couldn't find an approach that worked properly, but
> maybe the approach was different than yours, I need to get back to the
> original discussion.
> 
> Back then, what we concluded is that people could use the "make
> --output-sync=target" option, or "make -Otarget" in short. This option
> buffers the output of make on a per-target basis, and displays this
> output at once when the target has completed. The advantage is that the
> output is no longer garbled, but the drawback is that you don't see
> "live" what is happening: if a package takes 20 minutes to build,
> during 20 minutes you see nothing, and at the end of the 20 minutes,
> you get in one go the full build output of that package.
> 

I changed a bit an approach to log output per-package and per-step, so it
solves the issue of refreshing log for particular step by simply
removing the log file at the beginning of step. Actually these steps logs
might be merged into one at the end of make.

Regards,
Vadim Kochan

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Vadym Kochan]

Hi Thomas, All

On Mon, Feb 25, 2019 at 09:05:43AM +0100, Thomas Petazzoni wrote:
> Hello Vadim,
> 
> On Mon, 25 Feb 2019 03:10:03 +0200
> Vadim Kochan <vadim4j@gmail.com> wrote:
> 
> > I think that in case of per-packages parallel build the logs became
> > messy and meaningless so I send a patch which allows to do per-package
> > logging too:
> > 
> > 	https://patchwork.ozlabs.org/patch/1047549/
> > 
> > this is a POC so it just shows the idea.
> 
> Thanks for proposing this!
> 
> The problem of logging was discussed quite a lot back when I started to
> work on per-package directories. I think we investigated overriding the
> SHELL variable, but couldn't find an approach that worked properly, but
> maybe the approach was different than yours, I need to get back to the
> original discussion.
> 
> Back then, what we concluded is that people could use the "make
> --output-sync=target" option, or "make -Otarget" in short. This option
> buffers the output of make on a per-target basis, and displays this
> output at once when the target has completed. The advantage is that the
> output is no longer garbled, but the drawback is that you don't see
> "live" what is happening: if a package takes 20 minutes to build,
> during 20 minutes you see nothing, and at the end of the 20 minutes,
> you get in one go the full build output of that package.
> 
> Best regards,
> 

I updated the patch:

    https://patchwork.ozlabs.org/patch/1047620/

where I added per-package-and-per-step logging, I think it is possible
to merge them info one log file, but per-step approach allows to easy
update logs for perticular step.

But I 'd like to clarify if actually such approach make sense ?
Also how do you think about to add (using the approach from the patch)
prepending logs with a package name prefix (in bold) (not necessary for
parellel building but enabled by config option):

    [${pkg_name}] $log_line

after it may be possible to print the percentage of installed packages
like:

    [${percent}][${pkg_name}] $log_line

ofcourse all this is just for providing to user at runtime the info
which package is building and how many packages (in percentages) are
done. So, may be it looks useless, but I feel it would be nice to have.

Regards,
Vadim Kochan

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Yann E. MORIN]

Vadym, All,

On 2019-03-01 16:50 +0200, Vadym Kochan spake thusly:
> On Mon, Feb 25, 2019 at 09:05:43AM +0100, Thomas Petazzoni wrote:
> > On Mon, 25 Feb 2019 03:10:03 +0200
> > Vadim Kochan <vadim4j@gmail.com> wrote:
> > 
> > > I think that in case of per-packages parallel build the logs became
> > > messy and meaningless so I send a patch which allows to do per-package
> > > logging too:
> > > 
> > > 	https://patchwork.ozlabs.org/patch/1047549/
> > > 
> > > this is a POC so it just shows the idea.
> > The problem of logging was discussed quite a lot back when I started to
> > work on per-package directories. I think we investigated overriding the
> > SHELL variable, but couldn't find an approach that worked properly, but
> > maybe the approach was different than yours, I need to get back to the
> > original discussion.
> > 
> > Back then, what we concluded is that people could use the "make
> > --output-sync=target" option, or "make -Otarget" in short. This option
> > buffers the output of make on a per-target basis, and displays this
> > output at once when the target has completed. The advantage is that the
> > output is no longer garbled, but the drawback is that you don't see
> > "live" what is happening: if a package takes 20 minutes to build,
> > during 20 minutes you see nothing, and at the end of the 20 minutes,
> > you get in one go the full build output of that package.
> 
> I updated the patch:
> 
>     https://patchwork.ozlabs.org/patch/1047620/
> 
> where I added per-package-and-per-step logging, I think it is possible
> to merge them info one log file, but per-step approach allows to easy
> update logs for perticular step.
> 
> But I 'd like to clarify if actually such approach make sense ?

As Thomas said, we already have had some discussion about the logging,
and we also investigated using a shell wrapper, and a PoC similar to
yours was even attempted.

However, we eventually concluded that this was not going to work
reliably, so we decided against at that time. I could not find that
conclusion in the devdays meeting reports, so I guess it's burried
somewhere in the mailing list. I'll try to unearth those later in the
evening or the week-end...

The first issue I can readily remember, was that the SHELL variable is
passed down to all children, and some of them may re-use it to run
commands, and the wrapper would make those fail. I don't have the
details, though, as I said I'll try to dig them from the archives...

But overall, I agree with Thomas, that people that still want nice logs
call 'make -Otarget'.

> Also how do you think about to add (using the approach from the patch)
> prepending logs with a package name prefix (in bold) (not necessary for
> parellel building but enabled by config option):
> 
>     [${pkg_name}] $log_line
> 
> after it may be possible to print the percentage of installed packages
> like:
> 
>     [${percent}][${pkg_name}] $log_line
> 
> ofcourse all this is just for providing to user at runtime the info
> which package is building and how many packages (in percentages) are
> done. So, may be it looks useless, but I feel it would be nice to have.

Sorry, but I think this is really getting too far.

We already have a simple make wrapper, utils/brmake, which filters the
build log and redirects all to a file, and just prints the '>>>' lines
to stdout. I think this is as far as we should go with Buildroot.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Arnout Vandecappelle]

On 01/03/2019 18:18, Yann E. MORIN wrote:
> However, we eventually concluded that this was not going to work
> reliably, so we decided against at that time. I could not find that
> conclusion in the devdays meeting reports, so I guess it's burried
> somewhere in the mailing list. I'll try to unearth those later in the
> evening or the week-end...

 If I remember correct, didn't GNU make bypass the SHELL variable for simple
commands?

 Regards,
 Arnout

[Buildroot] [PATCH v7 0/8] Top-level parallel build support

[Thomas Petazzoni]

On Mon, 4 Mar 2019 08:24:33 +0100
Arnout Vandecappelle <arnout@mind.be> wrote:

> On 01/03/2019 18:18, Yann E. MORIN wrote:
> > However, we eventually concluded that this was not going to work
> > reliably, so we decided against at that time. I could not find that
> > conclusion in the devdays meeting reports, so I guess it's burried
> > somewhere in the mailing list. I'll try to unearth those later in the
> > evening or the week-end...  
> 
>  If I remember correct, didn't GNU make bypass the SHELL variable for simple
> commands?

Aah, yes, you have good memory, this definitely rings a bell.

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

[Buildroot] [PATCH v7 3/8] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello Jan,

Yes, some feedback on an almost year old comment!

On Tue, 08 Jan 2019 19:02:56 +0100
Jan Kundr?t <jan.kundrat@cesnet.cz> wrote:

> I gave this series (the ppsh-v7-work merged into master, actually, with a 
> revert of 51395b14ed1a23858eef5d7f2bcf3a03cca6dfb3). The only immediate 
> breakage I see so far on my config (ARM, systemd, glibc, linaro toolchain) 
> is related to systemd-networkd and its /etc/resolv.conf symlink handling, 
> but I am afraid that I see a bigger problem.
> 
> The basic skeleton defines a symlink (it's in 
> system/skeleton/etc/resolv.conf) pointing to ../tmp/resolv.conf . This is 
> overwritten in package/systemd/systemd.mk through 
> SYSTEMD_INSTALL_RESOLVCONF_HOOK, and indeed it results in a correct symlink 
> in the systemd's per-package target dir:
> 
>   per-package/systemd/target/etc/resolv.conf -> 
> ../run/systemd/resolve/resolv.conf
> 
> The problem is that at the rsync time, packages are processed in 
> alphabetical order. If the very last package to be rsynced (in my case, 
> this is zlib) does not transitively depend on systemd, then rsync will 
> update the /etc/resolv.conf symlink back to one obtained from the default 
> skeleton.

Indeed. Clearly, we have identified since quite a while that the
per-package directory mechanism only works if all packages install
distinct set of files, unless they have a dependency relationship.

> I think that this is -- potentially -- also a problem for any package "P2" 
> which calls `ln -sf` from its *_INSTALL_TARGET_HOOKS to overwrite stuff 
> which belongs to another package "P1". If any other package "P3" depends on 
> "P1" and not on "P2", *and* if P3's name sorts after P2, then the P1's 
> symlink gets preserved via P3.
> 
> This will not necessary be fixed by changing to do the rsync in a 
> dependency order because "P3" can still be rsynced after "P2".
> 
> What is the cleanest fix here? Should this symlink overriding go to 
> *_TARGET_FINALIZE_HOOKS? If the `ln -sf` was just in TARGET_FINALIZE_HOOKS, 
> that would mean that the corresponding per-package/*/target would *not* 
> contain these fixes which would be quite confusing, IMHO... OTOH, two hooks 
> for overwriting would be ugly.

For the case of resolv.conf, I think solving it through a
TARGET_FINALIZE_HOOKS is probably the most appropriate option for now.
We'll have to see later on if we have other cases like this, and if
there is a pattern that allows us to invent some useful bit of
additional infrastructure.

Or we could:

 (1) Move the /etc/resolv.conf symlink from system/skeleton/ to
     package/skeleton-init-sysv/skeleton/etc.

 (2) Tweak the systemd.mk so that it always creates resolv.conf, either
     pointing to ../run/systemd/resolve/resolv.conf when resolved is
     enabled, or to /tmp/resolv.conf otherwise.

This would avoid the overwriting entirely.

What do you think ?

Best regards,

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

[Buildroot] [PATCH v7 3/8] core: implement per-package SDK and target

[Carlos Santos]

On Tue, Nov 5, 2019 at 1:38 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Jan,
>
> Yes, some feedback on an almost year old comment!
>
> On Tue, 08 Jan 2019 19:02:56 +0100
> Jan Kundr?t <jan.kundrat@cesnet.cz> wrote:
>
> > I gave this series (the ppsh-v7-work merged into master, actually, with a
> > revert of 51395b14ed1a23858eef5d7f2bcf3a03cca6dfb3). The only immediate
> > breakage I see so far on my config (ARM, systemd, glibc, linaro toolchain)
> > is related to systemd-networkd and its /etc/resolv.conf symlink handling,
> > but I am afraid that I see a bigger problem.
> >
> > The basic skeleton defines a symlink (it's in
> > system/skeleton/etc/resolv.conf) pointing to ../tmp/resolv.conf . This is
> > overwritten in package/systemd/systemd.mk through
> > SYSTEMD_INSTALL_RESOLVCONF_HOOK, and indeed it results in a correct symlink
> > in the systemd's per-package target dir:
> >
> >   per-package/systemd/target/etc/resolv.conf ->
> > ../run/systemd/resolve/resolv.conf
> >
> > The problem is that at the rsync time, packages are processed in
> > alphabetical order. If the very last package to be rsynced (in my case,
> > this is zlib) does not transitively depend on systemd, then rsync will
> > update the /etc/resolv.conf symlink back to one obtained from the default
> > skeleton.
>
> Indeed. Clearly, we have identified since quite a while that the
> per-package directory mechanism only works if all packages install
> distinct set of files, unless they have a dependency relationship.
>
> > I think that this is -- potentially -- also a problem for any package "P2"
> > which calls `ln -sf` from its *_INSTALL_TARGET_HOOKS to overwrite stuff
> > which belongs to another package "P1". If any other package "P3" depends on
> > "P1" and not on "P2", *and* if P3's name sorts after P2, then the P1's
> > symlink gets preserved via P3.
> >
> > This will not necessary be fixed by changing to do the rsync in a
> > dependency order because "P3" can still be rsynced after "P2".
> >
> > What is the cleanest fix here? Should this symlink overriding go to
> > *_TARGET_FINALIZE_HOOKS? If the `ln -sf` was just in TARGET_FINALIZE_HOOKS,
> > that would mean that the corresponding per-package/*/target would *not*
> > contain these fixes which would be quite confusing, IMHO... OTOH, two hooks
> > for overwriting would be ugly.
>
> For the case of resolv.conf, I think solving it through a
> TARGET_FINALIZE_HOOKS is probably the most appropriate option for now.
> We'll have to see later on if we have other cases like this, and if
> there is a pattern that allows us to invent some useful bit of
> additional infrastructure.
>
> Or we could:
>
>  (1) Move the /etc/resolv.conf symlink from system/skeleton/ to
>      package/skeleton-init-sysv/skeleton/etc.
>
>  (2) Tweak the systemd.mk so that it always creates resolv.conf, either
>      pointing to ../run/systemd/resolve/resolv.conf when resolved is
>      enabled, or to /tmp/resolv.conf otherwise.
>
> This would avoid the overwriting entirely.
>
> What do you think ?
>
> Best regards,
>
> Thomas

This would prevent NetworkManager from updating resolv.conf. See
https://bugs.busybox.net/show_bug.cgi?id=9881

-- 
Carlos Santos <unixmania@gmail.com>

[Buildroot] [PATCH v7 3/8] core: implement per-package SDK and target

[Thomas Petazzoni]

Hello,

On Tue, 5 Nov 2019 16:05:50 -0300
Carlos Santos <unixmania@gmail.com> wrote:

> > Or we could:
> >
> >  (1) Move the /etc/resolv.conf symlink from system/skeleton/ to
> >      package/skeleton-init-sysv/skeleton/etc.
> >
> >  (2) Tweak the systemd.mk so that it always creates resolv.conf, either
> >      pointing to ../run/systemd/resolve/resolv.conf when resolved is
> >      enabled, or to /tmp/resolv.conf otherwise.
> >
> > This would avoid the overwriting entirely.
> >
> > What do you think ?
> >
> > Best regards,
> >
> This would prevent NetworkManager from updating resolv.conf. See
> https://bugs.busybox.net/show_bug.cgi?id=9881

Then I guess the TARGET_FINALIZE_HOOKS is the only solution here.

But thinking more about this issue, it really means that no package
should overwrite any file installed by one of its dependencies: this
means that the check-uniq-files stuff that Yann recently removed was in
fact useful.

Indeed, if we have the following scenario:

 - Package A installs file /foo/bar

 - Package B depends on A, and overwrites /foo/bar with some contents

 - Package C depends on A

Then, at the end in the target root filesystem, we will have A's
version of /foo/bar, and not B's version. Just like Jan explained, the
final rsync will rsync the per-package target directories in this order
(due to alphabetic ordering):

 A, B, C

So, in the end, it is the version of /foo/bar in the per-package
directory of C that wins, and because C depends on A, the version of
/foo/bar in C's per-package directory is the one coming from package A.

So, it is really important for packages *not* to overwrite any file
installed by any other package, even if said package is in its
dependencies.

Am I missing something here ?

Best regards,

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

[Buildroot] [PATCH v7 3/8] core: implement per-package SDK and target

[Jan Kundrát]

> Then I guess the TARGET_FINALIZE_HOOKS is the only solution here.

Yes, this sounds reasonable.

> But thinking more about this issue, it really means that no package
> should overwrite any file installed by one of its dependencies:

Agreed. Of course there are still scenarios where packages have to 
overwrite stuff which belongs to something else (think of packages 
"registering themselves" within a file managed by a package they depend 
on). These can be solved by explicitly configuring these packages so that 
this conflicting resource exists in just one location, i.e., outside of the 
per-package namespace.

I've been doing this with (still out-of-tree) sysrepo etc like this:

https://gerrit.cesnet.cz/plugins/gitiles/github/buildroot/buildroot/+/refs/heads/cesnet/2019-06-10/package/sysrepo/sysrepo.mk#41

How does merging the ppsh work look?

With kind regards,
Jan