Makefile: don't hang the build if there are no file lists

PATCH New Thomas De Schampheleire 4 participants 11 messages Mar 18, 2020 – Mar 28, 2020 Kbuild

lore

Share & tools

lore

Thomas De Schampheleire

author

2020-03-18 15:58 UTC #0 lore raw

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

From: Thomas Petazzoni <[email protected]>

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 <[email protected]>

Signed-off-by Thomas De Schampheleire <[email protected]>

---
 Makefile | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Makefile +10 -6

index 5455e6662e..29d30a4f70 100644
@@ -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

Signature · 1 line

2.24.1

Thomas De Schampheleire

author

2020-03-18 15:58 UTC #1 lore raw

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

From: Thomas De Schampheleire <[email protected]>

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 <[email protected]>

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

package/pkg-generic.mk +8 -8

index 47238f2649..20b07a7fa9 100644
@@ -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))

Signature · 1 line

2.24.1

Peter Korsgaard

2020-03-28 07:39 UTC #2 lore raw

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

Quoted 8 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas De Schampheleire <[email protected]> writes:
From: Thomas De Schampheleire <[email protected]> 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.

Quoted 4 lines

Change the instrumentation steps so that the file list is directly under the package dir, ignoring the subdir.
Signed-off-by Thomas De Schampheleire <[email protected]>

Committed to 2020.02.x, thanks.

This is not applicable to the older releases, right?

Signature · 1 line

Bye, Peter Korsgaard

Thomas De Schampheleire

author

2020-03-28 08:58 UTC #3 lore raw

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

Quoted 12 lines · On Sat, Mar 28, 2020, 08:39 Peter Korsgaard <[email protected]> wrote:

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas De Schampheleire <[email protected]>
Quoted 4 lines · writes:
From: Thomas De Schampheleire <[email protected]> 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
Quoted 3 lines
logic in the Makefile would not find it.
Change the instrumentation steps so that the file list is directly
under the

Quoted 9 lines

Quoted 3 lines
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>

Peter Korsgaard

2020-03-28 11:06 UTC #4 lore raw

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

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas De Schampheleire <[email protected]> writes:

Hi,

Quoted 7 lines

Quoted 4 lines
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!

Signature · 1 line

Bye, Peter Korsgaard

Thomas De Schampheleire

author

2020-03-18 15:58 UTC #5 lore raw

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

In reply to Thomas De Schampheleire

From: Thomas De Schampheleire <[email protected]>

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 <[email protected]>

---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Makefile +6 -6

index 29d30a4f70..1280066bab 100644
@@ -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)

Signature · 1 line

2.24.1

Peter Korsgaard

2020-03-28 07:39 UTC #6 lore raw

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

Quoted 11 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas De Schampheleire <[email protected]> writes:
From: Thomas De Schampheleire <[email protected]> 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 <[email protected]>

Committed to 2020.02.x, thanks.

Signature · 1 line

Bye, Peter Korsgaard

Yann E. MORIN

2020-03-18 21:49 UTC #7 lore raw

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

In reply to Thomas De Schampheleire

Thomas?, All,

On 2020-03-18 16:58 +0100, Thomas De Schampheleire spake thusly:

Quoted 46 lines

From: Thomas Petazzoni <[email protected]>
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 <[email protected]>
Signed-off-by Thomas De Schampheleire <[email protected]>
---
 Makefile | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
Makefile +4 -2
index 5455e6662e..29d30a4f70 100644
@@ -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

Makefile endif # merged /usr · line 820

Quoted 10 lines

Makefile +6 -4

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

Quoted 1 line

$(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.

Quoted 10 lines

.PHONY: target-post-image
target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize
Signature · 6 lines
2.24.1

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines

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

Thomas Petazzoni

2020-03-20 21:10 UTC #8 lore raw

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

Quoted 9 lines · On Wed, 18 Mar 2020 22:49:08 +0100 "Yann E. MORIN" <[email protected]>…

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:

Quoted 6 lines

Quoted 1 line
$(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

Signature · 3 lines

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

Yann E. MORIN

2020-03-20 21:29 UTC #9 lore raw

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

In reply to Thomas De Schampheleire

Thomas ?2, All,

On 2020-03-18 16:58 +0100, Thomas De Schampheleire spake thusly:

Quoted 13 lines

From: Thomas Petazzoni <[email protected]>
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

Quoted 8 lines

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 <[email protected]>
Signed-off-by Thomas De Schampheleire <[email protected]>

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

Regards,
Yann E. MORIN.

Quoted 45 lines

---
 Makefile | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Makefile +10 -6

index 5455e6662e..29d30a4f70 100644
@@ -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

Signature · 6 lines

2.24.1

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines

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

Peter Korsgaard

2020-03-28 07:37 UTC #10 lore raw

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

In reply to Thomas De Schampheleire

Quoted 14 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas De Schampheleire <[email protected]> writes:
From: Thomas Petazzoni <[email protected]>
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

Quoted 8 lines

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 <[email protected]>
Signed-off-by Thomas De Schampheleire <[email protected]>

Committed to 2020.02.x, thanks.

Signature · 1 line

Bye, Peter Korsgaard