* [Buildroot] [PATCHv2] package/uboot: detect missing user-supplied environment source files
@ 2017-12-06 22:58 Yann E. MORIN
2017-12-07 8:18 ` Cam Hutchison
0 siblings, 1 reply; 4+ messages in thread
From: Yann E. MORIN @ 2017-12-06 22:58 UTC (permalink / raw)
To: buildroot
Since 0542bb79e8 (uboot: Support multiple environment source files),
a missing user-supplied environment source files is no longer detected.
This is because we cat them all, and feed the concatenation to the stdin
of mkenvimage. So, if one source file is missing, the cat exits in error,
but the compound command exits with the exit code of the last command,
which is that of mkenvimage, which happens to be happy with whatever it
is fed on its stdin, even is empty.
We fix that by creating a temporary file, that we even leave afterward
for the user to inspect.
We also move it out of the _CMDS block and into a macro of its own, so
that it is easier to write and maintain.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Cam Hutchison <camh@xdna.net>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Changes v1 -> v2:
- move into its own macro (Thomas)
---
boot/uboot/uboot.mk | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
index a1fac7dcae..abb89da69b 100644
--- a/boot/uboot/uboot.mk
+++ b/boot/uboot/uboot.mk
@@ -238,6 +238,18 @@ define UBOOT_BUILD_OMAP_IFT
-c $(call qstrip,$(BR2_TARGET_UBOOT_OMAP_IFT_CONFIG))
endef
+ifneq ($(BR2_TARGET_UBOOT_ENVIMAGE),)
+define UBOOT_GENERATE_ENV_IMAGE
+ cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) \
+ >$(BINARIES_DIR)/uboot-env.txt
+ $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
+ $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
+ $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
+ -o $(BINARIES_DIR)/uboot-env.bin \
+ $(BINARIES_DIR)/uboot-env.txt
+endef
+endif
+
define UBOOT_INSTALL_IMAGES_CMDS
$(foreach f,$(UBOOT_BINS), \
cp -dpf $(@D)/$(f) $(BINARIES_DIR)/
@@ -249,12 +261,7 @@ define UBOOT_INSTALL_IMAGES_CMDS
cp -dpf $(@D)/$(f) $(BINARIES_DIR)/
)
)
- $(if $(BR2_TARGET_UBOOT_ENVIMAGE),
- cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) | \
- $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
- $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
- $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
- -o $(BINARIES_DIR)/uboot-env.bin -)
+ $(UBOOT_GENERATE_ENV_IMAGE)
$(if $(BR2_TARGET_UBOOT_BOOT_SCRIPT),
$(HOST_DIR)/bin/mkimage -C none -A $(MKIMAGE_ARCH) -T script \
-d $(call qstrip,$(BR2_TARGET_UBOOT_BOOT_SCRIPT_SOURCE)) \
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Buildroot] [PATCHv2] package/uboot: detect missing user-supplied environment source files
2017-12-06 22:58 [Buildroot] [PATCHv2] package/uboot: detect missing user-supplied environment source files Yann E. MORIN
@ 2017-12-07 8:18 ` Cam Hutchison
2017-12-07 9:13 ` Yann E. MORIN
0 siblings, 1 reply; 4+ messages in thread
From: Cam Hutchison @ 2017-12-07 8:18 UTC (permalink / raw)
To: buildroot
On 7 December 2017 at 09:58, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Since 0542bb79e8 (uboot: Support multiple environment source files),
> a missing user-supplied environment source files is no longer detected.
>
> This is because we cat them all, and feed the concatenation to the stdin
> of mkenvimage. So, if one source file is missing, the cat exits in error,
> but the compound command exits with the exit code of the last command,
> which is that of mkenvimage, which happens to be happy with whatever it
> is fed on its stdin, even is empty.
>
> We fix that by creating a temporary file, that we even leave afterward
> for the user to inspect.
>
> We also move it out of the _CMDS block and into a macro of its own, so
> that it is easier to write and maintain.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Cam Hutchison <camh@xdna.net>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>
> ---
> Changes v1 -> v2:
> - move into its own macro (Thomas)
> ---
> boot/uboot/uboot.mk | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> index a1fac7dcae..abb89da69b 100644
> --- a/boot/uboot/uboot.mk
> +++ b/boot/uboot/uboot.mk
> @@ -238,6 +238,18 @@ define UBOOT_BUILD_OMAP_IFT
> -c $(call qstrip,$(BR2_TARGET_UBOOT_OMAP_IFT_CONFIG))
> endef
>
> +ifneq ($(BR2_TARGET_UBOOT_ENVIMAGE),)
> +define UBOOT_GENERATE_ENV_IMAGE
> + cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) \
> + >$(BINARIES_DIR)/uboot-env.txt
One (very minor) concern is using the name "uboot-env.txt". It is
possible that someone has
a post-image script that creates a file of that name in BINARIES_DIR,
so this will clobber
that file. As a tmp file, perhaps it should have a tmp file name?
I have a post-image script that creates a file of this name, but it
goes at a higher-level in
a binaries-type dir - I have a multi-layered build script, and the
higher-level directory that
aggregates the layers protects my file from being clobbered, but I can
imagine that if I
made different decisions when writing my front-end script, I could
have easily placed
my uboot-env.txt file in buildroot's BINARIES_DIR.
Perhaps _uboot-env.txt? Or go all the way and use mktemp?
> + $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
> + $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
> + $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
> + -o $(BINARIES_DIR)/uboot-env.bin \
> + $(BINARIES_DIR)/uboot-env.txt
> +endef
> +endif
> +
> define UBOOT_INSTALL_IMAGES_CMDS
> $(foreach f,$(UBOOT_BINS), \
> cp -dpf $(@D)/$(f) $(BINARIES_DIR)/
> @@ -249,12 +261,7 @@ define UBOOT_INSTALL_IMAGES_CMDS
> cp -dpf $(@D)/$(f) $(BINARIES_DIR)/
> )
> )
> - $(if $(BR2_TARGET_UBOOT_ENVIMAGE),
> - cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) | \
> - $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
> - $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
> - $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
> - -o $(BINARIES_DIR)/uboot-env.bin -)
> + $(UBOOT_GENERATE_ENV_IMAGE)
> $(if $(BR2_TARGET_UBOOT_BOOT_SCRIPT),
> $(HOST_DIR)/bin/mkimage -C none -A $(MKIMAGE_ARCH) -T script \
> -d $(call qstrip,$(BR2_TARGET_UBOOT_BOOT_SCRIPT_SOURCE)) \
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCHv2] package/uboot: detect missing user-supplied environment source files
2017-12-07 8:18 ` Cam Hutchison
@ 2017-12-07 9:13 ` Yann E. MORIN
2017-12-08 23:35 ` Cam Hutchison
0 siblings, 1 reply; 4+ messages in thread
From: Yann E. MORIN @ 2017-12-07 9:13 UTC (permalink / raw)
To: buildroot
Cam, All,
On 2017-12-07 19:18 +1100, Cam Hutchison spake thusly:
> On 7 December 2017 at 09:58, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > Since 0542bb79e8 (uboot: Support multiple environment source files),
> > a missing user-supplied environment source files is no longer detected.
> >
> > This is because we cat them all, and feed the concatenation to the stdin
> > of mkenvimage. So, if one source file is missing, the cat exits in error,
> > but the compound command exits with the exit code of the last command,
> > which is that of mkenvimage, which happens to be happy with whatever it
> > is fed on its stdin, even is empty.
> >
> > We fix that by creating a temporary file, that we even leave afterward
> > for the user to inspect.
> >
> > We also move it out of the _CMDS block and into a macro of its own, so
> > that it is easier to write and maintain.
> >
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Cam Hutchison <camh@xdna.net>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >
> > ---
> > Changes v1 -> v2:
> > - move into its own macro (Thomas)
> > ---
> > boot/uboot/uboot.mk | 19 +++++++++++++------
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> > index a1fac7dcae..abb89da69b 100644
> > --- a/boot/uboot/uboot.mk
> > +++ b/boot/uboot/uboot.mk
> > @@ -238,6 +238,18 @@ define UBOOT_BUILD_OMAP_IFT
> > -c $(call qstrip,$(BR2_TARGET_UBOOT_OMAP_IFT_CONFIG))
> > endef
> >
> > +ifneq ($(BR2_TARGET_UBOOT_ENVIMAGE),)
> > +define UBOOT_GENERATE_ENV_IMAGE
> > + cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) \
> > + >$(BINARIES_DIR)/uboot-env.txt
>
> One (very minor) concern is using the name "uboot-env.txt". It is
> possible that someone has
> a post-image script that creates a file of that name in BINARIES_DIR,
> so this will clobber
> that file. As a tmp file, perhaps it should have a tmp file name?
>
> I have a post-image script that creates a file of this name, but it
> goes at a higher-level in
> a binaries-type dir - I have a multi-layered build script, and the
> higher-level directory that
> aggregates the layers protects my file from being clobbered, but I can
> imagine that if I
> made different decisions when writing my front-end script, I could
> have easily placed
> my uboot-env.txt file in buildroot's BINARIES_DIR.
>
> Perhaps _uboot-env.txt? Or go all the way and use mktemp?
Or just generate uboot-env.txt in $(@D)/uboot-env.buildroot like so:
define UBOOT_GENERATE_ENV_IMAGE
cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) \
>$(@D)/uboot-env.buildroot
$(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
$(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
$(if $(filter BIG,$(BR2_ENDIAN)),-b) \
-o $(BINARIES_DIR)/uboot-env.bin \
$(@D)/uboot-env.buildroot
endef
Thoughts?
Regards,
Yann E. MORIN.
> > + $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
> > + $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
> > + $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
> > + -o $(BINARIES_DIR)/uboot-env.bin \
> > + $(BINARIES_DIR)/uboot-env.txt
> > +endef
> > +endif
> > +
> > define UBOOT_INSTALL_IMAGES_CMDS
> > $(foreach f,$(UBOOT_BINS), \
> > cp -dpf $(@D)/$(f) $(BINARIES_DIR)/
> > @@ -249,12 +261,7 @@ define UBOOT_INSTALL_IMAGES_CMDS
> > cp -dpf $(@D)/$(f) $(BINARIES_DIR)/
> > )
> > )
> > - $(if $(BR2_TARGET_UBOOT_ENVIMAGE),
> > - cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) | \
> > - $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
> > - $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
> > - $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
> > - -o $(BINARIES_DIR)/uboot-env.bin -)
> > + $(UBOOT_GENERATE_ENV_IMAGE)
> > $(if $(BR2_TARGET_UBOOT_BOOT_SCRIPT),
> > $(HOST_DIR)/bin/mkimage -C none -A $(MKIMAGE_ARCH) -T script \
> > -d $(call qstrip,$(BR2_TARGET_UBOOT_BOOT_SCRIPT_SOURCE)) \
> > --
> > 2.11.0
> >
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCHv2] package/uboot: detect missing user-supplied environment source files
2017-12-07 9:13 ` Yann E. MORIN
@ 2017-12-08 23:35 ` Cam Hutchison
0 siblings, 0 replies; 4+ messages in thread
From: Cam Hutchison @ 2017-12-08 23:35 UTC (permalink / raw)
To: buildroot
On 7 December 2017 at 20:13, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Cam, All,
>
> On 2017-12-07 19:18 +1100, Cam Hutchison spake thusly:
>> On 7 December 2017 at 09:58, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>> > Since 0542bb79e8 (uboot: Support multiple environment source files),
>> > a missing user-supplied environment source files is no longer detected.
>> >
>> > This is because we cat them all, and feed the concatenation to the stdin
>> > of mkenvimage. So, if one source file is missing, the cat exits in error,
>> > but the compound command exits with the exit code of the last command,
>> > which is that of mkenvimage, which happens to be happy with whatever it
>> > is fed on its stdin, even is empty.
>> >
>> > We fix that by creating a temporary file, that we even leave afterward
>> > for the user to inspect.
>> >
>> > We also move it out of the _CMDS block and into a macro of its own, so
>> > that it is easier to write and maintain.
>> >
>> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>> > Cc: Cam Hutchison <camh@xdna.net>
>> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> >
>> > ---
>> > Changes v1 -> v2:
>> > - move into its own macro (Thomas)
>> > ---
>> > boot/uboot/uboot.mk | 19 +++++++++++++------
>> > 1 file changed, 13 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
>> > index a1fac7dcae..abb89da69b 100644
>> > --- a/boot/uboot/uboot.mk
>> > +++ b/boot/uboot/uboot.mk
>> > @@ -238,6 +238,18 @@ define UBOOT_BUILD_OMAP_IFT
>> > -c $(call qstrip,$(BR2_TARGET_UBOOT_OMAP_IFT_CONFIG))
>> > endef
>> >
>> > +ifneq ($(BR2_TARGET_UBOOT_ENVIMAGE),)
>> > +define UBOOT_GENERATE_ENV_IMAGE
>> > + cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) \
>> > + >$(BINARIES_DIR)/uboot-env.txt
>>
>> One (very minor) concern is using the name "uboot-env.txt". It is
>> possible that someone has
>> a post-image script that creates a file of that name in BINARIES_DIR,
>> so this will clobber
>> that file. As a tmp file, perhaps it should have a tmp file name?
>>
>> I have a post-image script that creates a file of this name, but it
>> goes at a higher-level in
>> a binaries-type dir - I have a multi-layered build script, and the
>> higher-level directory that
>> aggregates the layers protects my file from being clobbered, but I can
>> imagine that if I
>> made different decisions when writing my front-end script, I could
>> have easily placed
>> my uboot-env.txt file in buildroot's BINARIES_DIR.
>>
>> Perhaps _uboot-env.txt? Or go all the way and use mktemp?
>
> Or just generate uboot-env.txt in $(@D)/uboot-env.buildroot like so:
>
> define UBOOT_GENERATE_ENV_IMAGE
> cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) \
> >$(@D)/uboot-env.buildroot
> $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
> $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
> $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
> -o $(BINARIES_DIR)/uboot-env.bin \
> $(@D)/uboot-env.buildroot
> endef
>
> Thoughts?
I like this better.
LGTM
> Regards,
> Yann E. MORIN.
>
>> > + $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
>> > + $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
>> > + $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
>> > + -o $(BINARIES_DIR)/uboot-env.bin \
>> > + $(BINARIES_DIR)/uboot-env.txt
>> > +endef
>> > +endif
>> > +
>> > define UBOOT_INSTALL_IMAGES_CMDS
>> > $(foreach f,$(UBOOT_BINS), \
>> > cp -dpf $(@D)/$(f) $(BINARIES_DIR)/
>> > @@ -249,12 +261,7 @@ define UBOOT_INSTALL_IMAGES_CMDS
>> > cp -dpf $(@D)/$(f) $(BINARIES_DIR)/
>> > )
>> > )
>> > - $(if $(BR2_TARGET_UBOOT_ENVIMAGE),
>> > - cat $(call qstrip,$(BR2_TARGET_UBOOT_ENVIMAGE_SOURCE)) | \
>> > - $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
>> > - $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
>> > - $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
>> > - -o $(BINARIES_DIR)/uboot-env.bin -)
>> > + $(UBOOT_GENERATE_ENV_IMAGE)
>> > $(if $(BR2_TARGET_UBOOT_BOOT_SCRIPT),
>> > $(HOST_DIR)/bin/mkimage -C none -A $(MKIMAGE_ARCH) -T script \
>> > -d $(call qstrip,$(BR2_TARGET_UBOOT_BOOT_SCRIPT_SOURCE)) \
>> > --
>> > 2.11.0
>> >
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> '------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-08 23:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-06 22:58 [Buildroot] [PATCHv2] package/uboot: detect missing user-supplied environment source files Yann E. MORIN
2017-12-07 8:18 ` Cam Hutchison
2017-12-07 9:13 ` Yann E. MORIN
2017-12-08 23:35 ` Cam Hutchison
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.