package/uboot: detect missing user-supplied environment source files

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" <[email protected]>

Cc Cam Hutchison <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
Changes v1 -> v2:
- move into its own macro (Thomas)

---
 boot/uboot/uboot.mk | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

boot/uboot/uboot.mk +0 -0

index a1fac7dcae..abb89da69b 100644

boot/uboot/uboot.mk +9 -7

+       $(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

Quoted 31 lines

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" <[email protected]>

Cc Cam Hutchison <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
Changes v1 -> v2:
- move into its own macro (Thomas)

---
 boot/uboot/uboot.mk | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

boot/uboot/uboot.mk +0 -0

index a1fac7dcae..abb89da69b 100644

boot/uboot/uboot.mk define UBOOT_BUILD_OMAP_IFT · line 244

Quoted 7 lines

boot/uboot/uboot.mk +4 -0

@@ -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?

Quoted 28 lines

boot/uboot/uboot.mk +9 -7

+       $(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

Cam, All,

On 2017-12-07 19:18 +1100, Cam Hutchison spake thusly:

Quoted 56 lines · Yann E. MORIN

Quoted 31 lines

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" <[email protected]>

Cc Cam Hutchison <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
Changes v1 -> v2:
- move into its own macro (Thomas)

---
 boot/uboot/uboot.mk | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

boot/uboot/uboot.mk +0 -0

index a1fac7dcae..abb89da69b 100644

boot/uboot/uboot.mk define UBOOT_BUILD_OMAP_IFT · line 244

Quoted 7 lines

boot/uboot/uboot.mk +4 -0

@@ -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.

Quoted 1 line

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.

Quoted 20 lines

Quoted 20 lines

boot/uboot/uboot.mk +8 -5

+       $(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) \

Quoted 8 lines

Quoted 8 lines

boot/uboot/uboot.mk +1 -2

-                       -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.  |

'------------------------------^-------^------------------^--------------------'