[Buildroot] [PATCH 00/11] Overwritten file detection, improvements to file listing logic

[Buildroot] [PATCH 00/11] Overwritten file detection, improvements to file listing logic

[Thomas Petazzoni]

Hello,

The initial starting point for this series was that I wanted to detect
when a package overwrites files installed by another package, which is
something that is not compatible with per-package directory support. I
indeed have a complex build configuration which I'm converting to use
TLP, and this build configuration does lots of overwrites, which I
want to detect and fix.

While doing this overwrite detection, I realized that our existing
logic to detect which files are installed by each package is not
reliable: if a target package installs something in $(HOST_DIR), those
files installed in $(HOST_DIR) are not tracked. Getting this list of
files correct is also needed to properly support the
-reinstall,-rebuild,-reconfigure targets in a per-package
configuration (which this series does not address yet).

So this series goes on like this (I'm only highlighting the main
commits, not all preparation commits and details)

 - We create a new "install" step in PATCH 3. It is executed after all
   installation steps have executed. As of PATCH 3, it does nothing
   except creating a stamp file, but it will allow us to do some logic
   once a package is finished installing. Indeed, so far we had no
   place where we could run some logic after all installation steps of
   a package have completed.

 - In PATCH 5, we rework the pkg_size logic that detects files
   installed by each package to rely on the new "install"
   step. Instead of capturing the list of files "before" at the
   beginning of install-target, install-staging, install-images,
   install-host, and the list of files "after" at the end of these
   steps, we capture the list of files "before" at the beginning of
   the configure step, and the list of files "after" during the new
   "install" step.

 - In PATCH 8, we introduce the logic to detect files overwritten by
   other packages, using a simple md5sum hash.

 - In PATCH 9-11, we add test cases for the "check binary
   architecture" functionality and the "overwritten files detection"
   functionality.

This has received some basic testing so far, I'm definitely interested
in getting feedback from people, simply by testing if it works for
them.

All these changes are also available on the Git branch at:

  https://github.com/tpetazzoni/buildroot/tree/detect-overwrite

Best regards,

Thomas Petazzoni

Thomas Petazzoni (11):
  package/pkg-generic.mk: use $(PKG)_NAME in step_pkg_size_after
  package/pkg-generic.mk: drop useless $(1) argument in
    step_pkg_size_{before,after}
  package/pkg-generic.mk: introduce final 'install' step
  package/pkg-generic.mk: create directories upfront in the configure
    step
  package/pkg-generic.mk: rework pkg_size logic with the "installed"
    step
  package/pkg-generic.mk: exclude the staging sub-directory
  package/pkg-generic.mk: move pkg_size_{before,after} and
    check_bin_arch functions
  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

 .gitlab-ci.yml                                |   3 +
 package/pkg-generic.mk                        | 146 ++++++++++--------
 package/pkg-utils.mk                          |   1 -
 support/testing/infra/__init__.py             |  13 +-
 .../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 |  47 ++++++
 16 files changed, 213 insertions(+), 65 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.25.4

[Buildroot] [PATCH 01/11] package/pkg-generic.mk: use $(PKG)_NAME in step_pkg_size_after

[Thomas Petazzoni]

Since we're already using $(PKG)_DIR in step_pkg_size_after, we can
also just use $(PKG)_NAME. This allows to make $(1) useless, which
means it can be dropped in a follow-up commit.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 8cd5a7ff62..d8df34ed14 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -76,7 +76,7 @@ define step_pkg_size_after
 	LC_ALL=C comm -13 \
 		$($(PKG)_DIR)/.files-list$(3).before \
 		$($(PKG)_DIR)/.files-list$(3).after \
-		| sed -r -e 's/^[^,]+/$(1)/' \
+		| sed -r -e 's/^[^,]+/$($(PKG)_NAME)/' \
 		> $($(PKG)_DIR)/.files-list$(3).txt
 	rm -f $($(PKG)_DIR)/.files-list$(3).before
 	rm -f $($(PKG)_DIR)/.files-list$(3).after
-- 
2.25.4

[Buildroot] [PATCH 02/11] package/pkg-generic.mk: drop useless $(1) argument in step_pkg_size_{before, after}

[Thomas Petazzoni]

The $(1) argument passed to step_pkg_size_{before,after}, which
contains the package name, is no longer used. We simply use $(PKG) to
get the upper-case version of the package name.

So, let's drop this first argument that isn't needed.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index d8df34ed14..fc43807bb0 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -57,45 +57,43 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
 
 # Hooks to collect statistics about installed files
 
-# $(1): package name
-# $(2): base directory to search in
-# $(3): suffix of file (optional)
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
 define step_pkg_size_before
-	cd $(2); \
+	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-list$(3).before
+		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).before
 endef
 
-# $(1): package name
-# $(2): base directory to search in
-# $(3): suffix of file (optional)
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
 define step_pkg_size_after
-	cd $(2); \
+	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-list$(3).after
+		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).after
 	LC_ALL=C comm -13 \
-		$($(PKG)_DIR)/.files-list$(3).before \
-		$($(PKG)_DIR)/.files-list$(3).after \
+		$($(PKG)_DIR)/.files-list$(2).before \
+		$($(PKG)_DIR)/.files-list$(2).after \
 		| sed -r -e 's/^[^,]+/$($(PKG)_NAME)/' \
-		> $($(PKG)_DIR)/.files-list$(3).txt
-	rm -f $($(PKG)_DIR)/.files-list$(3).before
-	rm -f $($(PKG)_DIR)/.files-list$(3).after
+		> $($(PKG)_DIR)/.files-list$(2).txt
+	rm -f $($(PKG)_DIR)/.files-list$(2).before
+	rm -f $($(PKG)_DIR)/.files-list$(2).after
 endef
 
 define step_pkg_size
 	$(if $(filter start-install-target,$(1)-$(2)),\
-		$(call step_pkg_size_before,$(3),$(TARGET_DIR)))
+		$(call step_pkg_size_before,$(TARGET_DIR)))
 	$(if $(filter start-install-staging,$(1)-$(2)),\
-		$(call step_pkg_size_before,$(3),$(STAGING_DIR),-staging))
+		$(call step_pkg_size_before,$(STAGING_DIR),-staging))
 	$(if $(filter start-install-host,$(1)-$(2)),\
-		$(call step_pkg_size_before,$(3),$(HOST_DIR),-host))
+		$(call step_pkg_size_before,$(HOST_DIR),-host))
 
 	$(if $(filter end-install-target,$(1)-$(2)),\
-		$(call step_pkg_size_after,$(3),$(TARGET_DIR)))
+		$(call step_pkg_size_after,$(TARGET_DIR)))
 	$(if $(filter end-install-staging,$(1)-$(2)),\
-		$(call step_pkg_size_after,$(3),$(STAGING_DIR),-staging))
+		$(call step_pkg_size_after,$(STAGING_DIR),-staging))
 	$(if $(filter end-install-host,$(1)-$(2)),\
-		$(call step_pkg_size_after,$(3),$(HOST_DIR),-host))
+		$(call step_pkg_size_after,$(HOST_DIR),-host))
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
 
-- 
2.25.4

[Buildroot] [PATCH 03/11] package/pkg-generic.mk: introduce final 'install' step

[Thomas Petazzoni]

We currently have four different install steps: target installation,
staging installation, images installation and host installation. These
steps are directly triggered from the $(1)-install make target, so
there is no place where we can run some logic once all installation
steps have completed.

However, as part of improving the reliability of the logic done in
step_pkg_size_before and step_pkg_size_after to detect the files
installed by packages, we would in fact need to run some logic after
all installation steps have completed. This will allow us to make sure
that all files are detected, even if a host package installs something
in the target directory, or if a target package installs something in
the host directory.

To achieve this, this commit implements a new stamp file,
.stamp_installed, which is a step that depends on all four install
steps. Currently, this step does nothing except creating the stamp
file.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index fc43807bb0..43f6f86b40 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -374,6 +374,11 @@ $(BUILD_DIR)/%/.stamp_target_installed:
 	@$(call step_end,install-target)
 	$(Q)touch $@
 
+# Final installation step, completed when all installation steps
+# (host, images, staging, target) have completed
+$(BUILD_DIR)/%/.stamp_installed:
+	$(Q)touch $@
+
 # Remove package sources
 $(BUILD_DIR)/%/.stamp_dircleaned:
 	$(if $(BR2_PER_PACKAGE_DIRECTORIES),rm -Rf $(PER_PACKAGE_DIR)/$(NAME))
@@ -705,6 +710,7 @@ $(2)_FINAL_RECURSIVE_RDEPENDENCIES = $$(sort \
 	$$($(2)_FINAL_RECURSIVE_RDEPENDENCIES__X))
 
 # define sub-target stamps
+$(2)_TARGET_INSTALL =           $$($(2)_DIR)/.stamp_installed
 $(2)_TARGET_INSTALL_TARGET =	$$($(2)_DIR)/.stamp_target_installed
 $(2)_TARGET_INSTALL_STAGING =	$$($(2)_DIR)/.stamp_staging_installed
 $(2)_TARGET_INSTALL_IMAGES =	$$($(2)_DIR)/.stamp_images_installed
@@ -760,14 +766,23 @@ endif
 
 # human-friendly targets and target sequencing
 $(1):			$(1)-install
+$(1)-install:		$$($(2)_TARGET_INSTALL)
 
 ifeq ($$($(2)_TYPE),host)
-$(1)-install:	        $(1)-install-host
+$$($(2)_TARGET_INSTALL): $$($(2)_TARGET_INSTALL_HOST)
 else
 $(2)_INSTALL_STAGING	?= NO
 $(2)_INSTALL_IMAGES	?= NO
 $(2)_INSTALL_TARGET	?= YES
-$(1)-install:		$(1)-install-staging $(1)-install-target $(1)-install-images
+ifeq ($$($(2)_INSTALL_TARGET),YES)
+$$($(2)_TARGET_INSTALL): $$($(2)_TARGET_INSTALL_TARGET)
+endif
+ifeq ($$($(2)_INSTALL_STAGING),YES)
+$$($(2)_TARGET_INSTALL): $$($(2)_TARGET_INSTALL_STAGING)
+endif
+ifeq ($$($(2)_INSTALL_IMAGES),YES)
+$$($(2)_TARGET_INSTALL): $$($(2)_TARGET_INSTALL_IMAGES)
+endif
 endif
 
 ifeq ($$($(2)_INSTALL_TARGET),YES)
@@ -936,6 +951,7 @@ $(1)-reconfigure:	$(1)-clean-for-reconfigure $(1)
 
 # define the PKG variable for all targets, containing the
 # uppercase package variable prefix
+$$($(2)_TARGET_INSTALL):		PKG=$(2)
 $$($(2)_TARGET_INSTALL_TARGET):		PKG=$(2)
 $$($(2)_TARGET_INSTALL_STAGING):	PKG=$(2)
 $$($(2)_TARGET_INSTALL_IMAGES):		PKG=$(2)
-- 
2.25.4

[Buildroot] [PATCH 04/11] package/pkg-generic.mk: create directories upfront in the configure step

[Thomas Petazzoni]

We currently create HOST_DIR, TARGET_DIR, STAGING_DIR and BINARIES_DIR
in their respective installation steps. However, as we are about to
change how the logic to capture files installed by packages is
implemented, we will need these directories to exist at the configure
step to keep things simple.

Note that when BR2_PER_PACKAGE_DIRECTORIES=y, the HOST_DIR, TARGET_DIR
and STAGING_DIR are anyway already created at the configure step, when
populating the per-package HOST_DIR and TARGET_DIR. This also means
that we can drop the "mkdir" from per-package-rsync.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/pkg-generic.mk | 5 +----
 package/pkg-utils.mk   | 1 -
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 43f6f86b40..efdc32e355 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -249,6 +249,7 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
 
 # Configure
 $(BUILD_DIR)/%/.stamp_configured:
+	@mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR) $(BINARIES_DIR)
 	@$(call step_start,configure)
 	@$(call MESSAGE,"Configuring")
 	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
@@ -271,7 +272,6 @@ $(BUILD_DIR)/%/.stamp_built::
 
 # Install to host dir
 $(BUILD_DIR)/%/.stamp_host_installed:
-	@mkdir -p $(HOST_DIR)
 	@$(call step_start,install-host)
 	@$(call MESSAGE,"Installing to host directory")
 	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
@@ -301,7 +301,6 @@ $(BUILD_DIR)/%/.stamp_host_installed:
 # empty when we use an internal toolchain.
 #
 $(BUILD_DIR)/%/.stamp_staging_installed:
-	@mkdir -p $(STAGING_DIR)
 	@$(call step_start,install-staging)
 	@$(call MESSAGE,"Installing to staging directory")
 	$(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
@@ -344,7 +343,6 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 
 # Install to images dir
 $(BUILD_DIR)/%/.stamp_images_installed:
-	@mkdir -p $(BINARIES_DIR)
 	@$(call step_start,install-image)
 	@$(call MESSAGE,"Installing to images directory")
 	$(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep))
@@ -355,7 +353,6 @@ $(BUILD_DIR)/%/.stamp_images_installed:
 
 # Install to target dir
 $(BUILD_DIR)/%/.stamp_target_installed:
-	@mkdir -p $(TARGET_DIR)
 	@$(call step_start,install-target)
 	@$(call MESSAGE,"Installing to target")
 	$(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 66504d0be2..dd7b06ac62 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -156,7 +156,6 @@ ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
 # $2: 'host' or 'target'
 # $3: destination directory
 define per-package-rsync
-	mkdir -p $(3)
 	$(foreach pkg,$(1),\
 		rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
 		$(PER_PACKAGE_DIR)/$(pkg)/$(2)/ \
-- 
2.25.4

[Buildroot] [PATCH 05/11] package/pkg-generic.mk: rework pkg_size logic with the "installed" step

[Thomas Petazzoni]

This commits reworks the pkg_size logic to no longer use the
GLOBAL_INSTRUMENTATION_HOOKS mechanism, but instead be directly
implemented within the configure step and install step.

The problem with the current implementation in the
GLOBAL_INSTRUMENTATION_HOOKS is that we only capture what is installed
in $(HOST_DIR) during the "host installation step", what is installed
in $(TARGET_DIR) during the "target installation step" and what is
installed in "$(STAGING_DIR)" during the staging installation step.

While this seems reasonable in principle, it is in fact not completely
true. For example, "toolchain", which is a target package, installs
tons of files in $(HOST_DIR). "qt5base", which is also a target
package, also installs things in $(HOST_DIR). Same with the "syslinux"
package.

The idea behind this patch is pretty simple:

 - At the beginning of the configure step, right after the per-package
   directories have been populated (if BR2_PER_PACKAGE_DIRECTORIES=y),
   we capture the state of the HOST_DIR, TARGET_DIR and STAGING_DIR.

 - At the end of all install steps (which is possible thanks to the
   newly introduced "install" step), we capture again the state of
   HOST_DIR, TARGET_DIR and STAGING_DIR, and compare it to what we
   have saved at the configure step.

So regardless of whether a file has been installed in $(HOST_DIR)
during the target or staging installation steps of a target package,
or if a host package has installed a file in $(TARGET_DIR), we will
detect it.

The pkg_size_before and pkg_size_after macros are intentionally left
where they are (even if they now fall in the middle of the
GLOBAL_INSTRUMENTATION_HOOKS implementations) to minimize the diffstat
and facilitate review.

Note that we also have to change check_bin_arch to be explicitly
called from the install step rather than through a
GLOBAL_INSTRUMENTATION_HOOKS as it depends on the .files-list.txt file
produced by the pkg_size_after function.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index efdc32e355..3fb1e5f8c3 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -59,7 +59,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
 
 # $(1): base directory to search in
 # $(2): suffix of file (optional)
-define step_pkg_size_before
+define pkg_size_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-list$(2).before
@@ -67,7 +67,7 @@ endef
 
 # $(1): base directory to search in
 # $(2): suffix of file (optional)
-define step_pkg_size_after
+define pkg_size_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-list$(2).after
@@ -80,35 +80,14 @@ define step_pkg_size_after
 	rm -f $($(PKG)_DIR)/.files-list$(2).after
 endef
 
-define step_pkg_size
-	$(if $(filter start-install-target,$(1)-$(2)),\
-		$(call step_pkg_size_before,$(TARGET_DIR)))
-	$(if $(filter start-install-staging,$(1)-$(2)),\
-		$(call step_pkg_size_before,$(STAGING_DIR),-staging))
-	$(if $(filter start-install-host,$(1)-$(2)),\
-		$(call step_pkg_size_before,$(HOST_DIR),-host))
-
-	$(if $(filter end-install-target,$(1)-$(2)),\
-		$(call step_pkg_size_after,$(TARGET_DIR)))
-	$(if $(filter end-install-staging,$(1)-$(2)),\
-		$(call step_pkg_size_after,$(STAGING_DIR),-staging))
-	$(if $(filter end-install-host,$(1)-$(2)),\
-		$(call step_pkg_size_after,$(HOST_DIR),-host))
-endef
-GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
-
-# Relies on step_pkg_size, so must be after
 define check_bin_arch
-	$(if $(filter end-install-target,$(1)-$(2)),\
-		support/scripts/check-bin-arch -p $(3) \
-			-l $($(PKG)_DIR)/.files-list.txt \
-			$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
-			-r $(TARGET_READELF) \
-			-a $(BR2_READELF_ARCH_NAME))
+	support/scripts/check-bin-arch -p $($(PKG)_NAME) \
+		-l $($(PKG)_DIR)/.files-list.txt \
+		$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
+		-r $(TARGET_READELF) \
+		-a $(BR2_READELF_ARCH_NAME)
 endef
 
-GLOBAL_INSTRUMENTATION_HOOKS += check_bin_arch
-
 # This hook checks that host packages that need libraries that we build
 # have a proper DT_RPATH or DT_RUNPATH tag
 define check_host_rpath
@@ -253,6 +232,9 @@ $(BUILD_DIR)/%/.stamp_configured:
 	@$(call step_start,configure)
 	@$(call MESSAGE,"Configuring")
 	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
+	@$(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),$(STAGING_DIR))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
@@ -374,6 +356,10 @@ $(BUILD_DIR)/%/.stamp_target_installed:
 # Final installation step, completed when all installation steps
 # (host, images, staging, target) have completed
 $(BUILD_DIR)/%/.stamp_installed:
+	@$(call pkg_size_after,$(TARGET_DIR))
+	@$(call pkg_size_after,$(STAGING_DIR),-staging)
+	@$(call pkg_size_after,$(HOST_DIR),-host)
+	@$(call check_bin_arch)
 	$(Q)touch $@
 
 # Remove package sources
-- 
2.25.4

[Buildroot] [PATCH 06/11] package/pkg-generic.mk: exclude the staging sub-directory

[Thomas Petazzoni]

Now that we are checking the host directory changes throughout all
installation steps and not just during the "host installation step",
it means that changes done within the staging directory (which is a
subdir of the host directory) are also visible in the
.files-list-host.txt file.

Note that this problem already potentially occurs if a host package is
installing files in the staging directory: they would be listed in
.files-list-host.txt even without the changes in this series.

To fix this up, we simply exclude files that are beneath the
$(STAGING_SUBDIR). Note that we do that in all cases, so when
searching $(HOST_DIR), $(HOST_DIR)/$(STAGING_SUBDIR) is excluded, but
when searching $(TARGET_DIR), $(TARGET_DIR)/$(STAGING_SUBDIR) is
excluded, and when search $(STAGING_DIR),
$(STAGING_DIR)/$(STAGING_SUBDIR) is excluded. This is not a problem in
practice since $(TARGET_DIR)/$(STAGING_SUBDIR) and
$(STAGING_DIR)/$(STAGING_SUBDIR) don't exist, but it's not very
nice. However, it allows to keep the code simple.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 3fb1e5f8c3..2ae269bb3d 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -61,7 +61,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
 # $(2): suffix of file (optional)
 define pkg_size_before
 	cd $(1); \
-	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
+	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
 		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).before
 endef
 
@@ -69,7 +69,7 @@ endef
 # $(2): suffix of file (optional)
 define pkg_size_after
 	cd $(1); \
-	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
+	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
 		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).after
 	LC_ALL=C comm -13 \
 		$($(PKG)_DIR)/.files-list$(2).before \
-- 
2.25.4

[Buildroot] [PATCH 07/11] package/pkg-generic.mk: move pkg_size_{before, after} and check_bin_arch functions

[Thomas Petazzoni]

These functions are no longer using the GLOBAL_INSTRUMENTATION_HOOKS
mechanism, so it doesn't make much sense for them to be in the section
of pkg-generic.mk related to those hooks.

Move them to the "Helper functions" section.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 2ae269bb3d..6e06d735ad 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -55,39 +55,6 @@ define step_time
 endef
 GLOBAL_INSTRUMENTATION_HOOKS += step_time
 
-# Hooks to collect statistics about installed files
-
-# $(1): base directory to search in
-# $(2): suffix of file (optional)
-define pkg_size_before
-	cd $(1); \
-	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
-		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).before
-endef
-
-# $(1): base directory to search in
-# $(2): suffix of file (optional)
-define pkg_size_after
-	cd $(1); \
-	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
-		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).after
-	LC_ALL=C comm -13 \
-		$($(PKG)_DIR)/.files-list$(2).before \
-		$($(PKG)_DIR)/.files-list$(2).after \
-		| sed -r -e 's/^[^,]+/$($(PKG)_NAME)/' \
-		> $($(PKG)_DIR)/.files-list$(2).txt
-	rm -f $($(PKG)_DIR)/.files-list$(2).before
-	rm -f $($(PKG)_DIR)/.files-list$(2).after
-endef
-
-define check_bin_arch
-	support/scripts/check-bin-arch -p $($(PKG)_NAME) \
-		-l $($(PKG)_DIR)/.files-list.txt \
-		$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
-		-r $(TARGET_READELF) \
-		-a $(BR2_READELF_ARCH_NAME)
-endef
-
 # This hook checks that host packages that need libraries that we build
 # have a proper DT_RPATH or DT_RUNPATH tag
 define check_host_rpath
@@ -135,6 +102,39 @@ define fixup-libtool-files
 endef
 endif
 
+# Functions to collect statistics about installed files
+
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
+define pkg_size_before
+	cd $(1); \
+	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
+		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).before
+endef
+
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
+define pkg_size_after
+	cd $(1); \
+	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
+		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).after
+	LC_ALL=C comm -13 \
+		$($(PKG)_DIR)/.files-list$(2).before \
+		$($(PKG)_DIR)/.files-list$(2).after \
+		| sed -r -e 's/^[^,]+/$($(PKG)_NAME)/' \
+		> $($(PKG)_DIR)/.files-list$(2).txt
+	rm -f $($(PKG)_DIR)/.files-list$(2).before
+	rm -f $($(PKG)_DIR)/.files-list$(2).after
+endef
+
+define check_bin_arch
+	support/scripts/check-bin-arch -p $($(PKG)_NAME) \
+		-l $($(PKG)_DIR)/.files-list.txt \
+		$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
+		-r $(TARGET_READELF) \
+		-a $(BR2_READELF_ARCH_NAME)
+endef
+
 ################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
-- 
2.25.4

[Buildroot] [PATCH 08/11] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR

[Thomas Petazzoni]

With per-package directory support, it is absolutely critical that a
given package does not overwrite files already installed by another
package. However, 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.

Due to this calculation and verification having some overhead, it is
only enabled when BR2_PER_PACKAGE_DIRECTORIES=y. Note that having
BR2_PER_PACKAGE_DIRECTORIES=y also means that on average the HOST_DIR
and TARGET_DIR contain less files than on non-per-package build: we
only have in HOST_DIR and TARGET_DIR the dependencies of the current
package being built. 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.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6e06d735ad..82af4afaee 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -102,6 +102,27 @@ define fixup-libtool-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
+	cd $(1); \
+	LC_ALL=C find . -type f -exec md5sum {} \; > $($(PKG)_DIR)/.files$(2).md5
+endef
+
+# $(1): base directory to search in
+# $(2): suffix of file (optional)
+define pkg_detect_overwrite_after
+	cd $(1); \
+	if test -s $($(PKG)_DIR)/.files$(2).md5 ; then \
+		LC_ALL=C 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
@@ -235,6 +256,8 @@ $(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 pkg_detect_overwrite_before,$(TARGET_DIR))
+	@$(call pkg_detect_overwrite_before,$(HOST_DIR),-host)
 	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
@@ -360,6 +383,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.25.4

[Buildroot] [PATCH 09/11] support/testing/infra: add log_file_path() function

[Thomas Petazzoni]

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>
---
 support/testing/infra/__init__.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/support/testing/infra/__init__.py b/support/testing/infra/__init__.py
index 6392aa679b..aaee055ee6 100644
--- a/support/testing/infra/__init__.py
+++ b/support/testing/infra/__init__.py
@@ -10,14 +10,23 @@ 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"""
+    if logtofile:
+        return "{}-{}.log".format(builddir, stage)
+    else:
+        return 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+')
+    logf = log_file_path(builddir, stage, logtofile)
+    if logf:
+        fhandle = open(logf, 'a+')
     else:
         fhandle = sys.stdout
     return fhandle
-- 
2.25.4

[Buildroot] [PATCH 10/11] support/testing/tests: add test for check_bin_arch

[Thomas Petazzoni]

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>
---
 .gitlab-ci.yml                                |  1 +
 .../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 +++++++++++++++++++
 7 files changed, 42 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/.gitlab-ci.yml b/.gitlab-ci.yml
index fa8e077a07..dd69fb9d50 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -354,6 +354,7 @@ tests.boot.test_syslinux.TestSysLinuxX86EFI: { extends: .runtime_test }
 tests.boot.test_syslinux.TestSysLinuxX86LegacyBios: { extends: .runtime_test }
 tests.boot.test_syslinux.TestSysLinuxX86_64EFI: { extends: .runtime_test }
 tests.boot.test_syslinux.TestSysLinuxX86_64LegacyBios: { extends: .runtime_test }
+tests.core.test_bad_arch.DetectBadArchTest: { extends: .runtime_test }
 tests.core.test_file_capabilities.TestFileCapabilities: { extends: .runtime_test }
 tests.core.test_hardening.TestFortifyConserv: { extends: .runtime_test }
 tests.core.test_hardening.TestFortifyNone: { extends: .runtime_test }
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..8f4bd57b0e
--- /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 "Advanced Micro Devices X86-64", should be "ARM"'
+            logf = open(logf_path, "r")
+            ret = subprocess.call(["grep", "-q", s], stdin=logf)
+            self.assertEqual(ret, 0)
-- 
2.25.4

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

[Thomas Petazzoni]

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 .gitlab-ci.yml                                |  2 +
 .../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 | 47 +++++++++++++++++++
 7 files changed, 76 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/.gitlab-ci.yml b/.gitlab-ci.yml
index dd69fb9d50..64266d8783 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -356,6 +356,8 @@ tests.boot.test_syslinux.TestSysLinuxX86_64EFI: { extends: .runtime_test }
 tests.boot.test_syslinux.TestSysLinuxX86_64LegacyBios: { extends: .runtime_test }
 tests.core.test_bad_arch.DetectBadArchTest: { extends: .runtime_test }
 tests.core.test_file_capabilities.TestFileCapabilities: { extends: .runtime_test }
+tests.core.test_file_overwrite.DetectHostFileOverwriteTest: { extends: .runtime_test }
+tests.core.test_file_overwrite.DetectTargetFileOverwriteTest: { extends: .runtime_test }
 tests.core.test_hardening.TestFortifyConserv: { extends: .runtime_test }
 tests.core.test_hardening.TestFortifyNone: { extends: .runtime_test }
 tests.core.test_hardening.TestRelro: { extends: .runtime_test }
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..526ed55e43
--- /dev/null
+++ b/support/testing/tests/core/test_file_overwrite.py
@@ -0,0 +1,47 @@
+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 = './etc/passwd: FAILED'
+            logf = open(logf_path, "r")
+            ret = subprocess.call(["grep", "-q", s], stdin=logf)
+            self.assertEqual(ret, 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/libpkgconf.pc: FAILED'
+            logf = open(logf_path, "r")
+            ret = subprocess.call(["grep", "-q", s], stdin=logf)
+            self.assertEqual(ret, 0)
-- 
2.25.4

[Buildroot] [PATCH 05/11] package/pkg-generic.mk: rework pkg_size logic with the "installed" step

[Yann E. MORIN]

On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
> This commits reworks the pkg_size logic to no longer use the
> GLOBAL_INSTRUMENTATION_HOOKS mechanism, but instead be directly
> implemented within the configure step and install step.
> 
> The problem with the current implementation in the
> GLOBAL_INSTRUMENTATION_HOOKS is that we only capture what is installed
> in $(HOST_DIR) during the "host installation step", what is installed
> in $(TARGET_DIR) during the "target installation step" and what is
> installed in "$(STAGING_DIR)" during the staging installation step.
> 
> While this seems reasonable in principle, it is in fact not completely
> true. For example, "toolchain", which is a target package, installs
> tons of files in $(HOST_DIR). "qt5base", which is also a target
> package, also installs things in $(HOST_DIR). Same with the "syslinux"
> package.
> 
> The idea behind this patch is pretty simple:
> 
>  - At the beginning of the configure step, right after the per-package
>    directories have been populated (if BR2_PER_PACKAGE_DIRECTORIES=y),
>    we capture the state of the HOST_DIR, TARGET_DIR and STAGING_DIR.
> 
>  - At the end of all install steps (which is possible thanks to the
>    newly introduced "install" step), we capture again the state of
>    HOST_DIR, TARGET_DIR and STAGING_DIR, and compare it to what we
>    have saved at the configure step.

I don't understand wy tht can't be achieved in the instrumentation hooks
mechanism:

    $(if $(filter start-configure,$(1)-$(2)),\
        $(call step_pkg_size_before,$(TARGET_DIR))\
        $(call step_pkg_size_before,$(STAGING_DIR),-staging)\
        $(call step_pkg_size_before,$(HOST_DIR),-host))

    $(if $(filter end-install,$(1)-$(2)),\
        $(call step_pkg_size_after,$(TARGET_DIR))\
        $(call step_pkg_size_after,$(STAGING_DIR),-staging)\
        $(call step_pkg_size_after,$(HOST_DIR),-host))

Of course, it means you have to add a call the instrumentation hooks
from the new 'install' step.

> So regardless of whether a file has been installed in $(HOST_DIR)
> during the target or staging installation steps of a target package,
> or if a host package has installed a file in $(TARGET_DIR), we will
> detect it.
> 
> The pkg_size_before and pkg_size_after macros are intentionally left
> where they are (even if they now fall in the middle of the
> GLOBAL_INSTRUMENTATION_HOOKS implementations) to minimize the diffstat
> and facilitate review.
> 
> Note that we also have to change check_bin_arch to be explicitly
> called from the install step rather than through a
> GLOBAL_INSTRUMENTATION_HOOKS as it depends on the .files-list.txt file
> produced by the pkg_size_after function.

And that would solve this as well.

Regards,
Yann E. MORIN.

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  package/pkg-generic.mk | 42 ++++++++++++++----------------------------
>  1 file changed, 14 insertions(+), 28 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index efdc32e355..3fb1e5f8c3 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -59,7 +59,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
>  
>  # $(1): base directory to search in
>  # $(2): suffix of file (optional)
> -define step_pkg_size_before
> +define pkg_size_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-list$(2).before
> @@ -67,7 +67,7 @@ endef
>  
>  # $(1): base directory to search in
>  # $(2): suffix of file (optional)
> -define step_pkg_size_after
> +define pkg_size_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-list$(2).after
> @@ -80,35 +80,14 @@ define step_pkg_size_after
>  	rm -f $($(PKG)_DIR)/.files-list$(2).after
>  endef
>  
> -define step_pkg_size
> -	$(if $(filter start-install-target,$(1)-$(2)),\
> -		$(call step_pkg_size_before,$(TARGET_DIR)))
> -	$(if $(filter start-install-staging,$(1)-$(2)),\
> -		$(call step_pkg_size_before,$(STAGING_DIR),-staging))
> -	$(if $(filter start-install-host,$(1)-$(2)),\
> -		$(call step_pkg_size_before,$(HOST_DIR),-host))
> -
> -	$(if $(filter end-install-target,$(1)-$(2)),\
> -		$(call step_pkg_size_after,$(TARGET_DIR)))
> -	$(if $(filter end-install-staging,$(1)-$(2)),\
> -		$(call step_pkg_size_after,$(STAGING_DIR),-staging))
> -	$(if $(filter end-install-host,$(1)-$(2)),\
> -		$(call step_pkg_size_after,$(HOST_DIR),-host))
> -endef
> -GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
> -
> -# Relies on step_pkg_size, so must be after
>  define check_bin_arch
> -	$(if $(filter end-install-target,$(1)-$(2)),\
> -		support/scripts/check-bin-arch -p $(3) \
> -			-l $($(PKG)_DIR)/.files-list.txt \
> -			$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
> -			-r $(TARGET_READELF) \
> -			-a $(BR2_READELF_ARCH_NAME))
> +	support/scripts/check-bin-arch -p $($(PKG)_NAME) \
> +		-l $($(PKG)_DIR)/.files-list.txt \
> +		$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
> +		-r $(TARGET_READELF) \
> +		-a $(BR2_READELF_ARCH_NAME)
>  endef
>  
> -GLOBAL_INSTRUMENTATION_HOOKS += check_bin_arch
> -
>  # This hook checks that host packages that need libraries that we build
>  # have a proper DT_RPATH or DT_RUNPATH tag
>  define check_host_rpath
> @@ -253,6 +232,9 @@ $(BUILD_DIR)/%/.stamp_configured:
>  	@$(call step_start,configure)
>  	@$(call MESSAGE,"Configuring")
>  	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
> +	@$(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),$(STAGING_DIR))
>  	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
>  	$($(PKG)_CONFIGURE_CMDS)
> @@ -374,6 +356,10 @@ $(BUILD_DIR)/%/.stamp_target_installed:
>  # Final installation step, completed when all installation steps
>  # (host, images, staging, target) have completed
>  $(BUILD_DIR)/%/.stamp_installed:
> +	@$(call pkg_size_after,$(TARGET_DIR))
> +	@$(call pkg_size_after,$(STAGING_DIR),-staging)
> +	@$(call pkg_size_after,$(HOST_DIR),-host)
> +	@$(call check_bin_arch)
>  	$(Q)touch $@
>  
>  # Remove package sources
> -- 
> 2.25.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 06/11] package/pkg-generic.mk: exclude the staging sub-directory

[Yann E. MORIN]

On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
> Now that we are checking the host directory changes throughout all
> installation steps and not just during the "host installation step",
> it means that changes done within the staging directory (which is a
> subdir of the host directory) are also visible in the
> .files-list-host.txt file.
> 
> Note that this problem already potentially occurs if a host package is
> installing files in the staging directory: they would be listed in
> .files-list-host.txt even without the changes in this series.
> 
> To fix this up, we simply exclude files that are beneath the
> $(STAGING_SUBDIR). Note that we do that in all cases, so when
> searching $(HOST_DIR), $(HOST_DIR)/$(STAGING_SUBDIR) is excluded, but
> when searching $(TARGET_DIR), $(TARGET_DIR)/$(STAGING_SUBDIR) is
> excluded, and when search $(STAGING_DIR),
> $(STAGING_DIR)/$(STAGING_SUBDIR) is excluded. This is not a problem in
> practice since $(TARGET_DIR)/$(STAGING_SUBDIR) and
> $(STAGING_DIR)/$(STAGING_SUBDIR) don't exist, but it's not very
> nice. However, it allows to keep the code simple.

I think this might be an error, as we would be missing the detection of
files installed in $(DESTDIR)/$(PREFIX) or such...

However, I think we can do a bit better, see below...

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  package/pkg-generic.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 3fb1e5f8c3..2ae269bb3d 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -61,7 +61,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
>  # $(2): suffix of file (optional)
>  define pkg_size_before
>  	cd $(1); \
> -	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> +	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
>  		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).before

Without the 'cd', and only ever ignoring staging:

    find $(1) -not -path '$(1)/$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%P\n' \

Notice the %P format specifier (upper-case 'P') instead of the previous
one (lower-case 'p').

Of course, totally untested (but would have the nice side-effect of
getting rid of the leadin ./ on all file names...

Regards,
Yann E. MORIN.

>  endef
>  
> @@ -69,7 +69,7 @@ endef
>  # $(2): suffix of file (optional)
>  define pkg_size_after
>  	cd $(1); \
> -	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> +	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
>  		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).after
>  	LC_ALL=C comm -13 \
>  		$($(PKG)_DIR)/.files-list$(2).before \
> -- 
> 2.25.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 07/11] package/pkg-generic.mk: move pkg_size_{before, after} and check_bin_arch functions

[Yann E. MORIN]

Thomas, All,

On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
> These functions are no longer using the GLOBAL_INSTRUMENTATION_HOOKS
> mechanism, so it doesn't make much sense for them to be in the section
> of pkg-generic.mk related to those hooks.

I think this would no longer be the case if you do as I suggested in
patch 5...

I'd like to keep the instrumentation, well, instrumentation...

Regards,
Yann E. MORIN.

> Move them to the "Helper functions" section.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  package/pkg-generic.mk | 66 +++++++++++++++++++++---------------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 2ae269bb3d..6e06d735ad 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -55,39 +55,6 @@ define step_time
>  endef
>  GLOBAL_INSTRUMENTATION_HOOKS += step_time
>  
> -# Hooks to collect statistics about installed files
> -
> -# $(1): base directory to search in
> -# $(2): suffix of file (optional)
> -define pkg_size_before
> -	cd $(1); \
> -	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> -		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).before
> -endef
> -
> -# $(1): base directory to search in
> -# $(2): suffix of file (optional)
> -define pkg_size_after
> -	cd $(1); \
> -	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> -		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).after
> -	LC_ALL=C comm -13 \
> -		$($(PKG)_DIR)/.files-list$(2).before \
> -		$($(PKG)_DIR)/.files-list$(2).after \
> -		| sed -r -e 's/^[^,]+/$($(PKG)_NAME)/' \
> -		> $($(PKG)_DIR)/.files-list$(2).txt
> -	rm -f $($(PKG)_DIR)/.files-list$(2).before
> -	rm -f $($(PKG)_DIR)/.files-list$(2).after
> -endef
> -
> -define check_bin_arch
> -	support/scripts/check-bin-arch -p $($(PKG)_NAME) \
> -		-l $($(PKG)_DIR)/.files-list.txt \
> -		$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
> -		-r $(TARGET_READELF) \
> -		-a $(BR2_READELF_ARCH_NAME)
> -endef
> -
>  # This hook checks that host packages that need libraries that we build
>  # have a proper DT_RPATH or DT_RUNPATH tag
>  define check_host_rpath
> @@ -135,6 +102,39 @@ define fixup-libtool-files
>  endef
>  endif
>  
> +# Functions to collect statistics about installed files
> +
> +# $(1): base directory to search in
> +# $(2): suffix of file (optional)
> +define pkg_size_before
> +	cd $(1); \
> +	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> +		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).before
> +endef
> +
> +# $(1): base directory to search in
> +# $(2): suffix of file (optional)
> +define pkg_size_after
> +	cd $(1); \
> +	LC_ALL=C find . -not -path './$(STAGING_SUBDIR)/*' \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> +		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(2).after
> +	LC_ALL=C comm -13 \
> +		$($(PKG)_DIR)/.files-list$(2).before \
> +		$($(PKG)_DIR)/.files-list$(2).after \
> +		| sed -r -e 's/^[^,]+/$($(PKG)_NAME)/' \
> +		> $($(PKG)_DIR)/.files-list$(2).txt
> +	rm -f $($(PKG)_DIR)/.files-list$(2).before
> +	rm -f $($(PKG)_DIR)/.files-list$(2).after
> +endef
> +
> +define check_bin_arch
> +	support/scripts/check-bin-arch -p $($(PKG)_NAME) \
> +		-l $($(PKG)_DIR)/.files-list.txt \
> +		$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
> +		-r $(TARGET_READELF) \
> +		-a $(BR2_READELF_ARCH_NAME)
> +endef
> +
>  ################################################################################
>  # Implicit targets -- produce a stamp file for each step of a package build
>  ################################################################################
> -- 
> 2.25.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 08/11] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR

[Yann E. MORIN]

Thomas, All,

On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
> With per-package directory support, it is absolutely critical that a
> given package does not overwrite files already installed by another
> package. However, 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.
> 
> Due to this calculation and verification having some overhead, it is
> only enabled when BR2_PER_PACKAGE_DIRECTORIES=y. Note that having
> BR2_PER_PACKAGE_DIRECTORIES=y also means that on average the HOST_DIR
> and TARGET_DIR contain less files than on non-per-package build: we
> only have in HOST_DIR and TARGET_DIR the dependencies of the current
> package being built. This helps a bit in mitigating the additional
> cost of this verification.

That paragraph mixes two things: the condition and the mitigation, but
misses explaining why we have that condition. What about:

    Due to this calculation and verification having some overhead, it is
    only enabled when BR2_PER_PACKAGE_DIRECTORIES=y. Indeed, without
    BR2_PER_PACKAGE_DIRECTORIES=y, the build order is guaranteed, and
    sequential, and thus files that are overwritten while always be so
    in order: adding to files, as well as replacing file will always be
    visible to subsequent packages. But with BR2_PER_PACKAGE_DIRECTORIES=y,
    this no longer works when we end up doing the final aggregation of
    target/ and staging/ and host/ : files will only have the content of
    the last dependency chain copied during the aggregation.

    BR2_PER_PACKAGE_DIRECTORIES=y also means that on average the
    HOST_DIR and TARGET_DIR contain less files [...]

> 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.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  package/pkg-generic.mk | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 6e06d735ad..82af4afaee 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -102,6 +102,27 @@ define fixup-libtool-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
> +	cd $(1); \
> +	LC_ALL=C find . -type f -exec md5sum {} \; > $($(PKG)_DIR)/.files$(2).md5
> +endef

We don;t care about having absolute or relative filenames stores in the
.md5 file, so:

    LC_ALL=C find $(1) -type f -exec md5sum {} \; > $($(PKG)_DIR)/.files$(2).md5

> +# $(1): base directory to search in
> +# $(2): suffix of file (optional)
> +define pkg_detect_overwrite_after
> +	cd $(1); \
> +	if test -s $($(PKG)_DIR)/.files$(2).md5 ; then \
> +		LC_ALL=C md5sum --quiet -c $($(PKG)_DIR)/.files$(2).md5 || \

Ditto.

> +		{ echo "ERROR: package $($(PKG)_NAME) has overwritten files installed by a previous package, aborting."; exit 1; } ; \

Do we absolutely want that to be a hard-error?

There are cases where modified files are not a problem. For example, the
gun info database is updated with each package that install info pages,
but we don;t care about that database, neither on target or staging, nor
on host.

Regards,
Yann E. MORIN.

> +	fi
> +endef
> +endif
> +
>  # Functions to collect statistics about installed files
>  
>  # $(1): base directory to search in
> @@ -235,6 +256,8 @@ $(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 pkg_detect_overwrite_before,$(TARGET_DIR))
> +	@$(call pkg_detect_overwrite_before,$(HOST_DIR),-host)
>  	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
>  	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
>  	$($(PKG)_CONFIGURE_CMDS)
> @@ -360,6 +383,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.25.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 05/11] package/pkg-generic.mk: rework pkg_size logic with the "installed" step

[Thomas Petazzoni]

Hello,

Thanks for taking the time to review this stuff!

On Fri, 1 May 2020 22:44:41 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> I don't understand wy tht can't be achieved in the instrumentation hooks
> mechanism:
> 
>     $(if $(filter start-configure,$(1)-$(2)),\
>         $(call step_pkg_size_before,$(TARGET_DIR))\
>         $(call step_pkg_size_before,$(STAGING_DIR),-staging)\
>         $(call step_pkg_size_before,$(HOST_DIR),-host))
> 
>     $(if $(filter end-install,$(1)-$(2)),\
>         $(call step_pkg_size_after,$(TARGET_DIR))\
>         $(call step_pkg_size_after,$(STAGING_DIR),-staging)\
>         $(call step_pkg_size_after,$(HOST_DIR),-host))
> 
> Of course, it means you have to add a call the instrumentation hooks
> from the new 'install' step.

This wouldn't work for the configure step. See the code:

$(BUILD_DIR)/%/.stamp_configured:
	@$(call step_start,configure)
	@$(call MESSAGE,"Configuring")
	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))

The hooks are called *before* we populate the per package directories.
But we need to capture the list of files *after* we populate the per
package directories.

In addition, I don't really understand why we use those global
instrumentation hooks within the generic infra itself. It is just a lot
clearer to me to make those function calls directly than to have this
indirection through hooks.

For me, it's like using hooks for the configure, build or install steps
of a generic package. For example, do we typically do:

ifeq ($(something),y)
define FOO_BUILD_THIS
	...
endef
endif

define FOO_BUILD_CMDS
	... do stuff ...
	$(FOO_BUILD_THIS)
endef

or this:

define FOO_BUILD_CMDS
	... do stuff ...
endef

ifeq ($(something),y)
define FOO_BUILD_THIS
	...
endef
FOO_POST_BUILD_HOOKS += FOO_BUILD_THIS
endef

We typically do the former, because there is no point in using hooks in
this situation. Hooks are useful to inject some logic to a step of
logic you don't have control over. When you have control over that step
of logic, hooks are kind of useless.

That being said, if you have a proposal to solve the ordering of things
in the configure step between the instrumentation hook and the
per-package directory population, I'm definitely fine with changing to
use instrumentation hooks.

Thanks!

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

[Buildroot] [PATCH 00/11] Overwritten file detection, improvements to file listing logic

[Yann E. MORIN]

Thomas, All,

On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
[--SNIP--]
> Thomas Petazzoni (11):
>   package/pkg-generic.mk: use $(PKG)_NAME in step_pkg_size_after
>   package/pkg-generic.mk: drop useless $(1) argument in
>     step_pkg_size_{before,after}
>   package/pkg-generic.mk: introduce final 'install' step
>   package/pkg-generic.mk: create directories upfront in the configure
>     step
>   package/pkg-generic.mk: rework pkg_size logic with the "installed"
>     step
>   package/pkg-generic.mk: exclude the staging sub-directory
>   package/pkg-generic.mk: move pkg_size_{before,after} and
>     check_bin_arch functions

Patches 1..7 applied to master, thanks.

I take back my comments on patch 5, you were right. As a consequence, I
also take back my comments on patch 7.

I take back my suggestion on patch 6: it does not work perfectly, and
the output changes slightly, so scritps that interpert the content would
have to be changed

>   package/pkg-generic.mk: detect files overwritten in TARGET_DIR and
>     HOST_DIR

For that one, I still have open questions about it, so I did not apply.

Thanks! :-)

Regards,
Yann E. MORIN.

>   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
> 
>  .gitlab-ci.yml                                |   3 +
>  package/pkg-generic.mk                        | 146 ++++++++++--------
>  package/pkg-utils.mk                          |   1 -
>  support/testing/infra/__init__.py             |  13 +-
>  .../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 |  47 ++++++
>  16 files changed, 213 insertions(+), 65 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.25.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 08/11] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR

[Yann E. MORIN]

Thomas, All,

On 2020-05-01 23:23 +0200, Yann E. MORIN spake thusly:
> On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
[--SNIP--]
> > +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> > +# $(1): base directory to search in
> > +# $(2): suffix of file (optional)
> > +define pkg_detect_overwrite_before
> > +	cd $(1); \
> > +	LC_ALL=C find . -type f -exec md5sum {} \; > $($(PKG)_DIR)/.files$(2).md5

Note that this is very slow: it spawns a new md5sum process for each
file it encounters.

There is a better solution, though:

    find $(1) -type f -print0 |xargs -0 md5sum > $($(PKG)_DIR)/.files$(2).md5

Your original code took 11s (the second time, with a hot VFS cache),
while my proposal got it down to 2s (again, hot VFS cache).

We could also try to parallelise the job:

    find $(1) -type f -print0 |xargs -P $(PARALLEL_JOBS) -0 md5sum > $($(PKG)_DIR)/.files$(2).md5

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 09/11] support/testing/infra: add log_file_path() function

[Yann E. MORIN]

Thomas, All,

Some pythonery below...

On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
> 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>
> ---
>  support/testing/infra/__init__.py | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/support/testing/infra/__init__.py b/support/testing/infra/__init__.py
> index 6392aa679b..aaee055ee6 100644
> --- a/support/testing/infra/__init__.py
> +++ b/support/testing/infra/__init__.py
> @@ -10,14 +10,23 @@ 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"""
> +    if logtofile:
> +        return "{}-{}.log".format(builddir, stage)
> +    else:
> +        return None

    def log_file_path(builddir, stage, logtofile=True):
        """
        blabla
        """
        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+')
> +    logf = log_file_path(builddir, stage, logtofile)
> +    if logf:
> +        fhandle = open(logf, 'a+')
>      else:
>          fhandle = sys.stdout
>      return fhandle

    def open_log_file(builddir, stage, logtofile=True):
        """
        blabla
        """
        return open(log_file_path(builddir, stage, logtofile), 'a+') if logtofile else sys.stdout

Regards,
Yann E. MORIN.

> -- 
> 2.25.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 08/11] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 > Thomas, All,
 > On 2020-05-01 23:23 +0200, Yann E. MORIN spake thusly:
 >> On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
 > [--SNIP--]
 >> > +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
 >> > +# $(1): base directory to search in
 >> > +# $(2): suffix of file (optional)
 >> > +define pkg_detect_overwrite_before
 >> > +	cd $(1); \
 >> > +	LC_ALL=C find . -type f -exec md5sum {} \; > $($(PKG)_DIR)/.files$(2).md5

 > Note that this is very slow: it spawns a new md5sum process for each
 > file it encounters.

 > There is a better solution, though:

 >     find $(1) -type f -print0 |xargs -0 md5sum > $($(PKG)_DIR)/.files$(2).md5

And perhaps add -r / --no-run-if-empty to xargs for good measure, even
if it is unlikely to be completely empty.

 > Your original code took 11s (the second time, with a hot VFS cache),
 > while my proposal got it down to 2s (again, hot VFS cache).

 > We could also try to parallelise the job:

 >     find $(1) -type f -print0 |xargs -P $(PARALLEL_JOBS) -0 md5sum > $($(PKG)_DIR)/.files$(2).md5

Will that not scramble the order of the hashes in the list?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 08/11] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR

[Yann E. MORIN]

Peter, All,

On 2020-07-25 07:58 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>  > Thomas, All,
>  > On 2020-05-01 23:23 +0200, Yann E. MORIN spake thusly:
>  >> On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
>  > [--SNIP--]
>  >> > +ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
>  >> > +# $(1): base directory to search in
>  >> > +# $(2): suffix of file (optional)
>  >> > +define pkg_detect_overwrite_before
>  >> > +	cd $(1); \
>  >> > +	LC_ALL=C find . -type f -exec md5sum {} \; > $($(PKG)_DIR)/.files$(2).md5
> 
>  > Note that this is very slow: it spawns a new md5sum process for each
>  > file it encounters.
> 
>  > There is a better solution, though:
> 
>  >     find $(1) -type f -print0 |xargs -0 md5sum > $($(PKG)_DIR)/.files$(2).md5
> 
> And perhaps add -r / --no-run-if-empty to xargs for good measure, even

Yes.

> if it is unlikely to be completely empty.

It would be for the skeleton and the host skeleton.

>  > Your original code took 11s (the second time, with a hot VFS cache),
>  > while my proposal got it down to 2s (again, hot VFS cache).
> 
>  > We could also try to parallelise the job:
> 
>  >     find $(1) -type f -print0 |xargs -P $(PARALLEL_JOBS) -0 md5sum > $($(PKG)_DIR)/.files$(2).md5
> 
> Will that not scramble the order of the hashes in the list?

But do we even care ? The order returned by find is not even guaranteed,
as this depends on the order of the entries in the directory anyway,
so...

Also, note that xargs will not run an md5sum process for each file, but
will coalesce multiple calls into a single one.

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 08/11] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 >> > Your original code took 11s (the second time, with a hot VFS cache),
 >> > while my proposal got it down to 2s (again, hot VFS cache).
 >> 
 >> > We could also try to parallelise the job:
 >> 
 >> >     find $(1) -type f -print0 |xargs -P $(PARALLEL_JOBS) -0 md5sum > $($(PKG)_DIR)/.files$(2).md5
 >> 
 >> Will that not scramble the order of the hashes in the list?

 > But do we even care ? The order returned by find is not even guaranteed,
 > as this depends on the order of the entries in the directory anyway,
 > so...

Ahh yes, I was thinking that we diffed the before/after files, but we
directly run md5sum with the before file, so it shouldn't matter.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 10/11] support/testing/tests: add test for check_bin_arch

[Yann E. MORIN]

On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
> 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>
> ---
[--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..8f4bd57b0e
> --- /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 "Advanced Micro Devices X86-64", should be "ARM"'
> +            logf = open(logf_path, "r")
> +            ret = subprocess.call(["grep", "-q", s], stdin=logf)
> +            self.assertEqual(ret, 0)

No need for a sub-process, we can do it quite efficiently in python:

        if logf_path:
            s = 'blabla'
            with open(logf_path, "r") as f:
                lines = [l for l in f.readlines() if l.startswith(s)]
            self.assertEqual(len(lines), 0)

Regards,
Yann E. MORIN.

> -- 
> 2.25.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 11/11] support/testing/tests: add test for file overwrite detection

[Yann E. MORIN]

Thomas, All,

On 2020-04-30 11:52 +0200, Thomas Petazzoni spake thusly:
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
[--SNIP--]
> 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..526ed55e43
> --- /dev/null
> +++ b/support/testing/tests/core/test_file_overwrite.py
> @@ -0,0 +1,47 @@
> +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 = './etc/passwd: FAILED'
> +            logf = open(logf_path, "r")
> +            ret = subprocess.call(["grep", "-q", s], stdin=logf)
> +            self.assertEqual(ret, 0)

This does not need a subprocess, and can be done efficiently in python,
like suggested in my review of patch 10.

Note thatif the search pattern does not occur at the beginning of the
line, we can do:

    s = 'blabla'
    for open(log_path, "r") as f:
        lines = [l for l in f.readlines() if s in l]

Ditto for the other test, of course. ;-)

Regards,
Yann E. MORIN.

> +
> +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/libpkgconf.pc: FAILED'
> +            logf = open(logf_path, "r")
> +            ret = subprocess.call(["grep", "-q", s], stdin=logf)
> +            self.assertEqual(ret, 0)


> -- 
> 2.25.4
> 

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