[Buildroot] [PATCH v2 00/18] Overwritten file detection and fixes, one more step to TLP build

[Buildroot] [PATCH v2 00/18] Overwritten file detection and fixes, one more step to TLP build

[Herve Codina]

Hi,

This v2 series performs overwrite fixes (patches 1 to 9) before adding
the overwrite detection tooling at patch 10.

It renames <PKG>_PER_PACKAGE_TWEAK_HOOKS to <PKG>_POST_PREPARE_HOOKS.

It avoids code duplication when fixing <pkg>_{reconfigure,rebuilf,reinstall} in
PATCH 14.

One more patch was added in this v2 series (PATCH 15) to removed a no more
used file and 3 patches related to test (PATCH 16 to 18) are added too.

More changes details are given in each individual patches.

Best regards,
Herv? Codina

Herve Codina (14):
  package/e2fsprogs: fix fsck overwrite in HOST_DIR
  package/pkg-generic.mk: Remove Info documents dir entry
  package/pkg-generic.mk: perform .la files fixup in per-package
    HOST_DIR
  package/pkg-generic: add post-prepare hooks
  package/apr-util: use post-prepare hook
  package/apache: move APACHE_FIXUP_APR_LIBTOOL to post-prepare hook
  package/pkg-python: remove _sysconfigdata*.pyc files when
    _sysconfigdata*.py are changed
  package/pkg-generic.mk: move python fixup to generic package
    infrastructure
  package/owfs: remove Python sysconfigdata fixup
  package/pkg-generic.mk: generate final rsync exclude file list
  Makefile: rsync global {TARGET,HOST}_DIR using exclusion file list
  Makefile: breaks hardlinks in global {TARGET,HOST}_DIR on per-package
    build
  package/pkg-generic.mk: fix per-package
    <pkg>-{reconfigure,rebuild,reinstall}
  package/pkg-generic.mk: remove .files-final-rsync.before temporary
    file

Thomas Petazzoni (4):
  package/pkg-generic.mk: detect files overwritten in TARGET_DIR and
    HOST_DIR
  support/testing/infra: add log_file_path() function
  support/testing/tests: add test for check_bin_arch
  support/testing/tests: add test for file overwrite detection

 Makefile                                      |  33 ++++-
 package/apache/apache.mk                      |   2 +-
 package/apr-util/apr-util.mk                  |  12 +-
 package/e2fsprogs/e2fsprogs.mk                |   1 +
 package/owfs/owfs.mk                          |   9 --
 package/pkg-generic.mk                        | 129 +++++++++++++++++-
 package/pkg-python.mk                         |  10 --
 support/testing/infra/__init__.py             |  11 +-
 .../br2-external/detect-bad-arch/Config.in    |   1 +
 .../detect-bad-arch/external.desc             |   1 +
 .../br2-external/detect-bad-arch/external.mk  |   1 +
 .../package/detect-bad-arch/Config.in         |   4 +
 .../detect-bad-arch/detect-bad-arch.mk        |  15 ++
 .../br2-external/detect-overwrite/Config.in   |   1 +
 .../detect-overwrite/external.desc            |   1 +
 .../br2-external/detect-overwrite/external.mk |   1 +
 .../package/detect-overwrite/Config.in        |   5 +
 .../detect-overwrite/detect-overwrite.mk      |  19 +++
 support/testing/tests/core/test_bad_arch.py   |  19 +++
 .../testing/tests/core/test_file_overwrite.py |  48 +++++++
 20 files changed, 287 insertions(+), 36 deletions(-)
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/Config.in
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/external.desc
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/external.mk
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/Config.in
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/detect-bad-arch.mk
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/Config.in
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/external.desc
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/external.mk
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/Config.in
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/detect-overwrite.mk
 create mode 100644 support/testing/tests/core/test_bad_arch.py
 create mode 100644 support/testing/tests/core/test_file_overwrite.py

-- 
2.31.1

[Buildroot] [PATCH v2 01/18] package/e2fsprogs: fix fsck overwrite in HOST_DIR

[Herve Codina]

host-e2fsprogs package overwrites the fsck program and some
manpages previously installed by host-util-linux package.

This patch simply disables fsck in host-e2fsprogs.

host-e2fsprogs is used to build final ext{2,3,4} images.
The missing host-e2fsprogs fsck tool (filesystem integrity check
tool) in HOST_DIR should not lead to issues.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
Changes v1 to v2:
 - Fixed commit message typos
 - Added 'Reviewed-by: Thomas Petazzoni'
 - Added 'Reviewed-by: Yann E. MORIN'

 package/e2fsprogs/e2fsprogs.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
index 0a69690e2f..bd147e737e 100644
--- a/package/e2fsprogs/e2fsprogs.mk
+++ b/package/e2fsprogs/e2fsprogs.mk
@@ -27,6 +27,7 @@ HOST_E2FSPROGS_CONF_OPTS = \
 	--disable-defrag \
 	--disable-e2initrd-helper \
 	--disable-fuse2fs \
+	--disable-fsck \
 	--disable-libblkid \
 	--disable-libuuid \
 	--disable-testio-debug \
-- 
2.31.1

[Buildroot] [PATCH v2 02/18] package/pkg-generic.mk: Remove Info documents dir entry

[Herve Codina]

Some packages (autotools for instance) install documentation
files using install-info. This program adds an entry in
the Info directory file (share/info/dir) and this causes
TARGET_DIR and/or HOST_DIR overwrite.

In order to avoid this overwrite this patch removes the Info
directory file right after any installation.

In order to be as generic as possible, this patch introduces
a new tooling to remove useless and conflicting files based
on the file and/or directory list <PKG>_DROP_FILES_OR_DIRS.
share/info/dir file is added for every packages in this list.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
Changes v1 to v2:
 - Reworked adding <PKG>_DROP_FILES_OR_DIRS and associated tooling.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 45589bcbb4..05b2095664 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -135,6 +135,22 @@ define check_bin_arch
 		-a $(BR2_READELF_ARCH_NAME)
 endef
 
+# Functions to remove conflicting and useless files
+
+# $1: base directory (target, staging, host)
+define remove-conflicting-useless-files
+	$(Q)$(RM) -rf $(patsubst %, $(1)/%, $($(PKG)_DROP_FILES_OR_DIRS))
+endef
+define REMOVE_CONFLICTING_USELESS_FILES_IN_HOST
+	$(call remove-conflicting-useless-files,$(HOST_DIR))
+endef
+define REMOVE_CONFLICTING_USELESS_FILES_IN_STAGING
+	$(call remove-conflicting-useless-files,$(STAGING_DIR))
+endef
+define REMOVE_CONFLICTING_USELESS_FILES_IN_TARGET
+	$(call remove-conflicting-useless-files,$(TARGET_DIR))
+endef
+
 ################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
@@ -823,6 +839,16 @@ $$(error "Package $(1) defines host variant before target variant!")
 endif
 endif
 
+# Globaly remove following conflicting and useless files
+$(2)_DROP_FILES_OR_DIRS += /share/info/dir
+
+ifeq ($$($(2)_TYPE),host)
+$(2)_POST_INSTALL_HOOKS += REMOVE_CONFLICTING_USELESS_FILES_IN_HOST
+else
+$(2)_POST_INSTALL_STAGING_HOOKS += REMOVE_CONFLICTING_USELESS_FILES_IN_STAGING
+$(2)_POST_INSTALL_TARGET_HOOKS += REMOVE_CONFLICTING_USELESS_FILES_IN_TARGET
+endif
+
 # human-friendly targets and target sequencing
 $(1):			$(1)-install
 $(1)-install:		$$($(2)_TARGET_INSTALL)
-- 
2.31.1

[Buildroot] [PATCH v2 03/18] package/pkg-generic.mk: perform .la files fixup in per-package HOST_DIR

[Herve Codina]

fixup-libtool-files was called on per-package STAGING_DIR.
Some host-xxxx packages have their .la files with directories
pointing outside their own per-package directory.

This commit, calling fixup-libtool-files on HOST_DIR, fixes this
issue.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
Changes v1 to v2:
 - Used '... -print0 | xargs -0 -r ...' construction.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 05b2095664..88707e2f8d 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -97,7 +97,8 @@ endif
 # $2: staging directory of the package
 ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
 define fixup-libtool-files
-	$(Q)find $(2)/usr/lib* -name "*.la" | xargs --no-run-if-empty \
+	$(Q)find $(2) \( -path '$(2)/lib*' -o -path '$(2)/usr/lib*' \) \
+		-name "*.la" -print0 | xargs -0 --no-run-if-empty \
 		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$(1)/:g"
 endef
 endif
@@ -251,6 +252,7 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call pkg_size_before,$(TARGET_DIR))
 	@$(call pkg_size_before,$(STAGING_DIR),-staging)
 	@$(call pkg_size_before,$(HOST_DIR),-host)
+	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
 	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
-- 
2.31.1

[Buildroot] [PATCH v2 04/18] package/pkg-generic: add post-prepare hooks

[Herve Codina]

Currently, when a package needs to modify files it inherits from its
dependencies, because they contain paths, we can only do that in a
pre- or post-configure hook.

However, whatever is done as part of those hooks, will be accounted
to the package itself, and thus will trigger file-overwrite detection.

So, we need a way to be able to actually modify files before we
start monitoring changes in those files.

We introduce a new set of hooks that an individual package can set,
or that a package infra can set, and that are called right before
we snapshot the state of target, and host (to which staging belongs),

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
Changes v1 to v2:
 - Renamed <PKG>_PER_PACKAGE_TWEAK_HOOKS to <PKG>_POST_PREPARE_HOOKS.
 - Reworked commit message.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 88707e2f8d..6fbd4006da 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -254,6 +254,7 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call pkg_size_before,$(HOST_DIR),-host)
 	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
 	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
+	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
@@ -834,6 +835,7 @@ $(2)_PRE_LEGAL_INFO_HOOKS       ?=
 $(2)_POST_LEGAL_INFO_HOOKS      ?=
 $(2)_TARGET_FINALIZE_HOOKS      ?=
 $(2)_ROOTFS_PRE_CMD_HOOKS       ?=
+$(2)_POST_PREPARE_HOOKS         ?=
 
 ifeq ($$($(2)_TYPE),target)
 ifneq ($$(HOST_$(2)_KCONFIG_VAR),)
-- 
2.31.1

[Buildroot] [PATCH v2 05/18] package/apr-util: use post-prepare hook

[Herve Codina]

Original APR_UTIL_FIX_RULES_MK_LIBTOOL tweaked libtool and rules.mk.

libtool is provided by a dependency (apr). It needs to be tweaked
and, as an apr-util external file, this tweak is relevant in
<PKG>_POST_PREPARE_HOOKS.

rules.mk is generated by apr-util configure step and it is private
to apr-util. The modification performed needs to be kept in
<PKG>_POST_CONFIGURE_HOOKS.

This commit splits original APR_UTIL_FIX_RULES_MK_LIBTOOL and
attaches each part to the correct hook.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
Changes v1 to v2:
 - Renamed <PKG>_PER_PACKAGE_TWEAK_HOOKS to <PKG>_POST_PREPARE_HOOKS
 - Added 'Reviewed-by: Yann E. MORIN'

 package/apr-util/apr-util.mk | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/package/apr-util/apr-util.mk b/package/apr-util/apr-util.mk
index db4df91564..f6ce7ac064 100644
--- a/package/apr-util/apr-util.mk
+++ b/package/apr-util/apr-util.mk
@@ -18,13 +18,17 @@ APR_UTIL_CONF_OPTS = \
 APR_UTIL_CONFIG_SCRIPTS = apu-1-config
 
 ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
-define APR_UTIL_FIX_RULES_MK_LIBTOOL
-	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \
-		$(@D)/build/rules.mk
+define APR_UTIL_FIX_LIBTOOL
 	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \
 		$(STAGING_DIR)/usr/build-1/libtool
 endef
-APR_UTIL_POST_CONFIGURE_HOOKS += APR_UTIL_FIX_RULES_MK_LIBTOOL
+APR_UTIL_POST_PREPARE_HOOKS += APR_UTIL_FIX_LIBTOOL
+
+define APR_UTIL_FIX_RULES_MK
+	$(SED) 's,$(PER_PACKAGE_DIR)/apr/,$(PER_PACKAGE_DIR)/apr-util/,g' \
+		$(@D)/build/rules.mk
+endef
+APR_UTIL_POST_CONFIGURE_HOOKS += APR_UTIL_FIX_RULES_MK
 endif
 
 # When iconv is available, then use it to provide charset conversion
-- 
2.31.1

[Buildroot] [PATCH v2 06/18] package/apache: move APACHE_FIXUP_APR_LIBTOOL to post-prepare hook

[Herve Codina]

APACHE_FIXUP_APR_LIBTOOL tweaks files for per package directory build.
This is typically the kind of operation expected to be in
post-prepare hook.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
Changes v1 to v2:
 - Renamed <PKG>_PER_PACKAGE_TWEAK_HOOKS to <PKG>_POST_PREPARE_HOOKS
 - Added 'Reviewed-by: Yann E. MORIN'

 package/apache/apache.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/apache/apache.mk b/package/apache/apache.mk
index 7dbd1a4512..dc66cd660e 100644
--- a/package/apache/apache.mk
+++ b/package/apache/apache.mk
@@ -23,7 +23,7 @@ define APACHE_FIXUP_APR_LIBTOOL
 	$(SED) "s@$(PER_PACKAGE_DIR)/[^/]\+/@$(PER_PACKAGE_DIR)/apache/@g" \
 		$(STAGING_DIR)/usr/build-1/libtool
 endef
-APACHE_POST_CONFIGURE_HOOKS += APACHE_FIXUP_APR_LIBTOOL
+APACHE_POST_PREPARE_HOOKS += APACHE_FIXUP_APR_LIBTOOL
 endif
 
 APACHE_CONF_ENV= \
-- 
2.31.1

[Buildroot] [PATCH v2 07/18] package/pkg-python: remove _sysconfigdata*.pyc files when _sysconfigdata*.py are changed

[Herve Codina]

In order to avoid _sysconfigdata*.pyc overwrites when they are generated based on
_sysconfigdata*.py changes, this commit simply removes _sysconfigdata*.pyc
whenever _sysconfigdata*.py are changed.

As they are removed, overwrite detection will no longer trig and coherency between
the two files (.py and .pyc) is ensured.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
Changes from v1 to v2:
 - Used separate shells for the two 'find' commands.
 - Used '-deleted' to remove files

 package/pkg-python.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/pkg-python.mk b/package/pkg-python.mk
index 59a48e5a87..1e4fd5ba33 100644
--- a/package/pkg-python.mk
+++ b/package/pkg-python.mk
@@ -97,6 +97,8 @@ define PKG_PYTHON_FIXUP_SYSCONFIGDATA
 	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
 		-name "_sysconfigdata*.py" | xargs --no-run-if-empty \
 		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g"
+	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
+		-name "_sysconfigdata*.pyc" -delete
 endef
 endif
 
-- 
2.31.1

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Herve Codina]

Fixing _sysconfigdata*.{py,pyc} was previously done by python package
infrastructure. Some packages use python stuff without using python
package infrastructure.
These packages perform overwrites and need the specific python fixup
to fix them.

In order to be sure to fix all of these packages, the python fixup
is moved to the generic package infrastructure and applied to all
packages.
This follows the same principle as for the .la libtool files fixup.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
Changes v1 to v2:
 - Used '... -print0 | xargs -0 -r ...' construction.
 - Used '-deleted' to remove files.

 package/pkg-generic.mk | 17 +++++++++++++++++
 package/pkg-python.mk  | 12 ------------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6fbd4006da..8c19522fd8 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -103,6 +103,21 @@ define fixup-libtool-files
 endef
 endif
 
+# Make sure python _sysconfigdata*.py files only reference the current
+# per-package directory.
+
+# $1: package name (lower case)
+# $2: staging or host directory of the package
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+define fixup-python-files
+	$(Q)find $(2) \( -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*' \) \
+		-name "_sysconfigdata*.py" -print0 | xargs -0 --no-run-if-empty \
+		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$(1)/:g"
+	$(Q)find $(2) \( -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*' \) \
+		-name "_sysconfigdata*.pyc" -delete
+endef
+endif
+
 # Functions to collect statistics about installed files
 
 # $(1): base directory to search in
@@ -254,6 +269,8 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call pkg_size_before,$(HOST_DIR),-host)
 	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
 	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
+	$(call fixup-python-files,$(NAME),$(HOST_DIR))
+	$(call fixup-python-files,$(NAME),$(STAGING_DIR))
 	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
diff --git a/package/pkg-python.mk b/package/pkg-python.mk
index 1e4fd5ba33..e6b81bdfd3 100644
--- a/package/pkg-python.mk
+++ b/package/pkg-python.mk
@@ -92,16 +92,6 @@ HOST_PKG_PYTHON_SETUPTOOLS_INSTALL_OPTS = \
 	--root=/ \
 	--single-version-externally-managed
 
-ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
-define PKG_PYTHON_FIXUP_SYSCONFIGDATA
-	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
-		-name "_sysconfigdata*.py" | xargs --no-run-if-empty \
-		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g"
-	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
-		-name "_sysconfigdata*.pyc" -delete
-endef
-endif
-
 ################################################################################
 # inner-python-package -- defines how the configuration, compilation
 # and installation of a Python package should be done, implements a
@@ -246,8 +236,6 @@ $(2)_PYTHON_INTERPRETER = $$(HOST_DIR)/bin/$$($(2)_NEEDS_HOST_PYTHON)
 endif
 endif
 
-$(2)_PRE_CONFIGURE_HOOKS += PKG_PYTHON_FIXUP_SYSCONFIGDATA
-
 #
 # Build step. Only define it if not already defined by the package .mk
 # file.
-- 
2.31.1

[Buildroot] [PATCH v2 09/18] package/owfs: remove Python sysconfigdata fixup

[Herve Codina]

This fixup is done at pkg-generic level for all packages.
So, it is no more needed in owfs package.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
Changes v1 to v2:
 - Added 'Reviewed-by: Yann E. MORIN'

 package/owfs/owfs.mk | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/package/owfs/owfs.mk b/package/owfs/owfs.mk
index ffc0b3098d..10543d3698 100644
--- a/package/owfs/owfs.mk
+++ b/package/owfs/owfs.mk
@@ -88,15 +88,6 @@ OWFS_DEPENDENCIES += python host-swig
 # override the PYSITEDIR variable in make.
 OWFS_EXTRA_MAKE_OPTS += PYSITEDIR=/usr/lib/python$(PYTHON_VERSION_MAJOR)/site-packages
 
-ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
-define OWFS_FIXUP_PYTHON_SYSCONFIGDATA
-	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
-		-name "_sysconfigdata*.py" | xargs --no-run-if-empty \
-		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/owfs/:g"
-endef
-OWFS_PRE_CONFIGURE_HOOKS += OWFS_FIXUP_PYTHON_SYSCONFIGDATA
-endif
-
 else
 OWFS_CONF_OPTS += --disable-owpython --without-python
 endif
-- 
2.31.1

[Buildroot] [PATCH v2 10/18] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR

[Herve Codina]

From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Without per-package directory support, a package can happily overwrite
files installed by other packages. Indeed, because the build order
between packages is always guaranteed, Buildroot will always produce
the same output.

However, with per-package directory support, it is absolutely critical
that a given package does not overwrite files already installed by
another package, due to how the final aggregation is done to create
the complete target/, staging/ and host/ folders. Unfortunately, we
currently don't have anything in Buildroot that detects this
situation.

We used to have check-uniq-files, but it was dropped in commit
2496189a4207173e4cd5bbab90256f911175ee57.

This commit is a new implementation of such a detection, which is
based on calculating and verifying MD5 hashes of installed files: the
calculation is done at the beginning of the configure step, the
verification during the newly introduced "install" step that takes
place after all installation steps.

Since preventing file overwrites is really only needed when
per-package directory support is used, and due to this verification
having some overhead, it is only enabled when
BR2_PER_PACKAGE_DIRECTORIES=y. This additional verification cost is
however not too bad as on average, with per-package directory support,
the per-package target/ and host/ directories will contain less files
than with a build that doesn't use per-package directory support. This
helps a bit in mitigating the additional cost of this verification.

Note that we are not handling separately HOST_DIR and STAGING_DIR,
like we're doing with the pkg_size_{before,after} functions. Instead,
the verification on HOST_DIR walks down into the STAGING_DIR.

During per-package build, original files are modified by
fixup-libtool-files and fixup-python-files calls.
But since these fixups modify files using sed --in-place, these
modifications are done using a temporary file and a call to rename.
Rename breaks the hardlink to the original file and leave the temporary
file in per-package TARGET dir.
As the original file is not modified, this is no longer considered as
an overwrite. This patch simply considers that what is done by
fixup-libtool-files and fixup-python-files is part of the original
snapshot used to detect overwrites. And so, the original snapshot is
taken after fixup-libtool-files and fixup-python-files calls.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
This commit is retreived from Thomas's work.
The first version was discussed
https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-9-thomas.petazzoni at bootlin.com/
This new version was not already submitted by Thomas or I missed it.
Compared to the first version, this patch has an improved commit message and
generates the md5sum snapshot using
 'LC_ALL=C find $(1) -type f -print0 | xargs -0 -r md5sum > $($(PKG)_DIR)/.files$(2).md5;'
instead of
 'cd $(1); LC_ALL=C find . -type f -exec md5sum {} \; > $($(PKG)_DIR)/.files$(2).md5'

Changes v1 to v2:
 - Added note about why fixup-{libtool,python}-files are not considered overwrites
   and so take the overwrite snapshot after fixup-{libtool,python}-files call.
 - Removed 'LC_ALL=C'

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 8c19522fd8..3e1030a495 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -118,6 +118,25 @@ define fixup-python-files
 endef
 endif
 
+# Functions to detect overwritten files
+
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
+define pkg_detect_overwrite_before
+	find $(1) -type f -print0 | xargs -0 -r md5sum > $($(PKG)_DIR)/.files$(2).md5;
+endef
+
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
+define pkg_detect_overwrite_after
+	if test -s $($(PKG)_DIR)/.files$(2).md5 ; then \
+		md5sum --quiet -c $($(PKG)_DIR)/.files$(2).md5 || \
+		{ echo "ERROR: package $($(PKG)_NAME) has overwritten files installed by a previous package, aborting."; exit 1; } ; \
+	fi
+endef
+endif
+
 # Functions to collect statistics about installed files
 
 # $(1): base directory to search in
@@ -272,6 +291,8 @@ $(BUILD_DIR)/%/.stamp_configured:
 	$(call fixup-python-files,$(NAME),$(HOST_DIR))
 	$(call fixup-python-files,$(NAME),$(STAGING_DIR))
 	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
+	@$(call pkg_detect_overwrite_before,$(TARGET_DIR))
+	@$(call pkg_detect_overwrite_before,$(HOST_DIR),-host)
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
@@ -396,6 +417,8 @@ $(BUILD_DIR)/%/.stamp_installed:
 	@$(call pkg_size_after,$(STAGING_DIR),-staging)
 	@$(call pkg_size_after,$(HOST_DIR),-host)
 	@$(call check_bin_arch)
+	@$(call pkg_detect_overwrite_after,$(TARGET_DIR))
+	@$(call pkg_detect_overwrite_after,$(HOST_DIR),-host)
 	$(Q)touch $@
 
 # Remove package sources
-- 
2.31.1

[Buildroot] [PATCH v2 11/18] package/pkg-generic.mk: generate final rsync exclude file list

[Herve Codina]

The final rsync performed at {host,target}-finalize steps
need to be done by rsyncing files generated by each packages
without looking at files generated by other packages the current
package depends on. This is needed to avoid overwrites in final
{HOST,TARGET}_DIR.

In order to prepare the final rsync, an exclusion list is generated.
This list lists files that are not generated by the current package and
so files that need to be excluded from the final rsync.

Note also that the files list was not based on .files-list.{before,after}.
During .file-list.{before,after} built for host directory, staging
sub-directory (ie <toolchain>/sysroot) is filtered out.
The final rsync exclusion list needs to take into account the full
{host,target} directory to avoid final overwrites.

Using an empty directory for per-package installation directory would be the
simplest way to find what a package installs.
However, as it has been discussed in the past, this is fraught with unworkable
issues. For example, some paths may be hard-coded at configure time and/or build
time, and thus the package would still install in the original stagin we presented
it with (for target/, this is not an isue, because target/ is never looked at
during configure or build, only at install time). Or a package installation
process just assumes that the directory structure exists (bad, but eh...).

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
Changes v1 to v2:
 - Added a note in the commit message related to why an empty directory is not
   used to find what a package installs.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 3e1030a495..6157f6e540 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -162,6 +162,30 @@ define pkg_size_after
 	rm -f $($(PKG)_DIR)/.files-list$(2).after
 endef
 
+# Functions to collect final rsync exclusion files
+
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
+define pkg_final_rsync_before
+	cd $(1); \
+	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
+		| LC_ALL=C sort > $($(PKG)_DIR)/.files-final-rsync$(2).before
+endef
+
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
+define pkg_final_rsync_after
+	cd $(1); \
+	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
+		| LC_ALL=C sort > $($(PKG)_DIR)/.files-final-rsync$(2).after
+	LC_ALL=C comm -2 \
+		$($(PKG)_DIR)/.files-final-rsync$(2).before \
+		$($(PKG)_DIR)/.files-final-rsync$(2).after \
+		| sed -r -e 's/^[^,]+,./- /' \
+		> $($(PKG)_DIR)/.files-final-rsync$(2).exclude_rsync
+	rm -f $($(PKG)_DIR)/.files-final-rsync$(2).after
+endef
+
 define check_bin_arch
 	support/scripts/check-bin-arch -p $($(PKG)_NAME) \
 		-l $($(PKG)_DIR)/.files-list.txt \
@@ -283,6 +307,8 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call MESSAGE,"Configuring")
 	$(Q)mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR) $(BINARIES_DIR)
 	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
+	@$(call pkg_final_rsync_before,$(TARGET_DIR))
+	@$(call pkg_final_rsync_before,$(HOST_DIR),-host)
 	@$(call pkg_size_before,$(TARGET_DIR))
 	@$(call pkg_size_before,$(STAGING_DIR),-staging)
 	@$(call pkg_size_before,$(HOST_DIR),-host)
@@ -417,6 +443,8 @@ $(BUILD_DIR)/%/.stamp_installed:
 	@$(call pkg_size_after,$(STAGING_DIR),-staging)
 	@$(call pkg_size_after,$(HOST_DIR),-host)
 	@$(call check_bin_arch)
+	@$(call pkg_final_rsync_after,$(TARGET_DIR))
+	@$(call pkg_final_rsync_after,$(HOST_DIR),-host)
 	@$(call pkg_detect_overwrite_after,$(TARGET_DIR))
 	@$(call pkg_detect_overwrite_after,$(HOST_DIR),-host)
 	$(Q)touch $@
-- 
2.31.1

[Buildroot] [PATCH v2 12/18] Makefile: rsync global {TARGET, HOST}_DIR using exclusion file list

[Herve Codina]

On a per-package build, rsync final {TARGET,HOST}_DIR using
exclusion file list computed for each packages.

This exclusion list allows to rsync only files generated by a given
package. This is needed to avoid any overwrites in final
{TARGET,HOST}_DIR.

As we rsync only files generated by each packages, we need to
to do this rsync recursively on each dependencies to collect
all needed files. This is done based on existing
<PKG>_FINAL_RECURSIVE_DEPENDENCIES

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
Changes v1 to v2:
 - Added a note in the commit message related to why this exclusion
   list is needed.
 - Added $(Q) to mkdir and rsync
 - Changed indentations to make calls more readable
 
 Makefile | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 74c238cb20..380fdbb3cb 100644
--- a/Makefile
+++ b/Makefile
@@ -740,10 +740,33 @@ TARGET_DIR_FILES_LISTS = $(sort $(wildcard $(BUILD_DIR)/*/.files-list.txt))
 HOST_DIR_FILES_LISTS = $(sort $(wildcard $(BUILD_DIR)/*/.files-list-host.txt))
 STAGING_DIR_FILES_LISTS = $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.txt))
 
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+# rsync the contents of per-package directories
+# $1: space-separated list of packages to rsync from
+# $2: 'host' or 'target'
+# $3: destination directory
+# $4: exclude file list to use
+define per-package-rsync-delta
+	$(Q)mkdir -p $(3)
+	$(foreach pkg,$(1),\
+		$(Q)rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(strip $(2))/ \
+			--filter='merge $($(call UPPERCASE,$(pkg))_DIR)/$(strip $(4))' \
+			$(PER_PACKAGE_DIR)/$(pkg)/$(strip $(2))/ \
+			$(3)$(sep))
+endef
+endif
+
 .PHONY: host-finalize
 host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
 	@$(call MESSAGE,"Finalizing host directory")
-	$(call per-package-rsync,$(sort $(PACKAGES)),host,$(HOST_DIR))
+	$(call per-package-rsync-delta, \
+		$(sort $(foreach pkg, $(PACKAGES), \
+			$(pkg) $($(call UPPERCASE,$(pkg))_FINAL_RECURSIVE_DEPENDENCIES)) \
+		), \
+		host, \
+		$(HOST_DIR), \
+		.files-final-rsync-host.exclude_rsync \
+	)
 
 .PHONY: staging-finalize
 staging-finalize: $(STAGING_DIR_SYMLINK)
@@ -751,7 +774,14 @@ staging-finalize: $(STAGING_DIR_SYMLINK)
 .PHONY: target-finalize
 target-finalize: $(PACKAGES) $(TARGET_DIR) host-finalize
 	@$(call MESSAGE,"Finalizing target directory")
-	$(call per-package-rsync,$(sort $(PACKAGES)),target,$(TARGET_DIR))
+	$(call per-package-rsync-delta, \
+		$(sort $(foreach pkg, $(PACKAGES), \
+			$(pkg) $($(call UPPERCASE,$(pkg))_FINAL_RECURSIVE_DEPENDENCIES)) \
+		), \
+		target, \
+		$(TARGET_DIR), \
+		.files-final-rsync.exclude_rsync \
+	)
 	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
 	rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \
 		$(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \
-- 
2.31.1

[Buildroot] [PATCH v2 13/18] Makefile: breaks hardlinks in global {TARGET, HOST}_DIR on per-package build

[Herve Codina]

Without this patch, a make <pkg>_rebuild detects overwrites. Indeed, in
target_finalize steps some modifications are done on installed files (ie
strip or TARGET_FINALIZE_HOOKS for instance).

In order to avoid these modifications seen from per-package {TARGET,HOST}_DIR
and so been analyzed as some overwrites, global {TARGET,HOST}_DIR is built
using a full copy of the involved per-package files instead of hardlinks.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
Changed v1 to v2:
 - Added 'Reviewed-by: Yann E. MORIN'

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

diff --git a/Makefile b/Makefile
index 380fdbb3cb..cc67d29eb3 100644
--- a/Makefile
+++ b/Makefile
@@ -749,7 +749,7 @@ ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
 define per-package-rsync-delta
 	$(Q)mkdir -p $(3)
 	$(foreach pkg,$(1),\
-		$(Q)rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(strip $(2))/ \
+		$(Q)rsync -a \
 			--filter='merge $($(call UPPERCASE,$(pkg))_DIR)/$(strip $(4))' \
 			$(PER_PACKAGE_DIR)/$(pkg)/$(strip $(2))/ \
 			$(3)$(sep))
-- 
2.31.1

[Buildroot] [PATCH v2 14/18] package/pkg-generic.mk: fix per-package <pkg>-{reconfigure, rebuild, reinstall}

[Herve Codina]

Many overwrites are detected on <pkg>-{reconfigure,rebuild,reinstall}.
Indeed, files previously installed by a package were detected as
overwritten on next reconfigure, rebuild or reinstall.

To avoid this, we recreate per-package host and target dir from
scratch as it was done during the first configure step.

In order not to duplicate the code, this commit shares common code
in prepare-pre-configure macro.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
Changes v1 to v2:
 - Avoid code duplication using prepare-pre-configure macro

 package/pkg-generic.mk | 44 ++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6157f6e540..ee2d68f2f7 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -210,6 +210,23 @@ define REMOVE_CONFLICTING_USELESS_FILES_IN_TARGET
 	$(call remove-conflicting-useless-files,$(TARGET_DIR))
 endef
 
+# Function to prepare package
+
+define prepare-pre-configure
+	@$(call pkg_final_rsync_before,$(TARGET_DIR))
+	@$(call pkg_final_rsync_before,$(HOST_DIR),-host)
+	@$(call pkg_size_before,$(TARGET_DIR))
+	@$(call pkg_size_before,$(STAGING_DIR),-staging)
+	@$(call pkg_size_before,$(HOST_DIR),-host)
+	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
+	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
+	$(call fixup-python-files,$(NAME),$(HOST_DIR))
+	$(call fixup-python-files,$(NAME),$(STAGING_DIR))
+	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
+	@$(call pkg_detect_overwrite_before,$(TARGET_DIR))
+	@$(call pkg_detect_overwrite_before,$(HOST_DIR),-host)
+endef
+
 ################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
@@ -307,18 +324,7 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call MESSAGE,"Configuring")
 	$(Q)mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR) $(BINARIES_DIR)
 	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
-	@$(call pkg_final_rsync_before,$(TARGET_DIR))
-	@$(call pkg_final_rsync_before,$(HOST_DIR),-host)
-	@$(call pkg_size_before,$(TARGET_DIR))
-	@$(call pkg_size_before,$(STAGING_DIR),-staging)
-	@$(call pkg_size_before,$(HOST_DIR),-host)
-	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
-	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
-	$(call fixup-python-files,$(NAME),$(HOST_DIR))
-	$(call fixup-python-files,$(NAME),$(STAGING_DIR))
-	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))
-	@$(call pkg_detect_overwrite_before,$(TARGET_DIR))
-	@$(call pkg_detect_overwrite_before,$(HOST_DIR),-host)
+	$(call prepare-pre-configure)
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
@@ -483,6 +489,13 @@ define pkg-graph-depends
 		$$(GRAPHS_DIR)/$$(@).dot
 endef
 
+ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
+define empty-per-package-directory
+	rm -rf $(HOST_DIR) $(TARGET_DIR)
+	mkdir -p $(HOST_DIR) $(TARGET_DIR)
+endef
+endif
+
 ################################################################################
 # inner-generic-package -- generates the make targets needed to build a
 # generic package
@@ -1086,6 +1099,8 @@ $(1)-all-legal-info:	$$(foreach p,$$($(2)_FINAL_ALL_DEPENDENCIES),$$(p)-all-lega
 
 $(1)-dirclean:		$$($(2)_TARGET_DIRCLEAN)
 
+$(1)-clean-for-reinstall: PKG=$(2)
+$(1)-clean-for-reinstall: NAME=$(1)
 $(1)-clean-for-reinstall:
 ifneq ($$($(2)_OVERRIDE_SRCDIR),)
 			rm -f $$($(2)_TARGET_RSYNC)
@@ -1095,6 +1110,11 @@ endif
 			rm -f $$($(2)_TARGET_INSTALL_TARGET)
 			rm -f $$($(2)_TARGET_INSTALL_IMAGES)
 			rm -f $$($(2)_TARGET_INSTALL_HOST)
+			$$(call empty-per-package-directory)
+			$$(call prepare-per-package-directory,$$($(2)_FINAL_DOWNLOAD_DEPENDENCIES))
+			$$(call prepare-per-package-directory,$$($(2)_FINAL_EXTRACT_DEPENDENCIES))
+			$$(call prepare-per-package-directory,$$($(2)_FINAL_DEPENDENCIES))
+			$$(call prepare-pre-configure)
 
 $(1)-reinstall:		$(1)-clean-for-reinstall $(1)
 
-- 
2.31.1

[Buildroot] [PATCH v2 15/18] package/pkg-generic.mk: remove .files-final-rsync.before temporary file

[Herve Codina]

Keeping the .files-final-rsync.before file between make invocations
was needed to ensure that this file will be present in case of
'make <pkg>_{reconfigure,rebuild,reinstall}' invocation.

The .files-final-rsync.before is now recreated on
<pkg>_{reconfigure,rebuild,reinstall} and we don't need to keep it
between make invocation.

This patch simply removes this file as soon as it is no longer needed.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
New patch in this v2 series

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index ee2d68f2f7..14d76e65c0 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -183,6 +183,7 @@ define pkg_final_rsync_after
 		$($(PKG)_DIR)/.files-final-rsync$(2).after \
 		| sed -r -e 's/^[^,]+,./- /' \
 		> $($(PKG)_DIR)/.files-final-rsync$(2).exclude_rsync
+	rm -f $($(PKG)_DIR)/.files-final-rsync$(2).before
 	rm -f $($(PKG)_DIR)/.files-final-rsync$(2).after
 endef
 
-- 
2.31.1

[Buildroot] [PATCH v2 16/18] support/testing/infra: add log_file_path() function

[Herve Codina]

From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Some tests will need to grep through the build log to verify that some
features are working are expected. In order to allow them to open the
build log, we provide a new function called log_file_path(), which
returns the path to the log file if available.

We also use this function in open_log_file().

Note that open_log_file() cannot be used directly to grep through the
log file at the end of a build: because it opens in "a+" mode, it
greps starting from the end of the file.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
New patch in this v2 series

This patch is retrieved from Thomas's work.
The first version was discussed
https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-10-thomas.petazzoni at bootlin.com/

This new version was not already submitted by Thomas or I missed it.
Compared to the first version, this patch use a more python-like
syntax as proposed by Yann in the previous review.

 support/testing/infra/__init__.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/support/testing/infra/__init__.py b/support/testing/infra/__init__.py
index b10a7601a3..1f003f24c6 100644
--- a/support/testing/infra/__init__.py
+++ b/support/testing/infra/__init__.py
@@ -10,17 +10,18 @@ ARTIFACTS_URL = "http://autobuild.buildroot.net/artefacts/"
 BASE_DIR = os.path.realpath(os.path.join(os.path.dirname(__file__), "../../.."))
 
 
+def log_file_path(builddir, stage, logtofile=True):
+    """Return path to log file"""
+    return "{}-{}.log".format(builddir, stage) if logtofile else None
+
+
 def open_log_file(builddir, stage, logtofile=True):
     """
     Open a file for logging and return its handler.
     If logtofile is True, returns sys.stdout. Otherwise opens a file
     with a suitable name in the build directory.
     """
-    if logtofile:
-        fhandle = open("{}-{}.log".format(builddir, stage), 'a+')
-    else:
-        fhandle = sys.stdout
-    return fhandle
+    return open(log_file_path(builddir, stage, logtofile), 'a+') if logtofile else sys.stdout
 
 
 def basepath(relpath=""):
-- 
2.31.1

[Buildroot] [PATCH v2 17/18] support/testing/tests: add test for check_bin_arch

[Herve Codina]

From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

This tests build a bogus package that installs a binary built for the
host architecture into $(TARGET_DIR), which should cause a build
failure, at least as long as the host architecture isn't ARM.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
New patch in this v2 series

This patch is retrieved from Thomas's work.
The first version was discussed
https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-10-thomas.petazzoni at bootlin.com/

Compared to the first version, as proposed by Yann in the previous
review, this patch does not launch a subprocess (grep) to search the
string.

Note also that the .gitlab-ci.yml modification present in the previous
version is not present in this patch.

 .../br2-external/detect-bad-arch/Config.in    |  1 +
 .../detect-bad-arch/external.desc             |  1 +
 .../br2-external/detect-bad-arch/external.mk  |  1 +
 .../package/detect-bad-arch/Config.in         |  4 ++++
 .../detect-bad-arch/detect-bad-arch.mk        | 15 +++++++++++++++
 support/testing/tests/core/test_bad_arch.py   | 19 +++++++++++++++++++
 6 files changed, 41 insertions(+)
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/Config.in
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/external.desc
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/external.mk
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/Config.in
 create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/detect-bad-arch.mk
 create mode 100644 support/testing/tests/core/test_bad_arch.py

diff --git a/support/testing/tests/core/br2-external/detect-bad-arch/Config.in b/support/testing/tests/core/br2-external/detect-bad-arch/Config.in
new file mode 100644
index 0000000000..530c077bbe
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-bad-arch/Config.in
@@ -0,0 +1 @@
+source "$BR2_EXTERNAL_DETECT_BAD_ARCH_PATH/package/detect-bad-arch/Config.in"
diff --git a/support/testing/tests/core/br2-external/detect-bad-arch/external.desc b/support/testing/tests/core/br2-external/detect-bad-arch/external.desc
new file mode 100644
index 0000000000..3c4232c90d
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-bad-arch/external.desc
@@ -0,0 +1 @@
+name: DETECT_BAD_ARCH
diff --git a/support/testing/tests/core/br2-external/detect-bad-arch/external.mk b/support/testing/tests/core/br2-external/detect-bad-arch/external.mk
new file mode 100644
index 0000000000..71b9821ddc
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-bad-arch/external.mk
@@ -0,0 +1 @@
+include $(sort $(wildcard $(BR2_EXTERNAL_DETECT_BAD_ARCH_PATH)/package/*/*.mk))
diff --git a/support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/Config.in b/support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/Config.in
new file mode 100644
index 0000000000..9893e9afc1
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/Config.in
@@ -0,0 +1,4 @@
+config BR2_PACKAGE_DETECT_BAD_ARCH
+	bool
+	default y
+
diff --git a/support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/detect-bad-arch.mk b/support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/detect-bad-arch.mk
new file mode 100644
index 0000000000..5e78c55f1f
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/detect-bad-arch.mk
@@ -0,0 +1,15 @@
+################################################################################
+#
+# detect-bad-arch
+#
+################################################################################
+
+define DETECT_BAD_ARCH_BUILD_CMDS
+	echo "int main(void) { return 0; }" | $(HOSTCC) -x c -o $(@D)/foo -
+endef
+
+define DETECT_BAD_ARCH_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 0755 $(@D)/foo $(TARGET_DIR)/usr/bin/foo
+endef
+
+$(eval $(generic-package))
diff --git a/support/testing/tests/core/test_bad_arch.py b/support/testing/tests/core/test_bad_arch.py
new file mode 100644
index 0000000000..0be7fdc9b7
--- /dev/null
+++ b/support/testing/tests/core/test_bad_arch.py
@@ -0,0 +1,19 @@
+import infra
+import infra.basetest
+import subprocess
+
+
+class DetectBadArchTest(infra.basetest.BRConfigTest):
+    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + infra.basetest.MINIMAL_CONFIG
+    br2_external = [infra.filepath("tests/core/br2-external/detect-bad-arch")]
+
+    def test_run(self):
+        with self.assertRaises(SystemError):
+            self.b.build()
+        logf_path = infra.log_file_path(self.b.builddir, "build",
+                                        infra.basetest.BRConfigTest.logtofile)
+        if logf_path:
+            s = 'ERROR: architecture for "/usr/bin/foo" is'
+            with open(logf_path, "r") as f:
+                lines = [l for l in f.readlines() if l.startswith(s)]
+            self.assertEqual(len(lines), 0)
-- 
2.31.1

[Buildroot] [PATCH v2 18/18] support/testing/tests: add test for file overwrite detection

[Herve Codina]

From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
New patch in this v2 series

This patch is retrieved from Thomas's work.
The first version was discussed
https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-12-thomas.petazzoni at bootlin.com/

Compared to the first version, as proposed by Yann in the previous
review, this patch does not launch a subprocess (grep) to find the
string.
Additionally, the strings searched were changed from a specific
file ('path/file: FAILED') to the "ERROR: package ..." overwrite
detection message.

Note also that the .gitlab-ci.yml modification present in the previous
version is not present in this patch.

 .../br2-external/detect-overwrite/Config.in   |  1 +
 .../detect-overwrite/external.desc            |  1 +
 .../br2-external/detect-overwrite/external.mk |  1 +
 .../package/detect-overwrite/Config.in        |  5 ++
 .../detect-overwrite/detect-overwrite.mk      | 19 ++++++++
 .../testing/tests/core/test_file_overwrite.py | 48 +++++++++++++++++++
 6 files changed, 75 insertions(+)
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/Config.in
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/external.desc
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/external.mk
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/Config.in
 create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/detect-overwrite.mk
 create mode 100644 support/testing/tests/core/test_file_overwrite.py

diff --git a/support/testing/tests/core/br2-external/detect-overwrite/Config.in b/support/testing/tests/core/br2-external/detect-overwrite/Config.in
new file mode 100644
index 0000000000..b5514510bd
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-overwrite/Config.in
@@ -0,0 +1 @@
+source "$BR2_EXTERNAL_DETECT_OVERWRITE_PATH/package/detect-overwrite/Config.in"
diff --git a/support/testing/tests/core/br2-external/detect-overwrite/external.desc b/support/testing/tests/core/br2-external/detect-overwrite/external.desc
new file mode 100644
index 0000000000..6fedc276e8
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-overwrite/external.desc
@@ -0,0 +1 @@
+name: DETECT_OVERWRITE
diff --git a/support/testing/tests/core/br2-external/detect-overwrite/external.mk b/support/testing/tests/core/br2-external/detect-overwrite/external.mk
new file mode 100644
index 0000000000..90927b33ef
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-overwrite/external.mk
@@ -0,0 +1 @@
+include $(sort $(wildcard $(BR2_EXTERNAL_DETECT_OVERWRITE_PATH)/package/*/*.mk))
diff --git a/support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/Config.in b/support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/Config.in
new file mode 100644
index 0000000000..fff8b0320f
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/Config.in
@@ -0,0 +1,5 @@
+config BR2_PACKAGE_DETECT_OVERWRITE
+	bool "detect-overwrite"
+
+config BR2_PACKAGE_HOST_DETECT_OVERWRITE
+	bool "host-detect-overwrite"
diff --git a/support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/detect-overwrite.mk b/support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/detect-overwrite.mk
new file mode 100644
index 0000000000..c6df2a339d
--- /dev/null
+++ b/support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/detect-overwrite.mk
@@ -0,0 +1,19 @@
+################################################################################
+#
+# detect-overwrite
+#
+################################################################################
+
+define DETECT_OVERWRITE_INSTALL_TARGET_CMDS
+	grep -q "^foo" $(TARGET_DIR)/etc/passwd || \
+		echo "foo" >> $(TARGET_DIR)/etc/passwd
+endef
+
+HOST_DETECT_OVERWRITE_DEPENDENCIES = host-pkgconf
+
+define HOST_DETECT_OVERWRITE_INSTALL_CMDS
+	$(SED) 's/manipulating/tweaking/' $(HOST_DIR)/lib/pkgconfig/libpkgconf.pc
+endef
+
+$(eval $(generic-package))
+$(eval $(host-generic-package))
diff --git a/support/testing/tests/core/test_file_overwrite.py b/support/testing/tests/core/test_file_overwrite.py
new file mode 100644
index 0000000000..96553cb483
--- /dev/null
+++ b/support/testing/tests/core/test_file_overwrite.py
@@ -0,0 +1,48 @@
+import infra
+import infra.basetest
+import subprocess
+
+
+class DetectTargetFileOverwriteTest(infra.basetest.BRConfigTest):
+    config = \
+        infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
+        infra.basetest.MINIMAL_CONFIG + \
+        """
+        BR2_PER_PACKAGE_DIRECTORIES=y
+        BR2_PACKAGE_DETECT_OVERWRITE=y
+        """
+    br2_external = [infra.filepath("tests/core/br2-external/detect-overwrite")]
+
+    def test_run(self):
+        with self.assertRaises(SystemError):
+            self.b.build()
+        logf_path = infra.log_file_path(self.b.builddir, "build",
+                                        infra.basetest.BRConfigTest.logtofile)
+        if logf_path:
+            s = 'ERROR: package detect-overwrite has overwritten files installed by a previous package, aborting.'
+            with open(logf_path, "r") as f:
+                lines = [l for l in f.readlines() if l.startswith(s)]
+            self.assertNotEqual(len(lines), 0)
+
+
+class DetectHostFileOverwriteTest(infra.basetest.BRConfigTest):
+    config = \
+        infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
+        infra.basetest.MINIMAL_CONFIG + \
+        """
+        BR2_PER_PACKAGE_DIRECTORIES=y
+        BR2_PACKAGE_HOST_DETECT_OVERWRITE=y
+        """
+    br2_external = [infra.filepath("tests/core/br2-external/detect-overwrite")]
+
+    def test_run(self):
+        with self.assertRaises(SystemError):
+            self.b.build()
+        logf_path = infra.log_file_path(self.b.builddir, "build",
+                                        infra.basetest.BRConfigTest.logtofile)
+        if logf_path:
+            s = './lib/pkgconfig/hco_libpkgconf.pc: FAILED'
+            s = 'ERROR: package host-detect-overwrite has overwritten files installed by a previous package, aborting.'
+            with open(logf_path, "r") as f:
+                lines = [l for l in f.readlines() if l.startswith(s)]
+            self.assertNotEqual(len(lines), 0)
-- 
2.31.1

[Buildroot] [PATCH v2 02/18] package/pkg-generic.mk: Remove Info documents dir entry

[Yann E. MORIN]

Herv?, All,

On 2021-07-06 16:24 +0200, Herve Codina spake thusly:
> Some packages (autotools for instance) install documentation
> files using install-info. This program adds an entry in
> the Info directory file (share/info/dir) and this causes
> TARGET_DIR and/or HOST_DIR overwrite.
> 
> In order to avoid this overwrite this patch removes the Info
> directory file right after any installation.
> 
> In order to be as generic as possible, this patch introduces
> a new tooling to remove useless and conflicting files based
> on the file and/or directory list <PKG>_DROP_FILES_OR_DIRS.
> share/info/dir file is added for every packages in this list.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
> Changes v1 to v2:
>  - Reworked adding <PKG>_DROP_FILES_OR_DIRS and associated tooling.
> 
>  package/pkg-generic.mk | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 45589bcbb4..05b2095664 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -135,6 +135,22 @@ define check_bin_arch
>  		-a $(BR2_READELF_ARCH_NAME)
>  endef
>  
> +# Functions to remove conflicting and useless files
> +
> +# $1: base directory (target, staging, host)
> +define remove-conflicting-useless-files
> +	$(Q)$(RM) -rf $(patsubst %, $(1)/%, $($(PKG)_DROP_FILES_OR_DIRS))

This is very ambiguous: in case $(PKG)_DROP_FILES_OR_DIRS ends up
being empty, this code wil lexpand to "rm -rf". This does nothing,
fortunately, but calling rm -rf always makes me nervous, especially
in conjunction with adding $(1)/ in front of it, because what happens
when the caller forgets to pass the argument? Meh, maybe I'm getting
too paranoid. Or too old... Or both...

Yes, this is the exact line I wrote in my previous review, but it was
mostly just to kickstart the idea...

But here, I don't think checking would be superfluous.

Also, $(PKG)_DROP_FILES_OR_DIRS is supposed to contain absolute path,
e.g. /share/info/dir, so there already is a '/', so no need to add one.

So, what about:

    $(if $($(PKG)_DROP_FILES_OR_DIRS), \
        $(Q)$(RM) -rf $(patsubst %, $(1)%, $($(PKG)_DROP_FILES_OR_DIRS)))

No need to respin, I can do that when applying...

And of course, the big question: do we want to expose that to packages,
so they can set it by themselves? In which case, that lacks an entry in
the manual... But I don't think we should expose it; packages that want
to remove files can do do with post-install hooks. Here we really are
speaking about files that by design Buildroot does not want to handle...

Regards,
Yann E. MORIN.

> +endef
> +define REMOVE_CONFLICTING_USELESS_FILES_IN_HOST
> +	$(call remove-conflicting-useless-files,$(HOST_DIR))
> +endef
> +define REMOVE_CONFLICTING_USELESS_FILES_IN_STAGING
> +	$(call remove-conflicting-useless-files,$(STAGING_DIR))
> +endef
> +define REMOVE_CONFLICTING_USELESS_FILES_IN_TARGET
> +	$(call remove-conflicting-useless-files,$(TARGET_DIR))
> +endef
> +
>  ################################################################################
>  # Implicit targets -- produce a stamp file for each step of a package build
>  ################################################################################
> @@ -823,6 +839,16 @@ $$(error "Package $(1) defines host variant before target variant!")
>  endif
>  endif
>  
> +# Globaly remove following conflicting and useless files
> +$(2)_DROP_FILES_OR_DIRS += /share/info/dir
> +
> +ifeq ($$($(2)_TYPE),host)
> +$(2)_POST_INSTALL_HOOKS += REMOVE_CONFLICTING_USELESS_FILES_IN_HOST
> +else
> +$(2)_POST_INSTALL_STAGING_HOOKS += REMOVE_CONFLICTING_USELESS_FILES_IN_STAGING
> +$(2)_POST_INSTALL_TARGET_HOOKS += REMOVE_CONFLICTING_USELESS_FILES_IN_TARGET
> +endif
> +
>  # human-friendly targets and target sequencing
>  $(1):			$(1)-install
>  $(1)-install:		$$($(2)_TARGET_INSTALL)
> -- 
> 2.31.1
> 

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

[Buildroot] [PATCH v2 07/18] package/pkg-python: remove _sysconfigdata*.pyc files when _sysconfigdata*.py are changed

[Yann E. MORIN]

Herv?, All,

On 2021-07-06 16:24 +0200, Herve Codina spake thusly:
> In order to avoid _sysconfigdata*.pyc overwrites when they are generated based on
> _sysconfigdata*.py changes, this commit simply removes _sysconfigdata*.pyc
> whenever _sysconfigdata*.py are changed.
> 
> As they are removed, overwrite detection will no longer trig and coherency between

*trigger
s/coherency/consistency/  (this is a faux-amis).

No need to respin, I can fix when applying...

Regards,
Yann E. MORIN.

> the two files (.py and .pyc) is ensured.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
> Changes from v1 to v2:
>  - Used separate shells for the two 'find' commands.
>  - Used '-deleted' to remove files
> 
>  package/pkg-python.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/package/pkg-python.mk b/package/pkg-python.mk
> index 59a48e5a87..1e4fd5ba33 100644
> --- a/package/pkg-python.mk
> +++ b/package/pkg-python.mk
> @@ -97,6 +97,8 @@ define PKG_PYTHON_FIXUP_SYSCONFIGDATA
>  	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
>  		-name "_sysconfigdata*.py" | xargs --no-run-if-empty \
>  		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g"
> +	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
> +		-name "_sysconfigdata*.pyc" -delete
>  endef
>  endif
>  
> -- 
> 2.31.1
> 

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

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Yann E. MORIN]

Herv?, All,

On 2021-07-06 16:24 +0200, Herve Codina spake thusly:
> Fixing _sysconfigdata*.{py,pyc} was previously done by python package
> infrastructure. Some packages use python stuff without using python
> package infrastructure.
> These packages perform overwrites and need the specific python fixup
> to fix them.
> 
> In order to be sure to fix all of these packages, the python fixup
> is moved to the generic package infrastructure and applied to all
> packages.
> This follows the same principle as for the .la libtool files fixup.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
> Changes v1 to v2:
>  - Used '... -print0 | xargs -0 -r ...' construction.
>  - Used '-deleted' to remove files.
> 
>  package/pkg-generic.mk | 17 +++++++++++++++++
>  package/pkg-python.mk  | 12 ------------
>  2 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 6fbd4006da..8c19522fd8 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -103,6 +103,21 @@ define fixup-libtool-files
>  endef
>  endif
>  
> +# Make sure python _sysconfigdata*.py files only reference the current
> +# per-package directory.
> +
> +# $1: package name (lower case)
> +# $2: staging or host directory of the package
> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> +define fixup-python-files
> +	$(Q)find $(2) \( -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*' \) \
> +		-name "_sysconfigdata*.py" -print0 | xargs -0 --no-run-if-empty \
> +		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$(1)/:g"
> +	$(Q)find $(2) \( -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*' \) \
> +		-name "_sysconfigdata*.pyc" -delete

Those two find commands are not equivalent to the original ones.

Before, we had:
    find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* [...]

After, we'd have, after replacing $(2):
    find $(HOST_DIR) \( -path '$(HOST_DIR)/lib/python*' -o -path '$(HOST_DIR)/usr/lib/python*' \) [...]
    find $(STAGING_DIR) \( -path '$(STAGING_DIR)/lib/python*' -o -path '$(STAGING_DIR)/usr/lib/python*' \) [...]

So this is slightly different... I don't get why you needed to duplicate
the calls, calling the macro twice... And even then, it would have still
been simpler to pass the two locations as starting points to find:

    find $(HOST_DIR)/lib/python* $(HOST_DIR)/usr/lib/python* [...]
    find $(STAGING_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* [...]

But really, $(HOST_DIR)/usr is just a symlink to . so there is no reason
to search it. Also, previously $(STAGING_DIR)/lib/ was not in the search
list, but now it is...

> +endef
> +endif
> +
>  # Functions to collect statistics about installed files
>  
>  # $(1): base directory to search in
> @@ -254,6 +269,8 @@ $(BUILD_DIR)/%/.stamp_configured:
>  	@$(call pkg_size_before,$(HOST_DIR),-host)
>  	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
>  	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
> +	$(call fixup-python-files,$(NAME),$(HOST_DIR))
> +	$(call fixup-python-files,$(NAME),$(STAGING_DIR))
>  	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))

Even if the libtool fixups are not moved to the new _POST_PREPARE_HOOKS,
I still find it sad that we do not leverage those new hooks for the new
fixups... :-/

But since I said "OK-ish" in my previous review, I am going to stand by
it. A little reluctantly still... :-/

But then, re-reading the above about the two find commands: if you had
just moved the macro out of pkg-python.mk and into here, and just added:
    $(2)_POST_PREPARE_HOOKS += PKG_PYTHON_FIXUP_SYSCONFIGDATA

Then that would have been much simpler: 1) you would not have changed
the breadth of the search, and 2) you would have used the new hooks.

Oh, but you need the package name, that is passed as $(1)? No problem,
it is already available as $($(PKG)_NAME), like was done in the original
fixup macro (see below).

>  	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
>  	$($(PKG)_CONFIGURE_CMDS)
> diff --git a/package/pkg-python.mk b/package/pkg-python.mk
> index 1e4fd5ba33..e6b81bdfd3 100644
> --- a/package/pkg-python.mk
> +++ b/package/pkg-python.mk
> @@ -92,16 +92,6 @@ HOST_PKG_PYTHON_SETUPTOOLS_INSTALL_OPTS = \
>  	--root=/ \
>  	--single-version-externally-managed
>  
> -ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> -define PKG_PYTHON_FIXUP_SYSCONFIGDATA
> -	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
> -		-name "_sysconfigdata*.py" | xargs --no-run-if-empty \
> -		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g"
> -	find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* \
> -		-name "_sysconfigdata*.pyc" -delete
> -endef
> -endif
> -
>  ################################################################################
>  # inner-python-package -- defines how the configuration, compilation
>  # and installation of a Python package should be done, implements a
> @@ -246,8 +236,6 @@ $(2)_PYTHON_INTERPRETER = $$(HOST_DIR)/bin/$$($(2)_NEEDS_HOST_PYTHON)
>  endif
>  endif
>  
> -$(2)_PRE_CONFIGURE_HOOKS += PKG_PYTHON_FIXUP_SYSCONFIGDATA

See here: the fixup macro was already a hook! ;-)

Regards,
Yann E. MORIN.

>  #
>  # Build step. Only define it if not already defined by the package .mk
>  # file.
> -- 
> 2.31.1
> 

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

[Buildroot] [PATCH v2 17/18] support/testing/tests: add test for check_bin_arch

[Yann E. MORIN]

Herv?, All,

On 2021-07-06 16:25 +0200, Herve Codina spake thusly:
> From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> 
> This tests build a bogus package that installs a binary built for the
> host architecture into $(TARGET_DIR), which should cause a build
> failure, at least as long as the host architecture isn't ARM.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
> New patch in this v2 series
> 
> This patch is retrieved from Thomas's work.
> The first version was discussed
> https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-10-thomas.petazzoni at bootlin.com/

Correct URL is:
    https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-11-thomas.petazzoni at bootlin.com/

> Compared to the first version, as proposed by Yann in the previous
> review, this patch does not launch a subprocess (grep) to search the
> string.
> 
> Note also that the .gitlab-ci.yml modification present in the previous
> version is not present in this patch.

Yes, because the file is generated now. \o/

[--SNIP--]
> diff --git a/support/testing/tests/core/test_bad_arch.py b/support/testing/tests/core/test_bad_arch.py
> new file mode 100644
> index 0000000000..0be7fdc9b7
> --- /dev/null
> +++ b/support/testing/tests/core/test_bad_arch.py
> @@ -0,0 +1,19 @@
> +import infra
> +import infra.basetest
> +import subprocess
> +
> +
> +class DetectBadArchTest(infra.basetest.BRConfigTest):
> +    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + infra.basetest.MINIMAL_CONFIG
> +    br2_external = [infra.filepath("tests/core/br2-external/detect-bad-arch")]
> +
> +    def test_run(self):
> +        with self.assertRaises(SystemError):
> +            self.b.build()
> +        logf_path = infra.log_file_path(self.b.builddir, "build",
> +                                        infra.basetest.BRConfigTest.logtofile)
> +        if logf_path:
> +            s = 'ERROR: architecture for "/usr/bin/foo" is'
> +            with open(logf_path, "r") as f:
> +                lines = [l for l in f.readlines() if l.startswith(s)]
> +            self.assertEqual(len(lines), 0)

So, this tests explicitly installs a host executable in target/, yet the
assert is written to ensure there is no error: it compares against zero.

So this test-case will fail, by design.

But test should not fail; they should succeed. In this case, the assert
should check that there indeed is an error:

    self.assertEqual(len(lines), 1)

Or did I miss something?

Regards,
Yann E. MORIN.

> -- 
> 2.31.1
> 

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

[Buildroot] [PATCH v2 00/18] Overwritten file detection and fixes, one more step to TLP build

[Yann E. MORIN]

Herv?, All,

On 2021-07-06 16:24 +0200, Herve Codina spake thusly:
> This v2 series performs overwrite fixes (patches 1 to 9) before adding
> the overwrite detection tooling at patch 10.
> 
> It renames <PKG>_PER_PACKAGE_TWEAK_HOOKS to <PKG>_POST_PREPARE_HOOKS.
> 
> It avoids code duplication when fixing <pkg>_{reconfigure,rebuilf,reinstall} in
> PATCH 14.
> 
> One more patch was added in this v2 series (PATCH 15) to removed a no more
> used file and 3 patches related to test (PATCH 16 to 18) are added too.
> 
> More changes details are given in each individual patches.
> 
> Best regards,
> Herv? Codina
> 
> Herve Codina (14):
>   package/e2fsprogs: fix fsck overwrite in HOST_DIR
>   package/pkg-generic.mk: Remove Info documents dir entry
>   package/pkg-generic.mk: perform .la files fixup in per-package
>     HOST_DIR
>   package/pkg-generic: add post-prepare hooks
>   package/apr-util: use post-prepare hook
>   package/apache: move APACHE_FIXUP_APR_LIBTOOL to post-prepare hook
>   package/pkg-python: remove _sysconfigdata*.pyc files when
>     _sysconfigdata*.py are changed

I've applied those 7 first patches, thanks!

>   package/pkg-generic.mk: move python fixup to generic package
>     infrastructure

I still had some concerns about that one, that I had totally missed in
my previous review, sorry... So I stopped applying the series there.

If you agree with my suggestion, just say so as a reply to that patch,
and I'll do the changes when applying.

Regards,
Yann E. MORIN.

>   package/owfs: remove Python sysconfigdata fixup
>   package/pkg-generic.mk: generate final rsync exclude file list
>   Makefile: rsync global {TARGET,HOST}_DIR using exclusion file list
>   Makefile: breaks hardlinks in global {TARGET,HOST}_DIR on per-package
>     build
>   package/pkg-generic.mk: fix per-package
>     <pkg>-{reconfigure,rebuild,reinstall}
>   package/pkg-generic.mk: remove .files-final-rsync.before temporary
>     file
> 
> Thomas Petazzoni (4):
>   package/pkg-generic.mk: detect files overwritten in TARGET_DIR and
>     HOST_DIR
>   support/testing/infra: add log_file_path() function
>   support/testing/tests: add test for check_bin_arch
>   support/testing/tests: add test for file overwrite detection
> 
>  Makefile                                      |  33 ++++-
>  package/apache/apache.mk                      |   2 +-
>  package/apr-util/apr-util.mk                  |  12 +-
>  package/e2fsprogs/e2fsprogs.mk                |   1 +
>  package/owfs/owfs.mk                          |   9 --
>  package/pkg-generic.mk                        | 129 +++++++++++++++++-
>  package/pkg-python.mk                         |  10 --
>  support/testing/infra/__init__.py             |  11 +-
>  .../br2-external/detect-bad-arch/Config.in    |   1 +
>  .../detect-bad-arch/external.desc             |   1 +
>  .../br2-external/detect-bad-arch/external.mk  |   1 +
>  .../package/detect-bad-arch/Config.in         |   4 +
>  .../detect-bad-arch/detect-bad-arch.mk        |  15 ++
>  .../br2-external/detect-overwrite/Config.in   |   1 +
>  .../detect-overwrite/external.desc            |   1 +
>  .../br2-external/detect-overwrite/external.mk |   1 +
>  .../package/detect-overwrite/Config.in        |   5 +
>  .../detect-overwrite/detect-overwrite.mk      |  19 +++
>  support/testing/tests/core/test_bad_arch.py   |  19 +++
>  .../testing/tests/core/test_file_overwrite.py |  48 +++++++
>  20 files changed, 287 insertions(+), 36 deletions(-)
>  create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/Config.in
>  create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/external.desc
>  create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/external.mk
>  create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/Config.in
>  create mode 100644 support/testing/tests/core/br2-external/detect-bad-arch/package/detect-bad-arch/detect-bad-arch.mk
>  create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/Config.in
>  create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/external.desc
>  create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/external.mk
>  create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/Config.in
>  create mode 100644 support/testing/tests/core/br2-external/detect-overwrite/package/detect-overwrite/detect-overwrite.mk
>  create mode 100644 support/testing/tests/core/test_bad_arch.py
>  create mode 100644 support/testing/tests/core/test_file_overwrite.py
> 
> -- 
> 2.31.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

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

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Yann E. MORIN]

Herv?, All,

On 2021-07-06 21:50 +0200, Yann E. MORIN spake thusly:
> On 2021-07-06 16:24 +0200, Herve Codina spake thusly:
> > Fixing _sysconfigdata*.{py,pyc} was previously done by python package
> > infrastructure. Some packages use python stuff without using python
> > package infrastructure.
> > These packages perform overwrites and need the specific python fixup
> > to fix them.
> > 
> > In order to be sure to fix all of these packages, the python fixup
> > is moved to the generic package infrastructure and applied to all
> > packages.
[--SNIP--]
> But then, re-reading the above about the two find commands: if you had
> just moved the macro out of pkg-python.mk and into here, and just added:
>     $(2)_POST_PREPARE_HOOKS += PKG_PYTHON_FIXUP_SYSCONFIGDATA

And as usual, it would probably need another name, since it no longer is
in pkg-python.mk. This is a fixup, it's about python, so what about:

    $(2)_POST_PREPARE_HOOKS += FIXUP_PYTHON_SYSCONFIGDATA

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v2 17/18] support/testing/tests: add test for check_bin_arch

[Yann E. MORIN]

Herv?, All,

On 2021-07-06 22:20 +0200, Yann E. MORIN spake thusly:
> On 2021-07-06 16:25 +0200, Herve Codina spake thusly:
> > From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > 
> > This tests build a bogus package that installs a binary built for the
> > host architecture into $(TARGET_DIR), which should cause a build
> > failure, at least as long as the host architecture isn't ARM.
> > 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-11-thomas.petazzoni at bootlin.com/
[--SNIP--]
> > +        if logf_path:
> > +            s = 'ERROR: architecture for "/usr/bin/foo" is'
> > +            with open(logf_path, "r") as f:
> > +                lines = [l for l in f.readlines() if l.startswith(s)]
> > +            self.assertEqual(len(lines), 0)
> 
> So, this tests explicitly installs a host executable in target/, yet the
> assert is written to ensure there is no error: it compares against zero.
> So this test-case will fail, by design.

Ah, I think I see the issue: previously, the test by Thomas was calling
'grep', and was checking against '0', which meant that grep _had found_
the string...

So yeah, we really need to check that we _did_ find the error pattern:
    self.assertEqual(len(lines), 1)

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Herve Codina]

Hi,

On Tue, 6 Jul 2021 21:50:48 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> > +# Make sure python _sysconfigdata*.py files only reference the current
> > +# per-package directory.
> > +
> > +# $1: package name (lower case)
> > +# $2: staging or host directory of the package
> > +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> > +define fixup-python-files
> > +	$(Q)find $(2) \( -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*' \) \
> > +		-name "_sysconfigdata*.py" -print0 | xargs -0 --no-run-if-empty \
> > +		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$(1)/:g"
> > +	$(Q)find $(2) \( -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*' \) \
> > +		-name "_sysconfigdata*.pyc" -delete  
> 
> Those two find commands are not equivalent to the original ones.
> 
> Before, we had:
>     find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* [...]
> 
> After, we'd have, after replacing $(2):
>     find $(HOST_DIR) \( -path '$(HOST_DIR)/lib/python*' -o -path '$(HOST_DIR)/usr/lib/python*' \) [...]
>     find $(STAGING_DIR) \( -path '$(STAGING_DIR)/lib/python*' -o -path '$(STAGING_DIR)/usr/lib/python*' \) [...]
> 
> So this is slightly different... I don't get why you needed to duplicate
> the calls, calling the macro twice... And even then, it would have still
> been simpler to pass the two locations as starting points to find:
> 
>     find $(HOST_DIR)/lib/python* $(HOST_DIR)/usr/lib/python* [...]
>     find $(STAGING_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* [...]
> 
> But really, $(HOST_DIR)/usr is just a symlink to . so there is no reason
> to search it. Also, previously $(STAGING_DIR)/lib/ was not in the search
> list, but now it is...
> 
> > +endef
> > +endif
> > +
> >  # Functions to collect statistics about installed files
> >  
> >  # $(1): base directory to search in
> > @@ -254,6 +269,8 @@ $(BUILD_DIR)/%/.stamp_configured:
> >  	@$(call pkg_size_before,$(HOST_DIR),-host)
> >  	$(call fixup-libtool-files,$(NAME),$(HOST_DIR))
> >  	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
> > +	$(call fixup-python-files,$(NAME),$(HOST_DIR))
> > +	$(call fixup-python-files,$(NAME),$(STAGING_DIR))
> >  	$(foreach hook,$($(PKG)_POST_PREPARE_HOOKS),$(call $(hook))$(sep))  
> 
> Even if the libtool fixups are not moved to the new _POST_PREPARE_HOOKS,
> I still find it sad that we do not leverage those new hooks for the new
> fixups... :-/
> 
> But since I said "OK-ish" in my previous review, I am going to stand by
> it. A little reluctantly still... :-/
> 
> But then, re-reading the above about the two find commands: if you had
> just moved the macro out of pkg-python.mk and into here, and just added:
>     $(2)_POST_PREPARE_HOOKS += PKG_PYTHON_FIXUP_SYSCONFIGDATA
> 
> Then that would have been much simpler: 1) you would not have changed
> the breadth of the search, and 2) you would have used the new hooks.
> 
> Oh, but you need the package name, that is passed as $(1)? No problem,
> it is already available as $($(PKG)_NAME), like was done in the original
> fixup macro (see below).
> 


Well,

These finds are now called for all packages and .../lib/python* can be not
present. In this case, find fails.
Using '-path' avoid this failure if files/dirs are not present.

I wanted to mimic fixup-libtool-files.
And so performed 2 calls. One for HOST_DIR and the other for STAGING_DIR.

For HOST_DIR, the previous find searched in $(HOST_DIR)/lib,
for STAGING_DIR, it searched in $(STAGING_DIR)/usr/lib,

That's why I wrote the macro with 
-path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*'

We can simplify the call using:
  $(Q)find $(2) -path '$(2)/usr/lib/python*' -name ...
This assumes:
(1) python stuff will not install anything in $(STAGING_DIR)/lib/python*
(2) $(HOST_DIR)/usr will always be a symlink to $(HOST_DIR)/.


-- 
Herv? Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 17/18] support/testing/tests: add test for check_bin_arch

[Herve Codina]

Hi,

On Tue, 6 Jul 2021 23:25:18 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> > So, this tests explicitly installs a host executable in target/, yet the
> > assert is written to ensure there is no error: it compares against zero.
> > So this test-case will fail, by design.  
> 
> Ah, I think I see the issue: previously, the test by Thomas was calling
> 'grep', and was checking against '0', which meant that grep _had found_
> the string...
> 
> So yeah, we really need to check that we _did_ find the error pattern:
>     self.assertEqual(len(lines), 1)
> 

Oups,

I was focused on overwrite detection and I missed this one.
I am sorry.

I confirm, 'self.assertEqual(len(lines), 1)' fixed the test.

Herv?

-- 
Herv? Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Arnout Vandecappelle]

On 07/07/2021 13:48, Herve Codina wrote:
> Hi,
> 
> On Tue, 6 Jul 2021 21:50:48 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 
>>> +# Make sure python _sysconfigdata*.py files only reference the current
>>> +# per-package directory.
>>> +
>>> +# $1: package name (lower case)
>>> +# $2: staging or host directory of the package
>>> +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
>>> +define fixup-python-files
>>> +	$(Q)find $(2) \( -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*' \) \
>>> +		-name "_sysconfigdata*.py" -print0 | xargs -0 --no-run-if-empty \
>>> +		$(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$(1)/:g"
>>> +	$(Q)find $(2) \( -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*' \) \
>>> +		-name "_sysconfigdata*.pyc" -delete  
>>
>> Those two find commands are not equivalent to the original ones.
>>
>> Before, we had:
>>     find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* [...]
>>
>> After, we'd have, after replacing $(2):
>>     find $(HOST_DIR) \( -path '$(HOST_DIR)/lib/python*' -o -path '$(HOST_DIR)/usr/lib/python*' \) [...]
>>     find $(STAGING_DIR) \( -path '$(STAGING_DIR)/lib/python*' -o -path '$(STAGING_DIR)/usr/lib/python*' \) [...]
>>
>> So this is slightly different... I don't get why you needed to duplicate
>> the calls, calling the macro twice... And even then, it would have still
>> been simpler to pass the two locations as starting points to find:
>>
>>     find $(HOST_DIR)/lib/python* $(HOST_DIR)/usr/lib/python* [...]
>>     find $(STAGING_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python* [...]
>>
>> But really, $(HOST_DIR)/usr is just a symlink to . so there is no reason
>> to search it. Also, previously $(STAGING_DIR)/lib/ was not in the search
>> list, but now it is...

[snip]
> 
> These finds are now called for all packages and .../lib/python* can be not
> present. In this case, find fails.

 A simpler way of doing that would be:

	$(if $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*),
		$(Q)find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*


> Using '-path' avoid this failure if files/dirs are not present.
> 
> I wanted to mimic fixup-libtool-files.
> And so performed 2 calls. One for HOST_DIR and the other for STAGING_DIR.
> 
> For HOST_DIR, the previous find searched in $(HOST_DIR)/lib,
> for STAGING_DIR, it searched in $(STAGING_DIR)/usr/lib,
> 
> That's why I wrote the macro with 
> -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*'
> 
> We can simplify the call using:
>   $(Q)find $(2) -path '$(2)/usr/lib/python*' -name ...
> This assumes:
> (1) python stuff will not install anything in $(STAGING_DIR)/lib/python*

 This you can safely assume.

> (2) $(HOST_DIR)/usr will always be a symlink to $(HOST_DIR)/.

 Although for now you can assume this, ideally we would like to have the
possibility to get rid of the usr->. symlink. So if you can avoid relying on it
it would be better.

 Regards,
 Arnout

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Herve Codina]

Hi,

On Wed, 7 Jul 2021 14:21:16 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> > 
> > These finds are now called for all packages and .../lib/python* can be not
> > present. In this case, find fails.  
> 
>  A simpler way of doing that would be:
> 
> 	$(if $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*),
> 		$(Q)find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*
> 

I think this does not solve the problem.
find fails if at least one of the 2 paths are not present.

Something like this (not tested) is needed.

	$(if $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*),
 		$(Q)find $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*)



> 
> > Using '-path' avoid this failure if files/dirs are not present.
> > 
> > I wanted to mimic fixup-libtool-files.
> > And so performed 2 calls. One for HOST_DIR and the other for STAGING_DIR.
> > 
> > For HOST_DIR, the previous find searched in $(HOST_DIR)/lib,
> > for STAGING_DIR, it searched in $(STAGING_DIR)/usr/lib,
> > 
> > That's why I wrote the macro with 
> > -path '$(2)/lib/python*' -o -path '$(2)/usr/lib/python*'
> > 
> > We can simplify the call using:
> >   $(Q)find $(2) -path '$(2)/usr/lib/python*' -name ...
> > This assumes:
> > (1) python stuff will not install anything in $(STAGING_DIR)/lib/python*  
> 
>  This you can safely assume.

Ok.

> 
> > (2) $(HOST_DIR)/usr will always be a symlink to $(HOST_DIR)/.  
> 
>  Although for now you can assume this, ideally we would like to have the
> possibility to get rid of the usr->. symlink. So if you can avoid relying on it
> it would be better.

Right.
Assuming, the symlink not present, can we be sure that host python stuff will be
installed only in $(HOST_DIR)/lib/python* ?

Herv?

-- 
Herv? Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Arnout Vandecappelle]

On 07/07/2021 14:49, Herve Codina wrote:
> Hi,
> 
> On Wed, 7 Jul 2021 14:21:16 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:

[snip]

>>> (2) $(HOST_DIR)/usr will always be a symlink to $(HOST_DIR)/.  
>>
>>  Although for now you can assume this, ideally we would like to have the
>> possibility to get rid of the usr->. symlink. So if you can avoid relying on it
>> it would be better.
> 
> Right.
> Assuming, the symlink not present, can we be sure that host python stuff will be
> installed only in $(HOST_DIR)/lib/python* ?

 Yes - the usr symlink will only be removed if we're absolutely sure that
nothing installs in $(HOST_DIR)/usr (which means probably never...)

 Regards,
 Arnout

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Yann E. MORIN]

Herv?, Arnout, All,

On 2021-07-07 14:49 +0200, Herve Codina spake thusly:
> On Wed, 7 Jul 2021 14:21:16 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> > > These finds are now called for all packages and .../lib/python* can be not
> > > present. In this case, find fails.  

OK, I see.

However, the commit log was lacking these explanations, because the
change is not simply "the python fixup is moved to the generic package
infrastructure", because the fixups had to be adapted.

> >  A simpler way of doing that would be:
> > 	$(if $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*),
> > 		$(Q)find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*
> I think this does not solve the problem.
> find fails if at least one of the 2 paths are not present.
> 
> Something like this (not tested) is needed.
> 
> 	$(if $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*),
>  		$(Q)find $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*)

Yes, this would keep the pre-existing behaviour.

However, I think the traditional way we usually do that would be with a
foreach:

    $(foreach dir, $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*), \
        $(Q)find $(dir) -name "_sysconfigdata*.py" -print0 \
        |xargs -0 --no-run-if-empty \
            $(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g"
        $(Q)find $(dir) -name "_sysconfigdata*.pyc" -delete
    )

And thus you're back to a basic, non-parameterised macro that can be
used as a POST_PREPARE_HOOK. Wee! :-)

Note: optimisation: we should be able to use a single find call to do
both, though:

    $(foreach dir, $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*), \
        $(Q)find $(dir) \( -name "_sysconfigdata*.pyc" -delete \) \
            -o \( -name "_sysconfigdata*.py" -print0 \) \
        |xargs -0 --no-run-if-empty \
            $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
    )

Maybe the groupings '\( ... \)' are superfluous. Adapt appropriately...

> > > This assumes:
> > > (1) python stuff will not install anything in $(STAGING_DIR)/lib/python*  
> >  This you can safely assume.

Well, a package that would install its python bindings in /lib/python*
would anyway break at runtime, so this package would need to eb fixed to
install in /usr/lib/python*

So yes, nothing to look for in staging/lib/python*.

> > > (2) $(HOST_DIR)/usr will always be a symlink to $(HOST_DIR)/.  
> >  Although for now you can assume this, ideally we would like to have the
> > possibility to get rid of the usr->. symlink. So if you can avoid relying on it
> > it would be better.
> Right.
> Assuming, the symlink not present, can we be sure that host python stuff will be
> installed only in $(HOST_DIR)/lib/python* ?

I think, as Arnout said, that the usr symlink will always be present. So
we can also safely assume that there is nothing to look for in
host/usr/lib/python* either, because:

 1) that would have already been found in host/lib/python*

 2) that would also probably not work if usr was not a symlink, because
    our host python would not look there to find modules anyway

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Herve Codina]

Hi,

On Wed, 7 Jul 2021 22:12:57 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Herv?, Arnout, All,
> 
> On 2021-07-07 14:49 +0200, Herve Codina spake thusly:
> > On Wed, 7 Jul 2021 14:21:16 +0200
> > Arnout Vandecappelle <arnout@mind.be> wrote:  
> > > > These finds are now called for all packages and .../lib/python* can be not
> > > > present. In this case, find fails.    
> 
> OK, I see.
> 
> However, the commit log was lacking these explanations, because the
> change is not simply "the python fixup is moved to the generic package
> infrastructure", because the fixups had to be adapted.
> 
> > >  A simpler way of doing that would be:
> > > 	$(if $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*),
> > > 		$(Q)find $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*  
> > I think this does not solve the problem.
> > find fails if at least one of the 2 paths are not present.
> > 
> > Something like this (not tested) is needed.
> > 
> > 	$(if $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*),
> >  		$(Q)find $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*)  
> 
> Yes, this would keep the pre-existing behaviour.
> 
> However, I think the traditional way we usually do that would be with a
> foreach:
> 
>     $(foreach dir, $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*), \
>         $(Q)find $(dir) -name "_sysconfigdata*.py" -print0 \
>         |xargs -0 --no-run-if-empty \
>             $(SED) "s:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g"
>         $(Q)find $(dir) -name "_sysconfigdata*.pyc" -delete
>     )
> 
> And thus you're back to a basic, non-parameterised macro that can be
> used as a POST_PREPARE_HOOK. Wee! :-)
> 
> Note: optimisation: we should be able to use a single find call to do
> both, though:
> 
>     $(foreach dir, $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*), \
>         $(Q)find $(dir) \( -name "_sysconfigdata*.pyc" -delete \) \
>             -o \( -name "_sysconfigdata*.py" -print0 \) \
>         |xargs -0 --no-run-if-empty \
>             $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
>     )
> 
> Maybe the groupings '\( ... \)' are superfluous. Adapt appropriately...
> 

I tried this kind of things and I ran into trouble with $(wildcard ...)
Indeed, $(wildcard ...) does not see my directories.
A simple 'ls $(HOST_DIR)/lib/python*' just before $(foreach dir, $(wildcard ...), ...)
lists the directory.

Suppose this stupid package:
---- 8< -----
define TLP_PKGA_INSTALL_TARGET_CMDS
	echo "$$(date)" > $(TARGET_DIR)/pkga1
	echo "$$(date)" > $(TARGET_DIR)/pkga2
	mkdir -p $(TARGET_DIR)/foo/bla
	touch $(TARGET_DIR)/foo/bla/file1
	touch $(TARGET_DIR)/foo/bla/file2
endef

define TLP_PKGA_LIST_CMD
	echo "ls:" && ls $(TARGET_DIR)/foo/* || true
	echo "wildcard: $(wildcard $(TARGET_DIR)/foo/*)"
endef

TLP_PKGA_PRE_INSTALL_TARGET_HOOKS += TLP_PKGA_LIST_CMD
TLP_PKGA_POST_INSTALL_TARGET_HOOKS += TLP_PKGA_LIST_CMD


$(eval $(generic-package))
---- 8< -----

'make pkg' leads to:
	>>> pkg  Installing to target
	echo "ls:" && ls /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/* || true
	ls:
	ls: cannot access '/home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/*': No such file or directory
	echo "wildcard: "
	wildcard: 
	echo "$(date)" > /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/pkga1
	echo "$(date)" > /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/pkga2
	mkdir -p /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/bla
	touch /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/bla/file1
	touch /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/bla/file2
	echo "ls:" && ls /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/* || true
	ls:
	file1  file2
	echo "wildcard: "
	wildcard: 

$(wildcard ...) uses internal make caching.
Depending on when it is called related to make internal caching updates, it can
lead to unexpected results.

In the example provided, make cannot known that $(TARGET_DIR)/foo/bla was updated by
TLP_PKGA_INSTALL_TARGET_CMDS and so it didn't update its internal caching.
$(wildcard ...) does not see the new files.

Herve

-- 
Herv? Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Yann E. MORIN]

Herv?, All,

On 2021-07-08 17:38 +0200, Herve Codina spake thusly:
> On Wed, 7 Jul 2021 22:12:57 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
[--SNIP--]
> > Note: optimisation: we should be able to use a single find call to do
> > both, though:
> > 
> >     $(foreach dir, $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*), \
> >         $(Q)find $(dir) \( -name "_sysconfigdata*.pyc" -delete \) \
> >             -o \( -name "_sysconfigdata*.py" -print0 \) \
> >         |xargs -0 --no-run-if-empty \
> >             $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
> >     )
[--SNIP--]
> I tried this kind of things and I ran into trouble with $(wildcard ...)
> Indeed, $(wildcard ...) does not see my directories.
> A simple 'ls $(HOST_DIR)/lib/python*' just before $(foreach dir, $(wildcard ...), ...)
> lists the directory.
> 
> Suppose this stupid package:
> ---- 8< -----
> define TLP_PKGA_INSTALL_TARGET_CMDS
> 	echo "$$(date)" > $(TARGET_DIR)/pkga1
> 	echo "$$(date)" > $(TARGET_DIR)/pkga2
> 	mkdir -p $(TARGET_DIR)/foo/bla
> 	touch $(TARGET_DIR)/foo/bla/file1
> 	touch $(TARGET_DIR)/foo/bla/file2
> endef
> 
> define TLP_PKGA_LIST_CMD
> 	echo "ls:" && ls $(TARGET_DIR)/foo/* || true
> 	echo "wildcard: $(wildcard $(TARGET_DIR)/foo/*)"
> endef
> 
> TLP_PKGA_PRE_INSTALL_TARGET_HOOKS += TLP_PKGA_LIST_CMD
> TLP_PKGA_POST_INSTALL_TARGET_HOOKS += TLP_PKGA_LIST_CMD
> 
> 
> $(eval $(generic-package))
> ---- 8< -----
> 
> 'make pkg' leads to:
> 	>>> pkg  Installing to target
> 	echo "ls:" && ls /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/* || true
> 	ls:
> 	ls: cannot access '/home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/*': No such file or directory
> 	echo "wildcard: "
> 	wildcard: 
> 	echo "$(date)" > /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/pkga1
> 	echo "$(date)" > /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/pkga2
> 	mkdir -p /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/bla
> 	touch /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/bla/file1
> 	touch /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/bla/file2
> 	echo "ls:" && ls /home/hcodina/project/nagra/buildroot/output/per-package/tlp_pkga/target/foo/* || true
> 	ls:
> 	file1  file2
> 	echo "wildcard: "
> 	wildcard: 
> 
> $(wildcard ...) uses internal make caching.

It is not caching. The issue is when variables (or calls to functions)
are evaluated.

In makefile, the evaluation is done for a full recipe at once, not for
each line to be executed.

HOOKS, pre or post, are part of the same recipe and the corresponding
CMDS, so all three (pre hooks, cmds, and post hooks) are evaluated at
the same time, even before any single line of the recipe if executed.

So given this trivial Makefile:

    $ cat Makefile
    all:
        @touch toto
        @echo '"toto is $(wildcard toto)"'

    $ make
    "toto is "
    $ make
    "toto is toto"

So, in the end, with all these back-n-forth, and combining all the
explanations, we sould be able to complete the macro with:

    # Can't use $(foreach d, $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*, ...)
    # because those directories may be created in the same recipe this
    # macro wil be expanded in.
    # Additionally, either or both may be missing, which would make find
    # whine and fail.
    # So we just use HOST_DIR as a starting point, and filter on the two
    # directories of interest.
    define FIXUP_PYTHON_SYSCONFIGDATA
        $(Q)find $(HOST_DIR) \
            \( -path '$(HOST_DIR)/lib/python*' -o -path '$(STAGING_DIR)/usr/lib/python*' \) \
            \(    \( -name "_sysconfigdata*.pyc" -delete \) \
               -o \( -name "_sysconfigdata*.py" -print0 \) \) \
        |xargs -0 --no-run-if-empty \
            $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
    endef

And then, in inner-genenric-package:

    $(2)_POST_PREPARE_HOOKS += FIXUP_PYTHON_SYSCONFIGDATA

And then the commit log will need to have all the details why we use
this trick, and probably also a small comment above the macro defintion,
to explain it too.

Not that STAGING_DIR is not a starting point for find, because it is
below HOST_DIR, so we will eventually search STAGING_DIR.

Oh, and a final note: the existing hook is already sliently broken!
Indeed, if either $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*
is mising, find whines and fails. However, the breakage is ignored,
because find is on the left-hand side of a pipe, so its exit status is
ignored.

But of course, this is no longer the case with the new find that was
added by the previous commit, which really is the commit that introduces
the issues of find failing when a directory was missing, not this commit
specifically. I suppose you did not notice, because the prvious commit
was not tested on its own (and I can understand that, for I do that very
often too). But now autobuilders have caught it (and user have started
to notice too)...

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v2 08/18] package/pkg-generic.mk: move python fixup to generic package infrastructure

[Herve Codina]

On Thu, 8 Jul 2021 22:26:23 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Herv?, All,
> 
> On 2021-07-08 17:38 +0200, Herve Codina spake thusly:
> > On Wed, 7 Jul 2021 22:12:57 +0200
> > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:  
> [--SNIP--]
> > > Note: optimisation: we should be able to use a single find call to do
> > > both, though:
> > > 
> > >     $(foreach dir, $(wildcard $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*), \
> > >         $(Q)find $(dir) \( -name "_sysconfigdata*.pyc" -delete \) \
> > >             -o \( -name "_sysconfigdata*.py" -print0 \) \
> > >         |xargs -0 --no-run-if-empty \
> > >             $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
> > >     )  
> [--SNIP--]
> > I tried this kind of things and I ran into trouble with $(wildcard ...)
> > Indeed, $(wildcard ...) does not see my directories.
> > A simple 'ls $(HOST_DIR)/lib/python*' just before $(foreach dir, $(wildcard ...), ...)
> > lists the directory.
[...]
> It is not caching. The issue is when variables (or calls to functions)
> are evaluated.
> 
> In makefile, the evaluation is done for a full recipe at once, not for
> each line to be executed.
> 
> HOOKS, pre or post, are part of the same recipe and the corresponding
> CMDS, so all three (pre hooks, cmds, and post hooks) are evaluated at
> the same time, even before any single line of the recipe if executed.
> 
> So given this trivial Makefile:
> 
>     $ cat Makefile
>     all:
>         @touch toto
>         @echo '"toto is $(wildcard toto)"'
> 
>     $ make
>     "toto is "
>     $ make
>     "toto is toto"
> 

Thanks for this explanation.

> So, in the end, with all these back-n-forth, and combining all the
> explanations, we sould be able to complete the macro with:
> 
>     # Can't use $(foreach d, $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*, ...)
>     # because those directories may be created in the same recipe this
>     # macro wil be expanded in.
>     # Additionally, either or both may be missing, which would make find
>     # whine and fail.
>     # So we just use HOST_DIR as a starting point, and filter on the two
>     # directories of interest.
>     define FIXUP_PYTHON_SYSCONFIGDATA
>         $(Q)find $(HOST_DIR) \
>             \( -path '$(HOST_DIR)/lib/python*' -o -path '$(STAGING_DIR)/usr/lib/python*' \) \
>             \(    \( -name "_sysconfigdata*.pyc" -delete \) \
>                -o \( -name "_sysconfigdata*.py" -print0 \) \) \
>         |xargs -0 --no-run-if-empty \
>             $(SED) 's:$(PER_PACKAGE_DIR)/[^/]\+/:$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/:g'
>     endef
> 
> And then, in inner-genenric-package:
> 
>     $(2)_POST_PREPARE_HOOKS += FIXUP_PYTHON_SYSCONFIGDATA
> 
> And then the commit log will need to have all the details why we use
> this trick, and probably also a small comment above the macro defintion,
> to explain it too.
> 
> Not that STAGING_DIR is not a starting point for find, because it is
> below HOST_DIR, so we will eventually search STAGING_DIR.

Right, I will try this one.

> 
> Oh, and a final note: the existing hook is already sliently broken!
> Indeed, if either $(HOST_DIR)/lib/python* $(STAGING_DIR)/usr/lib/python*
> is mising, find whines and fails. However, the breakage is ignored,
> because find is on the left-hand side of a pipe, so its exit status is
> ignored.
> 
> But of course, this is no longer the case with the new find that was
> added by the previous commit, which really is the commit that introduces
> the issues of find failing when a directory was missing, not this commit
> specifically. I suppose you did not notice, because the prvious commit
> was not tested on its own (and I can understand that, for I do that very
> often too). But now autobuilders have caught it (and user have started
> to notice too)...

You right, I tested my first version of the patch (with the pipe) and missed
the issue introduced by removing the pipe in v2.
Indeed, I tested the whole series not each patches.

-- 
Herv? Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com