Patchwork Run post scripts referenced by relative paths

login
register
mail settings
Submitter Waldemar Rymarkiewicz
Date Jan. 28, 2014, 10:14 a.m.
Message ID <1390904090-27934-1-git-send-email-waldemar.rymarkiewicz@gmail.com>
Download mbox | patch
Permalink /patch/314665/
State Rejected
Headers show

Comments

Waldemar Rymarkiewicz - Jan. 28, 2014, 10:14 a.m.
This fix will let to run post scripts seamlessly while are referenced by
relative paths. Scripts referenced by full path still works.

Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@gmail.com>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Maxime Hadjinlian - Jan. 28, 2014, 10:37 a.m.
Hi Waldemar, all

On Tue, Jan 28, 2014 at 11:14 AM, Waldemar Rymarkiewicz
<waldemar.rymarkiewicz@gmail.com> wrote:
> This fix will let to run post scripts seamlessly while are referenced by
> relative paths. Scripts referenced by full path still works.
>
> Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@gmail.com>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 067458b..ef7f4c2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -554,7 +554,7 @@ endif
>
>         @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
>                 $(call MESSAGE,"Executing post-build script $(s)"); \
> -               $(USER_HOOKS_EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
> +               $(USER_HOOKS_EXTRA_ENV) $(abspath $(s)) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
>
>  ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
>  LOCALE_WHITELIST=$(BUILD_DIR)/locales.nopurge
> @@ -600,7 +600,7 @@ endif
>  target-post-image:
>         @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
>                 $(call MESSAGE,"Executing post-image script $(s)"); \
> -               $(USER_HOOKS_EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
> +               $(USER_HOOKS_EXTRA_ENV) $(abspath $(s)) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
>
>  toolchain-eclipse-register:
>         ./support/scripts/eclipse-register-toolchain `readlink -f $(O)` $(notdir $(TARGET_CROSS)) $(BR2_ARCH)
> --
> 1.8.3.2
>
I am not sure I fully get your patch, what is wrong with putting
path/to/my_script.sh ? You don't have to specify the full path, a
relative path works well.

> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Waldemar Rymarkiewicz - Jan. 28, 2014, 10:50 a.m.
Hi

>>
> I am not sure I fully get your patch, what is wrong with putting
> path/to/my_script.sh ? You don't have to specify the full path, a
> relative path works well.

Nothing is wrong to put full path to your scripts. Just a matter of
what you prefer.

Try to put script file in buildroot directory and set the post script
variable  using just the name of the script. I does not work for me
and I'm pretty sure it does not work for you as well. This is simply
how Makefile calls the scripts.

This patch is just addition to run post scripts referenced by relative
paths as well, nothing more.

Thanks,
/Waldek
Maxime Hadjinlian - Jan. 28, 2014, 11:08 a.m.
On Tue, Jan 28, 2014 at 11:50 AM, Waldemar Rymarkiewicz
<waldemar.rymarkiewicz@gmail.com> wrote:
> Hi
>
>>>
>> I am not sure I fully get your patch, what is wrong with putting
>> path/to/my_script.sh ? You don't have to specify the full path, a
>> relative path works well.
>
> Nothing is wrong to put full path to your scripts. Just a matter of
> what you prefer.
>
> Try to put script file in buildroot directory and set the post script
> variable  using just the name of the script. I does not work for me
> and I'm pretty sure it does not work for you as well. This is simply
> how Makefile calls the scripts.
Okay, I did not understood where the problem came from as I never put
my scripts at the root but in a special directory inside buildroot
boards folder so it never happened to me.
Thanks for the clarifications.
>
> This patch is just addition to run post scripts referenced by relative
> paths as well, nothing more.
>
> Thanks,
> /Waldek
Arnout Vandecappelle - Jan. 28, 2014, 5 p.m.
On 28/01/14 11:14, Waldemar Rymarkiewicz wrote:
> This fix will let to run post scripts seamlessly while are referenced by
> relative paths. Scripts referenced by full path still works.

  The commit message is wrong: relative paths work fine already, it is 
only a post-script in the current directory (= the buildroot directory) 
that doesn't work.

  Patch itself looks OK to me.

  Regards,
  Arnout

>
> Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@gmail.com>
> ---
>   Makefile | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 067458b..ef7f4c2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -554,7 +554,7 @@ endif
>
>   	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
>   		$(call MESSAGE,"Executing post-build script $(s)"); \
> -		$(USER_HOOKS_EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
> +		$(USER_HOOKS_EXTRA_ENV) $(abspath $(s)) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
>
>   ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
>   LOCALE_WHITELIST=$(BUILD_DIR)/locales.nopurge
> @@ -600,7 +600,7 @@ endif
>   target-post-image:
>   	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
>   		$(call MESSAGE,"Executing post-image script $(s)"); \
> -		$(USER_HOOKS_EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
> +		$(USER_HOOKS_EXTRA_ENV) $(abspath $(s)) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
>
>   toolchain-eclipse-register:
>   	./support/scripts/eclipse-register-toolchain `readlink -f $(O)` $(notdir $(TARGET_CROSS)) $(BR2_ARCH)
>
Yann E. MORIN - Jan. 28, 2014, 6:32 p.m.
Waldemar, All,

On 2014-01-28 11:50 +0100, Waldemar Rymarkiewicz spake thusly:
> > I am not sure I fully get your patch, what is wrong with putting
> > path/to/my_script.sh ? You don't have to specify the full path, a
> > relative path works well.
> 
> Nothing is wrong to put full path to your scripts. Just a matter of
> what you prefer.
> 
> Try to put script file in buildroot directory and set the post script
> variable  using just the name of the script. I does not work for me
> and I'm pretty sure it does not work for you as well. This is simply
> how Makefile calls the scripts.
> 
> This patch is just addition to run post scripts referenced by relative
> paths as well, nothing more.

Why don't you just set:
  BR2_ROOTFS_POST_BUILD_SCRIPT="./my-script ./my-second-script"

(Ditto for BR2_ROOTFS_POST_IMAGE_SCRIPT.)

I'd say, we don't need that change. I prefer the user be explixit about
what he intends to do, rather than Buildroot guessing.

Regards,
Yann E. MORIN.
Thomas Petazzoni - Jan. 28, 2014, 9:41 p.m.
Dear Yann E. MORIN,

On Tue, 28 Jan 2014 19:32:02 +0100, Yann E. MORIN wrote:

> On 2014-01-28 11:50 +0100, Waldemar Rymarkiewicz spake thusly:
> > > I am not sure I fully get your patch, what is wrong with putting
> > > path/to/my_script.sh ? You don't have to specify the full path, a
> > > relative path works well.
> > 
> > Nothing is wrong to put full path to your scripts. Just a matter of
> > what you prefer.
> > 
> > Try to put script file in buildroot directory and set the post script
> > variable  using just the name of the script. I does not work for me
> > and I'm pretty sure it does not work for you as well. This is simply
> > how Makefile calls the scripts.
> > 
> > This patch is just addition to run post scripts referenced by relative
> > paths as well, nothing more.
> 
> Why don't you just set:
>   BR2_ROOTFS_POST_BUILD_SCRIPT="./my-script ./my-second-script"
> 
> (Ditto for BR2_ROOTFS_POST_IMAGE_SCRIPT.)
> 
> I'd say, we don't need that change. I prefer the user be explixit about
> what he intends to do, rather than Buildroot guessing.

I agree.

Thomas
Waldemar Rymarkiewicz - Jan. 28, 2014, 10:06 p.m.
Hi,

On 28 January 2014 22:41, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Yann E. MORIN,
>
> On Tue, 28 Jan 2014 19:32:02 +0100, Yann E. MORIN wrote:
>
>> On 2014-01-28 11:50 +0100, Waldemar Rymarkiewicz spake thusly:
>> > > I am not sure I fully get your patch, what is wrong with putting
>> > > path/to/my_script.sh ? You don't have to specify the full path, a
>> > > relative path works well.
>> >
>> > Nothing is wrong to put full path to your scripts. Just a matter of
>> > what you prefer.
>> >
>> > Try to put script file in buildroot directory and set the post script
>> > variable  using just the name of the script. I does not work for me
>> > and I'm pretty sure it does not work for you as well. This is simply
>> > how Makefile calls the scripts.
>> >to
>> > This patch is just addition to run post scripts referenced by relative
>> > paths as well, nothing more.
>>
>> Why don't you just set:
>>   BR2_ROOTFS_POST_BUILD_SCRIPT="./my-script ./my-second-script"
>> (Ditto for BR2_ROOTFS_POST_IMAGE_SCRIPT.
>>
>> I'd say, we don't need that change. I prefer the user be explixit about
>> what he intends to do, rather than Buildroot guessing.
>
> I agree.

Let's leave it as it is now then.

The point was that neither manual nor menuconfig help explains how a
script should be referenced. By full path, relative or what. In my
case the easiest way was  just to put  "a space-separated list of
scripts" as per help message and it does not work. For someone who is
not dealing with buildroot frequently it can be an issue. Of course
you can always use ./your_script, but still it's not explained
anywhere. Abspath will fix it and it costs nothing, but I understand
your way of thinking.

Thanks,
/Waldek
Thomas Petazzoni - Jan. 28, 2014, 10:15 p.m.
Dear Waldemar Rymarkiewicz,

On Tue, 28 Jan 2014 23:06:05 +0100, Waldemar Rymarkiewicz wrote:

> The point was that neither manual nor menuconfig help explains how a
> script should be referenced. By full path, relative or what. In my
> case the easiest way was  just to put  "a space-separated list of
> scripts" as per help message and it does not work. For someone who is
> not dealing with buildroot frequently it can be an issue. Of course
> you can always use ./your_script, but still it's not explained
> anywhere. Abspath will fix it and it costs nothing, but I understand
> your way of thinking.

Your patches to the documentation and/or the help text of the
problematic Config.in options would definitely be welcome!

Thanks!

Thomas
Yann E. MORIN - Jan. 28, 2014, 10:17 p.m.
Waldemar, All,

On 2014-01-28 23:06 +0100, Waldemar Rymarkiewicz spake thusly:
> > On Tue, 28 Jan 2014 19:32:02 +0100, Yann E. MORIN wrote:
> >> Why don't you just set:
> >>   BR2_ROOTFS_POST_BUILD_SCRIPT="./my-script ./my-second-script"
> >> (Ditto for BR2_ROOTFS_POST_IMAGE_SCRIPT.
> >>
> >> I'd say, we don't need that change. I prefer the user be explixit about
> >> what he intends to do, rather than Buildroot guessing.

> Let's leave it as it is now then.
> 
> The point was that neither manual nor menuconfig help explains how a
> script should be referenced. By full path, relative or what. In my
> case the easiest way was  just to put  "a space-separated list of
> scripts" as per help message and it does not work. For someone who is
> not dealing with buildroot frequently it can be an issue. Of course
> you can always use ./your_script, but still it's not explained
> anywhere. Abspath will fix it and it costs nothing, but I understand
> your way of thinking.

Ah, but then we are ready to accept a patch to the manual to fix this! ;-)
The manual is located in docs/manual/ in your buildroot tree, FYI.

Your comment is prefectly valid, indeed, but would better be solved by
fixing the manual instead.

Regards,
Yann E. MORIN.

Patch

diff --git a/Makefile b/Makefile
index 067458b..ef7f4c2 100644
--- a/Makefile
+++ b/Makefile
@@ -554,7 +554,7 @@  endif
 
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
 		$(call MESSAGE,"Executing post-build script $(s)"); \
-		$(USER_HOOKS_EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
+		$(USER_HOOKS_EXTRA_ENV) $(abspath $(s)) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
 
 ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
 LOCALE_WHITELIST=$(BUILD_DIR)/locales.nopurge
@@ -600,7 +600,7 @@  endif
 target-post-image:
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
 		$(call MESSAGE,"Executing post-image script $(s)"); \
-		$(USER_HOOKS_EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
+		$(USER_HOOKS_EXTRA_ENV) $(abspath $(s)) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
 
 toolchain-eclipse-register:
 	./support/scripts/eclipse-register-toolchain `readlink -f $(O)` $(notdir $(TARGET_CROSS)) $(BR2_ARCH)