diff mbox

[v3] boot/uboot: fix build for sunxi targets

Message ID 20170309115247.16981-1-joerg.krause@embedded.rocks
State Rejected
Headers show

Commit Message

Jörg Krause March 9, 2017, 11:52 a.m. UTC
Since version 2017.01 U-Boot needs to run the binman tool on the host machine
for building the 'u-boot-sunxi-with-spl.bin' target. The binman tool is written
in Python 2 and therefore needs a Python 2 interpreter. The python
scripts sets the shebang to: `#!/usr/bin/env python`. For modern Linux
distributions `python` is set to the Python 3 interpreter. In this
case, building U-Boot for a sunxi-based target fails:

```
  BINMAN  u-boot-sunxi-with-spl.bin
  File "./tools/binman/binman", line 49
    print result
               ^
SyntaxError: Missing parentheses in call to 'print'
make[1]: *** [Makefile:1090: u-boot-sunxi-with-spl.bin] Error 1
```

Add a post patch hook to force the shebang to python2 in case the binman
file exists.

Reported upstream:
https://lists.denx.de/pipermail/u-boot/2017-March/283164.html

Tested for U-Boot versions:
 * 2016.07 (no binman tool, custom version)
 * 2017.01 (binman tool, latest version)
 * 2017.03-rc3 (binman tool, custom version)

Signed-off-by: Jörg Krause <joerg.krause@embedded.rocks>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
v3:
 * use a post patch hook to support custom U-Boot versions (suggested by Thomas)
v2:
 * add patch to fix shebang instead of adding a host dependency for
   python (suggested by Peter Korsgaard)
---
 boot/uboot/uboot.mk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Thomas Petazzoni March 20, 2017, 9:32 p.m. UTC | #1
Hello,

On Thu,  9 Mar 2017 12:52:47 +0100, Jörg Krause wrote:

> +# The binman tool was introduced in version 2017.01. The tool is a set
> +# python scripts and requires a python2 interpreter. Force the shebang
> +# python2 to ensure the correct interpreter is used on host systems
> +# where python defaults to the python3 interpreter. 
> +define UBOOT_TRY_SED_SHEBANG_BINMAN
> +	if test -f $(@D)/tools/binman/binman; then \
> +		sed -i '1s_^\(#!/usr/bin/env \).*_\1python2_' \
> +			$(@D)/tools/binman/binman; \
> +	fi
> +endef
> +UBOOT_POST_PATCH_HOOKS += UBOOT_TRY_SED_SHEBANG_BINMAN

Thanks for this. However, now that the issue has been fixed upstream,
I'm not sure I'm willing to make our uboot.mk more complicated to fix a
problem that:

 1. Only occurs with sunxi targets and a few x86 platforms

 2. Only occurs with 2017.01 exactly. binman didn't exist before, and
    2017.03 was fixed in time.

So I'm tempted to put the burden on people using 2017.01 on those few
platforms to carry the small patch (or upgrade to 2017.03).

What do you think?

Thanks,

Thomas
Jörg Krause March 21, 2017, 7:36 a.m. UTC | #2
Hi Thomas,

On Mon, 2017-03-20 at 22:32 +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu,  9 Mar 2017 12:52:47 +0100, Jörg Krause wrote:
> 
> > +# The binman tool was introduced in version 2017.01. The tool is a
> > set
> > +# python scripts and requires a python2 interpreter. Force the
> > shebang
> > +# python2 to ensure the correct interpreter is used on host
> > systems
> > +# where python defaults to the python3 interpreter. 
> > +define UBOOT_TRY_SED_SHEBANG_BINMAN
> > +	if test -f $(@D)/tools/binman/binman; then \
> > +		sed -i '1s_^\(#!/usr/bin/env \).*_\1python2_' \
> > +			$(@D)/tools/binman/binman; \
> > +	fi
> > +endef
> > +UBOOT_POST_PATCH_HOOKS += UBOOT_TRY_SED_SHEBANG_BINMAN
> 
> Thanks for this. However, now that the issue has been fixed upstream,
> I'm not sure I'm willing to make our uboot.mk more complicated to fix
> a
> problem that:
> 
>  1. Only occurs with sunxi targets and a few x86 platforms
> 
>  2. Only occurs with 2017.01 exactly. binman didn't exist before, and
>     2017.03 was fixed in time.
> 
> So I'm tempted to put the burden on people using 2017.01 on those few
> platforms to carry the small patch (or upgrade to 2017.03).
> 
> What do you think?

I'm fine with dumping this patch. I'll mark it as superseded.

Jörg
Thomas Petazzoni March 21, 2017, 8:46 a.m. UTC | #3
Hello,

On Tue, 21 Mar 2017 08:36:25 +0100, Jörg Krause wrote:

> I'm fine with dumping this patch. I'll mark it as superseded.

Thanks!

Thomas
diff mbox

Patch

diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
index 1e22eaa8e..bbf61fca5 100644
--- a/boot/uboot/uboot.mk
+++ b/boot/uboot/uboot.mk
@@ -36,6 +36,18 @@  BR_NO_CHECK_HASH_FOR += $(UBOOT_SOURCE)
 endif
 endif
 
+# The binman tool was introduced in version 2017.01. The tool is a set
+# python scripts and requires a python2 interpreter. Force the shebang
+# python2 to ensure the correct interpreter is used on host systems
+# where python defaults to the python3 interpreter. 
+define UBOOT_TRY_SED_SHEBANG_BINMAN
+	if test -f $(@D)/tools/binman/binman; then \
+		sed -i '1s_^\(#!/usr/bin/env \).*_\1python2_' \
+			$(@D)/tools/binman/binman; \
+	fi
+endef
+UBOOT_POST_PATCH_HOOKS += UBOOT_TRY_SED_SHEBANG_BINMAN
+
 ifeq ($(BR2_TARGET_UBOOT_FORMAT_BIN),y)
 UBOOT_BINS += u-boot.bin
 endif