Patchwork post-build: reimplement using a for loop and make verbose

login
register
mail settings
Submitter Luca Ceresoli
Date April 3, 2013, 8:28 a.m.
Message ID <1364977729-27519-1-git-send-email-luca@lucaceresoli.net>
Download mbox | patch
Permalink /patch/233307/
State Superseded
Headers show

Comments

Luca Ceresoli - April 3, 2013, 8:28 a.m.
Make the post-build system similar to the rootfs overlay system. This allows to
show each script filename before execution.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 Makefile |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
Thomas Petazzoni - April 3, 2013, 8:32 a.m.
Dear Luca Ceresoli,

On Wed,  3 Apr 2013 10:28:48 +0200, Luca Ceresoli wrote:
> Make the post-build system similar to the rootfs overlay system. This allows to
> show each script filename before execution.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

I don't have a strong opinion on whether this is useful or not, but if
it gets applied, I'd like the same to be done for post-image scripts,
in order to keep the consistency.

Thanks!

Thomas
Danomi Manchego - April 4, 2013, 1:35 a.m.
Alternatively, you could just move the MESSAGE into the loop instead
of relying on shell loops.  That would keep things more in make, if
that's desired.

	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
		$(call MESSAGE,"Executing $(s)"); \
		$(s) $(TARGET_DIR)$(sep))

Danomi -
Luca Ceresoli - April 7, 2013, 1:58 p.m.
Thomas Petazzoni wrote:
> Dear Luca Ceresoli,
>
> On Wed,  3 Apr 2013 10:28:48 +0200, Luca Ceresoli wrote:
>> Make the post-build system similar to the rootfs overlay system. This allows to
>> show each script filename before execution.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> I don't have a strong opinion on whether this is useful or not, but if
> it gets applied, I'd like the same to be done for post-image scripts,
> in order to keep the consistency.

To me it is useful each time multiple post-build scripts are used, 
because is
makes it easy to understand what's happening and which scripts you are 
running
(and which you are not running).
E.g., I have multiple products with a common base, plus some "optional" 
features.
Some features needs a specific post-build script. Each product 
implements a subset
of the features, but these subsets overlap between different products.
When a build a product's rootfs it is useful to see if I am running all 
and only
the correct post-build scripts: the "common base" script, plus the 
scripts for the
wanted features.

Of course I agree this change should be done also in the post-image scripts.
I will submit a patchset that does also this.

Luca
Luca Ceresoli - April 8, 2013, 4:47 p.m.
Danomi Manchego wrote:
> Alternatively, you could just move the MESSAGE into the loop instead
> of relying on shell loops.  That would keep things more in make, if
> that's desired.
>
> 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
> 		$(call MESSAGE,"Executing $(s)"); \
> 		$(s) $(TARGET_DIR)$(sep))

It is actually cleanerand should also be more efficient(FWIW).

Sending a v2 patchset with this implementation.

Luca

Patch

diff --git a/Makefile b/Makefile
index c95bb1e..69996bc 100644
--- a/Makefile
+++ b/Makefile
@@ -509,11 +509,10 @@  endif
 			$${dir}/ $(TARGET_DIR); \
 	done
 
-ifneq ($(BR2_ROOTFS_POST_BUILD_SCRIPT),"")
-	@$(call MESSAGE,"Executing post-build script\(s\)")
-	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
-		$(s) $(TARGET_DIR)$(sep))
-endif
+	@for scr in $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)); do \
+		$(call MESSAGE,"Executing post-build script $${scr}"); \
+		$${scr} $(TARGET_DIR); \
+	done
 
 ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
 LOCALE_WHITELIST=$(BUILD_DIR)/locales.nopurge