diff mbox series

Makefile: Fix u-boot-nodtb.bin target

Message ID 20201007133940.21028-1-pali@kernel.org
State Accepted
Commit ae897022d7bdf1466584f54655efe89e2f67ca82
Delegated to: Tom Rini
Headers show
Series Makefile: Fix u-boot-nodtb.bin target | expand

Commit Message

Pali Rohár Oct. 7, 2020, 1:39 p.m. UTC
This change fixes two issues when building u-boot-nodtb.bin target:

* Remove intermediate binary u-boot-nodtb.bin from disk when static_rela
  call (which modifies u-boot-nodtb.bin binary) failed. It is required
  because previous objcopy call creates binary and static_rela finish it.

* Do not call static_rela cmd when u-boot-nodtb.bin binary was not
  created/updated by previous objcopy call.

Second fix would ensure that u-boot-nodtb.bin binary is not updated when
all prerequisites were up-to-date. And therefore final binary u-boot.bin
is not updated in case all prerequisites were not modified and were
up-to-date.

Now running 'make SOURCE_DATE_EPOCH=0 u-boot.bin' second time now does not
touch u-boot.bin binary in case nothing was modified, so GNU make can
correctly detect that everything is up-to-date.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 Makefile | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Simon Glass Oct. 12, 2020, 3:34 a.m. UTC | #1
On Wed, 7 Oct 2020 at 07:39, Pali Rohár <pali@kernel.org> wrote:
>
> This change fixes two issues when building u-boot-nodtb.bin target:
>
> * Remove intermediate binary u-boot-nodtb.bin from disk when static_rela
>   call (which modifies u-boot-nodtb.bin binary) failed. It is required
>   because previous objcopy call creates binary and static_rela finish it.
>
> * Do not call static_rela cmd when u-boot-nodtb.bin binary was not
>   created/updated by previous objcopy call.
>
> Second fix would ensure that u-boot-nodtb.bin binary is not updated when
> all prerequisites were up-to-date. And therefore final binary u-boot.bin
> is not updated in case all prerequisites were not modified and were
> up-to-date.
>
> Now running 'make SOURCE_DATE_EPOCH=0 u-boot.bin' second time now does not
> touch u-boot.bin binary in case nothing was modified, so GNU make can
> correctly detect that everything is up-to-date.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  Makefile | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tom Rini Oct. 24, 2020, 2:51 p.m. UTC | #2
On Wed, Oct 07, 2020 at 03:39:40PM +0200, Pali Rohár wrote:

> This change fixes two issues when building u-boot-nodtb.bin target:
> 
> * Remove intermediate binary u-boot-nodtb.bin from disk when static_rela
>   call (which modifies u-boot-nodtb.bin binary) failed. It is required
>   because previous objcopy call creates binary and static_rela finish it.
> 
> * Do not call static_rela cmd when u-boot-nodtb.bin binary was not
>   created/updated by previous objcopy call.
> 
> Second fix would ensure that u-boot-nodtb.bin binary is not updated when
> all prerequisites were up-to-date. And therefore final binary u-boot.bin
> is not updated in case all prerequisites were not modified and were
> up-to-date.
> 
> Now running 'make SOURCE_DATE_EPOCH=0 u-boot.bin' second time now does not
> touch u-boot.bin binary in case nothing was modified, so GNU make can
> correctly detect that everything is up-to-date.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!
Patrick DELAUNAY Nov. 2, 2020, 1:25 p.m. UTC | #3
Hi Tom and Pali,

> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Tom Rini
> Sent: samedi 24 octobre 2020 16:52
> 
> On Wed, Oct 07, 2020 at 03:39:40PM +0200, Pali Rohár wrote:
> 
> > This change fixes two issues when building u-boot-nodtb.bin target:
> >
> > * Remove intermediate binary u-boot-nodtb.bin from disk when static_rela
> >   call (which modifies u-boot-nodtb.bin binary) failed. It is required
> >   because previous objcopy call creates binary and static_rela finish it.
> >
> > * Do not call static_rela cmd when u-boot-nodtb.bin binary was not
> >   created/updated by previous objcopy call.
> >
> > Second fix would ensure that u-boot-nodtb.bin binary is not updated
> > when all prerequisites were up-to-date. And therefore final binary
> > u-boot.bin is not updated in case all prerequisites were not modified
> > and were up-to-date.
> >
> > Now running 'make SOURCE_DATE_EPOCH=0 u-boot.bin' second time now
> does
> > not touch u-boot.bin binary in case nothing was modified, so GNU make
> > can correctly detect that everything is up-to-date.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Applied to u-boot/master, thanks!

Since this patch is applyied on master branch, I have error when I compile with the verbose option V=1.

I don't found a correction for this issue until now... except revert this commit.

For information: I use stm32mp1 platform = ARMv7 32bits
I have the error:

scripts/Kbuild.include:220: *** Recursive variable 'echo-cmd' references itself (eventually).  Stop.
make[1]: *** Waiting for unfinished jobs....
make -f /local/home/frq07632/views/u-boot/scripts/Makefile.build obj=arch/arm/dts dtbs

 
> --
> Tom

Regards
Patrick
Pali Rohár Nov. 2, 2020, 2:15 p.m. UTC | #4
On Monday 02 November 2020 13:25:56 Patrick DELAUNAY wrote:
> Hi Tom and Pali,
> 
> > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Tom Rini
> > Sent: samedi 24 octobre 2020 16:52
> > 
> > On Wed, Oct 07, 2020 at 03:39:40PM +0200, Pali Rohár wrote:
> > 
> > > This change fixes two issues when building u-boot-nodtb.bin target:
> > >
> > > * Remove intermediate binary u-boot-nodtb.bin from disk when static_rela
> > >   call (which modifies u-boot-nodtb.bin binary) failed. It is required
> > >   because previous objcopy call creates binary and static_rela finish it.
> > >
> > > * Do not call static_rela cmd when u-boot-nodtb.bin binary was not
> > >   created/updated by previous objcopy call.
> > >
> > > Second fix would ensure that u-boot-nodtb.bin binary is not updated
> > > when all prerequisites were up-to-date. And therefore final binary
> > > u-boot.bin is not updated in case all prerequisites were not modified
> > > and were up-to-date.
> > >
> > > Now running 'make SOURCE_DATE_EPOCH=0 u-boot.bin' second time now
> > does
> > > not touch u-boot.bin binary in case nothing was modified, so GNU make
> > > can correctly detect that everything is up-to-date.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > 
> > Applied to u-boot/master, thanks!
> 
> Since this patch is applyied on master branch, I have error when I compile with the verbose option V=1.
> 
> I don't found a correction for this issue until now... except revert this commit.
> 
> For information: I use stm32mp1 platform = ARMv7 32bits
> I have the error:
> 
> scripts/Kbuild.include:220: *** Recursive variable 'echo-cmd' references itself (eventually).  Stop.
> make[1]: *** Waiting for unfinished jobs....
> make -f /local/home/frq07632/views/u-boot/scripts/Makefile.build obj=arch/arm/dts dtbs

Hello Patrick!

Now I was able to trigger this issue, but it seems to be present only
sometimes. Calling make more times hides that issue, so it looks like
something broken in Kconfig dependency generator for Makefiles... Or
maybe something dependent on GNU make...

I played a bit with it and after changing following line in that patch

shell_cmd = { $(echo-cmd) $(cmd_$(1)); }

by

shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); }

that issue disappeared. But because it triggered on my machine only
sometimes, I do not know if it finally fixes it.

Could you please try this change? And do you have reproducer how to
_always_ trigger it and not only sometimes?

> 
>  
> > --
> > Tom
> 
> Regards
> Patrick
Patrick DELAUNAY Nov. 4, 2020, 8:53 a.m. UTC | #5
Hi Pali,


> From: Pali Rohár <pali@kernel.org>
> Sent: lundi 2 novembre 2020 15:16
> 
> On Monday 02 November 2020 13:25:56 Patrick DELAUNAY wrote:
> > Hi Tom and Pali,
> >
> > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Tom Rini
> > > Sent: samedi 24 octobre 2020 16:52
> > >
> > > On Wed, Oct 07, 2020 at 03:39:40PM +0200, Pali Rohár wrote:
> > >
> > > > This change fixes two issues when building u-boot-nodtb.bin target:
> > > >
> > > > * Remove intermediate binary u-boot-nodtb.bin from disk when static_rela
> > > >   call (which modifies u-boot-nodtb.bin binary) failed. It is required
> > > >   because previous objcopy call creates binary and static_rela finish it.
> > > >
> > > > * Do not call static_rela cmd when u-boot-nodtb.bin binary was not
> > > >   created/updated by previous objcopy call.
> > > >
> > > > Second fix would ensure that u-boot-nodtb.bin binary is not
> > > > updated when all prerequisites were up-to-date. And therefore
> > > > final binary u-boot.bin is not updated in case all prerequisites
> > > > were not modified and were up-to-date.
> > > >
> > > > Now running 'make SOURCE_DATE_EPOCH=0 u-boot.bin' second time
> now
> > > does
> > > > not touch u-boot.bin binary in case nothing was modified, so GNU
> > > > make can correctly detect that everything is up-to-date.
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > Applied to u-boot/master, thanks!
> >
> > Since this patch is applyied on master branch, I have error when I compile with
> the verbose option V=1.
> >
> > I don't found a correction for this issue until now... except revert this commit.
> >
> > For information: I use stm32mp1 platform = ARMv7 32bits I have the
> > error:
> >
> > scripts/Kbuild.include:220: *** Recursive variable 'echo-cmd' references itself
> (eventually).  Stop.
> > make[1]: *** Waiting for unfinished jobs....
> > make -f /local/home/frq07632/views/u-boot/scripts/Makefile.build
> > obj=arch/arm/dts dtbs
> 
> Hello Patrick!
> 
> Now I was able to trigger this issue, but it seems to be present only sometimes.
> Calling make more times hides that issue, so it looks like something broken in
> Kconfig dependency generator for Makefiles... Or maybe something dependent on
> GNU make...
> 
> I played a bit with it and after changing following line in that patch
> 
> shell_cmd = { $(echo-cmd) $(cmd_$(1)); }
> 
> by
> 
> shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); }
> 
> that issue disappeared. But because it triggered on my machine only sometimes, I
> do not know if it finally fixes it.

on my machine (Ubuntu 18.04 / fast laptop with Core i7) it is 100% reproduce with:
1/ gcc version 9.2.1 20191025 (GNU Toolchain for the A-profile Architecture 9.2-2019.12 (arm-9.10))

2/ make --version
GNU Make 4.1
Built for x86_64-pc-linux-gnu

3/ the request
export CROSS_COMPILE="arm-none-linux-gnueabihf-"
make stm32mp15_basic_defconfig
make V=1 all

 > Could you please try this change? And do you have reproducer how to _always_
> trigger it and not only sometimes?

I test it with the change:

-shell_cmd = { $(echo-cmd) $(cmd_$(1)); }
+shell_cmd = { $(call echo-cmd, $(1)) $(cmd_$(1)); }

And I never reproduced the error again and command is correctly displayed with V=1:

arm-none-linux-gnueabihf-objcopy --gap-fill=0xff  -j .text -j .secure_text -j .secure_data -j .rodata -j .hash -j .data -j .got -j .got.plt -j .u_boot_list -j .rel.dyn -j .binman_sym_table -j .text_rest -j .dtb.init.rodata -j .efi_runtime -j .efi_runtime_rel -O binary   u-boot u-boot-nodtb.bin && {  true; } || rm -f u-boot-nodtb.bin

or without option 

OBJCOPY u-boot-nodtb.bin

So I think that the issue is fixed.

Regards
Patrick
Pali Rohár Nov. 4, 2020, 9:32 a.m. UTC | #6
On Wednesday 04 November 2020 08:53:07 Patrick DELAUNAY wrote:
> Hi Pali,
> 
> 
> > From: Pali Rohár <pali@kernel.org>
> > Sent: lundi 2 novembre 2020 15:16
> > 
> > On Monday 02 November 2020 13:25:56 Patrick DELAUNAY wrote:
> > > Hi Tom and Pali,
> > >
> > > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Tom Rini
> > > > Sent: samedi 24 octobre 2020 16:52
> > > >
> > > > On Wed, Oct 07, 2020 at 03:39:40PM +0200, Pali Rohár wrote:
> > > >
> > > > > This change fixes two issues when building u-boot-nodtb.bin target:
> > > > >
> > > > > * Remove intermediate binary u-boot-nodtb.bin from disk when static_rela
> > > > >   call (which modifies u-boot-nodtb.bin binary) failed. It is required
> > > > >   because previous objcopy call creates binary and static_rela finish it.
> > > > >
> > > > > * Do not call static_rela cmd when u-boot-nodtb.bin binary was not
> > > > >   created/updated by previous objcopy call.
> > > > >
> > > > > Second fix would ensure that u-boot-nodtb.bin binary is not
> > > > > updated when all prerequisites were up-to-date. And therefore
> > > > > final binary u-boot.bin is not updated in case all prerequisites
> > > > > were not modified and were up-to-date.
> > > > >
> > > > > Now running 'make SOURCE_DATE_EPOCH=0 u-boot.bin' second time
> > now
> > > > does
> > > > > not touch u-boot.bin binary in case nothing was modified, so GNU
> > > > > make can correctly detect that everything is up-to-date.
> > > > >
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > Applied to u-boot/master, thanks!
> > >
> > > Since this patch is applyied on master branch, I have error when I compile with
> > the verbose option V=1.
> > >
> > > I don't found a correction for this issue until now... except revert this commit.
> > >
> > > For information: I use stm32mp1 platform = ARMv7 32bits I have the
> > > error:
> > >
> > > scripts/Kbuild.include:220: *** Recursive variable 'echo-cmd' references itself
> > (eventually).  Stop.
> > > make[1]: *** Waiting for unfinished jobs....
> > > make -f /local/home/frq07632/views/u-boot/scripts/Makefile.build
> > > obj=arch/arm/dts dtbs
> > 
> > Hello Patrick!
> > 
> > Now I was able to trigger this issue, but it seems to be present only sometimes.
> > Calling make more times hides that issue, so it looks like something broken in
> > Kconfig dependency generator for Makefiles... Or maybe something dependent on
> > GNU make...
> > 
> > I played a bit with it and after changing following line in that patch
> > 
> > shell_cmd = { $(echo-cmd) $(cmd_$(1)); }
> > 
> > by
> > 
> > shell_cmd = { $(call echo-cmd,$(1)) $(cmd_$(1)); }
> > 
> > that issue disappeared. But because it triggered on my machine only sometimes, I
> > do not know if it finally fixes it.
> 
> on my machine (Ubuntu 18.04 / fast laptop with Core i7) it is 100% reproduce with:
> 1/ gcc version 9.2.1 20191025 (GNU Toolchain for the A-profile Architecture 9.2-2019.12 (arm-9.10))
> 
> 2/ make --version
> GNU Make 4.1
> Built for x86_64-pc-linux-gnu
> 
> 3/ the request
> export CROSS_COMPILE="arm-none-linux-gnueabihf-"
> make stm32mp15_basic_defconfig
> make V=1 all
> 
>  > Could you please try this change? And do you have reproducer how to _always_
> > trigger it and not only sometimes?
> 
> I test it with the change:
> 
> -shell_cmd = { $(echo-cmd) $(cmd_$(1)); }
> +shell_cmd = { $(call echo-cmd, $(1)) $(cmd_$(1)); }
> 
> And I never reproduced the error again and command is correctly displayed with V=1:
> 
> arm-none-linux-gnueabihf-objcopy --gap-fill=0xff  -j .text -j .secure_text -j .secure_data -j .rodata -j .hash -j .data -j .got -j .got.plt -j .u_boot_list -j .rel.dyn -j .binman_sym_table -j .text_rest -j .dtb.init.rodata -j .efi_runtime -j .efi_runtime_rel -O binary   u-boot u-boot-nodtb.bin && {  true; } || rm -f u-boot-nodtb.bin
> 
> or without option 
> 
> OBJCOPY u-boot-nodtb.bin
> 
> So I think that the issue is fixed.

Ok, thank you for testing! I will send a proper git format patch to ML.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index d4736bb501..91b6d2c2b3 100644
--- a/Makefile
+++ b/Makefile
@@ -884,7 +884,7 @@  cmd_static_rela = \
 	tools/relocate-rela $(3) $(4) $$start $$end
 else
 quiet_cmd_static_rela =
-cmd_static_rela =
+cmd_static_rela = true
 endif
 
 # Always append INPUTS so that arch config.mk's can add custom ones
@@ -1317,9 +1317,13 @@  init_sp_bss_offset_check: u-boot.dtb FORCE
 	fi
 endif
 
+shell_cmd = { $(echo-cmd) $(cmd_$(1)); }
+
+quiet_cmd_objcopy_uboot = OBJCOPY $@
+cmd_objcopy_uboot = $(cmd_objcopy) && $(call shell_cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE)) || rm -f $@
+
 u-boot-nodtb.bin: u-boot FORCE
-	$(call if_changed,objcopy)
-	$(call cmd,static_rela,$<,$@,$(CONFIG_SYS_TEXT_BASE))
+	$(call if_changed,objcopy_uboot)
 	$(BOARD_SIZE_CHECK)
 
 u-boot.ldr:	u-boot