[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[James Hilliard]

Fixes:
 - http://autobuild.buildroot.net/results/71bf937339705a520e047d12c146229ec42a76f2

Details:
 - https://bugs.python.org/issue36721

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 package/kmod/kmod.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/kmod/kmod.mk b/package/kmod/kmod.mk
index a5bcf2f2d6..a63fe99d91 100644
--- a/package/kmod/kmod.mk
+++ b/package/kmod/kmod.mk
@@ -45,6 +45,10 @@ endif
 ifeq ($(BR2_PACKAGE_PYTHON)$(BR2_PACKAGE_PYTHON3),y)
 KMOD_DEPENDENCIES += $(if $(BR2_PACKAGE_PYTHON),python,python3)
 KMOD_CONF_OPTS += --enable-python
+ifeq ($(BR2_PACKAGE_PYTHON3),y)
+KMOD_CONF_ENV += \
+	PYTHON_LIBS="`$(STAGING_DIR)/usr/bin/python3-config --libs --embed`"
+endif
 endif
 
 ifeq ($(BR2_PACKAGE_KMOD_TOOLS),y)
-- 
2.20.1

[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[Thomas Petazzoni]

On Sun, 20 Oct 2019 13:38:18 -0600
James Hilliard <james.hilliard1@gmail.com> wrote:

> Fixes:
>  - http://autobuild.buildroot.net/results/71bf937339705a520e047d12c146229ec42a76f2
> 
> Details:
>  - https://bugs.python.org/issue36721

Reading this, I am not sure your fix is correct (though I am not sure
what the correct fix is).

Indeed, in this bug report, Victor Stinner explains:

"""
There are two use cases for libpython:

* Build a C extension and load it in Python: use case called "pyext" by waf
* Embed Python into an application: use case called "pyembed" by waf
"""

The first case should not require -lpython3.8, and therefore
python-config no longer returns -lpython3.8, unless you pass --embed
(which means you are in the second case: you embed Python into an
application).

But kmod is only providing Python bindings: it's a C extension to be
loaded in Python, so we are in the first case, and we shouldn't need
--embed.

What do you think ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[James Hilliard]

On Mon, Oct 21, 2019 at 8:56 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Sun, 20 Oct 2019 13:38:18 -0600
> James Hilliard <james.hilliard1@gmail.com> wrote:
>
> > Fixes:
> >  - http://autobuild.buildroot.net/results/71bf937339705a520e047d12c146229ec42a76f2
> >
> > Details:
> >  - https://bugs.python.org/issue36721
>
> Reading this, I am not sure your fix is correct (though I am not sure
> what the correct fix is).
Yeah, not entirely sure but I saw other projects use this same fix effectively.
>
> Indeed, in this bug report, Victor Stinner explains:
>
> """
> There are two use cases for libpython:
>
> * Build a C extension and load it in Python: use case called "pyext" by waf
> * Embed Python into an application: use case called "pyembed" by waf
> """
>
> The first case should not require -lpython3.8, and therefore
> python-config no longer returns -lpython3.8, unless you pass --embed
> (which means you are in the second case: you embed Python into an
> application).
>
> But kmod is only providing Python bindings: it's a C extension to be
> loaded in Python, so we are in the first case, and we shouldn't need
> --embed.
>
> What do you think ?
Yeah, it's not very clear, this fix is the same as these essentially however:
https://gitlab.freedesktop.org/gstreamer/gst-python/merge_requests/14/diffs
https://gitlab.com/ita1024/waf/merge_requests/2236/diffs
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[Thomas Petazzoni]

On Mon, 21 Oct 2019 21:12:44 +0200
James Hilliard <james.hilliard1@gmail.com> wrote:

> > Reading this, I am not sure your fix is correct (though I am not sure
> > what the correct fix is).  
> Yeah, not entirely sure but I saw other projects use this same fix effectively.

Yes, it "works", but it's quite weird, because the whole point of
Victor Stinner's work was to make sure Python extensions were not
linked with libpython.

> > What do you think ?  
> Yeah, it's not very clear, this fix is the same as these essentially however:
> https://gitlab.freedesktop.org/gstreamer/gst-python/merge_requests/14/diffs

This is indeed a Python binding.

> https://gitlab.com/ita1024/waf/merge_requests/2236/diffs

This is a generic build system, and it only adds --embed when the
"pyembed" flag is used. It could simply be that this flag is used for
applications/libraries than embed Python, and not for Python
extensions. Indeed, the original entry in the Python issue tracker
mentioned that waf had "pyext" for Python extensions and "pyembed" for
apps/libs embedding Python.

It turns out that Victor Stinner (the author of the changes) is a
former colleague of mine from university times. I'll try to get some
feedback from him about this.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[Thomas Petazzoni]

Hello,

On Sun, 20 Oct 2019 13:38:18 -0600
James Hilliard <james.hilliard1@gmail.com> wrote:

> +ifeq ($(BR2_PACKAGE_PYTHON3),y)
> +KMOD_CONF_ENV += \
> +	PYTHON_LIBS="`$(STAGING_DIR)/usr/bin/python3-config --libs --embed`"
> +endif

For kmod, the problem is the same as libselinux, just not the same
option is used. libselinux is passing "-z defs", while kmod is passing
--no-undefined, but they are exactly the same option.

So, for kmod, I believe the proper fix is:

diff --git a/Makefile.am b/Makefile.am
index c5c2f06..8e9c90d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -173,7 +173,7 @@ CPYTHON_MODULE_CFLAGS = \
        $(AM_CFLAGS) -DCPYTHON_COMPILING_IN_PYPY=0 \
        $(PYTHON_NOWARN) $(PYTHON_CFLAGS) \
        -fvisibility=default
-CPYTHON_MODULE_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared
+CPYTHON_MODULE_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared -Wl,-z,undefs
 
 if BUILD_PYTHON
 pkgpyexec_LTLIBRARIES = \

Indeed, kmod globally uses -Wl,--no-undefined when linking, but
specifically for building the Python extension, we want to disable that
and allow undefined symbols to exist, so we override it with
-Wl,-z,undefs.

Best regards,

Thomas Petazzoni
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[Yann E. MORIN]

Fabrice, All,

On 2019-11-16 17:23 +0100, Yann E. MORIN spake thusly:
> On 2019-11-16 12:45 +0100, Fabrice Fontaine spake thusly:
> > Replace second patch that adds -Wl,-z,undefs by a patch that remove
> > -Wl,--no-undefined as some ld versions are ignoring this flag:
[--SNIP--]
> > -diff --git a/Makefile.am b/Makefile.am
> > -index c5c2f06..8e9c90d 100644
> > ---- a/Makefile.am
> > -+++ b/Makefile.am
> > -@@ -173,7 +173,7 @@ CPYTHON_MODULE_CFLAGS = \
> > - 	$(AM_CFLAGS) -DCPYTHON_COMPILING_IN_PYPY=0 \
> > - 	$(PYTHON_NOWARN) $(PYTHON_CFLAGS) \
> > - 	-fvisibility=default
> > --CPYTHON_MODULE_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared
> > -+CPYTHON_MODULE_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared -Wl,-z,undefs
> 
> What about this hack:
> 
>     CPYTHON_MODULE_LDFLAGS = (subst -Wl,--no-undefined,,$(AM_LDFLAGS)) ...

Does not work because of commas. The good hack would be:

    comma = ,
    CPYTHON_MODULE_LDFLAGS = (subst -Wl$(comma)--no-undefined,,$(AM_LDFLAGS)) ...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[Yann E. MORIN]

Fabrice, All,

On 2019-11-16 12:45 +0100, Fabrice Fontaine spake thusly:
> Replace second patch that adds -Wl,-z,undefs by a patch that remove
> -Wl,--no-undefined as some ld versions are ignoring this flag:

Alas, I'm afraid this patch will not be upstreamable, especially since
upstream did accept Thomas' patch.

> diff --git a/package/kmod/0002-Do-not-check-for-undefined-symbols-when-building-the.patch b/package/kmod/0002-Do-not-check-for-undefined-symbols-when-building-the.patch
> deleted file mode 100644
> index 3be40d4993..0000000000
> --- a/package/kmod/0002-Do-not-check-for-undefined-symbols-when-building-the.patch
> +++ /dev/null
> @@ -1,75 +0,0 @@
> -From 55a0a0aac503f5012ff2df7af37107544c757f19 Mon Sep 17 00:00:00 2001
> -From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> -Date: Tue, 22 Oct 2019 09:56:32 +0200
> -Subject: [PATCH kmod] Do not check for undefined symbols when building the
> - Python modules
[--SNIP--]
> -Upstream: https://lore.kernel.org/linux-modules/20191024174710.9441-1-thomas.petazzoni at bootlin.com/

Applied upstream:
    https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=1d14ef82f4a3be741bcdf6b1c6d51ce9dce43567

> -diff --git a/Makefile.am b/Makefile.am
> -index c5c2f06..8e9c90d 100644
> ---- a/Makefile.am
> -+++ b/Makefile.am
> -@@ -173,7 +173,7 @@ CPYTHON_MODULE_CFLAGS = \
> - 	$(AM_CFLAGS) -DCPYTHON_COMPILING_IN_PYPY=0 \
> - 	$(PYTHON_NOWARN) $(PYTHON_CFLAGS) \
> - 	-fvisibility=default
> --CPYTHON_MODULE_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared
> -+CPYTHON_MODULE_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared -Wl,-z,undefs

What about this hack:

    CPYTHON_MODULE_LDFLAGS = (subst -Wl,--no-undefined,,$(AM_LDFLAGS)) ...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] package/kmod: fix build with python 3.8

[Fabrice Fontaine]

Replace second patch that adds -Wl,-z,undefs by a patch that remove
-Wl,--no-undefined as some ld versions are ignoring this flag:

/home/naourr/work/instance-1/output-1/host/opt/ext-toolchain/bin/../lib/gcc/aarch64_be-linux-gnu/7.3.1/../../../../aarch64_be-linux-gnu/bin/ld: warning: -z undefs ignored.

Fixes:
 - http://autobuild.buildroot.org/results/06a6d865b6b7d8ebd793bde214f4a4c40e0962e1

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 ...-undefined-symbols-when-building-the.patch | 75 -------------------
 ...-configure.ac-remove-Wl-no-undefined.patch | 39 ++++++++++
 package/kmod/kmod.mk                          |  2 +-
 3 files changed, 40 insertions(+), 76 deletions(-)
 delete mode 100644 package/kmod/0002-Do-not-check-for-undefined-symbols-when-building-the.patch
 create mode 100644 package/kmod/0002-configure.ac-remove-Wl-no-undefined.patch

diff --git a/package/kmod/0002-Do-not-check-for-undefined-symbols-when-building-the.patch b/package/kmod/0002-Do-not-check-for-undefined-symbols-when-building-the.patch
deleted file mode 100644
index 3be40d4993..0000000000
--- a/package/kmod/0002-Do-not-check-for-undefined-symbols-when-building-the.patch
+++ /dev/null
@@ -1,75 +0,0 @@
-From 55a0a0aac503f5012ff2df7af37107544c757f19 Mon Sep 17 00:00:00 2001
-From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
-Date: Tue, 22 Oct 2019 09:56:32 +0200
-Subject: [PATCH kmod] Do not check for undefined symbols when building the
- Python modules
-
-kmod's configure.ac uses the -Wl,--no-undefined linker flag to verify
-at link time that all symbols of shared libraries are available, and
-that there are no undefined symbols.
-
-This make perfect sense for regular shared libraries. However, for
-Python extensions, which will be dlopen()ed inside the Python
-interpreter, it makes less sense.
-
-Since Python 3.8, there is a change in python-config script and
-Python's pkg-config file: it no longer links Python extensions with
-the libpython library. See
-https://docs.python.org/dev/whatsnew/3.8.html#debug-build-uses-the-same-abi-as-release-build
-which states:
-
-  On the other hand, pkg-config python3.8 --libs no longer contains
-  -lpython3.8. C extensions must not be linked to libpython (except on
-  Android and Cygwin, whose cases are handled by the script); this
-  change is backward incompatible on purpose. (Contributed by Victor
-  Stinner in bpo-36721.)
-
-So, when linking the kmod Python extensions, it currently fails with
-numerous unresolved symbols, that were previously provided by
-libpython:
-
-/home/test/autobuild/run/instance-3/output-1/host/opt/ext-toolchain/bin/../lib/gcc/powerpc64-buildroot-linux-gnu/7.4.0/../../../../powerpc64-buildroot-linux-gnu/bin/ld: libkmod/python/kmod/.libs/list_la-list.o: in function `__Pyx_PyObject_GetAttrStr':
-list.c:(.text.__Pyx_PyObject_GetAttrStr+0x48): undefined reference to `PyObject_GetAttr'
-/home/test/autobuild/run/instance-3/output-1/host/opt/ext-toolchain/bin/../lib/gcc/powerpc64-buildroot-linux-gnu/7.4.0/../../../../powerpc64-buildroot-linux-gnu/bin/ld: libkmod/python/kmod/.libs/list_la-list.o: in function `__pyx_tp_dealloc_4kmod_4list_ModListItem':
-list.c:(.text.__pyx_tp_dealloc_4kmod_4list_ModListItem+0x78): undefined reference to `PyObject_CallFinalizerFromDealloc'
-/home/test/autobuild/run/instance-3/output-1/host/opt/ext-toolchain/bin/../lib/gcc/powerpc64-buildroot-linux-gnu/7.4.0/../../../../powerpc64-buildroot-linux-gnu/bin/ld: libkmod/python/kmod/.libs/list_la-list.o: in function `__pyx_tp_dealloc_4kmod_4list_ModList':
-list.c:(.text.__pyx_tp_dealloc_4kmod_4list_ModList+0x30): undefined reference to `PyErr_Fetch'
-
-[Complete log at http://autobuild.buildroot.net/results/79a/79a5a0398723e8cfea0d0aa3dec5f7649aee4c63/build-end.log]
-
-Linking with libpython is no longer recommended: those symbols should
-remain unresolved in the Python extensions, as they wil be properly
-resolved when the Python extension gets loaded into the Python
-interpreter.
-
-Since we want to keep -Wl,--no-undefined globally in kmod, we leave
-the configure.ac file unchanged, and instead, specifically in the
-LDFLAGS used to build the Python extensions, we override
--Wl,--no-undefined with -Wl,-z,undefs. Ideally, -Wl,--no-undefined is
-the same as -Wl,-z,defs, and the effect of these options can be
-canceled on the linker command line by a following -Wl,-z,undefs (see
-the ld man page for details).
-
-Upstream: https://lore.kernel.org/linux-modules/20191024174710.9441-1-thomas.petazzoni at bootlin.com/
-Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
-Cc: Victor Stinner <victor.stinner@gmail.com>
----
- Makefile.am | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/Makefile.am b/Makefile.am
-index c5c2f06..8e9c90d 100644
---- a/Makefile.am
-+++ b/Makefile.am
-@@ -173,7 +173,7 @@ CPYTHON_MODULE_CFLAGS = \
- 	$(AM_CFLAGS) -DCPYTHON_COMPILING_IN_PYPY=0 \
- 	$(PYTHON_NOWARN) $(PYTHON_CFLAGS) \
- 	-fvisibility=default
--CPYTHON_MODULE_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared
-+CPYTHON_MODULE_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared -Wl,-z,undefs
- 
- if BUILD_PYTHON
- pkgpyexec_LTLIBRARIES = \
--- 
-2.21.0
-
diff --git a/package/kmod/0002-configure.ac-remove-Wl-no-undefined.patch b/package/kmod/0002-configure.ac-remove-Wl-no-undefined.patch
new file mode 100644
index 0000000000..e45cb93e61
--- /dev/null
+++ b/package/kmod/0002-configure.ac-remove-Wl-no-undefined.patch
@@ -0,0 +1,39 @@
+From 11eeeb760062ff6aa41d31078793f69de560243f Mon Sep 17 00:00:00 2001
+From: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+Date: Sat, 16 Nov 2019 12:29:21 +0100
+Subject: [PATCH] configure.ac: remove -Wl,--no-undefined
+
+Commit 1d14ef82f4a3be741bcdf6b1c6d51ce9dce43567 does not completely fix
+the build with python 3.8 as we still get link failure due to
+'-z undefs' being ignored by some versions of ld:
+
+/home/buildroot/autobuild/instance-0/output-1/host/opt/ext-toolchain/bin/../lib/gcc/mips-linux-gnu/5.3.0/../../../../mips-linux-gnu/bin/ld: warning: -z undefs ignored.
+
+/home/naourr/work/instance-1/output-1/host/opt/ext-toolchain/bin/../lib/gcc/aarch64_be-linux-gnu/7.3.1/../../../../aarch64_be-linux-gnu/bin/ld: warning: -z undefs ignored.
+
+So remove -Wl,--no-undefined from configure.ac to fix the issue
+
+Fixes:
+ - http://autobuild.buildroot.org/results/e9645d9969481b09f507f6e0d0b35faaa283eb60
+ - http://autobuild.buildroot.org/results/06a6d865b6b7d8ebd793bde214f4a4c40e0962e1
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+---
+ configure.ac | 1 -
+ 1 file changed, 1 deletion(-)
+
+diff --git a/configure.ac b/configure.ac
+index e885d79..bbeb513 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -266,7 +266,6 @@ AC_SUBST([OUR_CFLAGS], "$with_cflags $with_coverage_cflags")
+ 
+ CC_CHECK_FLAGS_APPEND([with_ldflags], [LDFLAGS], [ \
+ 		       -Wl,--as-needed \
+-		       -Wl,--no-undefined \
+ 		       -Wl,--gc-sections])
+ AC_SUBST([OUR_LDFLAGS], $with_ldflags)
+ 
+-- 
+2.24.0
+
diff --git a/package/kmod/kmod.mk b/package/kmod/kmod.mk
index e21f40a845..2f9df1834c 100644
--- a/package/kmod/kmod.mk
+++ b/package/kmod/kmod.mk
@@ -8,7 +8,7 @@ KMOD_VERSION = 26
 KMOD_SOURCE = kmod-$(KMOD_VERSION).tar.xz
 KMOD_SITE = $(BR2_KERNEL_MIRROR)/linux/utils/kernel/kmod
 KMOD_INSTALL_STAGING = YES
-# 0002-Do-not-check-for-undefined-symbols-when-building-the.patch
+# We're patching configure.ac
 KMOD_AUTORECONF = YES
 KMOD_DEPENDENCIES = host-pkgconf
 HOST_KMOD_DEPENDENCIES = host-pkgconf
-- 
2.24.0