Patchwork system: add option to pass extra args to post-build and post-image scripts

login
register
mail settings
Submitter Yann E. MORIN
Date July 9, 2013, 6:06 p.m.
Message ID <1373393196-19024-1-git-send-email-yann.morin.1998@free.fr>
Download mbox | patch
Permalink /patch/257851/
State Superseded
Headers show

Comments

Yann E. MORIN - July 9, 2013, 6:06 p.m.
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>
---
 Makefile         |  4 ++--
 system/Config.in | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)
Baruch Siach - July 9, 2013, 6:25 p.m.
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
Yann E. MORIN - July 9, 2013, 6:35 p.m.
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.
Yann E. MORIN - July 9, 2013, 6:54 p.m.
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.
Peter Korsgaard - July 9, 2013, 9:50 p.m.
>>>>> "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).
Thomas Petazzoni - July 10, 2013, 7:16 a.m.
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
Thomas Petazzoni - July 10, 2013, 9:29 a.m.
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
Peter Korsgaard - July 10, 2013, 9:53 a.m.
>>>>> "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.
Thomas Petazzoni - July 10, 2013, 12:49 p.m.
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
Yann E. MORIN - July 10, 2013, 4:17 p.m.
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.
Yann E. MORIN - July 10, 2013, 4:22 p.m.
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.

Patch

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