| Message ID | 20171013023556.25825-1-matthew.weber@rockwellcollins.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | uboot-tools: bugfix libfdt python build | expand |
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
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.
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
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
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
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
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
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.
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/
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
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