| Message ID | 1390904090-27934-1-git-send-email-waldemar.rymarkiewicz@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
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
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
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
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) >
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.
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
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
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
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.
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)
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(-)