[U-Boot] [PATCH v2] Makefile: fix broken pipe error for lcd4_lwmon5 board

[Simon Glass]

Hi Masahiro,

On 18 December 2013 03:00, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> Before this commit, a broken pipe error sometimes happened
> when building lcd4_lwmon5 board with Buildman.
>
> This commit re-writes build rules of
> u-boot.spr and u-boot-img-spl-at-end.bin
> more simply without using a pipe.
>
> Besides fixing a broken pipe error,
> this commit gives us other advantages:
>
>   - Do not generate intermidiate files, spl/u-boot-spl.img
>     and spl/u-boot-spl-pad.img for creating u-boot.spr
>
>   - Do not generate an intermidiate file, u-boot-pad.img
>     for creating u-boot-img-spl-at-end.bin
>
> Such intermidiate files were not deleted by "make clean" or "make mrpropr".
> Nor u-boot-pad.img was ignored by git.
>
> Signed-off-by: Masahiro Yamada <yamada.m at jp.panasonic.com>
> Acked-by: Stefan Roese <sr at denx.de>
> ---
>
> Hello Simon.
>
> I noticed a broken pipe error when I was using Buildman.
> The error happens here:
>
>   $(obj)u-boot-img-spl-at-end.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.img
>                 tr "\000" "\377" < /dev/zero | dd ibs=1 count=$(CONFIG_UBOOT_PAD_TO) \
>                         of=$(obj)u-boot-pad.img 2>/dev/null
>
> I cannot understand why this error happens on Buildman.
> (I have never seen it on MAKEALL script.)
> Does the same error happen on your machine too?
> Do you have any idea why this error happen?
>
> What I did is as follows:
>
>   $ git checkout master
>
>   $ git describe
>   v2014.01-rc2
>
>   $ git checkout -b cosmetic
>
>     Add many cosmetic commits. Do not change any source files.
>     (For example, changing README files)
>     I think about 20 commits are enough.
>
>   $ git branch --set-upstream-to=master
>
>   $ tool/buildman/buildman -b cosmetic lcd4_lwmon5
>   No section: 'make-flags'
>   Building 21 commits for 1 boards (1 thread, 8 jobs per thread)
>   Cloning repo for thread 0
>      14    7    0 /21     lcd4_lwmon5
>
>   $ tools/buildman/buildman -se -b cosmetic
>   No section: 'make-flags'
>   Summary of 21 commits for 1194 boards (8 threads, 1 job per thread)
>   01: Prepare v2014.01-rc2
>      powerpc: +   lcd4_lwmon5
>   +tr: write error: Broken pipe
>   +tr: write error
>   02: cosmetic1
>      powerpc:    lcd4_lwmon5
>   -tr: write error: Broken pipe
>   -tr: write error
>   03: cosmetic2
>   04: cosmetic3
>   05: cosmetic4
>   06: cosmetic5
>   07: cosmetic6
>   08: cosmetic7
>   09: cosmetic8
>   10: cosmetic9
>   11: cosmetic10
>   12: cosmetic11
>   13: cosmetic12
>   14: cosmetic13
>   15: cosmetic14
>   16: cosmetic15
>      powerpc: +   lcd4_lwmon5
>   +tr: write error: Broken pipe
>   +tr: write error
>   17: cosmetic16
>   18: cosmetic17
>   19: cosmetic18
>   20: cosmetic19
>   21: cosmetic20
>
> Here, a broken pipe error occurred at 01 and fixed at 02.
> It happend again at 16.

Since buildman does incremental builds by default, if nothing changes
in the the build, that Makefile rule may not be invoked. It is
possible that commits 03 to 15 don't update anything which causes the
file to be rebuilt.

As to the real problem, I'm not sure, but it could be that the stderr
redirect causes problems. Perhaps there is something wrong in
buildman's handling here, but I can't see a problem. Anyway your fixed
version of the Makefile looks much nicer to me.

>
> But the result changes every time.
> The error happens randomly.
>

That conflicts with my comment above. I suppose another possibility is
a race condition in the Makefile brought on by -j.

Regards,
Simon

>
> Changes in v2:
>   - Delete an unnecessary line
>            cp $(obj)u-boot.img $@
>
>  Makefile | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f03d116..c9d372b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -471,12 +471,10 @@ $(obj)u-boot.sb:       $(obj)u-boot.bin $(obj)spl/u-boot-spl.bin
>  $(obj)u-boot.spr:      $(obj)u-boot.img $(obj)spl/u-boot-spl.bin
>                 $(obj)tools/mkimage -A $(ARCH) -T firmware -C none \
>                 -a $(CONFIG_SPL_TEXT_BASE) -e $(CONFIG_SPL_TEXT_BASE) -n XLOADER \
> -               -d $(obj)spl/u-boot-spl.bin $(obj)spl/u-boot-spl.img
> -               tr "\000" "\377" < /dev/zero | dd ibs=1 count=$(CONFIG_SPL_PAD_TO) \
> -                       of=$(obj)spl/u-boot-spl-pad.img 2>/dev/null
> -               dd if=$(obj)spl/u-boot-spl.img of=$(obj)spl/u-boot-spl-pad.img \
> -                       conv=notrunc 2>/dev/null
> -               cat $(obj)spl/u-boot-spl-pad.img $(obj)u-boot.img > $@
> +               -d $(obj)spl/u-boot-spl.bin $@
> +               $(OBJCOPY) -I binary -O binary \
> +                       --pad-to=$(CONFIG_SPL_PAD_TO) --gap-fill=0xff $@
> +               cat $(obj)u-boot.img >> $@
>
>  ifneq ($(CONFIG_TEGRA),)
>  $(obj)u-boot-nodtb-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin
> @@ -499,11 +497,9 @@ $(obj)u-boot-img.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.img
>  # at the start padded up to the start of the SPL image. And then concat
>  # the SPL image to the end.
>  $(obj)u-boot-img-spl-at-end.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.img
> -               tr "\000" "\377" < /dev/zero | dd ibs=1 count=$(CONFIG_UBOOT_PAD_TO) \
> -                       of=$(obj)u-boot-pad.img 2>/dev/null
> -               dd if=$(obj)u-boot.img of=$(obj)u-boot-pad.img \
> -                       conv=notrunc 2>/dev/null
> -               cat $(obj)u-boot-pad.img $(obj)spl/u-boot-spl.bin > $@
> +               $(OBJCOPY) -I binary -O binary --pad-to=$(CONFIG_UBOOT_PAD_TO) \
> +                        --gap-fill=0xff $(obj)u-boot.img $@
> +               cat $(obj)spl/u-boot-spl.bin >> $@
>
>  ifeq ($(CONFIG_SANDBOX),y)
>  GEN_UBOOT = \
> --
> 1.8.3.2
>