| Message ID | 1373393196-19024-1-git-send-email-yann.morin.1998@free.fr |
|---|---|
| State | Superseded |
| Headers | show |
Hi Yann, On Tue, Jul 09, 2013 at 08:06:36PM +0200, Yann E. MORIN wrote: > From: "Yann E. MORIN" <yann.morin.1998@free.fr> > > It can be useful to have different configuration use the same post-build > and/or post-image scripts as they share a common infrastructure, but yet > have minor differentiation. > > This option allows passing zero or more additional arguments to each > post-build or post-image script. > > The same set of extra arguments are passed to all scripts, it is not > possible to pass different arguments to each script. > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> You should also change the "first and only argument" language in the help text of BR2_ROOTFS_POST_BUILD_SCRIPT and BR2_ROOTFS_POST_IMAGE_SCRIPT. baruch > --- > Makefile | 4 ++-- > system/Config.in | 16 ++++++++++++++++ > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 955e684..f6e9965 100644 > --- a/Makefile > +++ b/Makefile > @@ -512,7 +512,7 @@ endif > > @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \ > $(call MESSAGE,"Executing post-build script $(s)"); \ > - $(s) $(TARGET_DIR)$(sep)) > + $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep)) > > ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) > LOCALE_WHITELIST=$(BUILD_DIR)/locales.nopurge > @@ -558,7 +558,7 @@ endif > target-post-image: > @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \ > $(call MESSAGE,"Executing post-image script $(s)"); \ > - $(s) $(BINARIES_DIR)$(sep)) > + $(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) > diff --git a/system/Config.in b/system/Config.in > index c0e4f4a..da50f67 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -294,4 +294,20 @@ config BR2_ROOTFS_POST_IMAGE_SCRIPT > first and only argument. The script is executed from the > main Buildroot source directory as the current directory. > > +config BR2_ROOTFS_POST_SCRIPT_ARGS > + string "Extra post-{build,image} arguments" > + depends on BR2_ROOTFS_POST_BUILD_SCRIPT != "" || BR2_ROOTFS_POST_IMAGE_SCRIPT != "" > + help > + Pass these aditional arguments to each post-build or post-image > + scripts. > + > + Note that all the post-build and post-image scripts will be passed > + the same set of arguments, you can not pass different arguments to > + each script. > + > + Note also, as stated in their respective help text, that the first > + argument to each post-build or post-image script is the target/ > + directory $(TARGET_DIR). The arguments in this option will be > + passed *after*. > + > endmenu
Hello All! On 2013-07-09 20:06 +0200, Yann E. MORIN spake thusly: > It can be useful to have different configuration use the same post-build > and/or post-image scripts as they share a common infrastructure, but yet > have minor differentiation. > > This option allows passing zero or more additional arguments to each > post-build or post-image script. > > The same set of extra arguments are passed to all scripts, it is not > possible to pass different arguments to each script. I've already suggested moving the current _POST_BUILD_SCRIPT and _POST_IMAGE_SCRIPT options to Legacy, and introduce two new options that would accept a single script with arguments. This was refused (and rightfully) as that would greatly impact users depending on the current behaviour, and some alternate solution were proposed: - have a single script that would behave differently on how it sould be called, and have different symlinks pointing to that script; - have a script for each configuration, that 'sources' a functions file and call the required functions After playing a bit with both solutions, it turned out to be not entirely manageable, especially when the inrastructure put in place by the functions changes, since all scripts must be changed accordingly. So, I'm back with this yet-alternate solution, that passes a set of extra args to the scripts, so it becomes possible to have a single (or more!) script which can decide what to do, based on its arguments. In my case, the arguments are a description file of the target: - the steps to execute post-build - the partitioning scheme(s) of the storage device(s) Since it is using a description file, and the post scripts only read that file for deciding what to do, a change in the functions API has no impact on the main script. Granted, a change in the description format will require all description files to be updated, but that is *not* expected to happen too often. Besides, this new solution is fully backward-compatible, so does not break existing workflows. Ready to hear^Wread comments! ;-) Regards, Yann E. MORIN.
Baruch, All, On 2013-07-09 21:25 +0300, Baruch Siach spake thusly: > Hi Yann, > > On Tue, Jul 09, 2013 at 08:06:36PM +0200, Yann E. MORIN wrote: > > From: "Yann E. MORIN" <yann.morin.1998@free.fr> > > > > It can be useful to have different configuration use the same post-build > > and/or post-image scripts as they share a common infrastructure, but yet > > have minor differentiation. > > > > This option allows passing zero or more additional arguments to each > > post-build or post-image script. > > > > The same set of extra arguments are passed to all scripts, it is not > > possible to pass different arguments to each script. > > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > You should also change the "first and only argument" language in the help text > of BR2_ROOTFS_POST_BUILD_SCRIPT and BR2_ROOTFS_POST_IMAGE_SCRIPT. And update the manual while at it, indeed! Thanks for pointing this. :-) Regards, Yann E. MORIN.
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
Hi,
Yann> Besides, this new solution is fully backward-compatible, so does
Yann> not break existing workflows.
Yann> Ready to hear^Wread comments! ;-)
I don't really find the solution pretty, but I guess we cannot do any
better given the constraints, so OK from here (once you resend).
Dear Yann E. MORIN, On Tue, 9 Jul 2013 20:35:05 +0200, Yann E. MORIN wrote: > This was refused (and rightfully) as that would greatly impact users > depending on the current behaviour, and some alternate solution were > proposed: > - have a single script that would behave differently on how it sould > be called, and have different symlinks pointing to that script; > - have a script for each configuration, that 'sources' a functions > file and call the required functions > > After playing a bit with both solutions, it turned out to be not > entirely manageable, especially when the inrastructure put in place by > the functions changes, since all scripts must be changed accordingly. I am not quite sure to understand why the symlink solution doesn't work in your case. Could you elaborate on that? Regarding the patch itself, I'd say why not. I'm just wondering if it wouldn't be better to have separate arguments for both scripts. Not sure. Thanks! Thomas
On Wed, 10 Jul 2013 09:16:28 +0200, Thomas Petazzoni wrote: > > After playing a bit with both solutions, it turned out to be not > > entirely manageable, especially when the inrastructure put in place by > > the functions changes, since all scripts must be changed accordingly. > > I am not quite sure to understand why the symlink solution doesn't work > in your case. Could you elaborate on that? > > Regarding the patch itself, I'd say why not. I'm just wondering if it > wouldn't be better to have separate arguments for both scripts. Not > sure. Ok, I see that it has already been committed, without more discussion... Fair enough. Best regards, Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: Thomas> On Wed, 10 Jul 2013 09:16:28 +0200, Thomas Petazzoni wrote: >> Regarding the patch itself, I'd say why not. I'm just wondering if it >> wouldn't be better to have separate arguments for both scripts. Not >> sure. Thomas> Ok, I see that it has already been committed, without more Thomas> discussion... Fair enough. I commented here and on IRC that I didn't really find it nice, but didn't see any better way of doing it. Your comment seemed to be in line with that, so I applied. But I agree, perhaps it would be better to have seperate options for post-build/post-image.
Dear Peter Korsgaard, On Wed, 10 Jul 2013 11:53:52 +0200, Peter Korsgaard wrote: > Thomas> On Wed, 10 Jul 2013 09:16:28 +0200, Thomas Petazzoni wrote: > > >> Regarding the patch itself, I'd say why not. I'm just wondering if it > >> wouldn't be better to have separate arguments for both scripts. Not > >> sure. > > Thomas> Ok, I see that it has already been committed, without more > Thomas> discussion... Fair enough. > > I commented here and on IRC that I didn't really find it nice, but > didn't see any better way of doing it. Your comment seemed to be in line > with that, so I applied. > > But I agree, perhaps it would be better to have seperate options for post-build/post-image. Maybe, I don't know. But for this kind of patches, I'm rather surprised that it gets applied from one day to the next, but well, I'm not completely against the change, so good enough. Thomas
Thomas, All, On 2013-07-10 09:16 +0200, Thomas Petazzoni spake thusly: > On Tue, 9 Jul 2013 20:35:05 +0200, Yann E. MORIN wrote: > > This was refused (and rightfully) as that would greatly impact users > > depending on the current behaviour, and some alternate solution were > > proposed: > > - have a single script that would behave differently on how it sould > > be called, and have different symlinks pointing to that script; > > - have a script for each configuration, that 'sources' a functions > > file and call the required functions > > > > After playing a bit with both solutions, it turned out to be not > > entirely manageable, especially when the inrastructure put in place by > > the functions changes, since all scripts must be changed accordingly. > > I am not quite sure to understand why the symlink solution doesn't work > in your case. Could you elaborate on that? The basic idea behind usig a symlink is that: - there is a single script with all the infrastructure - the script decides what to do based on ${0} The script I'm using has no board-specific parts, only a handfull generic functions, and parses a board-specific file that describes what to do. So instead of a single file (the board description for that project), I need two: the symlink which will never change, and the board file. Since the symlink will never change, it is just lying there for no reason. For a few boards/projects, using a symlink can be enough, but as I will eventually be managing a few dozens, or even more, boards/projects, I'd end up with as many symlinks that serve no purpose except working around a limitation in Buildroot, limitation which can be easily raised in a backward-compatible way. Being able to pass a argument to the script means I have a single file to manage per board/project, and not carry this useless symlink. > Regarding the patch itself, I'd say why not. I'm just wondering if it > wouldn't be better to have separate arguments for both scripts. Not > sure. That was my original idea, too, but as an afterthought I decided not to, since the argument(s) passed will probably be something like the board and the project name (eg. ( rpi/tvheadend ) or ( rpi, tvheadend )), and that would be common to the postbuild/image scripts. But I have no strong opinion on this. Copy-pasting between each option will be easy enough! ;-) I can upgrade the infra to separate both if Peter and you want it. Regards, Yann E. MORIN.
Peter, Thomas, All, On 2013-07-10 14:49 +0200, Thomas Petazzoni spake thusly: > On Wed, 10 Jul 2013 11:53:52 +0200, Peter Korsgaard wrote: > > Thomas> On Wed, 10 Jul 2013 09:16:28 +0200, Thomas Petazzoni wrote: > > > > >> Regarding the patch itself, I'd say why not. I'm just wondering if it > > >> wouldn't be better to have separate arguments for both scripts. Not > > >> sure. > > > > Thomas> Ok, I see that it has already been committed, without more > > Thomas> discussion... Fair enough. > > > > I commented here and on IRC that I didn't really find it nice, but > > didn't see any better way of doing it. Your comment seemed to be in line > > with that, so I applied. > > > > But I agree, perhaps it would be better to have seperate options for post-build/post-image. > > Maybe, I don't know. But for this kind of patches, I'm rather surprised > that it gets applied from one day to the next, but well, I'm not > completely against the change, so good enough. Err, maybe I'm to blame here. I asked Peter on IRC if I could have some feedback on the patch. I did not expect him to apply it so soon, indeed. Since I was going to rewrite my entire infrastructure based on whether that patch would be accepted or not, I needed to know early if I could start the conversion, or think up yet another way. Peter, sorry if I sounded pushy yesterday. That was not my intention. :-/ But I have to admit I'm glad this made it in! ;-) Regards, Yann E. MORIN.
diff --git a/Makefile b/Makefile index 955e684..f6e9965 100644 --- a/Makefile +++ b/Makefile @@ -512,7 +512,7 @@ endif @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \ $(call MESSAGE,"Executing post-build script $(s)"); \ - $(s) $(TARGET_DIR)$(sep)) + $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep)) ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) LOCALE_WHITELIST=$(BUILD_DIR)/locales.nopurge @@ -558,7 +558,7 @@ endif target-post-image: @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \ $(call MESSAGE,"Executing post-image script $(s)"); \ - $(s) $(BINARIES_DIR)$(sep)) + $(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) diff --git a/system/Config.in b/system/Config.in index c0e4f4a..da50f67 100644 --- a/system/Config.in +++ b/system/Config.in @@ -294,4 +294,20 @@ config BR2_ROOTFS_POST_IMAGE_SCRIPT first and only argument. The script is executed from the main Buildroot source directory as the current directory. +config BR2_ROOTFS_POST_SCRIPT_ARGS + string "Extra post-{build,image} arguments" + depends on BR2_ROOTFS_POST_BUILD_SCRIPT != "" || BR2_ROOTFS_POST_IMAGE_SCRIPT != "" + help + Pass these aditional arguments to each post-build or post-image + scripts. + + Note that all the post-build and post-image scripts will be passed + the same set of arguments, you can not pass different arguments to + each script. + + Note also, as stated in their respective help text, that the first + argument to each post-build or post-image script is the target/ + directory $(TARGET_DIR). The arguments in this option will be + passed *after*. + endmenu