[Buildroot] [PATCH] fs: add missing $$(sep) to pre- and post-command hooks code

[Buildroot] [PATCH] fs: add missing $$(sep) to pre- and post-command hooks code

[Andrey Smirnov]

When multiple hooks are registred, both pre-a and post-command hooks'
foreach loops need to have a separator at the end in order for the
code to work as intended. Without the separator all hooks end up as a
one single line command thus making all but the first hook into
no-ops.

Fixes: 4628b6f3b4 ("fs: add pre- and post-command hooks")
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Romain Naour <romain.naour@gmail.com>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/common.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/common.mk b/fs/common.mk
index 9a7758ff49..5b612a3f41 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -96,13 +96,13 @@ endif
 		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
 		echo $$(s) $$(TARGET_DIR) $$(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $$(FAKEROOT_SCRIPT)$$(sep))
 	$$(foreach hook,$$(ROOTFS_PRE_CMD_HOOKS),\
-		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT))
+		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
 ifeq ($$(BR2_REPRODUCIBLE),y)
 	echo "find $$(TARGET_DIR) -print0 | xargs -0 -r touch -hd @$$(SOURCE_DATE_EPOCH)" >> $$(FAKEROOT_SCRIPT)
 endif
 	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
 	$$(foreach hook,$$(ROOTFS_POST_CMD_HOOKS),\
-		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT))
+		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
 	chmod a+x $$(FAKEROOT_SCRIPT)
 	PATH=$$(BR_PATH) $$(HOST_DIR)/bin/fakeroot -- $$(FAKEROOT_SCRIPT)
 	$$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
-- 
2.13.6

[Buildroot] [PATCH] fs: add missing $$(sep) to pre- and post-command hooks code

[Yann E. MORIN]

Andrey, All,

On 2017-11-03 11:49 -0700, Andrey Smirnov spake thusly:
> When multiple hooks are registred, both pre-a and post-command hooks'
> foreach loops need to have a separator at the end in order for the
> code to work as intended. Without the separator all hooks end up as a
> one single line command thus making all but the first hook into
> no-ops.

I had a hard time figuring out why they were ignored (and I was the one
writing that). And it is pretty trivial: each hook is printed with
PRINTF, which expands the hook in single quote, and there is no format
string at all.

So two hooks get expanded into (all on one line):
    printf 'something expnded from first hook\n' print 'something else'

and so on for the third and subsquent hooks. This should have been part
of the commit message, I guess...

> Fixes: 4628b6f3b4 ("fs: add pre- and post-command hooks")
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Romain Naour <romain.naour@gmail.com>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Yet, could you explain the use-case where you need other such hooks?

Regards,
Yann E. MORIN.

> ---
>  fs/common.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/common.mk b/fs/common.mk
> index 9a7758ff49..5b612a3f41 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -96,13 +96,13 @@ endif
>  		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
>  		echo $$(s) $$(TARGET_DIR) $$(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $$(FAKEROOT_SCRIPT)$$(sep))
>  	$$(foreach hook,$$(ROOTFS_PRE_CMD_HOOKS),\
> -		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT))
> +		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
>  ifeq ($$(BR2_REPRODUCIBLE),y)
>  	echo "find $$(TARGET_DIR) -print0 | xargs -0 -r touch -hd @$$(SOURCE_DATE_EPOCH)" >> $$(FAKEROOT_SCRIPT)
>  endif
>  	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
>  	$$(foreach hook,$$(ROOTFS_POST_CMD_HOOKS),\
> -		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT))
> +		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
>  	chmod a+x $$(FAKEROOT_SCRIPT)
>  	PATH=$$(BR_PATH) $$(HOST_DIR)/bin/fakeroot -- $$(FAKEROOT_SCRIPT)
>  	$$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
> -- 
> 2.13.6
> 

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

[Buildroot] [PATCH] fs: add missing $$(sep) to pre- and post-command hooks code

[Arnout Vandecappelle]

On 03-11-17 19:49, Andrey Smirnov wrote:
> When multiple hooks are registred, both pre-a and post-command hooks'
> foreach loops need to have a separator at the end in order for the
> code to work as intended. Without the separator all hooks end up as a
> one single line command thus making all but the first hook into
> no-ops.
> 
> Fixes: 4628b6f3b4 ("fs: add pre- and post-command hooks")
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Romain Naour <romain.naour@gmail.com>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

 Applied to master, thanks.

 Regards,
 Arnout

> ---
>  fs/common.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/common.mk b/fs/common.mk
> index 9a7758ff49..5b612a3f41 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -96,13 +96,13 @@ endif
>  		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
>  		echo $$(s) $$(TARGET_DIR) $$(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $$(FAKEROOT_SCRIPT)$$(sep))
>  	$$(foreach hook,$$(ROOTFS_PRE_CMD_HOOKS),\
> -		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT))
> +		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
>  ifeq ($$(BR2_REPRODUCIBLE),y)
>  	echo "find $$(TARGET_DIR) -print0 | xargs -0 -r touch -hd @$$(SOURCE_DATE_EPOCH)" >> $$(FAKEROOT_SCRIPT)
>  endif
>  	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
>  	$$(foreach hook,$$(ROOTFS_POST_CMD_HOOKS),\
> -		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT))
> +		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
>  	chmod a+x $$(FAKEROOT_SCRIPT)
>  	PATH=$$(BR_PATH) $$(HOST_DIR)/bin/fakeroot -- $$(FAKEROOT_SCRIPT)
>  	$$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] fs: add missing $$(sep) to pre- and post-command hooks code

[Andrey Smirnov]

On Fri, Nov 3, 2017 at 3:08 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Andrey, All,
>
> On 2017-11-03 11:49 -0700, Andrey Smirnov spake thusly:
>> When multiple hooks are registred, both pre-a and post-command hooks'
>> foreach loops need to have a separator at the end in order for the
>> code to work as intended. Without the separator all hooks end up as a
>> one single line command thus making all but the first hook into
>> no-ops.
>
> I had a hard time figuring out why they were ignored (and I was the one
> writing that). And it is pretty trivial: each hook is printed with
> PRINTF, which expands the hook in single quote, and there is no format
> string at all.
>
> So two hooks get expanded into (all on one line):
>     printf 'something expnded from first hook\n' print 'something else'
>
> and so on for the third and subsquent hooks. This should have been part
> of the commit message, I guess...
>
>> Fixes: 4628b6f3b4 ("fs: add pre- and post-command hooks")
>> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
>> Cc: Romain Naour <romain.naour@gmail.com>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>
> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Yet, could you explain the use-case where you need other such hooks?
>

Can't say I necessarily have a use-case for it, but I was
experimenting with overriding certain aspects of
skeleton-init-systemd's behavior using custom hooks and realized that
none of them were working. Hence this patch.

Thanks,
Andrey Smirnov

[Buildroot] [PATCH] fs: add missing $$(sep) to pre- and post-command hooks code

[Peter Korsgaard]

>>>>> "Andrey" == Andrey Smirnov <andrew.smirnov@gmail.com> writes:

 > When multiple hooks are registred, both pre-a and post-command hooks'
 > foreach loops need to have a separator at the end in order for the
 > code to work as intended. Without the separator all hooks end up as a
 > one single line command thus making all but the first hook into
 > no-ops.

 > Fixes: 4628b6f3b4 ("fs: add pre- and post-command hooks")
 > Cc: Yann E. MORIN <yann.morin.1998@free.fr>
 > Cc: Romain Naour <romain.naour@gmail.com>
 > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Committed to 2017.08.x, thanks.

-- 
Bye, Peter Korsgaard