[Buildroot] [PATCH 1/3] Makefile: don't hang the build if there are no file lists

[Buildroot] [PATCH 1/3] Makefile: don't hang the build if there are no file lists

[Thomas De Schampheleire]

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

In very limited configurations, it is possible to have a case where no
.files-list-staging.txt files are created. In this case:

	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.txt)) > \
		$(BUILD_DIR)/packages-file-list-staging.txt

becomes:

	cat > \
		$(BUILD_DIR)/packages-file-list-staging.txt

which of course makes the build hang.. forever.

So we fix this by checking the list is not empty. To keep the code
readable, we introduce an intermediate variable to store the list of
these files.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 Makefile | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 5455e6662e..29d30a4f70 100644
--- a/Makefile
+++ b/Makefile
@@ -728,6 +728,10 @@ $(TARGETS_ROOTFS): target-finalize
 # Avoid the rootfs name leaking down the dependency chain
 target-finalize: ROOTFS=
 
+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))
+
 .PHONY: host-finalize
 host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
 	@$(call MESSAGE,"Finalizing host directory")
@@ -808,12 +812,12 @@ endif # merged /usr
 
 	touch $(TARGET_DIR)/usr
 
-	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list.txt)) > \
-		$(BUILD_DIR)/packages-file-list.txt
-	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-host.txt)) > \
-		$(BUILD_DIR)/packages-file-list-host.txt
-	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.txt)) > \
-		$(BUILD_DIR)/packages-file-list-staging.txt
+	$(if $(TARGET_DIR_FILES_LISTS), \
+		cat $(TARGET_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list.txt)
+	$(if $(HOST_DIR_FILES_LISTS), \
+		cat $(HOST_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list-host.txt)
+	$(if $(STAGING_DIR_FILES_LISTS), \
+		cat $(STAGING_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list-staging.txt)
 
 .PHONY: target-post-image
 target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
-- 
2.24.1

[Buildroot] [PATCH 2/3] Makefile: fix package file list if FOO_SUBDIR is set

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

If a package sets a FOO_SUBDIR (meaning its sources are not under
output/build/foo-123 but under output/build/foo-123/$(FOO_SUBDIR)), the
.files-list.txt file were also created under FOO_SUBDIR, due to which the
logic in the Makefile would not find it.

Change the instrumentation steps so that the file list is directly under the
package dir, ignoring the subdir.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/pkg-generic.mk | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 47238f2649..20b07a7fa9 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -63,7 +63,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
 define step_pkg_size_before
 	cd $(2); \
 	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
-		| LC_ALL=C sort > $($(PKG)_BUILDDIR)/.files-list$(3).before
+		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(3).before
 endef
 
 # $(1): package name
@@ -72,14 +72,14 @@ endef
 define step_pkg_size_after
 	cd $(2); \
 	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
-		| LC_ALL=C sort > $($(PKG)_BUILDDIR)/.files-list$(3).after
+		| LC_ALL=C sort > $($(PKG)_DIR)/.files-list$(3).after
 	LC_ALL=C comm -13 \
-		$($(PKG)_BUILDDIR)/.files-list$(3).before \
-		$($(PKG)_BUILDDIR)/.files-list$(3).after \
+		$($(PKG)_DIR)/.files-list$(3).before \
+		$($(PKG)_DIR)/.files-list$(3).after \
 		| sed -r -e 's/^[^,]+/$(1)/' \
-		> $($(PKG)_BUILDDIR)/.files-list$(3).txt
-	rm -f $($(PKG)_BUILDDIR)/.files-list$(3).before
-	rm -f $($(PKG)_BUILDDIR)/.files-list$(3).after
+		> $($(PKG)_DIR)/.files-list$(3).txt
+	rm -f $($(PKG)_DIR)/.files-list$(3).before
+	rm -f $($(PKG)_DIR)/.files-list$(3).after
 endef
 
 define step_pkg_size
@@ -103,7 +103,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_pkg_size
 define check_bin_arch
 	$(if $(filter end-install-target,$(1)-$(2)),\
 		support/scripts/check-bin-arch -p $(3) \
-			-l $($(PKG)_BUILDDIR)/.files-list.txt \
+			-l $($(PKG)_DIR)/.files-list.txt \
 			$(foreach i,$($(PKG)_BIN_ARCH_EXCLUDE),-i "$(i)") \
 			-r $(TARGET_READELF) \
 			-a $(BR2_READELF_ARCH_NAME))
-- 
2.24.1

[Buildroot] [PATCH 3/3] Makefile: assemble package file lists before calling post-build scripts

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Post-build scripts may want to do something based on the list of files
installed by a package. However, since commit
0e2be4db8ab01d479177a3a187c22525752195ae the final packages-file-lists.txt
file is only created _after_ the post-build scripts.

Move the assembly of the file lists upwards, before the post-build scripts.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 29d30a4f70..1280066bab 100644
--- a/Makefile
+++ b/Makefile
@@ -806,12 +806,6 @@ endif # merged /usr
 		$(call MESSAGE,"Copying overlay $(d)"); \
 		$(call SYSTEM_RSYNC,$(d),$(TARGET_DIR))$(sep))
 
-	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
-		$(call MESSAGE,"Executing post-build script $(s)"); \
-		$(EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
-
-	touch $(TARGET_DIR)/usr
-
 	$(if $(TARGET_DIR_FILES_LISTS), \
 		cat $(TARGET_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list.txt)
 	$(if $(HOST_DIR_FILES_LISTS), \
@@ -819,6 +813,12 @@ endif # merged /usr
 	$(if $(STAGING_DIR_FILES_LISTS), \
 		cat $(STAGING_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list-staging.txt)
 
+	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
+		$(call MESSAGE,"Executing post-build script $(s)"); \
+		$(EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
+
+	touch $(TARGET_DIR)/usr
+
 .PHONY: target-post-image
 target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
 	@rm -f $(ROOTFS_COMMON_TAR)
-- 
2.24.1

[Buildroot] [PATCH 1/3] Makefile: don't hang the build if there are no file lists

[Yann E. MORIN]

Thomas?, All,

On 2020-03-18 16:58 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> 
> In very limited configurations, it is possible to have a case where no
> .files-list-staging.txt files are created. In this case:
> 
> 	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.txt)) > \
> 		$(BUILD_DIR)/packages-file-list-staging.txt
> 
> becomes:
> 
> 	cat > \
> 		$(BUILD_DIR)/packages-file-list-staging.txt
> 
> which of course makes the build hang.. forever.
> 
> So we fix this by checking the list is not empty. To keep the code
> readable, we introduce an intermediate variable to store the list of
> these files.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  Makefile | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 5455e6662e..29d30a4f70 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -728,6 +728,10 @@ $(TARGETS_ROOTFS): target-finalize
>  # Avoid the rootfs name leaking down the dependency chain
>  target-finalize: ROOTFS=
>  
> +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))
> +
>  .PHONY: host-finalize
>  host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
>  	@$(call MESSAGE,"Finalizing host directory")
> @@ -808,12 +812,12 @@ endif # merged /usr
>  
>  	touch $(TARGET_DIR)/usr
>  
> -	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list.txt)) > \
> -		$(BUILD_DIR)/packages-file-list.txt
> -	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-host.txt)) > \
> -		$(BUILD_DIR)/packages-file-list-host.txt
> -	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.txt)) > \
> -		$(BUILD_DIR)/packages-file-list-staging.txt
> +	$(if $(TARGET_DIR_FILES_LISTS), \
> +		cat $(TARGET_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list.txt)
> +	$(if $(HOST_DIR_FILES_LISTS), \
> +		cat $(HOST_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list-host.txt)
> +	$(if $(STAGING_DIR_FILES_LISTS), \
> +		cat $(STAGING_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list-staging.txt)

So, if there is no file instaleld in staging, the packages-file-list-staging.txt
file is not created. However, in followup patches, you make it (as well
as the other two) available to post-build scripts.

This is not nce, as the scripts will have to be carefull to test if the
files exist.

I would like to suggest an alternative, that guarantees the files are
created, even if empty:

    $(if $(STAGING_DIR_FILES_LISTS), \
        cat $(STAGING_DIR_FILES_LISTS)) >$(BUILD_DIR)/packages-file-list-staging.txt

Notice how the redirection is outside the conditional, so that if there
is no file, the commadn will be:

    > $(BUILD_DIR)/packages-file-list-staging.txt

Which will create an empty file.

Thoughts? Shall I do that when applying the series or will you want to
respin?

Regards,
Yann E. MORIN.

>  
>  .PHONY: target-post-image
>  target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
> -- 
> 2.24.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 1/3] Makefile: don't hang the build if there are no file lists

[Thomas Petazzoni]

On Wed, 18 Mar 2020 22:49:08 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> I would like to suggest an alternative, that guarantees the files are
> created, even if empty:
> 
>     $(if $(STAGING_DIR_FILES_LISTS), \
>         cat $(STAGING_DIR_FILES_LISTS)) >$(BUILD_DIR)/packages-file-list-staging.txt
> 
> Notice how the redirection is outside the conditional, so that if there
> is no file, the commadn will be:
> 
>     > $(BUILD_DIR)/packages-file-list-staging.txt  
> 
> Which will create an empty file.
> 
> Thoughts? Shall I do that when applying the series or will you want to
> respin?

What you suggest sounds good to me. Feel free to tweak that when
applying. Thanks!

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

[Buildroot] [PATCH 1/3] Makefile: don't hang the build if there are no file lists

[Yann E. MORIN]

Thomas ?2, All,

On 2020-03-18 16:58 +0100, Thomas De Schampheleire spake thusly:
> From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> 
> In very limited configurations, it is possible to have a case where no
> .files-list-staging.txt files are created. In this case:
> 
> 	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.txt)) > \
> 		$(BUILD_DIR)/packages-file-list-staging.txt
> 
> becomes:
> 
> 	cat > \
> 		$(BUILD_DIR)/packages-file-list-staging.txt
> 
> which of course makes the build hang.. forever.
> 
> So we fix this by checking the list is not empty. To keep the code
> readable, we introduce an intermediate variable to store the list of
> these files.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Series applied to master, after fixing this patch as previously
discussed in the thread. Thanks.

Regards,
Yann E. MORIN.

> ---
>  Makefile | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 5455e6662e..29d30a4f70 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -728,6 +728,10 @@ $(TARGETS_ROOTFS): target-finalize
>  # Avoid the rootfs name leaking down the dependency chain
>  target-finalize: ROOTFS=
>  
> +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))
> +
>  .PHONY: host-finalize
>  host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
>  	@$(call MESSAGE,"Finalizing host directory")
> @@ -808,12 +812,12 @@ endif # merged /usr
>  
>  	touch $(TARGET_DIR)/usr
>  
> -	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list.txt)) > \
> -		$(BUILD_DIR)/packages-file-list.txt
> -	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-host.txt)) > \
> -		$(BUILD_DIR)/packages-file-list-host.txt
> -	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.txt)) > \
> -		$(BUILD_DIR)/packages-file-list-staging.txt
> +	$(if $(TARGET_DIR_FILES_LISTS), \
> +		cat $(TARGET_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list.txt)
> +	$(if $(HOST_DIR_FILES_LISTS), \
> +		cat $(HOST_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list-host.txt)
> +	$(if $(STAGING_DIR_FILES_LISTS), \
> +		cat $(STAGING_DIR_FILES_LISTS) > $(BUILD_DIR)/packages-file-list-staging.txt)
>  
>  .PHONY: target-post-image
>  target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
> -- 
> 2.24.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 1/3] Makefile: don't hang the build if there are no file lists

[Peter Korsgaard]

>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

 > From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > In very limited configurations, it is possible to have a case where no
 > .files-list-staging.txt files are created. In this case:

 > 	cat $(sort $(wildcard $(BUILD_DIR)/*/.files-list-staging.txt)) > \
 > 		$(BUILD_DIR)/packages-file-list-staging.txt

 > becomes:

 > 	cat > \
 > 		$(BUILD_DIR)/packages-file-list-staging.txt

 > which of course makes the build hang.. forever.

 > So we fix this by checking the list is not empty. To keep the code
 > readable, we introduce an intermediate variable to store the list of
 > these files.

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Committed to 2020.02.x, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 2/3] Makefile: fix package file list if FOO_SUBDIR is set

[Peter Korsgaard]

>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

 > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
 > If a package sets a FOO_SUBDIR (meaning its sources are not under
 > output/build/foo-123 but under output/build/foo-123/$(FOO_SUBDIR)), the
 > .files-list.txt file were also created under FOO_SUBDIR, due to which the
 > logic in the Makefile would not find it.

 > Change the instrumentation steps so that the file list is directly under the
 > package dir, ignoring the subdir.

 > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Committed to 2020.02.x, thanks.

This is not applicable to the older releases, right?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 3/3] Makefile: assemble package file lists before calling post-build scripts

[Peter Korsgaard]

>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

 > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
 > Post-build scripts may want to do something based on the list of files
 > installed by a package. However, since commit
 > 0e2be4db8ab01d479177a3a187c22525752195ae the final packages-file-lists.txt
 > file is only created _after_ the post-build scripts.

 > Move the assembly of the file lists upwards, before the post-build scripts.

 > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Committed to 2020.02.x, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 2/3] Makefile: fix package file list if FOO_SUBDIR is set

[Thomas De Schampheleire]

On Sat, Mar 28, 2020, 08:39 Peter Korsgaard <peter@korsgaard.com> wrote:

> >>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com>
> writes:
>
>  > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>  > If a package sets a FOO_SUBDIR (meaning its sources are not under
>  > output/build/foo-123 but under output/build/foo-123/$(FOO_SUBDIR)), the
>  > .files-list.txt file were also created under FOO_SUBDIR, due to which
> the
>  > logic in the Makefile would not find it.
>
>  > Change the instrumentation steps so that the file list is directly
> under the
>  > package dir, ignoring the subdir.
>
>  > Signed-off-by: Thomas De Schampheleire <
> thomas.de_schampheleire at nokia.com>
>
> Committed to 2020.02.x, thanks.
>
> This is not applicable to the older releases, right?
>

No indeed, I think this code that handles the file lists differently was
new in 2020.02.

Thanks,
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20200328/44b4c0f9/attachment.html>

[Buildroot] [PATCH 2/3] Makefile: fix package file list if FOO_SUBDIR is set

[Peter Korsgaard]

>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

Hi,

 >> Committed to 2020.02.x, thanks.
 >> 
 >> This is not applicable to the older releases, right?
 >> 

 > No indeed, I think this code that handles the file lists differently was
 > new in 2020.02.

Ok, thanks!

-- 
Bye, Peter Korsgaard