All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support
@ 2018-11-14 10:55 Thomas Petazzoni
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 1/6] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use Thomas Petazzoni
                   ` (8 more replies)
  0 siblings, 9 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-14 10:55 UTC (permalink / raw)
  To: buildroot

Hello,

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

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

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

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

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

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

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

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

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

 - Rebased on top of the latest master.

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

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

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

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

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

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

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

   See below for more details.

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

 - Rebased on the latest Buildroot next branch

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

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

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

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

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

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

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

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

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

The main problem that remains is fixing packages that don't work well
with per-package folders. I have already sent a few fixes (such as
adding missing host-pkgconf dependencies, or fixing the freetype
package), but I have setup a separate autobuilder instance to test
this, and it uncovered a number of other problematic packages:

 - apr-util doesn't build, because some hardcoded paths from the "apr"
   per-package folders leak into the apr-util build.

 - Meson packages currently do not build, because pkg-meson.mk doesn't
   use $$(HOST_DIR) to reference cross-compilation.conf, but more
   importantly because cross-compilation.conf uses hardcoded paths.

 - 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. I started working on
   this topic, and will be sending a separate e-mail about this.

Best regards,

Thomas

Thomas Petazzoni (6):
  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
  core: implement per-package SDK and target

 Config.in                        | 18 ++++++
 Makefile                         | 94 ++++++++++++++++++++------------
 fs/common.mk                     |  3 +-
 package/pkg-generic.mk           | 27 ++++++++-
 support/scripts/check-host-rpath |  8 ++-
 5 files changed, 110 insertions(+), 40 deletions(-)

-- 
2.19.1

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 1/6] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
@ 2018-11-14 10:55 ` Thomas Petazzoni
  2018-11-15 20:49   ` Yann E. MORIN
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 2/6] support/scripts/check-host-rpath: split condition on two statements Thomas Petazzoni
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-14 10:55 UTC (permalink / raw)
  To: buildroot

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

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

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

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes since v2:
 - New patch
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 2/6] support/scripts/check-host-rpath: split condition on two statements
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 1/6] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use Thomas Petazzoni
@ 2018-11-14 10:55 ` Thomas Petazzoni
  2018-11-15 20:58   ` Yann E. MORIN
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic Thomas Petazzoni
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-14 10:55 UTC (permalink / raw)
  To: buildroot

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

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

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 support/scripts/check-host-rpath | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 1/6] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use Thomas Petazzoni
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 2/6] support/scripts/check-host-rpath: split condition on two statements Thomas Petazzoni
@ 2018-11-14 10:55 ` Thomas Petazzoni
  2018-11-15 21:09   ` Yann E. MORIN
  2018-11-16  1:21   ` Matthew Weber
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 4/6] Makefile: move .NOTPARALLEL statement after including .config file Thomas Petazzoni
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-14 10:55 UTC (permalink / raw)
  To: buildroot

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

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

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

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

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

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

diff --git a/Makefile b/Makefile
index 2819d44124..e675ac26aa 100644
--- a/Makefile
+++ b/Makefile
@@ -572,10 +572,6 @@ $(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\
 
 endif
 
-.PHONY: dirs
-dirs: $(BUILD_DIR) $(STAGING_DIR) $(BASE_TARGET_DIR) \
-	$(HOST_DIR) $(HOST_DIR_SYMLINK) $(BINARIES_DIR)
-
 $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
 	$(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTCXX="$(HOSTCXX_NOCCACHE)" syncconfig
 
@@ -605,11 +601,6 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY)
 		--transform='s#^\.#$(BR2_SDK_PREFIX)#' \
 		-C $(HOST_DIR) "."
 
-# Populating the staging with the base directories is handled by the skeleton package
-$(STAGING_DIR):
-	@mkdir -p $(STAGING_DIR)
-	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
-
 RSYNC_VCS_EXCLUSIONS = \
 	--exclude .svn --exclude .git --exclude .hg --exclude .bzr \
 	--exclude CVS
@@ -710,8 +701,14 @@ $(TARGETS_ROOTFS): target-finalize
 # Avoid the rootfs name leaking down the dependency chain
 target-finalize: ROOTFS=
 
+host-finalize: $(HOST_DIR_SYMLINK)
+
+.PHONY: staging-finalize
+staging-finalize:
+	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
+
 .PHONY: target-finalize
-target-finalize: $(PACKAGES)
+target-finalize: $(PACKAGES) host-finalize
 	@$(call MESSAGE,"Finalizing target directory")
 	# Check files that are touched by more than one package
 	./support/scripts/check-uniq-files -t target $(BUILD_DIR)/packages-file-list.txt
@@ -782,7 +779,7 @@ endif
 	touch $(TARGET_DIR)/usr
 
 .PHONY: target-post-image
-target-post-image: $(TARGETS_ROOTFS) target-finalize
+target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
 	@rm -f $(ROOTFS_COMMON_TAR)
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
 		$(call MESSAGE,"Executing post-image script $(s)"); \
@@ -811,7 +808,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config
 
 .PHONY: legal-info
-legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \
+legal-info: legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \
 		$(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST)
 	@cat support/legal-info/README.header >>$(LEGAL_REPORT)
 	@if [ -r $(LEGAL_WARNINGS) ]; then \
diff --git a/fs/common.mk b/fs/common.mk
index 2a5a202a89..96658428ba 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -145,6 +145,7 @@ $$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2)
 $$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot
 $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES)
 	@$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)")
+	mkdir -p $$(@D)
 	rm -rf $$(ROOTFS_$(2)_DIR)
 	mkdir -p $$(ROOTFS_$(2)_DIR)
 	echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f34f46afc8..309fd8cd48 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -173,6 +173,7 @@ $(BUILD_DIR)/%/.stamp_extracted:
 $(BUILD_DIR)/%/.stamp_rsynced:
 	@$(call step_start,rsync)
 	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
+	@mkdir -p $(@D)
 	$(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep))
 	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
 	rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $($(PKG)_OVERRIDE_SRCDIR_RSYNC_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D)
@@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built::
 $(BUILD_DIR)/%/.stamp_host_installed:
 	@$(call step_start,install-host)
 	@$(call MESSAGE,"Installing to host directory")
+	@mkdir -p $(HOST_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
@@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed:
 $(BUILD_DIR)/%/.stamp_staging_installed:
 	@$(call step_start,install-staging)
 	@$(call MESSAGE,"Installing to staging directory")
+	@mkdir -p $(STAGING_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_STAGING_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
@@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 # Install to images dir
 $(BUILD_DIR)/%/.stamp_images_installed:
 	@$(call step_start,install-image)
+	@mkdir -p $(BINARIES_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep))
 	@$(call MESSAGE,"Installing to images directory")
 	+$($(PKG)_INSTALL_IMAGES_CMDS)
@@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed:
 $(BUILD_DIR)/%/.stamp_target_installed:
 	@$(call step_start,install-target)
 	@$(call MESSAGE,"Installing to target")
+	@mkdir -p $(TARGET_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_TARGET_CMDS)
 	$(if $(BR2_INIT_SYSTEMD),\
@@ -735,7 +740,7 @@ $$($(2)_TARGET_BUILD):	$$($(2)_TARGET_CONFIGURE)
 $(1)-configure:			$$($(2)_TARGET_CONFIGURE)
 $$($(2)_TARGET_CONFIGURE):	| $$($(2)_FINAL_DEPENDENCIES)
 
-$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare
+$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | prepare
 $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies
 
 ifeq ($$($(2)_OVERRIDE_SRCDIR),)
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 4/6] Makefile: move .NOTPARALLEL statement after including .config file
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
                   ` (2 preceding siblings ...)
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic Thomas Petazzoni
@ 2018-11-14 10:55 ` Thomas Petazzoni
  2018-11-15 21:37   ` Yann E. MORIN
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 5/6] Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR Thomas Petazzoni
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-14 10:55 UTC (permalink / raw)
  To: buildroot

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

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

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

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 5/6] Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
                   ` (3 preceding siblings ...)
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 4/6] Makefile: move .NOTPARALLEL statement after including .config file Thomas Petazzoni
@ 2018-11-14 10:55 ` Thomas Petazzoni
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target Thomas Petazzoni
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-14 10:55 UTC (permalink / raw)
  To: buildroot

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

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

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

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

TARGET_DIR_WARNING_FILE is used in three places:

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

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

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

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

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
                   ` (4 preceding siblings ...)
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 5/6] Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR Thomas Petazzoni
@ 2018-11-14 10:55 ` Thomas Petazzoni
  2018-11-15 16:41   ` Andreas Naumann
  2018-11-15 14:37 ` [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-14 10:55 UTC (permalink / raw)
  To: buildroot

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

There are two main benefits:

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

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

It works as follows:

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

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

   This output/per-package/ folder is PER_PACKAGE_DIR.

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

 - Of course, packages have dependencies, so those dependencies must
   be installed in the per-package host and target folders. To do so,
   we simply rsync (using hard links to save space and time) the host
   and target folders of the direct dependencies of the package to the
   current package host and target folders.

   We only need to take care of direct dependencies (and not
   recursively all dependencies), because we accumulate into those
   per-package host and target folders the files installed by the
   dependencies. Note that this only works because we make the
   assumption that one package does *not* overwrite files installed by
   another package.

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

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

 - The filesystem image creation code cannot work.

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

In order to fix this, this commit extends the target-finalize step so
that it starts by populating output/target and output/host by
rsync-ing into them the target and host directories of all packages
listed in the $(PACKAGES) variable. It is necessary to do this
sequentially in the target-finalize step and not in each
package. Doing it in package installation means that it can be done in
parallel. In that case, there is a chance that two rsyncs are creating
the same hardlink or directory at the same time, which makes one of
them fail.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes since v2:
 - Account for <pkg>_EXTRACT_DEPENDENCIES in the extract step of the
   package, by rsync'ing their host and target directories to the
   current package host/target directory.

Changes since v1:
 - Remove the LD_LIBRARY_PATH change since we're now longer relying on
   LD_LIBRARY_PATH to allow host programs to find their libraries.
 - Improve commit log according to Arnout suggestions
 - Remove -u option from rsync calls in the main Makefile, suggested
   by Arnout
 - Drop entirely the definitions of <pkg>_TARGET_DIR,
   <pkg>_STAGING_DIR and <pkg>_HOST_DIR, and instead make the global
   TARGET_DIR, HOST_DIR variables point to the per-package directories
   when PKG is defined. Suggested by Arnout.
---
 Config.in                        | 18 ++++++++++++++++
 Makefile                         | 37 +++++++++++++++++++++++++++-----
 fs/common.mk                     |  2 +-
 package/pkg-generic.mk           | 20 ++++++++++++++++-
 support/scripts/check-host-rpath |  5 ++++-
 5 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/Config.in b/Config.in
index 03e4eb3928..69e34ec256 100644
--- a/Config.in
+++ b/Config.in
@@ -700,6 +700,24 @@ config BR2_REPRODUCIBLE
 	  This is labeled as an experimental feature, as not all
 	  packages behave properly to ensure reproducibility.
 
+config BR2_PER_PACKAGE_FOLDERS
+	bool "Use per-package folders (experimental)"
+	help
+	  This option will change the build process of Buildroot
+	  package to use per-package target and host directories.
+
+	  This is useful for two related purposes:
+
+	    - Cleanly isolate the build of each package, so that a
+	      given package only "sees" the dependencies it has
+	      explicitly expressed, and not other packages that may
+	      have by chance been built before.
+
+	    - Enable top-level parallel build.
+
+	  This is labeled as an experimental feature, as not all
+	  packages behave properly with per-package folders.
+
 endmenu
 
 comment "Security Hardening Options"
diff --git a/Makefile b/Makefile
index 23032988a5..f3c0e2326e 100644
--- a/Makefile
+++ b/Makefile
@@ -207,7 +207,8 @@ BINARIES_DIR := $(BASE_DIR)/images
 # The target directory is common to all packages,
 # but there is one that is specific to each filesystem.
 BASE_TARGET_DIR := $(BASE_DIR)/target
-TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(BASE_TARGET_DIR))
+PER_PACKAGE_DIR := $(BASE_DIR)/per-package
+
 # initial definition so that 'make clean' works for most users, even without
 # .config. HOST_DIR will be overwritten later when .config is included.
 HOST_DIR := $(BASE_DIR)/host
@@ -230,6 +231,7 @@ ifeq ($(filter $(noconfig_targets),$(MAKECMDGOALS)),)
 -include $(BR2_CONFIG)
 endif
 
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),)
 # Parallel execution of this Makefile is disabled because it changes
 # the packages building order, that can be a problem for two reasons:
 # - If a package has an unspecified optional dependency and that
@@ -245,6 +247,13 @@ endif
 # use the -j<jobs> option when building, e.g:
 #      make -j$((`getconf _NPROCESSORS_ONLN`+1))
 .NOTPARALLEL:
+endif
+
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/target,$(BASE_TARGET_DIR)))
+else
+TARGET_DIR = $(if $(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(BASE_TARGET_DIR))
+endif
 
 # timezone and locale may affect build output
 ifeq ($(BR2_REPRODUCIBLE),y)
@@ -455,7 +464,11 @@ LZCAT := $(call qstrip,$(BR2_LZCAT))
 TAR_OPTIONS = $(call qstrip,$(BR2_TAR_OPTIONS)) -xf
 
 # packages compiled for the host go here
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+HOST_DIR = $(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/host,$(call qstrip,$(BR2_HOST_DIR)))
+else
 HOST_DIR := $(call qstrip,$(BR2_HOST_DIR))
+endif
 
 ifneq ($(HOST_DIR),$(BASE_DIR)/host)
 HOST_DIR_SYMLINK = $(BASE_DIR)/host
@@ -701,14 +714,28 @@ $(TARGETS_ROOTFS): target-finalize
 # Avoid the rootfs name leaking down the dependency chain
 target-finalize: ROOTFS=
 
-host-finalize: $(HOST_DIR_SYMLINK)
+host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+	@$(call MESSAGE,"Creating global host directory")
+	$(foreach pkg,$(PACKAGES),\
+		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
+		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
+		$(HOST_DIR)$(sep))
+endif
 
 .PHONY: staging-finalize
 staging-finalize:
 	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
 
 .PHONY: target-finalize
-target-finalize: $(PACKAGES) host-finalize
+target-finalize: $(PACKAGES) $(TARGET_DIR) host-finalize
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+	@$(call MESSAGE,"Creating global target directory")
+	$(foreach pkg,$(PACKAGES),\
+		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
+		$(PER_PACKAGE_DIR)/$(pkg)/target/ \
+		$(TARGET_DIR)$(sep))
+endif
 	@$(call MESSAGE,"Finalizing target directory")
 	# Check files that are touched by more than one package
 	./support/scripts/check-uniq-files -t target $(BUILD_DIR)/packages-file-list.txt
@@ -972,7 +999,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 
 # staging and target directories do NOT list these as
 # dependencies anywhere else
-$(BUILD_DIR) $(BASE_TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
+$(BUILD_DIR) $(BASE_TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST) $(PER_PACKAGE_DIR):
 	@mkdir -p $@
 
 # outputmakefile generates a Makefile in the output directory, if using a
@@ -1011,7 +1038,7 @@ printvars:
 clean:
 	rm -rf $(BASE_TARGET_DIR) $(BINARIES_DIR) $(HOST_DIR) $(HOST_DIR_SYMLINK) \
 		$(BUILD_DIR) $(BASE_DIR)/staging \
-		$(LEGAL_INFO_DIR) $(GRAPHS_DIR)
+		$(LEGAL_INFO_DIR) $(GRAPHS_DIR) $(PER_PACKAGE_DIR)
 
 .PHONY: distclean
 distclean: clean
diff --git a/fs/common.mk b/fs/common.mk
index 96658428ba..fc4be3cc05 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -172,7 +172,7 @@ rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1)
 
 ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
 TARGETS_ROOTFS += rootfs-$(1)
-PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES))
+PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES)) $(ROOTFS_COMMON_DEPENDENCIES)
 endif
 
 # Check for legacy POST_TARGETS rules
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 309fd8cd48..8ec5e8db73 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
 
@@ -126,6 +126,21 @@ endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_user
 endif
 
+# $1: deps
+ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
+define prepare-per-package-folder
+	mkdir -p $(HOST_DIR) $(TARGET_DIR)
+	$(foreach pkg,$(1),\
+		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
+		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
+		$(HOST_DIR)$(sep))
+	$(foreach pkg,$(1),\
+		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
+		$(PER_PACKAGE_DIR)/$(pkg)/target/ \
+		$(TARGET_DIR)$(sep))
+endef
+endif
+
 ################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
@@ -133,6 +148,7 @@ endif
 # Retrieve the archive
 $(BUILD_DIR)/%/.stamp_downloaded:
 	@$(call step_start,download)
+	$(call prepare-per-package-folder,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES))
 	$(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
 # Only show the download message if it isn't already downloaded
 	$(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \
@@ -159,6 +175,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call step_start,extract)
 	@$(call MESSAGE,"Extracting")
+	$(call prepare-per-package-folder,$($(PKG)_FINAL_EXTRACT_DEPENDENCIES))
 	$(foreach hook,$($(PKG)_PRE_EXTRACT_HOOKS),$(call $(hook))$(sep))
 	$(Q)mkdir -p $(@D)
 	$($(PKG)_EXTRACT_CMDS)
@@ -219,6 +236,7 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
 $(BUILD_DIR)/%/.stamp_configured:
 	@$(call step_start,configure)
 	@$(call MESSAGE,"Configuring")
+	$(call prepare-per-package-folder,$($(PKG)_FINAL_DEPENDENCIES))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
index 6c5767da05..787a1763b0 100755
--- a/support/scripts/check-host-rpath
+++ b/support/scripts/check-host-rpath
@@ -11,6 +11,7 @@ export LC_ALL=C
 main() {
     local pkg="${1}"
     local hostdir="${2}"
+    local perpackagedir="${3}"
     local file ret
 
     # Remove duplicate and trailing '/' for proper match
@@ -20,7 +21,7 @@ main() {
     while read file; do
         is_elf "${file}" || continue
         elf_needs_rpath "${file}" "${hostdir}" || continue
-        check_elf_has_rpath "${file}" "${hostdir}" && continue
+        check_elf_has_rpath "${file}" "${hostdir}" "${perpackagedir}" && continue
         if [ ${ret} -eq 0 ]; then
             ret=1
             printf "***\n"
@@ -57,6 +58,7 @@ elf_needs_rpath() {
 check_elf_has_rpath() {
     local file="${1}"
     local hostdir="${2}"
+    local perpackagedir="${3}"
     local rpath dir
 
     while read rpath; do
@@ -65,6 +67,7 @@ check_elf_has_rpath() {
             dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
             [ "${dir}" = "${hostdir}/lib" ] && return 0
             [ "${dir}" = "\$ORIGIN/../lib" ] && return 0
+            [[ ${dir} =~ ${perpackagedir}/[^/]*/host/lib ]] && return 0
         done
     done < <( readelf -d "${file}"                                              \
               |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
                   ` (5 preceding siblings ...)
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target Thomas Petazzoni
@ 2018-11-15 14:37 ` Thomas Petazzoni
  2018-11-15 16:41 ` Andreas Naumann
  2018-11-19 13:30 ` Arnout Vandecappelle
  8 siblings, 0 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-15 14:37 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 14 Nov 2018 11:55:51 +0100, Thomas Petazzoni wrote:

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

FYI, I did a test build of some given configuration (randomly chosen),
in the following situations, which provided the following results

 - Per-package folder support disabled

real    5m11.261s
user    8m58.011s
sys     1m54.650s

 - Per-package folder support enabled, sequential build

real    5m15.744s
user    9m4.848s
sys     1m55.885s

 - Per-package folder support enabled, parallel build

real    4m17.059s
user    9m55.024s
sys     2m2.344s

So, there is a small performance hit in doing all the rsyncs needed for
per-package folders: 4.5 seconds on a build of ~5 minutes.

However, this performance hit is largely compensated by the parallelism
of the build. It is worth mentioning that this specific configuration
only lead to a small amount of parallelism, as can be seen in the
attached PDF files.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: no-parallel.pdf
Type: application/pdf
Size: 17122 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20181115/57d59566/attachment.pdf>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: parallel.pdf
Type: application/pdf
Size: 16636 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20181115/57d59566/attachment-0001.pdf>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
                   ` (6 preceding siblings ...)
  2018-11-15 14:37 ` [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
@ 2018-11-15 16:41 ` Andreas Naumann
  2018-11-16 14:43   ` Thomas Petazzoni
  2018-11-19 13:30 ` Arnout Vandecappelle
  8 siblings, 1 reply; 35+ messages in thread
From: Andreas Naumann @ 2018-11-15 16:41 UTC (permalink / raw)
  To: buildroot

Hi Thomas,


Am 14.11.18 um 11:55 schrieb Thomas Petazzoni:
> Hello,
> 
> Here is a fourth iteration of the per-package SDK and target directory
> implementation.
> 
> If you're interested in testing it, you can find the patch series at:
> 
>    http://git.bootlin.com/users/thomas-petazzoni/buildroot/log/?h=ppsh-v4
> 


thank you for the updated version. I tried to run it with my configs. 
For that I needed
a) a generic patch for all kconfig packages, see my answer on the final 
patch
b) linux-specific patch posted two days ago: [PATCH v2 1/1] linux: Make 
dtc install step more reliable

On my local machine things seemed to work, however final rootfs creation 
failed in our CI environment due to broken host- binaries, e.g. 
mkfs.ubifs not finding some required lib (liblzo2.so.2 in that 
particular case).
It seems some things with rpath manipulation/checking do not yet work 
properly. Looking into the binary with ldd I see libs used from the 
host-machine that probably should come from buildroots host/usr/lib...
	linux-vdso.so.1 =>  (0x00007ffc06fcc000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f760428f000)
	liblzo2.so.2 => not found
	libuuid.so.1 => /lib/x86_64-linux-gnu/libuuid.so.1 (0x00007f760408a000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f7603d81000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f76039b7000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f76044a9000)

I couldnt find out in reasonable time where things go wrong so I'd be 
thankful for a pointer on how to debug this.

regards,
Andreas

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target Thomas Petazzoni
@ 2018-11-15 16:41   ` Andreas Naumann
  2018-11-16 13:47     ` Thomas Petazzoni
  0 siblings, 1 reply; 35+ messages in thread
From: Andreas Naumann @ 2018-11-15 16:41 UTC (permalink / raw)
  To: buildroot

Hi,

Am 14.11.18 um 11:55 schrieb Thomas Petazzoni:
...
> +# $1: deps
> +ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
> +define prepare-per-package-folder
> +	mkdir -p $(HOST_DIR) $(TARGET_DIR)
> +	$(foreach pkg,$(1),\
> +		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> +		$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> +		$(HOST_DIR)$(sep))
> +	$(foreach pkg,$(1),\
> +		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
> +		$(PER_PACKAGE_DIR)/$(pkg)/target/ \
> +		$(TARGET_DIR)$(sep))
> +endef
> +endif
> +
>   ################################################################################
>   # Implicit targets -- produce a stamp file for each step of a package build
>   ################################################################################
> @@ -133,6 +148,7 @@ endif
>   # Retrieve the archive
>   $(BUILD_DIR)/%/.stamp_downloaded:
>   	@$(call step_start,download)
> +	$(call prepare-per-package-folder,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES))
>   	$(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
>   # Only show the download message if it isn't already downloaded
>   	$(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \
> @@ -159,6 +175,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
>   $(BUILD_DIR)/%/.stamp_extracted:
>   	@$(call step_start,extract)
>   	@$(call MESSAGE,"Extracting")
> +	$(call prepare-per-package-folder,$($(PKG)_FINAL_EXTRACT_DEPENDENCIES))
>   	$(foreach hook,$($(PKG)_PRE_EXTRACT_HOOKS),$(call $(hook))$(sep))
>   	$(Q)mkdir -p $(@D)
>   	$($(PKG)_EXTRACT_CMDS)
> @@ -219,6 +236,7 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
>   $(BUILD_DIR)/%/.stamp_configured:
>   	@$(call step_start,configure)
>   	@$(call MESSAGE,"Configuring")
> +	$(call prepare-per-package-folder,$($(PKG)_FINAL_DEPENDENCIES))
>   	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
>   	$($(PKG)_CONFIGURE_CMDS)
>   	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))

This is much clearer to me than v3, however there is a problem with 
kconfig packages. Unfortunately, the kconfig infrastructure is running 
the 'make xxx_defconfig' step before the configure-step is "officially" 
started. This is due to the kconfig_fixup target being a dependency of 
-configure. This in turn requires availability of the toolchain and 
other prerequisites that are part of _FINAL_DEPENDENCIES earlier as 
prepared in your above patch.

I made a patch which moves the _FINAL_DEPENDENCIES preparation to an 
additional .stamp-configure-prepare step just before .stamp_configured. 
That works but is not particularly beautiful.
I guess a more proper solution would be to somehow move the 
kconfig_fixup code into the configure-step. Maybe use the 
pre-configure-hook, any suggestions?


regards,
Andreas

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 1/6] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 1/6] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use Thomas Petazzoni
@ 2018-11-15 20:49   ` Yann E. MORIN
  0 siblings, 0 replies; 35+ messages in thread
From: Yann E. MORIN @ 2018-11-15 20:49 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2018-11-14 11:55 +0100, Thomas Petazzoni spake thusly:
> As we are going to move to per-package SDK, the location of CCACHE and
> therefore the definitions of HOSTCC and HOSTCXX need to be evaluated
> at the time of use and not at the time of assignment. Indeed, the
> value of HOST_DIR changes from one package to the other.
> 
> Therefore, we need to change from := to =.
> 
> In addition, while doing A := $(something) $(A) is possible, doing A =
> $(something) $(A) is not legal. So, instead of defining HOSTCC in
> terms of the current HOSTCC variable, we re-use HOSTCC_NOCCACHE
> instead.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

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

Regards,
Yann E. MORIN.

> ---
> Changes since v2:
>  - New patch
> ---
>  Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 94adff3406..2819d44124 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -471,11 +471,11 @@ BR_PATH = "$(HOST_DIR)/bin:$(HOST_DIR)/sbin:$(PATH)"
>  TARGET_DIR_WARNING_FILE = $(BASE_TARGET_DIR)/THIS_IS_NOT_YOUR_ROOT_FILESYSTEM
>  
>  ifeq ($(BR2_CCACHE),y)
> -CCACHE := $(HOST_DIR)/bin/ccache
> +CCACHE = $(HOST_DIR)/bin/ccache
>  BR_CACHE_DIR ?= $(call qstrip,$(BR2_CCACHE_DIR))
>  export BR_CACHE_DIR
> -HOSTCC := $(CCACHE) $(HOSTCC)
> -HOSTCXX := $(CCACHE) $(HOSTCXX)
> +HOSTCC = $(CCACHE) $(HOSTCC_NOCCACHE)
> +HOSTCXX = $(CCACHE) $(HOSTCXX_NOCCACHE)
>  else
>  export BR_NO_CCACHE
>  endif
> -- 
> 2.19.1
> 

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 2/6] support/scripts/check-host-rpath: split condition on two statements
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 2/6] support/scripts/check-host-rpath: split condition on two statements Thomas Petazzoni
@ 2018-11-15 20:58   ` Yann E. MORIN
  0 siblings, 0 replies; 35+ messages in thread
From: Yann E. MORIN @ 2018-11-15 20:58 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2018-11-14 11:55 +0100, Thomas Petazzoni spake thusly:
> Inside the check_elf_has_rpath(), we check if the host binary has a
> correct RPATH, which should be either an absolute path to
> $(HOST_DIR)/lib, or a relative path using $ORIGIN. Those two
> conditions are checked in a single statements, but as we are going to
> add a third condition, let's split this up a bit:
> 
>  - If we have a RPATH to $(HOST_DIR)/lib -> we're good, return 0
>  - If we have a RPATH to $ORIGIN/../lib -> we're good, return 0
>  - Otherwise, we will exit the loop, and return 1
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

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

Regards,
Yann E. MORIN.

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

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic Thomas Petazzoni
@ 2018-11-15 21:09   ` Yann E. MORIN
  2018-11-16 14:08     ` Thomas Petazzoni
  2018-11-16  1:21   ` Matthew Weber
  1 sibling, 1 reply; 35+ messages in thread
From: Yann E. MORIN @ 2018-11-15 21:09 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2018-11-14 11:55 +0100, Thomas Petazzoni spake thusly:
> In the current code, the creation of the main output folders
> (BUILD_DIR, STAGING_DIR, HOST_DIR, TARGET_DIR, etc.) is done by a
> global "dirs" target. While this works fine in the current situation,
> it doesn't work well in a context where per-package host and target
> directories are used.
> 
> For example, with the current code and per-package host directories,
> the output/staging symbolic link ends up being created as a link to
> the per-package package sysroot directory of the first package being
> built, instead of the global sysroot.
> 
> This commit reworks the creation of those directories by having the
> package/pkg-generic.mk code ensure that the build directory, target
> directory, host directory, staging directory and binaries directory
> exist before they are needed.

I don't think we do need anything special to create HOST_DIR,
STAGING_DIR and TARGET_DIR: 

  - host-skeleton is the first package to run, and that one does ensure
    the existence of $(HOST_DIR) by the mere act of creating it (which
    it cirrently does not do, but should)

  - skeleton is the first package to install in target/ and staging, and
    currently the skeleton-custom and skeleton-init-common should both
    create target/ and staging/

> Two new targets, host-finalize and staging-finalize are added in the
> main Makefile to create the compatibility symlinks for host and
> staging directories. They will be extended later with additional logic
> for per-package directories.
> 
> Thanks to those changes, the global "dirs" target is entirely removed.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  Makefile               | 21 +++++++++------------
>  fs/common.mk           |  1 +
>  package/pkg-generic.mk |  7 ++++++-
>  3 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 2819d44124..e675ac26aa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -572,10 +572,6 @@ $(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\
>  
>  endif
>  
> -.PHONY: dirs
> -dirs: $(BUILD_DIR) $(STAGING_DIR) $(BASE_TARGET_DIR) \
> -	$(HOST_DIR) $(HOST_DIR_SYMLINK) $(BINARIES_DIR)
> -
>  $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
>  	$(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTCXX="$(HOSTCXX_NOCCACHE)" syncconfig
>  
> @@ -605,11 +601,6 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY)
>  		--transform='s#^\.#$(BR2_SDK_PREFIX)#' \
>  		-C $(HOST_DIR) "."
>  
> -# Populating the staging with the base directories is handled by the skeleton package
> -$(STAGING_DIR):
> -	@mkdir -p $(STAGING_DIR)
> -	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> -
>  RSYNC_VCS_EXCLUSIONS = \
>  	--exclude .svn --exclude .git --exclude .hg --exclude .bzr \
>  	--exclude CVS
> @@ -710,8 +701,14 @@ $(TARGETS_ROOTFS): target-finalize
>  # Avoid the rootfs name leaking down the dependency chain
>  target-finalize: ROOTFS=
>  
> +host-finalize: $(HOST_DIR_SYMLINK)
> +
> +.PHONY: staging-finalize
> +staging-finalize:
> +	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> +
>  .PHONY: target-finalize
> -target-finalize: $(PACKAGES)
> +target-finalize: $(PACKAGES) host-finalize

Shouldn't we ensure that host-finalize be done before staging-finalize
too?

We would also need to ensure that packages are all installed before
running staging-finalize.

So,=maybe we need to add:

    staging-finalize: $(PACKAGES) host-finalize

Regards,
Yann E. MORIN.

>  	@$(call MESSAGE,"Finalizing target directory")
>  	# Check files that are touched by more than one package
>  	./support/scripts/check-uniq-files -t target $(BUILD_DIR)/packages-file-list.txt
> @@ -782,7 +779,7 @@ endif
>  	touch $(TARGET_DIR)/usr
>  
>  .PHONY: target-post-image
> -target-post-image: $(TARGETS_ROOTFS) target-finalize
> +target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
>  	@rm -f $(ROOTFS_COMMON_TAR)
>  	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
>  		$(call MESSAGE,"Executing post-image script $(s)"); \
> @@ -811,7 +808,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR)
>  	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config
>  
>  .PHONY: legal-info
> -legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \
> +legal-info: legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \
>  		$(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST)
>  	@cat support/legal-info/README.header >>$(LEGAL_REPORT)
>  	@if [ -r $(LEGAL_WARNINGS) ]; then \
> diff --git a/fs/common.mk b/fs/common.mk
> index 2a5a202a89..96658428ba 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -145,6 +145,7 @@ $$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2)
>  $$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot
>  $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES)
>  	@$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)")
> +	mkdir -p $$(@D)
>  	rm -rf $$(ROOTFS_$(2)_DIR)
>  	mkdir -p $$(ROOTFS_$(2)_DIR)
>  	echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f34f46afc8..309fd8cd48 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -173,6 +173,7 @@ $(BUILD_DIR)/%/.stamp_extracted:
>  $(BUILD_DIR)/%/.stamp_rsynced:
>  	@$(call step_start,rsync)
>  	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
> +	@mkdir -p $(@D)
>  	$(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep))
>  	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
>  	rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $($(PKG)_OVERRIDE_SRCDIR_RSYNC_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D)
> @@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built::
>  $(BUILD_DIR)/%/.stamp_host_installed:
>  	@$(call step_start,install-host)
>  	@$(call MESSAGE,"Installing to host directory")
> +	@mkdir -p $(HOST_DIR)
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
>  	+$($(PKG)_INSTALL_CMDS)
>  	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
> @@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed:
>  $(BUILD_DIR)/%/.stamp_staging_installed:
>  	@$(call step_start,install-staging)
>  	@$(call MESSAGE,"Installing to staging directory")
> +	@mkdir -p $(STAGING_DIR)
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
>  	+$($(PKG)_INSTALL_STAGING_CMDS)
>  	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
> @@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
>  # Install to images dir
>  $(BUILD_DIR)/%/.stamp_images_installed:
>  	@$(call step_start,install-image)
> +	@mkdir -p $(BINARIES_DIR)
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep))
>  	@$(call MESSAGE,"Installing to images directory")
>  	+$($(PKG)_INSTALL_IMAGES_CMDS)
> @@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed:
>  $(BUILD_DIR)/%/.stamp_target_installed:
>  	@$(call step_start,install-target)
>  	@$(call MESSAGE,"Installing to target")
> +	@mkdir -p $(TARGET_DIR)
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
>  	+$($(PKG)_INSTALL_TARGET_CMDS)
>  	$(if $(BR2_INIT_SYSTEMD),\
> @@ -735,7 +740,7 @@ $$($(2)_TARGET_BUILD):	$$($(2)_TARGET_CONFIGURE)
>  $(1)-configure:			$$($(2)_TARGET_CONFIGURE)
>  $$($(2)_TARGET_CONFIGURE):	| $$($(2)_FINAL_DEPENDENCIES)
>  
> -$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare
> +$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | prepare
>  $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies
>  
>  ifeq ($$($(2)_OVERRIDE_SRCDIR),)
> -- 
> 2.19.1
> 

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 4/6] Makefile: move .NOTPARALLEL statement after including .config file
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 4/6] Makefile: move .NOTPARALLEL statement after including .config file Thomas Petazzoni
@ 2018-11-15 21:37   ` Yann E. MORIN
  2018-11-16  8:53     ` Thomas Petazzoni
  0 siblings, 1 reply; 35+ messages in thread
From: Yann E. MORIN @ 2018-11-15 21:37 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2018-11-14 11:55 +0100, Thomas Petazzoni spake thusly:
> In a follow-up commit, we will make the .NOTPARALLEL statement
> conditional on a Config.in option, so we need to move it further down.

Ultimately, I don't see why we do need a config option to turn top-level
parallel build on/off. It will ultimately be the user's choice to do so
when calling 'make -jN' no?

However, what we do need for now, is a config option that enabled
per-package directories (PPD, formerly known as PPS, per-package
staging). When that is turned off, we must not allow top-level
parallel build.

I was thinking that we would not need that option either, because we
would enable PPD when we detect that the user is doing top-level parllel
build, by inspecting $(MAKEFLAGS) to see if they contained -j.

However, make is our fiend here, because it does not include -j in the
MAKFLAGS variable, but the one that is exported:

    $ cat Makefile
    $(info var MAKEFLAGS='$(MAKEFLAGS)')
    all:
        @echo "env MAKEFLAGS='$${MAKEFLAGS}'"

    $ make -j1
    var MAKEFLAGS=''
    env MAKEFLAGS=''

    $ make -j3
    var MAKEFLAGS=''
    env MAKEFLAGS=' -j --jobserver-fds=3,4'

    $ make -j
    var MAKEFLAGS=''
    env MAKEFLAGS=' -j'

And there is no other variable in make that specifies that either...

So yes, this means we have no way from the Makefile to detect whether
we're parallel or not. Sigh. :-(

So we do need that option for PPD.

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

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

Regards,
Yann E. MORIN.

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

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic
  2018-11-14 10:55 ` [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic Thomas Petazzoni
  2018-11-15 21:09   ` Yann E. MORIN
@ 2018-11-16  1:21   ` Matthew Weber
  2018-11-16 14:15     ` Thomas Petazzoni
  1 sibling, 1 reply; 35+ messages in thread
From: Matthew Weber @ 2018-11-16  1:21 UTC (permalink / raw)
  To: buildroot

Thomas,

On 11/14/18, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> In the current code, the creation of the main output folders
> (BUILD_DIR, STAGING_DIR, HOST_DIR, TARGET_DIR, etc.) is done by a
> global "dirs" target. While this works fine in the current situation,
> it doesn't work well in a context where per-package host and target
> directories are used.
>
> For example, with the current code and per-package host directories,
> the output/staging symbolic link ends up being created as a link to
> the per-package package sysroot directory of the first package being
> built, instead of the global sysroot.
>
> This commit reworks the creation of those directories by having the
> package/pkg-generic.mk code ensure that the build directory, target
> directory, host directory, staging directory and binaries directory
> exist before they are needed.
>
> Two new targets, host-finalize and staging-finalize are added in the
> main Makefile to create the compatibility symlinks for host and
> staging directories. They will be extended later with additional logic
> for per-package directories.
>
> Thanks to those changes, the global "dirs" target is entirely removed.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  Makefile               | 21 +++++++++------------
>  fs/common.mk           |  1 +
>  package/pkg-generic.mk |  7 ++++++-
>  3 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 2819d44124..e675ac26aa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -572,10 +572,6 @@ $(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\
>
>  endif
>
> -.PHONY: dirs
> -dirs: $(BUILD_DIR) $(STAGING_DIR) $(BASE_TARGET_DIR) \
> -	$(HOST_DIR) $(HOST_DIR_SYMLINK) $(BINARIES_DIR)
> -
>  $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG)
>  	$(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)"
> HOSTCXX="$(HOSTCXX_NOCCACHE)" syncconfig
>
> @@ -605,11 +601,6 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY)
>  		--transform='s#^\.#$(BR2_SDK_PREFIX)#' \
>  		-C $(HOST_DIR) "."
>
> -# Populating the staging with the base directories is handled by the
> skeleton package
> -$(STAGING_DIR):
> -	@mkdir -p $(STAGING_DIR)
> -	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> -

With per package disabled, the staging directory never gets created
early enough for packages to be able to reference content.
Specifically I noticed docker couldn't find -lsystemd.  I'm assuming
it's because of the new target dependency on target-post-image?

>  RSYNC_VCS_EXCLUSIONS = \
>  	--exclude .svn --exclude .git --exclude .hg --exclude .bzr \
>  	--exclude CVS
> @@ -710,8 +701,14 @@ $(TARGETS_ROOTFS): target-finalize
>  # Avoid the rootfs name leaking down the dependency chain
>  target-finalize: ROOTFS=
>
> +host-finalize: $(HOST_DIR_SYMLINK)
> +
> +.PHONY: staging-finalize
> +staging-finalize:
> +	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> +
>  .PHONY: target-finalize
> -target-finalize: $(PACKAGES)
> +target-finalize: $(PACKAGES) host-finalize
>  	@$(call MESSAGE,"Finalizing target directory")
>  	# Check files that are touched by more than one package
>  	./support/scripts/check-uniq-files -t target
> $(BUILD_DIR)/packages-file-list.txt
> @@ -782,7 +779,7 @@ endif
>  	touch $(TARGET_DIR)/usr
>
>  .PHONY: target-post-image
> -target-post-image: $(TARGETS_ROOTFS) target-finalize
> +target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
>  	@rm -f $(ROOTFS_COMMON_TAR)
>  	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
>  		$(call MESSAGE,"Executing post-image script $(s)"); \
> @@ -811,7 +808,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR)
>  	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config
>
>  .PHONY: legal-info
> -legal-info: dirs legal-info-clean legal-info-prepare $(foreach
> p,$(PACKAGES),$(p)-all-legal-info) \
> +legal-info: legal-info-clean legal-info-prepare $(foreach
> p,$(PACKAGES),$(p)-all-legal-info) \
>  		$(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST)
>  	@cat support/legal-info/README.header >>$(LEGAL_REPORT)
>  	@if [ -r $(LEGAL_WARNINGS) ]; then \
> diff --git a/fs/common.mk b/fs/common.mk
> index 2a5a202a89..96658428ba 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -145,6 +145,7 @@ $$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2)
>  $$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot
>  $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES)
>  	@$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)")
> +	mkdir -p $$(@D)
>  	rm -rf $$(ROOTFS_$(2)_DIR)
>  	mkdir -p $$(ROOTFS_$(2)_DIR)
>  	echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f34f46afc8..309fd8cd48 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -173,6 +173,7 @@ $(BUILD_DIR)/%/.stamp_extracted:
>  $(BUILD_DIR)/%/.stamp_rsynced:
>  	@$(call step_start,rsync)
>  	@$(call MESSAGE,"Syncing from source dir $(SRCDIR)")
> +	@mkdir -p $(@D)
>  	$(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep))
>  	@test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1)
>  	rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS)
> $($(PKG)_OVERRIDE_SRCDIR_RSYNC_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D)
> @@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built::
>  $(BUILD_DIR)/%/.stamp_host_installed:
>  	@$(call step_start,install-host)
>  	@$(call MESSAGE,"Installing to host directory")
> +	@mkdir -p $(HOST_DIR)
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
>  	+$($(PKG)_INSTALL_CMDS)
>  	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
> @@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed:
>  $(BUILD_DIR)/%/.stamp_staging_installed:
>  	@$(call step_start,install-staging)
>  	@$(call MESSAGE,"Installing to staging directory")
> +	@mkdir -p $(STAGING_DIR)
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
>  	+$($(PKG)_INSTALL_STAGING_CMDS)
>  	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call
> $(hook))$(sep))
> @@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
>  # Install to images dir
>  $(BUILD_DIR)/%/.stamp_images_installed:
>  	@$(call step_start,install-image)
> +	@mkdir -p $(BINARIES_DIR)
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep))
>  	@$(call MESSAGE,"Installing to images directory")
>  	+$($(PKG)_INSTALL_IMAGES_CMDS)
> @@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed:
>  $(BUILD_DIR)/%/.stamp_target_installed:
>  	@$(call step_start,install-target)
>  	@$(call MESSAGE,"Installing to target")
> +	@mkdir -p $(TARGET_DIR)
>  	$(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
>  	+$($(PKG)_INSTALL_TARGET_CMDS)
>  	$(if $(BR2_INIT_SYSTEMD),\
> @@ -735,7 +740,7 @@ $$($(2)_TARGET_BUILD):	$$($(2)_TARGET_CONFIGURE)
>  $(1)-configure:			$$($(2)_TARGET_CONFIGURE)
>  $$($(2)_TARGET_CONFIGURE):	| $$($(2)_FINAL_DEPENDENCIES)
>
> -$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare
> +$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | prepare
>  $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies
>
>  ifeq ($$($(2)_OVERRIDE_SRCDIR),)
> --
> 2.19.1
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 4/6] Makefile: move .NOTPARALLEL statement after including .config file
  2018-11-15 21:37   ` Yann E. MORIN
@ 2018-11-16  8:53     ` Thomas Petazzoni
  0 siblings, 0 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-16  8:53 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu, 15 Nov 2018 22:37:58 +0100, Yann E. MORIN wrote:

> Ultimately, I don't see why we do need a config option to turn top-level
> parallel build on/off. It will ultimately be the user's choice to do so
> when calling 'make -jN' no?
> 
> However, what we do need for now, is a config option that enabled
> per-package directories (PPD, formerly known as PPS, per-package
> staging). When that is turned off, we must not allow top-level
> parallel build.
> 
> I was thinking that we would not need that option either, because we
> would enable PPD when we detect that the user is doing top-level parllel
> build, by inspecting $(MAKEFLAGS) to see if they contained -j.
> 
> However, make is our fiend here, because it does not include -j in the
> MAKFLAGS variable, but the one that is exported:
> 
>     $ cat Makefile
>     $(info var MAKEFLAGS='$(MAKEFLAGS)')
>     all:
>         @echo "env MAKEFLAGS='$${MAKEFLAGS}'"
> 
>     $ make -j1
>     var MAKEFLAGS=''
>     env MAKEFLAGS=''
> 
>     $ make -j3
>     var MAKEFLAGS=''
>     env MAKEFLAGS=' -j --jobserver-fds=3,4'
> 
>     $ make -j
>     var MAKEFLAGS=''
>     env MAKEFLAGS=' -j'
> 
> And there is no other variable in make that specifies that either...
> 
> So yes, this means we have no way from the Makefile to detect whether
> we're parallel or not. Sigh. :-(
> 
> So we do need that option for PPD.
> 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>  
> 
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Thanks for this research and the review. I think there is another
reason to have an option, at least for the moment: I'm pretty sure a
number of people not aware of top-level parallel build issues use
Buildroot with "make -jN" today, as they have learned that using make
-jN will "speed up their build" (I mean in general, not specifically
for Buildroot).

If we were to use make -jN as the criteria to decide if PPD should be
used or not, then suddenly all such users would start using PPD, which
is still way too experimental to expose it without the user's explicit
consent.

My plan is rather the following:

 (1) Have it under an explicit option for now.

 (2) Once we consider it stable, drop the option entirely, and always
     use PPD, even if the user doesn't use top-level parallel build.
     There's a slight additional cost, but there's also a huge benefit
     in terms of dependency checking/isolation.

Note that we might stay on step (1) for a little while, I think it will
take time to really make it fully usable.

Best regards,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-15 16:41   ` Andreas Naumann
@ 2018-11-16 13:47     ` Thomas Petazzoni
  2018-11-16 15:22       ` Thomas De Schampheleire
  0 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-16 13:47 UTC (permalink / raw)
  To: buildroot

Hello,

Yann was already in Cc, I've added Thomas De Schampheleire as well.
Yann, Thomas, there is some kconfig related discussion below, we
need you :-)

On Thu, 15 Nov 2018 17:41:35 +0100, Andreas Naumann wrote:

> This is much clearer to me than v3, however there is a problem with 
> kconfig packages. Unfortunately, the kconfig infrastructure is running 
> the 'make xxx_defconfig' step before the configure-step is "officially" 
> started. This is due to the kconfig_fixup target being a dependency of 
> -configure. This in turn requires availability of the toolchain and 
> other prerequisites that are part of _FINAL_DEPENDENCIES earlier as 
> prepared in your above patch.
> 
> I made a patch which moves the _FINAL_DEPENDENCIES preparation to an 
> additional .stamp-configure-prepare step just before .stamp_configured. 
> That works but is not particularly beautiful.

That is not too bad actually. Semantically speaking, preparing the
per-package folders is not really part of the configuration step. It
could be logical to do it in a "prepare" step.

The only thing that I dislike a bit with this is that it would no
longer be consistent with what we do for download and extract
dependencies: we prepare the per-package folders with download and
extract dependencies respectively in the download and extract steps. So
it would be quite logical to also do the same for the "configuration
dependencies" (which we name just "dependencies" in Buildroot).

So, this leaves us with 3 options:

 - Keep the dependency preparation within the download, extract and
   configure step, as proposed in this v4. This will require changing
   the kconfig logic to prepare the configuration file inside the
   "configure" step and not as a additional step injected before the
   "configure" step.

 - Keep the dependency preparation within the download and extract
   steps, and make an exception for the configure step, with a separate
   "prepare" step that comes before. Not nice in terms of consistency,
   as explained above.

 - Introduce "prepare download", "prepare extract" and "prepare
   configure" steps that would do the dependency preparation.

> I guess a more proper solution would be to somehow move the 
> kconfig_fixup code into the configure-step. Maybe use the 
> pre-configure-hook, any suggestions?

I don't recall why they need to be done before the configuration step,
but I'm pretty sure there is a reason for that.

Yann, Thomas, do you remember ?

Thanks a lot,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic
  2018-11-15 21:09   ` Yann E. MORIN
@ 2018-11-16 14:08     ` Thomas Petazzoni
  0 siblings, 0 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-16 14:08 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu, 15 Nov 2018 22:09:46 +0100, Yann E. MORIN wrote:

> I don't think we do need anything special to create HOST_DIR,
> STAGING_DIR and TARGET_DIR: 
> 
>   - host-skeleton is the first package to run, and that one does ensure
>     the existence of $(HOST_DIR) by the mere act of creating it (which
>     it cirrently does not do, but should)
> 
>   - skeleton is the first package to install in target/ and staging, and
>     currently the skeleton-custom and skeleton-init-common should both
>     create target/ and staging/

So, what you suggest is to remove the following hunks from my patch ?

@@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built::
 $(BUILD_DIR)/%/.stamp_host_installed:
 	@$(call step_start,install-host)
 	@$(call MESSAGE,"Installing to host directory")
+	@mkdir -p $(HOST_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
@@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed:
 $(BUILD_DIR)/%/.stamp_staging_installed:
 	@$(call step_start,install-staging)
 	@$(call MESSAGE,"Installing to staging directory")
+	@mkdir -p $(STAGING_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_STAGING_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
@@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 # Install to images dir
 $(BUILD_DIR)/%/.stamp_images_installed:
 	@$(call step_start,install-image)
+	@mkdir -p $(BINARIES_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep))
 	@$(call MESSAGE,"Installing to images directory")
 	+$($(PKG)_INSTALL_IMAGES_CMDS)
@@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed:
 $(BUILD_DIR)/%/.stamp_target_installed:
 	@$(call step_start,install-target)
 	@$(call MESSAGE,"Installing to target")
+	@mkdir -p $(TARGET_DIR)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_TARGET_CMDS)
 	$(if $(BR2_INIT_SYSTEMD),\

Is this what you mean ?

> > +host-finalize: $(HOST_DIR_SYMLINK)
> > +
> > +.PHONY: staging-finalize
> > +staging-finalize:
> > +	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> > +
> >  .PHONY: target-finalize
> > -target-finalize: $(PACKAGES)
> > +target-finalize: $(PACKAGES) host-finalize  
> 
> Shouldn't we ensure that host-finalize be done before staging-finalize
> too?

Why so ?

> We would also need to ensure that packages are all installed before
> running staging-finalize.
> 
> So,=maybe we need to add:
> 
>     staging-finalize: $(PACKAGES) host-finalize

Same, we don't really need all packages to be installed just to create
that dummy symlink.

In fact, maybe I should go one step further: STAGING_DIR is part of
HOST_DIR, so maybe I should simply create this compatibility "staging"
symlink as part of "host-finalize".

I.e:

STAGING_DIR_SYMLINK := $(BASE_DIR)/staging
$(STAGING_DIR_SYMLINK): $(BASE_DIR)
	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging

host-finalize: $(HOST_DIR_SYMLINK) $(STAGING_DIR_SYMLINK

What do you think about this idea ?

Thanks,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic
  2018-11-16  1:21   ` Matthew Weber
@ 2018-11-16 14:15     ` Thomas Petazzoni
  2018-11-16 15:14       ` Thomas Petazzoni
  0 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-16 14:15 UTC (permalink / raw)
  To: buildroot

Hello Matt,

Thanks for the feedback, and for testing the patch series.

On Thu, 15 Nov 2018 19:21:27 -0600, Matthew Weber wrote:

> > -# Populating the staging with the base directories is handled by the
> > skeleton package
> > -$(STAGING_DIR):
> > -	@mkdir -p $(STAGING_DIR)
> > -	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> > -  
> 
> With per package disabled, the staging directory never gets created
> early enough for packages to be able to reference content.
> Specifically I noticed docker couldn't find -lsystemd.  I'm assuming
> it's because of the new target dependency on target-post-image?

The STAGING_DIR itself is definitely created at the beginning of the
build, at the first package installing something into it.

However, with this patch, the $(BASE_DIR)/staging symlink indeed gets
created at the end of the build rather than at the beginning of the
build. However, this symlink is only a compatibility one. The code
should use $(STAGING_DIR) everywhere, which should not depend on this
"staging" compatibility symlink.

I've started a build here to reproduce the issue, but in general it
would be nice when you report issues to include an example defconfig
that exhibits the problem, as well as the build log. It would be a lot
easier to understand the specifics of the problem, and be able to
reproduce it.

Thanks,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support
  2018-11-15 16:41 ` Andreas Naumann
@ 2018-11-16 14:43   ` Thomas Petazzoni
  2018-11-19 14:17     ` Andreas Naumann
  0 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-16 14:43 UTC (permalink / raw)
  To: buildroot

Hello Andreas,

Thanks for your feedback!

On Thu, 15 Nov 2018 17:41:27 +0100, Andreas Naumann wrote:

> thank you for the updated version. I tried to run it with my configs. 

Thanks a lot for testing!

> For that I needed
> a) a generic patch for all kconfig packages, see my answer on the final 
> patch

For this one, we need to continue the discussion with Yann and Thomas
De Schampheleire to understand what is the right approach.

> b) linux-specific patch posted two days ago: [PATCH v2 1/1] linux: Make 
> dtc install step more reliable

This one is a package-specific fix, which can be merged separately from
the core of the PPSH series. I want to keep the PPSH series focused on
introducing PPSH support, and not make it grow infinitely with fixup
patches for lots of packages.

However, perhaps what we could do is to collect those patches in
branch, this way we could have two branches:

 - One with the PPSH core stuff

 - One with all the PPSH-related fixes, which are being
   submitted/applied progressively to Buildroot, even before PPSH
   itself is merged.

What do you think ?

> On my local machine things seemed to work, however final rootfs creation 
> failed in our CI environment due to broken host- binaries, e.g. 
> mkfs.ubifs not finding some required lib (liblzo2.so.2 in that 
> particular case).
> It seems some things with rpath manipulation/checking do not yet work 
> properly. Looking into the binary with ldd I see libs used from the 
> host-machine that probably should come from buildroots host/usr/lib...
> 	linux-vdso.so.1 =>  (0x00007ffc06fcc000)
> 	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f760428f000)
> 	liblzo2.so.2 => not found
> 	libuuid.so.1 => /lib/x86_64-linux-gnu/libuuid.so.1 (0x00007f760408a000)
> 	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f7603d81000)
> 	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f76039b7000)
> 	/lib64/ld-linux-x86-64.so.2 (0x00007f76044a9000)

This is weird, because it works fine here:

thomas at windsurf:~/projets/outputs/t$ ldd host/sbin/mkfs.ubifs 
	linux-vdso.so.1 (0x00007ffe8cd3b000)
	libz.so.1 => /home/thomas/projets/outputs/t/per-package/host-mtd/host/lib/libz.so.1 (0x00007f1a683ff000)
	liblzo2.so.2 => /home/thomas/projets/outputs/t/per-package/host-mtd/host/lib/liblzo2.so.2 (0x00007f1a683db000)
	libuuid.so.1 => /home/thomas/projets/outputs/t/per-package/host-mtd/host/lib/libuuid.so.1 (0x00007f1a683d2000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f1a6821e000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f1a68058000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f1a6841b000)

Could you dump the RPATH of the mkfs.ubifs library, like this:

thomas at windsurf:~/projets/outputs/t$ readelf -d host/sbin/mkfs.ubifs 

Dynamic section at offset 0x11dd0 contains 29 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libz.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [liblzo2.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libuuid.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000000f (RPATH)              Library rpath: [/home/thomas/projets/outputs/t/per-package/host-mtd/host/lib:/home/thomas/projets/outputs/t/per-package/host-util-linux/host/lib]
 0x000000000000000c (INIT)               0x402000
 0x000000000000000d (FINI)               0x40bb08
 0x0000000000000019 (INIT_ARRAY)         0x412dc0
 0x000000000000001b (INIT_ARRAYSZ)       8 (bytes)
 0x000000000000001a (FINI_ARRAY)         0x412dc8
 0x000000000000001c (FINI_ARRAYSZ)       8 (bytes)
 0x000000006ffffef5 (GNU_HASH)           0x400308
 0x0000000000000005 (STRTAB)             0x4009b8
 0x0000000000000006 (SYMTAB)             0x400340
 0x000000000000000a (STRSZ)              781 (bytes)
 0x000000000000000b (SYMENT)             24 (bytes)
 0x0000000000000015 (DEBUG)              0x0
 0x0000000000000003 (PLTGOT)             0x413000
 0x0000000000000002 (PLTRELSZ)           1464 (bytes)
 0x0000000000000014 (PLTREL)             RELA
 0x0000000000000017 (JMPREL)             0x400e58
 0x0000000000000007 (RELA)               0x400db0
 0x0000000000000008 (RELASZ)             168 (bytes)
 0x0000000000000009 (RELAENT)            24 (bytes)
 0x000000006ffffffe (VERNEED)            0x400d50
 0x000000006fffffff (VERNEEDNUM)         2
 0x000000006ffffff0 (VERSYM)             0x400cc6
 0x0000000000000000 (NULL)               0x0

Thanks a lot!

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic
  2018-11-16 14:15     ` Thomas Petazzoni
@ 2018-11-16 15:14       ` Thomas Petazzoni
  2018-11-20 22:08         ` Matthew Weber
  0 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-16 15:14 UTC (permalink / raw)
  To: buildroot

Hello,

On Fri, 16 Nov 2018 15:15:36 +0100, Thomas Petazzoni wrote:

> On Thu, 15 Nov 2018 19:21:27 -0600, Matthew Weber wrote:
> 
> > > -# Populating the staging with the base directories is handled by the
> > > skeleton package
> > > -$(STAGING_DIR):
> > > -	@mkdir -p $(STAGING_DIR)
> > > -	@ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> > > -    
> > 
> > With per package disabled, the staging directory never gets created
> > early enough for packages to be able to reference content.
> > Specifically I noticed docker couldn't find -lsystemd.  I'm assuming
> > it's because of the new target dependency on target-post-image?  
> 
> The STAGING_DIR itself is definitely created at the beginning of the
> build, at the first package installing something into it.
> 
> However, with this patch, the $(BASE_DIR)/staging symlink indeed gets
> created at the end of the build rather than at the beginning of the
> build. However, this symlink is only a compatibility one. The code
> should use $(STAGING_DIR) everywhere, which should not depend on this
> "staging" compatibility symlink.
> 
> I've started a build here to reproduce the issue, but in general it
> would be nice when you report issues to include an example defconfig
> that exhibits the problem, as well as the build log. It would be a lot
> easier to understand the specifics of the problem, and be able to
> reproduce it.

FWIW, the following defconfig:

BR2_arm=y
BR2_cortex_a8=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y
BR2_INIT_SYSTEMD=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_DOCKER_ENGINE=y
# BR2_TARGET_ROOTFS_TAR is not set

with ppsh-v4, but BR2_PER_PACKAGE_FOLDERS disabled, builds just fine,
and dockerd is linked with libsystemd.

Could you give more details about the issue you're seeing ?

Thanks,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-16 13:47     ` Thomas Petazzoni
@ 2018-11-16 15:22       ` Thomas De Schampheleire
  2018-11-16 19:57         ` Yann E. MORIN
  0 siblings, 1 reply; 35+ messages in thread
From: Thomas De Schampheleire @ 2018-11-16 15:22 UTC (permalink / raw)
  To: buildroot

On Fri, Nov 16, 2018, 14:47 Thomas Petazzoni <thomas.petazzoni@bootlin.com
wrote:

> Hello,
>
> Yann was already in Cc, I've added Thomas De Schampheleire as well.
> Yann, Thomas, there is some kconfig related discussion below, we
> need you :-)
>
> On Thu, 15 Nov 2018 17:41:35 +0100, Andreas Naumann wrote:
>
> > This is much clearer to me than v3, however there is a problem with
> > kconfig packages. Unfortunately, the kconfig infrastructure is running
> > the 'make xxx_defconfig' step before the configure-step is "officially"
> > started. This is due to the kconfig_fixup target being a dependency of
> > -configure. This in turn requires availability of the toolchain and
> > other prerequisites that are part of _FINAL_DEPENDENCIES earlier as
> > prepared in your above patch.
> >
> > I made a patch which moves the _FINAL_DEPENDENCIES preparation to an
> > additional .stamp-configure-prepare step just before .stamp_configured.
> > That works but is not particularly beautiful.
>
> That is not too bad actually. Semantically speaking, preparing the
> per-package folders is not really part of the configuration step. It
> could be logical to do it in a "prepare" step.
>
> The only thing that I dislike a bit with this is that it would no
> longer be consistent with what we do for download and extract
> dependencies: we prepare the per-package folders with download and
> extract dependencies respectively in the download and extract steps. So
> it would be quite logical to also do the same for the "configuration
> dependencies" (which we name just "dependencies" in Buildroot).
>
> So, this leaves us with 3 options:
>
>  - Keep the dependency preparation within the download, extract and
>    configure step, as proposed in this v4. This will require changing
>    the kconfig logic to prepare the configuration file inside the
>    "configure" step and not as a additional step injected before the
>    "configure" step.
>
>  - Keep the dependency preparation within the download and extract
>    steps, and make an exception for the configure step, with a separate
>    "prepare" step that comes before. Not nice in terms of consistency,
>    as explained above.
>
>  - Introduce "prepare download", "prepare extract" and "prepare
>    configure" steps that would do the dependency preparation.
>
> > I guess a more proper solution would be to somehow move the
> > kconfig_fixup code into the configure-step. Maybe use the
> > pre-configure-hook, any suggestions?
>
> I don't recall why they need to be done before the configuration step,
> but I'm pretty sure there is a reason for that.
>
> Yann, Thomas, do you remember ?
>

My mind triggers a big red warning light right now, so let's be careful :-)

The goal is that one can run 'make foo-menuconfig' from a clean tree,
without first processing (downloading, building, ...) the dependencies of
foo first.
If you put this stuff in the configure step of foo, you are bound by its
dependencies.

There may be other reasons too, not sure.

Best regards,
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20181116/705cfae7/attachment.html>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-16 15:22       ` Thomas De Schampheleire
@ 2018-11-16 19:57         ` Yann E. MORIN
  2018-11-18 21:55           ` Arnout Vandecappelle
  0 siblings, 1 reply; 35+ messages in thread
From: Yann E. MORIN @ 2018-11-16 19:57 UTC (permalink / raw)
  To: buildroot

Thomas?, All,

On 2018-11-16 16:22 +0100, Thomas De Schampheleire spake thusly:
> On Fri, Nov 16, 2018, 14:47 Thomas Petazzoni < [1]thomas.petazzoni@bootlin.com wrote:
>   On Thu, 15 Nov 2018 17:41:35 +0100, Andreas Naumann wrote:
[--SNIP--]
>   > I made a patch which moves the _FINAL_DEPENDENCIES preparation to an
>   > additional .stamp-configure-prepare step just before .stamp_configured.
>   > That works but is not particularly beautiful.
> 
>   That is not too bad actually. Semantically speaking, preparing the
>   per-package folders is not really part of the configuration step. It
>   could be logical to do it in a "prepare" step.

And IIRC, you alreqady proposed such a step, specifically to be able to
do the autopreconf for OOT building (which I am still working on, btw).

Maybe this step can be re-used for various pacakges infras, like the
kconfig one, to add preparation steps.

>   The only thing that I dislike a bit with this is that it would no
>   longer be consistent with what we do for download and extract
>   dependencies: we prepare the per-package folders with download and
>   extract dependencies respectively in the download and extract steps. So
>   it would be quite logical to also do the same for the "configuration
>   dependencies" (which we name just "dependencies" in Buildroot).
> 
>   So, this leaves us with 3 options:
> 
>   ?- Keep the dependency preparation within the download, extract and
>   ? ?configure step, as proposed in this v4. This will require changing
>   ? ?the kconfig logic to prepare the configuration file inside the
>   ? ?"configure" step and not as a additional step injected before the
>   ? ?"configure" step.
> 
>   ?- Keep the dependency preparation within the download and extract
>   ? ?steps, and make an exception for the configure step, with a separate
>   ? ?"prepare" step that comes before. Not nice in terms of consistency,
>   ? ?as explained above.
> 
>   ?- Introduce "prepare download", "prepare extract" and "prepare
>   ? ?configure" steps that would do the dependency preparation.
> 
>   > I guess a more proper solution would be to somehow move the
>   > kconfig_fixup code into the configure-step. Maybe use the
>   > pre-configure-hook, any suggestions?
> 
>   I don't recall why they need to be done before the configuration step,
>   but I'm pretty sure there is a reason for that.
> 
>   Yann, Thomas, do you remember ?
> 
> My mind triggers a big red warning light right now, so let's be careful :-)

Yes, we got bitten pretty hard when preparing the kconfig infra. But
what can we remember from an almost-5-year old work initiated after
FOSDEM? ;-)

> The goal is that one can run 'make foo-menuconfig' from a clean tree,
> without first processing (downloading, building, ...) the dependencies
> of foo first.
> If you put this stuff in the configure step of foo, you are bound byi
> its dependencies. There may be other reasons too, not sure.

I am all in favour of simplifying it if it can be made simpler by adding
an official extra 'prepare' step, that exists for all types of package
infras. This we'd have 4 levels of dependencies:

 1- download dependencies
 2- extract dependencies
 3- prepare dependencies
 4- configure dependencies

Currently, 1, 2, and 4 are implemented in a generic way and thus
available to all types of packages, while 3 is implemented only for
kconfig-package, in an ad-hoc way, and used only by the linux kernel to
ensure the toolchain is available before its kconfig is called.

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-16 19:57         ` Yann E. MORIN
@ 2018-11-18 21:55           ` Arnout Vandecappelle
  2018-11-19 10:48             ` Thomas Petazzoni
  0 siblings, 1 reply; 35+ messages in thread
From: Arnout Vandecappelle @ 2018-11-18 21:55 UTC (permalink / raw)
  To: buildroot



On 16/11/2018 20:57, Yann E. MORIN wrote:
> Thomas?, All,
> 
> On 2018-11-16 16:22 +0100, Thomas De Schampheleire spake thusly:
>> On Fri, Nov 16, 2018, 14:47 Thomas Petazzoni < [1]thomas.petazzoni@bootlin.com wrote:
>>   On Thu, 15 Nov 2018 17:41:35 +0100, Andreas Naumann wrote:
> [--SNIP--]
>>   > I made a patch which moves the _FINAL_DEPENDENCIES preparation to an
>>   > additional .stamp-configure-prepare step just before .stamp_configured.
>>   > That works but is not particularly beautiful.
>>
>>   That is not too bad actually. Semantically speaking, preparing the
>>   per-package folders is not really part of the configuration step. It
>>   could be logical to do it in a "prepare" step.
> 
> And IIRC, you alreqady proposed such a step, specifically to be able to
> do the autopreconf for OOT building (which I am still working on, btw).
> 
> Maybe this step can be re-used for various pacakges infras, like the
> kconfig one, to add preparation steps.
> 
>>   The only thing that I dislike a bit with this is that it would no
>>   longer be consistent with what we do for download and extract
>>   dependencies: we prepare the per-package folders with download and
>>   extract dependencies respectively in the download and extract steps. So
>>   it would be quite logical to also do the same for the "configuration
>>   dependencies" (which we name just "dependencies" in Buildroot).
>>
>>   So, this leaves us with 3 options:
>>
>>   ?- Keep the dependency preparation within the download, extract and
>>   ? ?configure step, as proposed in this v4. This will require changing
>>   ? ?the kconfig logic to prepare the configuration file inside the
>>   ? ?"configure" step and not as a additional step injected before the
>>   ? ?"configure" step.
>>
>>   ?- Keep the dependency preparation within the download and extract
>>   ? ?steps, and make an exception for the configure step, with a separate
>>   ? ?"prepare" step that comes before. Not nice in terms of consistency,
>>   ? ?as explained above.
>>
>>   ?- Introduce "prepare download", "prepare extract" and "prepare
>>   ? ?configure" steps that would do the dependency preparation.
>>
>>   > I guess a more proper solution would be to somehow move the
>>   > kconfig_fixup code into the configure-step. Maybe use the
>>   > pre-configure-hook, any suggestions?
>>
>>   I don't recall why they need to be done before the configuration step,
>>   but I'm pretty sure there is a reason for that.
>>
>>   Yann, Thomas, do you remember ?
>>
>> My mind triggers a big red warning light right now, so let's be careful :-)
> 
> Yes, we got bitten pretty hard when preparing the kconfig infra. But
> what can we remember from an almost-5-year old work initiated after
> FOSDEM? ;-)
> 
>> The goal is that one can run 'make foo-menuconfig' from a clean tree,
>> without first processing (downloading, building, ...) the dependencies
>> of foo first.
>> If you put this stuff in the configure step of foo, you are bound byi
>> its dependencies. There may be other reasons too, not sure.
> 
> I am all in favour of simplifying it if it can be made simpler by adding
> an official extra 'prepare' step, that exists for all types of package
> infras. This we'd have 4 levels of dependencies:
> 
>  1- download dependencies
>  2- extract dependencies
>  3- prepare dependencies
>  4- configure dependencies

 I think your mixing two things here. The "prepare" that ThomasP is talking
about is really the PPD creation. The additional step for autoreconf or kconfig
is something else - it's a step with commands that are defined by the package
(infra).


 Regards,
 Arnout


> 
> Currently, 1, 2, and 4 are implemented in a generic way and thus
> available to all types of packages, while 3 is implemented only for
> kconfig-package, in an ad-hoc way, and used only by the linux kernel to
> ensure the toolchain is available before its kconfig is called.
> 
> Regards,
> Yann E. MORIN.
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-18 21:55           ` Arnout Vandecappelle
@ 2018-11-19 10:48             ` Thomas Petazzoni
  2018-11-19 14:27               ` Andreas Naumann
                                 ` (2 more replies)
  0 siblings, 3 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-19 10:48 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 18 Nov 2018 22:55:26 +0100, Arnout Vandecappelle wrote:

> > Yes, we got bitten pretty hard when preparing the kconfig infra. But
> > what can we remember from an almost-5-year old work initiated after
> > FOSDEM? ;-)
> >   
> >> The goal is that one can run 'make foo-menuconfig' from a clean tree,
> >> without first processing (downloading, building, ...) the dependencies
> >> of foo first.
> >> If you put this stuff in the configure step of foo, you are bound byi
> >> its dependencies. There may be other reasons too, not sure.  
> > 
> > I am all in favour of simplifying it if it can be made simpler by adding
> > an official extra 'prepare' step, that exists for all types of package
> > infras. This we'd have 4 levels of dependencies:
> > 
> >  1- download dependencies
> >  2- extract dependencies
> >  3- prepare dependencies
> >  4- configure dependencies  
> 
>  I think your mixing two things here. The "prepare" that ThomasP is talking
> about is really the PPD creation. The additional step for autoreconf or kconfig
> is something else - it's a step with commands that are defined by the package
> (infra).

I don't think Yann is mixing things here. Indeed we are using the word
"prepare" for two different things: preparing the PPD folder, and
"doing some stuff before the configure step".

For the PPD folder, some preparation currently occurs at the download
step (for download dependencies), some preparation occurs at the
extract step (for the extract dependencies) and some preparation occurs
at the configuration step (for the normal dependencies).

The issue with the kconfig infrastructure is that it creates a new type
of dependency called "kconfig dependency", which should be ready to do
all the kconfig-related activities, but that we want separate from the
normal "configure dependencies" because we want to be able to do "make
linux-menuconfig" or "make busybox-menuconfig" without having to wait
for their dependencies to build.

So what Yann is proposing is that instead of having this "kconfig
dependency" stuff be shoe-horned into the dependency chain by
pkg-kconfig.mk, we should promote it to a new step in the build steps.
By the lack of better naming, Yann named that step "prepare", but it
has nothing to do with "preparing PPD".

Indeed, what we would do is:

 1. Download step
    Prepare PPD with download dependencies
    Do the normal download stuff

 2. Extract step
    Prepare PPD with extract dependencies
    Do the normal extract stuff

 3. Patch step
    Prepare PPD with patch dependencies
    Do the normal patch stuff

 4. Prepare step
    Prepare PPD with the prepare dependencies
    Do the prepare stuff (i.e only the kconfig stuff for now)

 5. Configure step
    Prepare PPD with the normal dependencies
    Do the normal configure stuff

Does that clarify Yann's proposal ?

I personally find it OK, even though it's a bit annoying to introduce
yet another step just for the sake of pkg-kconfig.

Also: I would not move autoreconf into this prepare step, but keep it
as it is today, within the configure step. I don't (today at least) see
the point/usefulness of moving the autoreconf into this prepare step.

Best regards,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support
  2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
                   ` (7 preceding siblings ...)
  2018-11-15 16:41 ` Andreas Naumann
@ 2018-11-19 13:30 ` Arnout Vandecappelle
  8 siblings, 0 replies; 35+ messages in thread
From: Arnout Vandecappelle @ 2018-11-19 13:30 UTC (permalink / raw)
  To: buildroot

 Hi Thomas,


On 14/11/2018 11:55, Thomas Petazzoni wrote:
> Hello,
> 
> Here is a fourth iteration of the per-package SDK and target directory
> implementation.

 It's very late to be coming with this, and I haven't thought it through, but I
realized something now...

 What we are doing is to create a per-package directory in which all of that
package's dependencies are collected. However, NixOS for example is doing the
reverse: each package gets installed in a separate staging directory, and
dependant packages are built with a long list of -I and -L flags to point to all
the dependencies. Well, to be exact, in many cases it's just PKG_CONFIG_LIBDIR
that needs to be set.

 The big advantage of such an approach is that no rewriting of absolute paths is
needed. It's perfectly fine if there are absolute paths, because we're not going
to change the location of any library or include directory.

 There are a few disadvantages to this approach that I already thought of:

- It doesn't fix the issue with qmake that the installation location is
hardcoded in the qmake configuration - but I guess a solution for that must exist.

- We're not doing exactly the same as NixOS, since NixOS will set --prefix to
the real installation location, while we will use --prefix=/usr and
DESTDIR=$($(PKG)_STAGING_DIR). I'm not sure what the impact is, but I'm pretty
sure it will have an impact. We can't use --prefix because we don't want that
absolute path on the target.

- We still need to combine everything for the SDK.

- Probably others...


 Still, given all the problems that we already found with PPD, it's something to
consider I think.

 Regards,
 Arnout



> 
> 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-v4
> 
> Changes since v4
> ================
> 
>  - Dropped patches that have been merged upstream, mainly the "extract
>    dependency" patches, but also a few other preparation patches.
> 
>  - Dropped the change of the RPATH handling. As discussed during
>    previous Buildroot Developers meeting, it is not a big problem if
>    during the build, the RPATH of a binary points to the library
>    folder of another package per-package folder. Therefore, instead of
>    fixing up RPATH at the end of each package installation, we keep
>    what Buildroot does today: fix them at the very end of the build.
> 
>  - Made the support for per-package folders optional. Indeed, we
>    realized there was no way to make sure it would be perfect from day
>    1. Even if the principle works, there are lots of package-specific
>    details to solve, and solving all of them before merging the base
>    per-package folder support is not reasonable. So for now, an option
>    BR2_PER_PACKAGE_FOLDERS is introduced, which defaults to off. When
>    this option is enabled, the .NOTPARALLEL statement of the main
>    Makefile goes away, which allows to do top-level parallel build.
> 
>  - Addition of a commit that reworks how the top-level directories are
>    created.
> 
>  - Introduction of a prepare-per-package-folder function in
>    pkg-generic.mk to encapsulate the logic to create the per-package
>    folders from the dependencies of a package.
> 
>  - Rebased on top of the latest master.
> 
> Changes since v3
> ================
> 
>  - Dropped patches that have been merged upstream:
>    pkgconf: use relative path to  STAGING_DIR instead of absolute path
>    toolchain: post-pone evaluation of  TOOLCHAIN_EXTERNAL_BIN
> 
>  - For now, removed the most controversial/complicated patches
>    (changing the rpath strategy, and switching to per-package
>    host/target directories). The goal for now is to merge only the
>    preparation patches.
> 
> Changes since v2
> ================
> 
>  - Solved the problem of all DEPENDENCIES_HOST_PREREQ targets:
>    host-ccache, host-tar, host-lzip, host-xz, host-fakedate.
> 
>    To achieve this, introduced <pkg>_EXTRACT_DEPENDENCIES and used
>    that to handle the dependencies on host-tar, host-lzip and host-xz.
> 
>    Used regular dependencies for host-ccache and host-fakedate.
> 
>    See below for more details.
> 
> Changes since v1
> ================
> 
>  - Rebased on the latest Buildroot next branch
> 
>  - The pkg-config changes are reworked according to Arnout's comments:
>    only @STAGING_SUBDIR@ is replaced in pkg-config.in to generate
>    pkg-config, the rest of the logic is internal to the script.
> 
>  - New patch to move the "host skeleton" logic into a proper
>    host-skeleton package.
> 
>  - New patch to have the CMake related files generated as part of the
>    toolchain package.
> 
>  - New patch to add a new "install" step that depends on the different
>    install steps (target, staging, images, host). This is needed to
>    later add some logic that is executed once all install steps of a
>    package have been done.
> 
>  - Fix the approach to solve the RPATH logic: instead of using
>    LD_LIBRARY_PATH, we actually fix with patchelf the RPATH of host
>    binaries right after their installation.
> 
>  - Misc other improvements in the per-package SDK implementation.
> 
> What is this all about ?
> ========================
> 
> See the cover letter of v3 for details:
> http://lists.busybox.net/pipermail/buildroot/2017-December/208384.html
> 
> What remains to be done?
> ========================
> 
> The main problem that remains is fixing packages that don't work well
> with per-package folders. I have already sent a few fixes (such as
> adding missing host-pkgconf dependencies, or fixing the freetype
> package), but I have setup a separate autobuilder instance to test
> this, and it uncovered a number of other problematic packages:
> 
>  - apr-util doesn't build, because some hardcoded paths from the "apr"
>    per-package folders leak into the apr-util build.
> 
>  - Meson packages currently do not build, because pkg-meson.mk doesn't
>    use $$(HOST_DIR) to reference cross-compilation.conf, but more
>    importantly because cross-compilation.conf uses hardcoded paths.
> 
>  - 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. I started working on
>    this topic, and will be sending a separate e-mail about this.
> 
> Best regards,
> 
> Thomas
> 
> Thomas Petazzoni (6):
>   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
>   core: implement per-package SDK and target
> 
>  Config.in                        | 18 ++++++
>  Makefile                         | 94 ++++++++++++++++++++------------
>  fs/common.mk                     |  3 +-
>  package/pkg-generic.mk           | 27 ++++++++-
>  support/scripts/check-host-rpath |  8 ++-
>  5 files changed, 110 insertions(+), 40 deletions(-)
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support
  2018-11-16 14:43   ` Thomas Petazzoni
@ 2018-11-19 14:17     ` Andreas Naumann
  0 siblings, 0 replies; 35+ messages in thread
From: Andreas Naumann @ 2018-11-19 14:17 UTC (permalink / raw)
  To: buildroot

Hi Thomas,

Am 16.11.18 um 15:43 schrieb Thomas Petazzoni:
> Hello Andreas,
> 
> Thanks for your feedback!
> 
> On Thu, 15 Nov 2018 17:41:27 +0100, Andreas Naumann wrote:
> 
>> thank you for the updated version. I tried to run it with my configs.
> 
> Thanks a lot for testing!
> 
>> For that I needed
>> a) a generic patch for all kconfig packages, see my answer on the final
>> patch
> 
> For this one, we need to continue the discussion with Yann and Thomas
> De Schampheleire to understand what is the right approach.

Ok.

> 
>> b) linux-specific patch posted two days ago: [PATCH v2 1/1] linux: Make
>> dtc install step more reliable
> 
> This one is a package-specific fix, which can be merged separately from
> the core of the PPSH series. I want to keep the PPSH series focused on
> introducing PPSH support, and not make it grow infinitely with fixup
> patches for lots of packages.
> 
> However, perhaps what we could do is to collect those patches in
> branch, this way we could have two branches:
> 
>   - One with the PPSH core stuff
> 
>   - One with all the PPSH-related fixes, which are being
>     submitted/applied progressively to Buildroot, even before PPSH
>     itself is merged.
> 
> What do you think ?

Ok, but right now I have only this one additional patch.

> 
>> On my local machine things seemed to work, however final rootfs creation
>> failed in our CI environment due to broken host- binaries, e.g.
>> mkfs.ubifs not finding some required lib (liblzo2.so.2 in that
>> particular case).
>> It seems some things with rpath manipulation/checking do not yet work
>> properly. Looking into the binary with ldd I see libs used from the
>> host-machine that probably should come from buildroots host/usr/lib...
>> 	linux-vdso.so.1 =>  (0x00007ffc06fcc000)
>> 	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f760428f000)
>> 	liblzo2.so.2 => not found
>> 	libuuid.so.1 => /lib/x86_64-linux-gnu/libuuid.so.1 (0x00007f760408a000)
>> 	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f7603d81000)
>> 	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f76039b7000)
>> 	/lib64/ld-linux-x86-64.so.2 (0x00007f76044a9000)
> 
> This is weird, because it works fine here:
> 
> thomas at windsurf:~/projets/outputs/t$ ldd host/sbin/mkfs.ubifs
> 	linux-vdso.so.1 (0x00007ffe8cd3b000)
> 	libz.so.1 => /home/thomas/projets/outputs/t/per-package/host-mtd/host/lib/libz.so.1 (0x00007f1a683ff000)
> 	liblzo2.so.2 => /home/thomas/projets/outputs/t/per-package/host-mtd/host/lib/liblzo2.so.2 (0x00007f1a683db000)
> 	libuuid.so.1 => /home/thomas/projets/outputs/t/per-package/host-mtd/host/lib/libuuid.so.1 (0x00007f1a683d2000)
> 	libm.so.6 => /lib64/libm.so.6 (0x00007f1a6821e000)
> 	libc.so.6 => /lib64/libc.so.6 (0x00007f1a68058000)
> 	/lib64/ld-linux-x86-64.so.2 (0x00007f1a6841b000)
> 
> Could you dump the RPATH of the mkfs.ubifs library, like this:
> 
> thomas at windsurf:~/projets/outputs/t$ readelf -d host/sbin/mkfs.ubifs
> 
> Dynamic section at offset 0x11dd0 contains 29 entries:
>    Tag        Type                         Name/Value
>   0x0000000000000001 (NEEDED)             Shared library: [libz.so.1]
>   0x0000000000000001 (NEEDED)             Shared library: [liblzo2.so.2]
>   0x0000000000000001 (NEEDED)             Shared library: [libuuid.so.1]
>   0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
>   0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
>   0x000000000000000f (RPATH)              Library rpath: [/home/thomas/projets/outputs/t/per-package/host-mtd/host/lib:/home/thomas/projets/outputs/t/per-package/host-util-linux/host/lib]
>   0x000000000000000c (INIT)               0x402000
>   0x000000000000000d (FINI)               0x40bb08
>   0x0000000000000019 (INIT_ARRAY)         0x412dc0
>   0x000000000000001b (INIT_ARRAYSZ)       8 (bytes)
>   0x000000000000001a (FINI_ARRAY)         0x412dc8
>   0x000000000000001c (FINI_ARRAYSZ)       8 (bytes)
>   0x000000006ffffef5 (GNU_HASH)           0x400308
>   0x0000000000000005 (STRTAB)             0x4009b8
>   0x0000000000000006 (SYMTAB)             0x400340
>   0x000000000000000a (STRSZ)              781 (bytes)
>   0x000000000000000b (SYMENT)             24 (bytes)
>   0x0000000000000015 (DEBUG)              0x0
>   0x0000000000000003 (PLTGOT)             0x413000
>   0x0000000000000002 (PLTRELSZ)           1464 (bytes)
>   0x0000000000000014 (PLTREL)             RELA
>   0x0000000000000017 (JMPREL)             0x400e58
>   0x0000000000000007 (RELA)               0x400db0
>   0x0000000000000008 (RELASZ)             168 (bytes)
>   0x0000000000000009 (RELAENT)            24 (bytes)
>   0x000000006ffffffe (VERNEED)            0x400d50
>   0x000000006fffffff (VERNEEDNUM)         2
>   0x000000006ffffff0 (VERSYM)             0x400cc6
>   0x0000000000000000 (NULL)               0x0
> 

I've done that and found that in the troubling case the RPATH is empty. 
This might be a problem of a custom wrapper I use which creates multiple 
images in different output folders. Digging deeper will take some time, 
thanks for the pointer!


regards,
Andreas

> Thanks a lot!
> 
> Thomas
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-19 10:48             ` Thomas Petazzoni
@ 2018-11-19 14:27               ` Andreas Naumann
  2018-11-19 19:49               ` Yann E. MORIN
  2018-11-20 16:19               ` Thomas Petazzoni
  2 siblings, 0 replies; 35+ messages in thread
From: Andreas Naumann @ 2018-11-19 14:27 UTC (permalink / raw)
  To: buildroot

Hi,

Am 19.11.18 um 11:48 schrieb Thomas Petazzoni:
> Hello,
> 
> On Sun, 18 Nov 2018 22:55:26 +0100, Arnout Vandecappelle wrote:
> 
>>> Yes, we got bitten pretty hard when preparing the kconfig infra. But
>>> what can we remember from an almost-5-year old work initiated after
>>> FOSDEM? ;-)
>>>    
>>>> The goal is that one can run 'make foo-menuconfig' from a clean tree,
>>>> without first processing (downloading, building, ...) the dependencies
>>>> of foo first.
>>>> If you put this stuff in the configure step of foo, you are bound byi
>>>> its dependencies. There may be other reasons too, not sure.
>>>
>>> I am all in favour of simplifying it if it can be made simpler by adding
>>> an official extra 'prepare' step, that exists for all types of package
>>> infras. This we'd have 4 levels of dependencies:
>>>
>>>   1- download dependencies
>>>   2- extract dependencies
>>>   3- prepare dependencies
>>>   4- configure dependencies
>>
>>   I think your mixing two things here. The "prepare" that ThomasP is talking
>> about is really the PPD creation. The additional step for autoreconf or kconfig
>> is something else - it's a step with commands that are defined by the package
>> (infra).
> 
> I don't think Yann is mixing things here. Indeed we are using the word
> "prepare" for two different things: preparing the PPD folder, and
> "doing some stuff before the configure step".
> 
> For the PPD folder, some preparation currently occurs at the download
> step (for download dependencies), some preparation occurs at the
> extract step (for the extract dependencies) and some preparation occurs
> at the configuration step (for the normal dependencies).
> 
> The issue with the kconfig infrastructure is that it creates a new type
> of dependency called "kconfig dependency", which should be ready to do
> all the kconfig-related activities, but that we want separate from the
> normal "configure dependencies" because we want to be able to do "make
> linux-menuconfig" or "make busybox-menuconfig" without having to wait
> for their dependencies to build.
> 
> So what Yann is proposing is that instead of having this "kconfig
> dependency" stuff be shoe-horned into the dependency chain by
> pkg-kconfig.mk, we should promote it to a new step in the build steps.
> By the lack of better naming, Yann named that step "prepare", but it
> has nothing to do with "preparing PPD".
> 
> Indeed, what we would do is:
> 
>   1. Download step
>      Prepare PPD with download dependencies
>      Do the normal download stuff
> 
>   2. Extract step
>      Prepare PPD with extract dependencies
>      Do the normal extract stuff
> 
>   3. Patch step
>      Prepare PPD with patch dependencies
>      Do the normal patch stuff
> 
>   4. Prepare step
>      Prepare PPD with the prepare dependencies
>      Do the prepare stuff (i.e only the kconfig stuff for now)
> 
>   5. Configure step
>      Prepare PPD with the normal dependencies
>      Do the normal configure stuff
> 
> Does that clarify Yann's proposal ?
> 
> I personally find it OK, even though it's a bit annoying to introduce
> yet another step just for the sake of pkg-kconfig.
> 
> Also: I would not move autoreconf into this prepare step, but keep it
> as it is today, within the configure step. I don't (today at least) see
> the point/usefulness of moving the autoreconf into this prepare step.

This is basically what i have done, but without explicitely introducing 
separate kconfig-dependencies, I hope this is readable:

 From cf583d9ed09c667f24f526edf890d81b374e1d83 Mon Sep 17 00:00:00 2001
From: Andreas Naumann <anaumann@ultratronik.de>
Date: Wed, 14 Nov 2018 17:27:31 +0100
Subject: [PATCH v2 1/1] core: Move per-package host preparation into 
separate
  step

Unfortunately, the kconfig infrastructure is running the 'make 
xxx_defconfig'
step before the configure-step is "officially" started. This is due to
the kconfig_fixup target.
This however requires availability of the toolchain and other prerequisites,
so this patch introduces an intermediate target to prepare the per-package
host dir.

Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
---
  package/pkg-generic.mk | 19 +++++++++++++++----
  package/pkg-kconfig.mk |  2 +-
  2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 8ec5e8db73..3014c13846 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -232,11 +232,18 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
  	$(if $(wildcard $(dir)),,\
  		$(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir))))

+# Prepare per-package host dir for configure
+$(BUILD_DIR)/%/.stamp_configure_prepared:
+	@$(call step_start,configure_prepared)
+	@$(call MESSAGE,"Prepare per-package host")
+	$(call prepare-per-package-folder,$($(PKG)_FINAL_DEPENDENCIES))
+	@$(call step_end,configure_prepared)
+	$(Q)touch $@
+
  # Configure
  $(BUILD_DIR)/%/.stamp_configured:
  	@$(call step_start,configure)
  	@$(call MESSAGE,"Configuring")
-	$(call prepare-per-package-folder,$($(PKG)_FINAL_DEPENDENCIES))
  	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
  	$($(PKG)_CONFIGURE_CMDS)
  	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
@@ -663,6 +670,7 @@ $(2)_TARGET_INSTALL_STAGING = 
$$($(2)_DIR)/.stamp_staging_installed
  $(2)_TARGET_INSTALL_IMAGES =	$$($(2)_DIR)/.stamp_images_installed
  $(2)_TARGET_INSTALL_HOST =      $$($(2)_DIR)/.stamp_host_installed
  $(2)_TARGET_BUILD =		$$($(2)_DIR)/.stamp_built
+$(2)_TARGET_CONFIGURE_PREP =	$$($(2)_DIR)/.stamp_configure_prepared
  $(2)_TARGET_CONFIGURE =		$$($(2)_DIR)/.stamp_configured
  $(2)_TARGET_RSYNC =	        $$($(2)_DIR)/.stamp_rsynced
  $(2)_TARGET_PATCH =		$$($(2)_DIR)/.stamp_patched
@@ -756,7 +764,8 @@ $$($(2)_TARGET_BUILD):	$$($(2)_TARGET_CONFIGURE)
  # dependency by using |.

  $(1)-configure:			$$($(2)_TARGET_CONFIGURE)
-$$($(2)_TARGET_CONFIGURE):	| $$($(2)_FINAL_DEPENDENCIES)
+$$($(2)_TARGET_CONFIGURE):	$$($(2)_TARGET_CONFIGURE_PREP)
+$$($(2)_TARGET_CONFIGURE_PREP): | $$($(2)_FINAL_DEPENDENCIES)

  $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | prepare
  $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies
@@ -768,7 +777,7 @@ ifeq ($$($(2)_OVERRIDE_SRCDIR),)
  #  extract
  #  patch
  #  configure
-$$($(2)_TARGET_CONFIGURE):	$$($(2)_TARGET_PATCH)
+$$($(2)_TARGET_CONFIGURE_PREP):	$$($(2)_TARGET_PATCH)

  $(1)-patch:		$$($(2)_TARGET_PATCH)
  $$($(2)_TARGET_PATCH):	$$($(2)_TARGET_EXTRACT)
@@ -807,7 +816,7 @@ else

  # Use an order-only dependency so the "<pkg>-clean-for-rebuild" rule
  # can remove the stamp file without triggering the configure step.
-$$($(2)_TARGET_CONFIGURE): | $$($(2)_TARGET_RSYNC)
+$$($(2)_TARGET_CONFIGURE_PREP): | $$($(2)_TARGET_RSYNC)

  $(1)-depends:		$$($(2)_FINAL_DEPENDENCIES)

@@ -878,6 +887,7 @@ $(1)-rebuild:		$(1)-clean-for-rebuild $(1)

  $(1)-clean-for-reconfigure: $(1)-clean-for-rebuild
  			rm -f $$($(2)_TARGET_CONFIGURE)
+			rm -f $$($(2)_TARGET_CONFIGURE_PREP)

  $(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)

@@ -889,6 +899,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_PREP):		PKG=$(2)
  $$($(2)_TARGET_RSYNC):			SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
  $$($(2)_TARGET_RSYNC):			PKG=$(2)
  $$($(2)_TARGET_PATCH):			PKG=$(2)
diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index d6c95b897e..5567df01d6 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -128,7 +128,7 @@ $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG): | $(1)-patch
  # Some packages may need additional tools to be present by the time their
  # kconfig structure is parsed (e.g. the linux kernel may need to call to
  # the compiler to test its features).
-$$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG): | $$($(2)_KCONFIG_DEPENDENCIES)
+$$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG): $$($(2)_TARGET_CONFIGURE_PREP) 
| $$($(2)_KCONFIG_DEPENDENCIES)

  # In order to get a usable, consistent configuration, some fixup may 
be needed.
  # The exact rules are specified by the package .mk file.
-- 
2.19.1




> 
> Best regards,
> 
> Thomas
> 

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-19 10:48             ` Thomas Petazzoni
  2018-11-19 14:27               ` Andreas Naumann
@ 2018-11-19 19:49               ` Yann E. MORIN
  2018-11-20 10:22                 ` Arnout Vandecappelle
  2018-11-20 16:19               ` Thomas Petazzoni
  2 siblings, 1 reply; 35+ messages in thread
From: Yann E. MORIN @ 2018-11-19 19:49 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2018-11-19 11:48 +0100, Thomas Petazzoni spake thusly:
> On Sun, 18 Nov 2018 22:55:26 +0100, Arnout Vandecappelle wrote:
[--SNIP--]
> > > I am all in favour of simplifying it if it can be made simpler by adding
> > > an official extra 'prepare' step, that exists for all types of package
> > > infras. This we'd have 4 levels of dependencies:
> > > 
> > >  1- download dependencies
> > >  2- extract dependencies
> > >  3- prepare dependencies
> > >  4- configure dependencies  
> > 
> >  I think your mixing two things here. The "prepare" that ThomasP is talking
> > about is really the PPD creation. The additional step for autoreconf or kconfig
> > is something else - it's a step with commands that are defined by the package
> > (infra).
> 
> I don't think Yann is mixing things here. Indeed we are using the word
> "prepare" for two different things: preparing the PPD folder, and
> "doing some stuff before the configure step".

Yeah, I was actually kinda mixing things here, in fact. ;-)

> For the PPD folder, some preparation currently occurs at the download
> step (for download dependencies), some preparation occurs at the
> extract step (for the extract dependencies) and some preparation occurs
> at the configuration step (for the normal dependencies).

Thanks, I was missing that piece. PPD preparation is split into as many
step as we have.

> The issue with the kconfig infrastructure is that it creates a new type
> of dependency called "kconfig dependency", which should be ready to do
> all the kconfig-related activities, but that we want separate from the
> normal "configure dependencies" because we want to be able to do "make
> linux-menuconfig" or "make busybox-menuconfig" without having to wait
> for their dependencies to build.

Exactly.

> So what Yann is proposing is that instead of having this "kconfig
> dependency" stuff be shoe-horned into the dependency chain by
> pkg-kconfig.mk, we should promote it to a new step in the build steps.

Yes.

> By the lack of better naming, Yann named that step "prepare", but it
> has nothing to do with "preparing PPD".

So, that's where I got it mixed up, indeed.

> Indeed, what we would do is:
> 
>  1. Download step
>     Prepare PPD with download dependencies
>     Do the normal download stuff
> 
>  2. Extract step
>     Prepare PPD with extract dependencies
>     Do the normal extract stuff
> 
>  3. Patch step
>     Prepare PPD with patch dependencies
>     Do the normal patch stuff
> 
>  4. Prepare step
>     Prepare PPD with the prepare dependencies
>     Do the prepare stuff (i.e only the kconfig stuff for now)
> 
>  5. Configure step
>     Prepare PPD with the normal dependencies
>     Do the normal configure stuff
> 
> Does that clarify Yann's proposal ?

Yes, that clarifies it perfectly. Thanks! :-)

> I personally find it OK, even though it's a bit annoying to introduce
> yet another step just for the sake of pkg-kconfig.
> 
> Also: I would not move autoreconf into this prepare step, but keep it
> as it is today, within the configure step. I don't (today at least) see
> the point/usefulness of moving the autoreconf into this prepare step.

For the PPD stuff? Definitely not.

But it will be usefull to run autoreconf in a step that is not the
configure step, so we are able to share the source tree between host and
target build, as well as stop rsyncing from _OVERRIDE_SRCDIR.

But then, I wonder if that is really the same as the 'prepare' step
above, because autoreconf will also require the same dependencies as the
configure step.

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-19 19:49               ` Yann E. MORIN
@ 2018-11-20 10:22                 ` Arnout Vandecappelle
  2018-11-20 10:29                   ` Thomas Petazzoni
  0 siblings, 1 reply; 35+ messages in thread
From: Arnout Vandecappelle @ 2018-11-20 10:22 UTC (permalink / raw)
  To: buildroot



On 19/11/2018 20:49, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2018-11-19 11:48 +0100, Thomas Petazzoni spake thusly:
[snip]
>> Indeed, what we would do is:
>>
>>  1. Download step
>>     Prepare PPD with download dependencies
>>     Do the normal download stuff
>>
>>  2. Extract step
>>     Prepare PPD with extract dependencies
>>     Do the normal extract stuff
>>
>>  3. Patch step
>>     Prepare PPD with patch dependencies
>>     Do the normal patch stuff
>>
>>  4. Prepare step
>>     Prepare PPD with the prepare dependencies
>>     Do the prepare stuff (i.e only the kconfig stuff for now)
>>
>>  5. Configure step
>>     Prepare PPD with the normal dependencies
>>     Do the normal configure stuff
>>
>> Does that clarify Yann's proposal ?
> 
> Yes, that clarifies it perfectly. Thanks! :-)
> 
>> I personally find it OK, even though it's a bit annoying to introduce
>> yet another step just for the sake of pkg-kconfig.

 Me too. And I'm not even sure if it really solves the pkg-kconfig problem. I
have a hard time remembering what all the issues were there.

 Perhaps, though, we should have a more general strategy of a per-infra
definition of what the steps are, instead of having a fixed sequence of steps.
I.e., have a pattern that should be followed by an infra-specific step to make
sure things are done correctly. So, for the kconfig case we would have an extra
kconfig_fixup step (we already have the stamp file, but it's not a real step
with hooks and the KCONFIG_DEPENDENCIES were strapped on as an afterthought),
and for the autoreconf case we would have an extra autoreconf step (instead of a
hook).


>> Also: I would not move autoreconf into this prepare step, but keep it
>> as it is today, within the configure step. I don't (today at least) see
>> the point/usefulness of moving the autoreconf into this prepare step.
> 
> For the PPD stuff? Definitely not.
> 
> But it will be usefull to run autoreconf in a step that is not the
> configure step, so we are able to share the source tree between host and
> target build, as well as stop rsyncing from _OVERRIDE_SRCDIR.

 But the problem is, for pkg-kconfig, we actually need the prepare stuff to be
done in the build directory, not the source directory... So it *still* doesn't
match.


> But then, I wonder if that is really the same as the 'prepare' step

 So no, it's not the same, but...

> above, because autoreconf will also require the same dependencies as the
> configure step.

 ... not because of this. Most dependencies are only really needed in the
configure step, it's only some of them (host-automake etc. obviously, but also
host-autoconf-archive and host-pkg-config for the .m4 files) that are needed for
the autoreconf. So if it is split, we would need to move some dependencies to
_PREPARE_DEPENDENCIES.


 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-20 10:22                 ` Arnout Vandecappelle
@ 2018-11-20 10:29                   ` Thomas Petazzoni
  2018-11-20 16:18                     ` Thomas Petazzoni
  0 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-20 10:29 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 20 Nov 2018 11:22:00 +0100, Arnout Vandecappelle wrote:

> >> I personally find it OK, even though it's a bit annoying to introduce
> >> yet another step just for the sake of pkg-kconfig.  
> 
>  Me too. And I'm not even sure if it really solves the pkg-kconfig problem. I
> have a hard time remembering what all the issues were there.
> 
>  Perhaps, though, we should have a more general strategy of a per-infra
> definition of what the steps are, instead of having a fixed sequence of steps.
> I.e., have a pattern that should be followed by an infra-specific step to make
> sure things are done correctly. So, for the kconfig case we would have an extra
> kconfig_fixup step (we already have the stamp file, but it's not a real step
> with hooks and the KCONFIG_DEPENDENCIES were strapped on as an afterthought),
> and for the autoreconf case we would have an extra autoreconf step (instead of a
> hook).

And so those "injected" steps should take care of preparing the
per-package folder with whatever dependencies they need ?

This is probably something that can be done to fix the pkg-kconfig
stuff without having to create a new "prepare" step in the generic
infrastructure.

>  But the problem is, for pkg-kconfig, we actually need the prepare stuff to be
> done in the build directory, not the source directory... So it *still* doesn't
> match.

Right.

> > above, because autoreconf will also require the same dependencies as the
> > configure step.  
> 
>  ... not because of this. Most dependencies are only really needed in the
> configure step, it's only some of them (host-automake etc. obviously, but also
> host-autoconf-archive and host-pkg-config for the .m4 files) that are needed for
> the autoreconf. So if it is split, we would need to move some dependencies to
> _PREPARE_DEPENDENCIES.

At this point, I don't want to change anything for autoreconf. It is
part of the configure step, it works as it is. I'd like to fix just the
pkg-kconfig situation. I'll try to do that within pkg-kconfig.mk.

Best regards,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-20 10:29                   ` Thomas Petazzoni
@ 2018-11-20 16:18                     ` Thomas Petazzoni
  0 siblings, 0 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-20 16:18 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 20 Nov 2018 11:29:00 +0100, Thomas Petazzoni wrote:

> >  Perhaps, though, we should have a more general strategy of a per-infra
> > definition of what the steps are, instead of having a fixed sequence of steps.
> > I.e., have a pattern that should be followed by an infra-specific step to make
> > sure things are done correctly. So, for the kconfig case we would have an extra
> > kconfig_fixup step (we already have the stamp file, but it's not a real step
> > with hooks and the KCONFIG_DEPENDENCIES were strapped on as an afterthought),
> > and for the autoreconf case we would have an extra autoreconf step (instead of a
> > hook).  
> 
> And so those "injected" steps should take care of preparing the
> per-package folder with whatever dependencies they need ?
> 
> This is probably something that can be done to fix the pkg-kconfig
> stuff without having to create a new "prepare" step in the generic
> infrastructure.

Just tried that and it works. It will be part of my v5, which I hope
to send either later today, worst case tomorrow. With this, I can run
"make linux-menuconfig" from a clean tree, and the KCONFIG_DEPENDENCIES
are properly copied to the linux per-package folder.

Best regards,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target
  2018-11-19 10:48             ` Thomas Petazzoni
  2018-11-19 14:27               ` Andreas Naumann
  2018-11-19 19:49               ` Yann E. MORIN
@ 2018-11-20 16:19               ` Thomas Petazzoni
  2 siblings, 0 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2018-11-20 16:19 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon, 19 Nov 2018 11:48:06 +0100, Thomas Petazzoni wrote:

>  3. Patch step
>     Prepare PPD with patch dependencies

In fact no: this is *not* needed. Patch dependencies are special:

$$($(2)_TARGET_PATCH):  | $$(patsubst %,%-patch,$$($(2)_FINAL_PATCH_DEPENDENCIES))

Contrary to other dependencies where we depend on the "dependent"
package to be fully built and installed, a patch dependency is a
dependency between the patch step of package A, and the patch step of
package B.

Since we only depend on the patch step of the "dependent" package to be
completed, and that up the patch step, packages are not allowed to
install stuff to $(HOST_DIR), $(TARGET_DIR) or $(STAGING_DIR), there is
nothing to do in terms of per-package folder preparation.

Best regards,

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

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic
  2018-11-16 15:14       ` Thomas Petazzoni
@ 2018-11-20 22:08         ` Matthew Weber
  2018-11-27  6:24           ` Christian Stewart
  0 siblings, 1 reply; 35+ messages in thread
From: Matthew Weber @ 2018-11-20 22:08 UTC (permalink / raw)
  To: buildroot

Thomas,


On Fri, Nov 16, 2018 at 9:14 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Fri, 16 Nov 2018 15:15:36 +0100, Thomas Petazzoni wrote:
>
> > On Thu, 15 Nov 2018 19:21:27 -0600, Matthew Weber wrote:
> >
> > > > -# Populating the staging with the base directories is handled by the
> > > > skeleton package
> > > > -$(STAGING_DIR):
> > > > - @mkdir -p $(STAGING_DIR)
> > > > - @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
> > > > -
> > >
> > > With per package disabled, the staging directory never gets created
> > > early enough for packages to be able to reference content.
> > > Specifically I noticed docker couldn't find -lsystemd.  I'm assuming
> > > it's because of the new target dependency on target-post-image?
> >
> > The STAGING_DIR itself is definitely created at the beginning of the
> > build, at the first package installing something into it.
> >
> > However, with this patch, the $(BASE_DIR)/staging symlink indeed gets
> > created at the end of the build rather than at the beginning of the
> > build. However, this symlink is only a compatibility one. The code
> > should use $(STAGING_DIR) everywhere, which should not depend on this
> > "staging" compatibility symlink.
> >
> > I've started a build here to reproduce the issue, but in general it
> > would be nice when you report issues to include an example defconfig
> > that exhibits the problem, as well as the build log. It would be a lot
> > easier to understand the specifics of the problem, and be able to
> > reproduce it.
>
> FWIW, the following defconfig:
>
> BR2_arm=y
> BR2_cortex_a8=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y
> BR2_INIT_SYSTEMD=y
> BR2_SYSTEM_BIN_SH_NONE=y
> # BR2_PACKAGE_BUSYBOX is not set
> BR2_PACKAGE_DOCKER_ENGINE=
> # BR2_TARGET_ROOTFS_TAR is not set

My testing was against next with ppsh-v4 applied.

BR2_aarch64=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_INIT_SYSTEMD=y
BR2_PACKAGE_DOCKER_ENGINE=y
BR2_PACKAGE_DOCKER_ENGINE_STATIC_CLIENT=y
# BR2_TARGET_ROOTFS_TAR is not set

Looking at it a bit further, the  "go build" linking fails when the
BR2_PACKAGE_DOCKER_ENGINE_STATIC_CLIENT is set.  Since there is a
pending patch for the docker package, I built with the current "next"
that doesn't have [1] and also with it applied.  I observed that [1]
fixed the static client build.  When they broke out the client from
the daemon the linking approach must have changed.

I guess what I observed doesn't rule out a "go" linker issue.  I'll
see if I can get it to dump the link search path.

[1] https://patchwork.ozlabs.org/patch/994635/

Matt

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic
  2018-11-20 22:08         ` Matthew Weber
@ 2018-11-27  6:24           ` Christian Stewart
  0 siblings, 0 replies; 35+ messages in thread
From: Christian Stewart @ 2018-11-27  6:24 UTC (permalink / raw)
  To: buildroot

Hi Thomas, Matthew,

Matthew Weber <matthew.weber@rockwellcollins.com> writes:
> BR2_aarch64=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_INIT_SYSTEMD=y
> BR2_PACKAGE_DOCKER_ENGINE=y
> BR2_PACKAGE_DOCKER_ENGINE_STATIC_CLIENT=y
> # BR2_TARGET_ROOTFS_TAR is not set
>
> Looking at it a bit further, the  "go build" linking fails when the
> BR2_PACKAGE_DOCKER_ENGINE_STATIC_CLIENT is set.  Since there is a
> pending patch for the docker package, I built with the current "next"
> that doesn't have [1] and also with it applied.  I observed that [1]
> fixed the static client build.  When they broke out the client from
> the daemon the linking approach must have changed.

The CLI is linked statically correctly with the new patch applied, which
also splits the CLI and daemon into separate patches and upgrades Docker
to the latest stable.

I'll submit the most recent revision of the patch tonight, after
finishing testing the new release with an odroid xu4, bananapi m1+, and
a pi 0.

Best,
Christian

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2018-11-27  6:24 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-14 10:55 [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
2018-11-14 10:55 ` [Buildroot] [PATCH next v4 1/6] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use Thomas Petazzoni
2018-11-15 20:49   ` Yann E. MORIN
2018-11-14 10:55 ` [Buildroot] [PATCH next v4 2/6] support/scripts/check-host-rpath: split condition on two statements Thomas Petazzoni
2018-11-15 20:58   ` Yann E. MORIN
2018-11-14 10:55 ` [Buildroot] [PATCH next v4 3/6] Makefile: rework main directory creation logic Thomas Petazzoni
2018-11-15 21:09   ` Yann E. MORIN
2018-11-16 14:08     ` Thomas Petazzoni
2018-11-16  1:21   ` Matthew Weber
2018-11-16 14:15     ` Thomas Petazzoni
2018-11-16 15:14       ` Thomas Petazzoni
2018-11-20 22:08         ` Matthew Weber
2018-11-27  6:24           ` Christian Stewart
2018-11-14 10:55 ` [Buildroot] [PATCH next v4 4/6] Makefile: move .NOTPARALLEL statement after including .config file Thomas Petazzoni
2018-11-15 21:37   ` Yann E. MORIN
2018-11-16  8:53     ` Thomas Petazzoni
2018-11-14 10:55 ` [Buildroot] [PATCH next v4 5/6] Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR Thomas Petazzoni
2018-11-14 10:55 ` [Buildroot] [PATCH next v4 6/6] core: implement per-package SDK and target Thomas Petazzoni
2018-11-15 16:41   ` Andreas Naumann
2018-11-16 13:47     ` Thomas Petazzoni
2018-11-16 15:22       ` Thomas De Schampheleire
2018-11-16 19:57         ` Yann E. MORIN
2018-11-18 21:55           ` Arnout Vandecappelle
2018-11-19 10:48             ` Thomas Petazzoni
2018-11-19 14:27               ` Andreas Naumann
2018-11-19 19:49               ` Yann E. MORIN
2018-11-20 10:22                 ` Arnout Vandecappelle
2018-11-20 10:29                   ` Thomas Petazzoni
2018-11-20 16:18                     ` Thomas Petazzoni
2018-11-20 16:19               ` Thomas Petazzoni
2018-11-15 14:37 ` [Buildroot] [PATCH next v4 0/6] Per-package host/target directory support Thomas Petazzoni
2018-11-15 16:41 ` Andreas Naumann
2018-11-16 14:43   ` Thomas Petazzoni
2018-11-19 14:17     ` Andreas Naumann
2018-11-19 13:30 ` Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.