uboot-tools: bugfix libfdt python build

Message ID 20171013023556.25825-1-matthew.weber@rockwellcollins.com
State Changes Requested
Headers show
Series
  • uboot-tools: bugfix libfdt python build
Related show

Commit Message

Matt Weber Oct. 13, 2017, 2:35 a.m.
The libfdt is using setup.py to perform the build
of it's wrapper.  This is being invoked with the
system's python default vs the HOST_DIR's.  This
patch completes the uboot-tools source update to
support an optional python intepreter override and
for buildroot to supply that value when doing a host
build.

Bug/Feature submitted upstream:
https://lists.denx.de/pipermail/u-boot/2017-October/309250.html

Fixes:
http://autobuild.buildroot.net/results/101/1014a6c2728ef49b332aa3ed169125a0d5a0ae1e/

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
 ...libfdt-give-setup-py-optional-interpreter.patch | 31 ++++++++++++++++++++++
 package/uboot-tools/uboot-tools.mk                 |  9 ++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 package/uboot-tools/0004-libfdt-give-setup-py-optional-interpreter.patch

Comments

Thomas Petazzoni Oct. 13, 2017, 2:11 p.m. | #1
Hello,

On Thu, 12 Oct 2017 21:35:56 -0500, Matt Weber wrote:

> +ifeq ($(BR2_PACKAGE_PYTHON3),y)
> +HOST_UBOOT_TOOLS_DEPENDENCIES += host-python3
> +else
> +HOST_UBOOT_TOOLS_DEPENDENCIES += host-python
> +endif

Gaah, adding host-python as a mandatory dependency of host-uboot-tools
is a big change. Any way around that ?

Thomas
Matt Weber Oct. 14, 2017, 1:01 a.m. | #2
Thomas,

On Fri, Oct 13, 2017 at 9:11 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
>
> Hello,
>
> On Thu, 12 Oct 2017 21:35:56 -0500, Matt Weber wrote:
>
> > +ifeq ($(BR2_PACKAGE_PYTHON3),y)
> > +HOST_UBOOT_TOOLS_DEPENDENCIES += host-python3
> > +else
> > +HOST_UBOOT_TOOLS_DEPENDENCIES += host-python
> > +endif
>
> Gaah, adding host-python as a mandatory dependency of host-uboot-tools
> is a big change. Any way around that ?
>

I looked at the uboot python issue a bit more and I'll send something
upstream.  I'm thinking the best option would be to define an optional
SWIG variable providing an absolute path to the tool and include an
update to the SWIG detection logic using it.

Then Buildroot could support an option to enable the libftd in the
menu and let that select python.  This option would be applicable to
host and/or target, as I noticed target uboot-tools was also failing
just not as often.

So the python intepretor path is still valid, but if the plan above
sounds good,  I'll rework and remove the unconditional dependency.
Matt Weber Oct. 14, 2017, 9:44 p.m. | #3
Thomas, Fabio, Peter,

On Fri, Oct 13, 2017 at 8:01 PM, Matthew Weber
<matthew.weber@rockwellcollins.com> wrote:
> Thomas,
>
> On Fri, Oct 13, 2017 at 9:11 AM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>>
>> Hello,
>>
>> On Thu, 12 Oct 2017 21:35:56 -0500, Matt Weber wrote:
>>
>> > +ifeq ($(BR2_PACKAGE_PYTHON3),y)
>> > +HOST_UBOOT_TOOLS_DEPENDENCIES += host-python3
>> > +else
>> > +HOST_UBOOT_TOOLS_DEPENDENCIES += host-python
>> > +endif
>>
>> Gaah, adding host-python as a mandatory dependency of host-uboot-tools
>> is a big change. Any way around that ?
>>
>
> I looked at the uboot python issue a bit more and I'll send something
> upstream.  I'm thinking the best option would be to define an optional
> SWIG variable providing an absolute path to the tool and include an
> update to the SWIG detection logic using it.
>
> Then Buildroot could support an option to enable the libftd in the
> menu and let that select python.  This option would be applicable to
> host and/or target, as I noticed target uboot-tools was also failing
> just not as often.
>
> So the python intepretor path is still valid, but if the plan above
> sounds good,  I'll rework and remove the unconditional dependency.
>

I dug further into the python/swig issue in uboot/uboot-tools and
their build of libfdt.  I ended up with the following conclusions.
1) Need to bump both packages to 2017.9 which I noticed was already
underway (Thanks Fabio).  This removes part of this patch submission
that fixed the python interpreter (already upstream after 9/3)
2) Need something like the following added respectively with the new
*_PYTHON_DEFS variable added to the MAKE OPTS in both uboot and
uboot-tools
(package/uboot-tools.mk)
# BR2_TARGET_UBOOT_NEEDS_PYLIBFDT enables python2/swig host dependencies
# as part of uboot's dependencies, however that means they occur after
# this package in the build sequence. We add them here conditionally,
# to move them prior to this package if the option is selected.  If
# it isn't selected, we still need to pass a swig path so that the pkg's
# logic to include the _libfdt.so target drops that target as the path
isn't valid.
HOST_UBOOT_TOOLS_PYTHON_DEFS = SWIG_TOOL="$(HOST_DIR)/bin/swig"
ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYLIBFDT),y)
HOST_UBOOT_TOOLS_PYTHON_DEFS +=
PYTHON="$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR)"
HOST_UBOOT_TOOLS_DEPENDENCIES += host-python host-swig
endif

(boot/uboot.mk)
UBOOT_PYTHON_DEFS = SWIG_TOOL="$(HOST_DIR)/bin/swig"
ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYLIBFDT),y)
UBOOT_PYTHON_DEFS += PYTHON="$(HOST_DIR)/bin/python$(PYTHON_VERSION_MAJOR)"
UBOOT_DEPENDENCIES += host-python host-swig
endif

3) I need to send a patch upstream to get the SWIG_TOOL variable used
instead of a "which swig" currently used in the tools/Makefile to
trigger the setup.py build of libfdt.so.  This doesn't fix all
versions of uboot, just uboot-tools or uboots newer then a version.
Thus I wonder for the older uboot versions that still have a host
system dependency requirement, we add something like below in the
uboot.mk above the ifeq ($(BR2_TARGET_UBOOT_NEEDS_PYLIBFDT),y)

# There is a flaw in the tools/Makefile with the swig
# checking line added on 2017-05-27 (ee95d10b).  It doesn't
# use a variable which could be set to the $(HOST_DIR)/bin/swig.
# Until this is resolved, depending on the uboot version,
# there is still a host system requirement to have the following
# packages installed (apt install swig libpython-dev).   Otherwise
# you have to always have uboot select swig/python to prevent a
# build failure, or remove swig from your host system.


If all those changes go well, it means no requirement to build python
with any uboot unless you want libfdt.

I plan to get the swig patch submitted uboot upstream this week and
supersede this patch with a new patchset dependent on the 2017.9 bump.
It will include the new MAKE OPT updates.

Matt
Fabio Estevam Oct. 15, 2017, 12:09 p.m. | #4
Hi Matthew,

On Sat, Oct 14, 2017 at 6:44 PM, Matthew Weber
<matthew.weber@rockwellcollins.com> wrote:

> I dug further into the python/swig issue in uboot/uboot-tools and
> their build of libfdt.  I ended up with the following conclusions.
> 1) Need to bump both packages to 2017.9 which I noticed was already
> underway (Thanks Fabio).  This removes part of this patch submission
> that fixed the python interpreter (already upstream after 9/3)

I sent a patch bumping u-boot-tools to 2011.09, but Peter and Arnout
reported build issues with it, so that's why it has not been applied:
http://patchwork.ozlabs.org/patch/812555/

I haven't had a chance to look at this issue, but if you are
interested, feel free to address the problem and submit the bump
patch.

Thanks
Matt Weber Oct. 17, 2017, 6:16 p.m. | #5
All,

On Sun, Oct 15, 2017 at 7:09 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Matthew,
>
> On Sat, Oct 14, 2017 at 6:44 PM, Matthew Weber
> <matthew.weber@rockwellcollins.com> wrote:
>
>> I dug further into the python/swig issue in uboot/uboot-tools and
>> their build of libfdt.  I ended up with the following conclusions.
>> 1) Need to bump both packages to 2017.9 which I noticed was already
>> underway (Thanks Fabio).  This removes part of this patch submission
>> that fixed the python interpreter (already upstream after 9/3)
>
> I sent a patch bumping u-boot-tools to 2011.09, but Peter and Arnout
> reported build issues with it, so that's why it has not been applied:
> http://patchwork.ozlabs.org/patch/812555/
>
> I haven't had a chance to look at this issue, but if you are
> interested, feel free to address the problem and submit the bump
> patch.
>

Looking at the current development on uboot upstream, the 2017.11
release will be the bump we should take.  With that they cleaned up
the tools having some default python/swig dependencies by having
libfdt enabled in uboot kconfig.  We'll still have to update the
BR2_TARGET_UBOOT_NEEDS_PYLIBFDT option in the uboot package so that it
includes providing the PYTHON path variable to the build.  The
upstream changes also removed the need for this patchset against
uboot-tools fixing python and swig patch issues.

I'll send a proposed patchset for a 2017.11 bump later this month
after testing with a -rc

Matt
Thomas Petazzoni Oct. 17, 2017, 6:54 p.m. | #6
Hello,

On Tue, 17 Oct 2017 13:16:01 -0500, Matthew Weber wrote:

> Looking at the current development on uboot upstream, the 2017.11
> release will be the bump we should take.  With that they cleaned up
> the tools having some default python/swig dependencies by having
> libfdt enabled in uboot kconfig.  We'll still have to update the
> BR2_TARGET_UBOOT_NEEDS_PYLIBFDT option in the uboot package so that it
> includes providing the PYTHON path variable to the build.  The
> upstream changes also removed the need for this patchset against
> uboot-tools fixing python and swig patch issues.

Thanks for looking into this!

> I'll send a proposed patchset for a 2017.11 bump later this month
> after testing with a -rc

Do we have a workaround in the mean time, to avoid trashing the
autobuilder results?

Thanks!

Thomas
Matt Weber Oct. 17, 2017, 7 p.m. | #7
Thomas,

On Tue, Oct 17, 2017 at 1:54 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Tue, 17 Oct 2017 13:16:01 -0500, Matthew Weber wrote:
>
>> Looking at the current development on uboot upstream, the 2017.11
>> release will be the bump we should take.  With that they cleaned up
>> the tools having some default python/swig dependencies by having
>> libfdt enabled in uboot kconfig.  We'll still have to update the
>> BR2_TARGET_UBOOT_NEEDS_PYLIBFDT option in the uboot package so that it
>> includes providing the PYTHON path variable to the build.  The
>> upstream changes also removed the need for this patchset against
>> uboot-tools fixing python and swig patch issues.
>
> Thanks for looking into this!
>
>> I'll send a proposed patchset for a 2017.11 bump later this month
>> after testing with a -rc
>
> Do we have a workaround in the mean time, to avoid trashing the
> autobuilder results?
>

Yep, I'll get a patch together for swig/python workaround that doesn't
require 2017.9 to fix the autobuilder.  Fabio, assuming we'll skip
2017.9 bump and just go 2017.11?

Matt
Fabio Estevam Oct. 17, 2017, 7:03 p.m. | #8
Hi Matthew,

On Tue, Oct 17, 2017 at 5:00 PM, Matthew Weber
<matthew.weber@rockwellcollins.com> wrote:

> Yep, I'll get a patch together for swig/python workaround that doesn't
> require 2017.9 to fix the autobuilder.  Fabio, assuming we'll skip
> 2017.9 bump and just go 2017.11?

Sounds good for me.
Matt Weber Oct. 17, 2017, 8:14 p.m. | #9
All,

On Tue, Oct 17, 2017 at 2:03 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Matthew,
>
> On Tue, Oct 17, 2017 at 5:00 PM, Matthew Weber
> <matthew.weber@rockwellcollins.com> wrote:
>
>> Yep, I'll get a patch together for swig/python workaround that doesn't
>> require 2017.9 to fix the autobuilder.  Fabio, assuming we'll skip
>> 2017.9 bump and just go 2017.11?
>
> Sounds good for me.

Superseded by https://patchwork.ozlabs.org/patch/827287/

Patch

diff --git a/package/uboot-tools/0004-libfdt-give-setup-py-optional-interpreter.patch b/package/uboot-tools/0004-libfdt-give-setup-py-optional-interpreter.patch
new file mode 100644
index 0000000..e6e3783
--- /dev/null
+++ b/package/uboot-tools/0004-libfdt-give-setup-py-optional-interpreter.patch
@@ -0,0 +1,31 @@ 
+From 95de35336e2a6b4a292b2950dbb092077dcda565 Mon Sep 17 00:00:00 2001
+From: Matt Weber <matthew.weber@rockwellcollins.com>
+Date: Thu, 12 Oct 2017 21:05:56 -0500
+Subject: [PATCH] libfdt: give setup.py optional interpreter
+
+If building in a sandboxed environment where a
+alternate python interpreter is desired. Allow
+configuring of the PYTHON variable to specify
+the interpreter to invoke setup.py.
+
+Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
+---
+ tools/Makefile | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/tools/Makefile b/tools/Makefile
+index 77706a9..30505dc 100644
+--- a/tools/Makefile
++++ b/tools/Makefile
+@@ -136,7 +136,7 @@ tools/_libfdt.so: $(LIBFDT_SRCS) $(LIBFDT_SWIG)
+ 		CPPFLAGS="$(_hostc_flags)" OBJDIR=tools \
+ 		SOURCES="$(LIBFDT_SRCS) tools/libfdt.i" \
+ 		SWIG_OPTS="-I$(srctree)/lib/libfdt -I$(srctree)/lib" \
+-		$(libfdt_tree)/pylibfdt/setup.py --quiet build_ext \
++		$(PYTHON) $(libfdt_tree)/pylibfdt/setup.py --quiet build_ext \
+ 			--build-lib tools
+ 
+ ifneq ($(CONFIG_MX23)$(CONFIG_MX28),)
+-- 
+1.8.3.1
+
diff --git a/package/uboot-tools/uboot-tools.mk b/package/uboot-tools/uboot-tools.mk
index 4f39f4a..4fa77ca 100644
--- a/package/uboot-tools/uboot-tools.mk
+++ b/package/uboot-tools/uboot-tools.mk
@@ -75,6 +75,12 @@  define UBOOT_TOOLS_INSTALL_TARGET_CMDS
 	$(UBOOT_TOOLS_INSTALL_DUMPIMAGE)
 endef
 
+ifeq ($(BR2_PACKAGE_PYTHON3),y)
+HOST_UBOOT_TOOLS_DEPENDENCIES += host-python3
+else
+HOST_UBOOT_TOOLS_DEPENDENCIES += host-python
+endif
+
 define HOST_UBOOT_TOOLS_CONFIGURE_CMDS
 	mkdir -p $(@D)/include/config
 	touch $(@D)/include/config/auto.conf
@@ -95,7 +101,8 @@  HOST_UBOOT_TOOLS_DEPENDENCIES += host-openssl
 endif
 
 define HOST_UBOOT_TOOLS_BUILD_CMDS
-	$(MAKE1) -C $(@D) $(HOST_UBOOT_TOOLS_MAKE_OPTS) tools-only
+	$(MAKE1) -C $(@D) $(HOST_UBOOT_TOOLS_MAKE_OPTS) \
+		PYTHON="$(HOST_DIR)/bin/python" tools-only
 endef
 
 define HOST_UBOOT_TOOLS_INSTALL_CMDS