[PATCH] Make barebox flashable image link for "multi-image" targets

[PATCH] Make barebox flashable image link for "multi-image" targets

[Trent Piepho]

For a target with multi-image support, no barebox-flash-image link is
made pointing to the finished flashable image.  This makes it hard for
buildroot to figure out which file is the barebox image.  And one has
to assume this link is generally useful and convenient to users of
barebox, or why would it have been created in the first place?

This patch to the multi-image build will create that link.

It only works when a single image is made.  If one is making multiple
images, then the concept of a single finished image no longer applies,
and no link is made.

Signed-off-by: Trent Piepho <tpiepho@kymetacorp.com>
---
 images/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/images/Makefile b/images/Makefile
index a5f589b..fcdf404 100644
--- a/images/Makefile
+++ b/images/Makefile
@@ -121,10 +121,16 @@ targets += $(foreach m, $(image-y), $(FILE_$(m)))
 
 SECONDARY: $(addprefix $(obj)/,$(targets))
 
-images: $(addprefix $(obj)/, $(image-y)) FORCE
+# Set if there is only one image in image-y
+flash-link := $(if $(filter 1,$(words $(image-y))),$(obj)/../barebox-flash-image)
+
+images: $(addprefix $(obj)/, $(image-y)) $(flash-link) FORCE
 	@echo "images built:"
 	@for i in $(image-y); do echo $$i; done
 
+$(flash-link): $(addprefix $(obj)/, $(image-y)) FORCE
+	$(call if_changed,ln)
+
 clean-files := *.pbl *.pblb *.pblx *.map start_*.imximg *.img barebox.z start_*.kwbimg \
 	start_*.kwbuartimg *.socfpgaimg *.mlo *.t20img *.t20img.cfg *.t30img \
 	*.t30img.cfg *.t124img *.t124img.cfg *.mlospi *.mlo *.mxsbs *.mxssd
-- 
1.8.3.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Make barebox flashable image link for "multi-image" targets

[Sascha Hauer]

On Tue, Oct 20, 2015 at 09:47:30PM +0000, Trent Piepho wrote:
> For a target with multi-image support, no barebox-flash-image link is
> made pointing to the finished flashable image.  This makes it hard for
> buildroot to figure out which file is the barebox image.  And one has
> to assume this link is generally useful and convenient to users of
> barebox, or why would it have been created in the first place?

Creating the barebox-flash-image link predates the multiimage support,
so by the time it was introduced it was not clear to me that there will
be no single image later anymore.

> 
> This patch to the multi-image build will create that link.
> 
> It only works when a single image is made.  If one is making multiple
> images, then the concept of a single finished image no longer applies,
> and no link is made.

Can't you make the image a config option in buildroot? Depending on only
having a single image selected in barebox doesn't sound like a good
idea.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Make barebox flashable image link for "multi-image" targets

[Trent Piepho]

On Wed, 2015-10-21 at 08:01 +0200, Sascha Hauer wrote:
> On Tue, Oct 20, 2015 at 09:47:30PM +0000, Trent Piepho wrote:
> > For a target with multi-image support, no barebox-flash-image link is
> > made pointing to the finished flashable image.  This makes it hard for
> > buildroot to figure out which file is the barebox image.  And one has
> > to assume this link is generally useful and convenient to users of
> > barebox, or why would it have been created in the first place?
> 
> Creating the barebox-flash-image link predates the multiimage support,
> so by the time it was introduced it was not clear to me that there will
> be no single image later anymore.

I imagine having multiple images is used far more often by developers of
barebox than developers of a device that uses barebox.  So I wouldn't
say the concept of a single image is dead.

> > 
> > This patch to the multi-image build will create that link.
> > 
> > It only works when a single image is made.  If one is making multiple
> > images, then the concept of a single finished image no longer applies,
> > and no link is made.
> 
> Can't you make the image a config option in buildroot? Depending on only
> having a single image selected in barebox doesn't sound like a good
> idea.

buildroot doesn't have an option that for that.  It uses
barebox-flash-image if it exists and then falls back to barebox.bin.
The latter will still exist in a multi-image build, but is not the
correct file!

My first thought was to add an option to buildroot for the name of the
flash image file.  But barebox is the one choosing this name and it
knows what it is.  So why should I have to manually copy this data out
of barebox and into buildroot and keep to up to date when it chages?
Barebox knows the value, have it tell buildroot what it is.  And there
is already the barebox-flash-image symlink system that does just this.
This way buildroot (and any other buildsystem or flash or test script
that uses a barebox image) gets told what file to use straight from the
authoritative source: the barebox build system that made the file.

So, that's why I did it this way.  And it's not like I'm adding a new
feature for this, just making an existing one work in more cases that it
did before.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Make barebox flashable image link for "multi-image" targets

[Sascha Hauer]

On Wed, Oct 21, 2015 at 06:09:10PM +0000, Trent Piepho wrote:
> On Wed, 2015-10-21 at 08:01 +0200, Sascha Hauer wrote:
> > On Tue, Oct 20, 2015 at 09:47:30PM +0000, Trent Piepho wrote:
> > > For a target with multi-image support, no barebox-flash-image link is
> > > made pointing to the finished flashable image.  This makes it hard for
> > > buildroot to figure out which file is the barebox image.  And one has
> > > to assume this link is generally useful and convenient to users of
> > > barebox, or why would it have been created in the first place?
> > 
> > Creating the barebox-flash-image link predates the multiimage support,
> > so by the time it was introduced it was not clear to me that there will
> > be no single image later anymore.
> 
> I imagine having multiple images is used far more often by developers of
> barebox than developers of a device that uses barebox.  So I wouldn't
> say the concept of a single image is dead.

Nope, it's not.

> 
> > > 
> > > This patch to the multi-image build will create that link.
> > > 
> > > It only works when a single image is made.  If one is making multiple
> > > images, then the concept of a single finished image no longer applies,
> > > and no link is made.
> > 
> > Can't you make the image a config option in buildroot? Depending on only
> > having a single image selected in barebox doesn't sound like a good
> > idea.
> 
> buildroot doesn't have an option that for that.  It uses
> barebox-flash-image if it exists and then falls back to barebox.bin.
> The latter will still exist in a multi-image build, but is not the
> correct file!
> 
> My first thought was to add an option to buildroot for the name of the
> flash image file.  But barebox is the one choosing this name and it
> knows what it is.  So why should I have to manually copy this data out
> of barebox and into buildroot and keep to up to date when it chages?
> Barebox knows the value, have it tell buildroot what it is.  And there
> is already the barebox-flash-image symlink system that does just this.
> This way buildroot (and any other buildsystem or flash or test script
> that uses a barebox image) gets told what file to use straight from the
> authoritative source: the barebox build system that made the file.
> 
> So, that's why I did it this way.  And it's not like I'm adding a new
> feature for this, just making an existing one work in more cases that it
> did before.

Yes, it works in more cases, but not in all. With your patch it works
exactly in the case when one board is selected. Once you intentionally
or by accident enable another board then buildroot would silently fall
back to barebox.bin and nothing works anymore. Some images do not even
have a separate config option, for example there are three nitrogen 6x
images which are all controlled by the CONFIG_MACH_NITROGEN6X option.

Applying a band aid solution only lowers the pressure to do the
necessary changes in buildroot. I am not a fan of this.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Make barebox flashable image link for "multi-image" targets

[Trent Piepho]

On Thu, 2015-10-22 at 09:06 +0200, Sascha Hauer wrote:
> > > > It only works when a single image is made.  If one is making multiple
> > > > images, then the concept of a single finished image no longer applies,
> > > > and no link is made.
> > > 
> > > Can't you make the image a config option in buildroot? Depending on only
> > > having a single image selected in barebox doesn't sound like a good
> > > idea.
> > 
> > buildroot doesn't have an option that for that.  It uses
> > barebox-flash-image if it exists and then falls back to barebox.bin.
> > The latter will still exist in a multi-image build, but is not the
> > correct file!
> > 
> > My first thought was to add an option to buildroot for the name of the
> > flash image file.  But barebox is the one choosing this name and it
> > knows what it is.  So why should I have to manually copy this data out
> > of barebox and into buildroot and keep to up to date when it chages?
> > Barebox knows the value, have it tell buildroot what it is.  And there
> > is already the barebox-flash-image symlink system that does just this.
> > This way buildroot (and any other buildsystem or flash or test script
> > that uses a barebox image) gets told what file to use straight from the
> > authoritative source: the barebox build system that made the file.
> > 
> > So, that's why I did it this way.  And it's not like I'm adding a new
> > feature for this, just making an existing one work in more cases that it
> > did before.
> 
> Yes, it works in more cases, but not in all. With your patch it works
> exactly in the case when one board is selected. Once you intentionally
> or by accident enable another board then buildroot would silently fall
> back to barebox.bin and nothing works anymore. Some images do not even
> have a separate config option, for example there are three nitrogen 6x
> images which are all controlled by the CONFIG_MACH_NITROGEN6X option.
> 
> Applying a band aid solution only lowers the pressure to do the
> necessary changes in buildroot. I am not a fan of this.

So should the existing support for barebox-flash-image for
non-multi-image boards be removed too?  If they switch a multi-image
based build, even if they support only one image, the link goes away as
you described above and one has the same problem.

And putting an option into barebox (and also every other script or
system that wants to do something with the barebox flash image), still
has the problem of duplicating information.  It's like putting an
identical #define in two C files instead of in one header that both use.
Barebox chose the image name, why not have it provide that information
automatically?

For instance, I have a script that puts barebox on an SD card.  I have
multiple different barebox builds for different devices.  I can point
the script at the barebox output dir and it can tell, because barebox
provides that information, what image file it needs to flash even
through the name isn't the same for the different builds.


What if, in the case of multiple images, the link is still made, but
points to a non-existent location like
"multiple-flash-images-generated".  That way anyone using the link will
get an error and know they need to come up with a different plan.  Like
not building multiple images when they only want one.

Another alternative would be to create a file that lists the images
generated one per line, like the final make output that lists them to
stdout.  Then the image name(s) could be retrieved automatically even in
the case of multiple images.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Make barebox flashable image link for "multi-image" targets

[Sascha Hauer]

On Thu, Oct 22, 2015 at 06:48:54PM +0000, Trent Piepho wrote:
> On Thu, 2015-10-22 at 09:06 +0200, Sascha Hauer wrote:
> > > > > It only works when a single image is made.  If one is making multiple
> > > > > images, then the concept of a single finished image no longer applies,
> > > > > and no link is made.
> > > > 
> > > > Can't you make the image a config option in buildroot? Depending on only
> > > > having a single image selected in barebox doesn't sound like a good
> > > > idea.
> > > 
> > > buildroot doesn't have an option that for that.  It uses
> > > barebox-flash-image if it exists and then falls back to barebox.bin.
> > > The latter will still exist in a multi-image build, but is not the
> > > correct file!
> > > 
> > > My first thought was to add an option to buildroot for the name of the
> > > flash image file.  But barebox is the one choosing this name and it
> > > knows what it is.  So why should I have to manually copy this data out
> > > of barebox and into buildroot and keep to up to date when it chages?
> > > Barebox knows the value, have it tell buildroot what it is.  And there
> > > is already the barebox-flash-image symlink system that does just this.
> > > This way buildroot (and any other buildsystem or flash or test script
> > > that uses a barebox image) gets told what file to use straight from the
> > > authoritative source: the barebox build system that made the file.
> > > 
> > > So, that's why I did it this way.  And it's not like I'm adding a new
> > > feature for this, just making an existing one work in more cases that it
> > > did before.
> > 
> > Yes, it works in more cases, but not in all. With your patch it works
> > exactly in the case when one board is selected. Once you intentionally
> > or by accident enable another board then buildroot would silently fall
> > back to barebox.bin and nothing works anymore. Some images do not even
> > have a separate config option, for example there are three nitrogen 6x
> > images which are all controlled by the CONFIG_MACH_NITROGEN6X option.
> > 
> > Applying a band aid solution only lowers the pressure to do the
> > necessary changes in buildroot. I am not a fan of this.
> 
> So should the existing support for barebox-flash-image for
> non-multi-image boards be removed too?  If they switch a multi-image
> based build, even if they support only one image, the link goes away as
> you described above and one has the same problem.
> 
> And putting an option into barebox (and also every other script or
> system that wants to do something with the barebox flash image), still
> has the problem of duplicating information.  It's like putting an
> identical #define in two C files instead of in one header that both use.
> Barebox chose the image name, why not have it provide that information
> automatically?
> 
> For instance, I have a script that puts barebox on an SD card.  I have
> multiple different barebox builds for different devices.  I can point
> the script at the barebox output dir and it can tell, because barebox
> provides that information, what image file it needs to flash even
> through the name isn't the same for the different builds.
> 
> 
> What if, in the case of multiple images, the link is still made, but
> points to a non-existent location like
> "multiple-flash-images-generated".  That way anyone using the link will
> get an error and know they need to come up with a different plan.  Like
> not building multiple images when they only want one.
> 
> Another alternative would be to create a file that lists the images
> generated one per line, like the final make output that lists them to
> stdout.  Then the image name(s) could be retrieved automatically even in
> the case of multiple images.

I like this last idea. This would allow to get a consistent behaviour
between single image and multi image builds.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox