Message ID | 20210125145742.42460-2-matthew.weber@rockwellcollins.com |
---|---|
State | Superseded |
Headers | show |
Series | [1/2] package/uboot-tools: resolve uboot env source file error | expand |
On Mon, 25 Jan 2021 08:57:42 -0600 Matt Weber <matthew.weber@rockwellcollins.com> wrote: > From: Kalpesh Panchal <kalpesh.panchal@rockwellcollins.com> > > The host build of uboot-tools can occur early in the build process and may > require the creation of BINARIES_DIR before generation of an enabled envimage > and/or boot script binary. > > Signed-off-by: Kalpesh Panchal <kalpesh.panchal@rockwellcollins.com> > Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com> Hum, while this works, I'm in fact not too happy with the proposed solution. I would prefer that we move the mkenvimage/$(MKIMAGE) invocations to a BUILD_CMDS step, that produces its results in $(@D), and then INSTALL_CMDS does a $(INSTALL) ... of that file, which with the -D option would create $(BINARIES_DIR). I know it's a bit bike-shedding, but it feels a little bit better. What do you think ? Thomas
Thomas/Yann, On Mon, Jan 25, 2021 at 3:18 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Mon, 25 Jan 2021 08:57:42 -0600 > Matt Weber <matthew.weber@rockwellcollins.com> wrote: > > > From: Kalpesh Panchal <kalpesh.panchal@rockwellcollins.com> > > > > The host build of uboot-tools can occur early in the build process and may > > require the creation of BINARIES_DIR before generation of an enabled envimage > > and/or boot script binary. > > > > Signed-off-by: Kalpesh Panchal <kalpesh.panchal@rockwellcollins.com> > > Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com> > > Hum, while this works, I'm in fact not too happy with the proposed > solution. I would prefer that we move the mkenvimage/$(MKIMAGE) > invocations to a BUILD_CMDS step, that produces its results in $(@D), > and then INSTALL_CMDS does a $(INSTALL) ... of that file, which with > the -D option would create $(BINARIES_DIR). > > I know it's a bit bike-shedding, but it feels a little bit better. What > do you think ? I've added Yann to the discussion as he suggested the mkdir :-) Whatever is better to maintain. -Matt
Matthew, Thomas, All, On 2021-01-25 15:22 -0600, Matthew Weber via buildroot spake thusly: > On Mon, Jan 25, 2021 at 3:18 PM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > On Mon, 25 Jan 2021 08:57:42 -0600 > > Matt Weber <matthew.weber@rockwellcollins.com> wrote: > > > From: Kalpesh Panchal <kalpesh.panchal@rockwellcollins.com> > > > The host build of uboot-tools can occur early in the build process and may > > > require the creation of BINARIES_DIR before generation of an enabled envimage > > > and/or boot script binary. > > > > > > Signed-off-by: Kalpesh Panchal <kalpesh.panchal@rockwellcollins.com> > > > Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com> > > > > Hum, while this works, I'm in fact not too happy with the proposed > > solution. I would prefer that we move the mkenvimage/$(MKIMAGE) > > invocations to a BUILD_CMDS step, that produces its results in $(@D), > > and then INSTALL_CMDS does a $(INSTALL) ... of that file, which with > > the -D option would create $(BINARIES_DIR). > > > > I know it's a bit bike-shedding, but it feels a little bit better. What > > do you think ? > > I've added Yann to the discussion as he suggested the mkdir :-) On principle, I do agree with Thomas. However, I would point out that this is way wider than just the script image. Indeed, we also have HOST_UBOOT_TOOLS_GENERATE_ENV_IMAGE which is only called at install time. If we wanted to mandate clean, separate build and install steps for the script image generation, so would we want for the environment image too. Which is a wider endeavour than just catering for the script image. Of course, if one would want to actually tackle the issue and actually separate the build and install steps, that would be awesome. Otherwise, I am fine with just the mkdir. Regards, Yann E. MORIN.
On Mon, 25 Jan 2021 22:42:55 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > On principle, I do agree with Thomas. > > However, I would point out that this is way wider than just the script > image. Indeed, we also have HOST_UBOOT_TOOLS_GENERATE_ENV_IMAGE which > is only called at install time. Well, the patch is also adding a mkdir inside the code generating the environment image. Thomas
All, On Mon, Jan 25, 2021 at 4:23 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Mon, 25 Jan 2021 22:42:55 +0100 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > On principle, I do agree with Thomas. > > > > However, I would point out that this is way wider than just the script > > image. Indeed, we also have HOST_UBOOT_TOOLS_GENERATE_ENV_IMAGE which > > is only called at install time. > > Well, the patch is also adding a mkdir inside the code generating the > environment image. v2 will be out in a bit, i'll mark this one as superseded. Regards, Matt
diff --git a/package/uboot-tools/uboot-tools.mk b/package/uboot-tools/uboot-tools.mk index 10cbd1cdd9..f9ff170266 100644 --- a/package/uboot-tools/uboot-tools.mk +++ b/package/uboot-tools/uboot-tools.mk @@ -128,6 +128,7 @@ endif define HOST_UBOOT_TOOLS_GENERATE_ENV_IMAGE $(HOST_UBOOT_TOOLS_GENERATE_ENV_DEFAULTS) + mkdir -p $(BINARIES_DIR) $(HOST_DIR)/bin/mkenvimage -s $(BR2_PACKAGE_HOST_UBOOT_TOOLS_ENVIMAGE_SIZE) \ $(if $(BR2_PACKAGE_HOST_UBOOT_TOOLS_ENVIMAGE_REDUNDANT),-r) \ $(if $(filter "BIG",$(BR2_ENDIAN)),-b) \ @@ -164,6 +165,7 @@ define HOST_UBOOT_TOOLS_INSTALL_CMDS $(INSTALL) -m 0755 -D $(@D)/tools/dumpimage $(HOST_DIR)/bin/dumpimage $(HOST_UBOOT_TOOLS_GENERATE_ENV_IMAGE) $(if $(BR2_PACKAGE_HOST_UBOOT_TOOLS_BOOT_SCRIPT), + mkdir -p $(BINARIES_DIR); \ $(MKIMAGE) -C none -A $(MKIMAGE_ARCH) -T script \ -d $(call qstrip,$(BR2_PACKAGE_HOST_UBOOT_TOOLS_BOOT_SCRIPT_SOURCE)) \ $(BINARIES_DIR)/boot.scr)