diff mbox series

package/nftables: fix the build of the pyhon bindings

Message ID 20230816180916.2608831-1-yann.morin.1998@free.fr
State Changes Requested
Headers show
Series package/nftables: fix the build of the pyhon bindings | expand

Commit Message

Yann E. MORIN Aug. 16, 2023, 6:09 p.m. UTC 
nftables provides python bindings; it uses setuptools to install them.
We currently install those bindings by telling nftables buildsystem,
autotools, to install the python bindings.

However, we do not pass any of the environment variables that are needed
for setuptools packages. When host-python-setuptools is installed before
nftables is built [0], this breaks the system at runtime, as the
bindings are not installed; only the egg is, resulting in runtime errors
like:

    # python -c 'import nftables'
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    ModuleNotFoundError: No module named 'nftables'

Upstream has been doing some changes on their python handling, but it is
not in a released version, and we can't backport those changes either,
due to other big changes.

Instead, we split the pyhon bindings to their own package.

For legacy hadnling, we make that package default to y, so that existing
(def)config still work. The only novelty is that it can be disabled now.

Many thanks to Julien for testing and finding the offending dependency,
to James for suggesting the package split, and to Adam for, well,
trigerring the issue in the first place! ;-p

[0] This can happen when another python package using setuptools is
    built before nftables. However, with PPD, this never happens because
    host-python-setuptools is never in the dependency chain of nftables.

Reported-by: Julien Olivain <ju.o@free.fr>
Suggested-by: James Hilliard <james.hilliard1@gmail.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Adam Duskett <aduskett@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/nftables/Config.in                    |  5 +++++
 package/nftables/nftables-python/Config.in    |  4 ++++
 .../nftables-python/nftables-python.hash      |  1 +
 .../nftables-python/nftables-python.mk        | 22 +++++++++++++++++++
 package/nftables/nftables.mk                  | 10 +++------
 5 files changed, 35 insertions(+), 7 deletions(-)
 create mode 100644 package/nftables/nftables-python/Config.in
 create mode 120000 package/nftables/nftables-python/nftables-python.hash
 create mode 100644 package/nftables/nftables-python/nftables-python.mk

Comments

Yann E. MORIN Aug. 16, 2023, 7:09 p.m. UTC | #1 
All,

There is still an unkown part to the tory, which is when it broke, or if
it always was broken. I'm stil invetigating this, and this will be added
to the commit log shortly (thus the patch is "changes-requested" in
patchwork).

Still, feedback on the changes are still appreciated.

Regards,
Yann E. MORIN.

On 2023-08-16 20:09 +0200, Yann E. MORIN spake thusly:
> nftables provides python bindings; it uses setuptools to install them.
> We currently install those bindings by telling nftables buildsystem,
> autotools, to install the python bindings.
> 
> However, we do not pass any of the environment variables that are needed
> for setuptools packages. When host-python-setuptools is installed before
> nftables is built [0], this breaks the system at runtime, as the
> bindings are not installed; only the egg is, resulting in runtime errors
> like:
> 
>     # python -c 'import nftables'
>     Traceback (most recent call last):
>       File "<string>", line 1, in <module>
>     ModuleNotFoundError: No module named 'nftables'
> 
> Upstream has been doing some changes on their python handling, but it is
> not in a released version, and we can't backport those changes either,
> due to other big changes.
> 
> Instead, we split the pyhon bindings to their own package.
> 
> For legacy hadnling, we make that package default to y, so that existing
> (def)config still work. The only novelty is that it can be disabled now.
> 
> Many thanks to Julien for testing and finding the offending dependency,
> to James for suggesting the package split, and to Adam for, well,
> trigerring the issue in the first place! ;-p
> 
> [0] This can happen when another python package using setuptools is
>     built before nftables. However, with PPD, this never happens because
>     host-python-setuptools is never in the dependency chain of nftables.
> 
> Reported-by: Julien Olivain <ju.o@free.fr>
> Suggested-by: James Hilliard <james.hilliard1@gmail.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Adam Duskett <aduskett@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  package/nftables/Config.in                    |  5 +++++
>  package/nftables/nftables-python/Config.in    |  4 ++++
>  .../nftables-python/nftables-python.hash      |  1 +
>  .../nftables-python/nftables-python.mk        | 22 +++++++++++++++++++
>  package/nftables/nftables.mk                  | 10 +++------
>  5 files changed, 35 insertions(+), 7 deletions(-)
>  create mode 100644 package/nftables/nftables-python/Config.in
>  create mode 120000 package/nftables/nftables-python/nftables-python.hash
>  create mode 100644 package/nftables/nftables-python/nftables-python.mk
> 
> diff --git a/package/nftables/Config.in b/package/nftables/Config.in
> index 8d172b7256..833a3a38a2 100644
> --- a/package/nftables/Config.in
> +++ b/package/nftables/Config.in
> @@ -13,5 +13,10 @@ config BR2_PACKAGE_NFTABLES
>  
>  	  http://www.netfilter.org/projects/nftables/index.html
>  
> +# Legacy: this used to be handled in nftables.mk
> +if BR2_PACKAGE_NFTABLES
> +source "package/nftables/nftables-python/Config.in"
> +endif
> +
>  comment "nftables needs a toolchain w/ wchar, headers >= 3.12"
>  	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_12
> diff --git a/package/nftables/nftables-python/Config.in b/package/nftables/nftables-python/Config.in
> new file mode 100644
> index 0000000000..b16e2d0ad1
> --- /dev/null
> +++ b/package/nftables/nftables-python/Config.in
> @@ -0,0 +1,4 @@
> +config BR2_PACKAGE_NFTABLES_PYTHON
> +	bool "python bindings"
> +	default y # legacy
> +	depends on BR2_PACKAGE_PYTHON3
> diff --git a/package/nftables/nftables-python/nftables-python.hash b/package/nftables/nftables-python/nftables-python.hash
> new file mode 120000
> index 0000000000..9ac74580e7
> --- /dev/null
> +++ b/package/nftables/nftables-python/nftables-python.hash
> @@ -0,0 +1 @@
> +../nftables.hash
> \ No newline at end of file
> diff --git a/package/nftables/nftables-python/nftables-python.mk b/package/nftables/nftables-python/nftables-python.mk
> new file mode 100644
> index 0000000000..908bacd99a
> --- /dev/null
> +++ b/package/nftables/nftables-python/nftables-python.mk
> @@ -0,0 +1,22 @@
> +################################################################################
> +#
> +# nftables-python
> +#
> +################################################################################
> +
> +# The following assignments work only because nftables.mk is included before
> +# this file is.
> +NFTABLES_PYTHON_VERSION = $(NFTABLES_VERSION)
> +NFTABLES_PYTHON_SOURCE = $(NFTABLES_SOURCE)
> +NFTABLES_PYTHON_SITE = $(NFTABLES_SITE)
> +NFTABLES_PYTHON_LICENSE = $(NFTABLES_LICENSE)
> +NFTABLES_PYTHON_LICENSE_FILES = $(NFTABLES_LICENSE_FILES)
> +
> +# We share the same source code as nftables
> +NFTABLES_PYTHON_DL_SUBDIR = nftables
> +
> +NFTABLES_PYTHON_SUBDIR = py
> +
> +NFTABLES_PYTHON_SETUP_TYPE = setuptools
> +
> +$(eval $(python-package))
> diff --git a/package/nftables/nftables.mk b/package/nftables/nftables.mk
> index b0a14bd429..aa88203ab5 100644
> --- a/package/nftables/nftables.mk
> +++ b/package/nftables/nftables.mk
> @@ -42,13 +42,6 @@ else
>  NFTABLES_CONF_OPTS += --without-json
>  endif
>  
> -ifeq ($(BR2_PACKAGE_PYTHON3),y)
> -NFTABLES_CONF_OPTS += --enable-python
> -NFTABLES_DEPENDENCIES += python3
> -else
> -NFTABLES_CONF_OPTS += --disable-python
> -endif
> -
>  NFTABLES_CONF_ENV = LIBS="$(NFTABLES_LIBS)"
>  
>  define NFTABLES_LINUX_CONFIG_FIXUPS
> @@ -58,3 +51,6 @@ define NFTABLES_LINUX_CONFIG_FIXUPS
>  endef
>  
>  $(eval $(autotools-package))
> +
> +# Legacy: we used to handle it in this .mk
> +include package/nftables/nftables-python/nftables-python.mk
> -- 
> 2.25.1
>
Julien Olivain Aug. 16, 2023, 7:59 p.m. UTC | #2 
Hi Yann, all,

Thanks for the patch!

I successfully tested this patch on master branch at commit 94edad7
with few known broken configurations. While at it, I also successfully
tested on branch next at commit eea0c9f, which was the point
on which I have initially hit the issue. I used the same commands as
reported in:
https://lists.buildroot.org/pipermail/buildroot/2023-August/672864.html

On 16/08/2023 20:09, Yann E. MORIN wrote:
> nftables provides python bindings; it uses setuptools to install them.
> We currently install those bindings by telling nftables buildsystem,
> autotools, to install the python bindings.
> 
> However, we do not pass any of the environment variables that are 
> needed
> for setuptools packages. When host-python-setuptools is installed 
> before
> nftables is built [0], this breaks the system at runtime, as the
> bindings are not installed; only the egg is, resulting in runtime 
> errors
> like:
> 
>     # python -c 'import nftables'
>     Traceback (most recent call last):
>       File "<string>", line 1, in <module>
>     ModuleNotFoundError: No module named 'nftables'
> 
> Upstream has been doing some changes on their python handling, but it 
> is
> not in a released version, and we can't backport those changes either,
> due to other big changes.
> 
> Instead, we split the pyhon bindings to their own package.
> 
> For legacy hadnling, we make that package default to y, so that 
> existing
> (def)config still work. The only novelty is that it can be disabled 
> now.
> 
> Many thanks to Julien for testing and finding the offending dependency,
> to James for suggesting the package split, and to Adam for, well,
> trigerring the issue in the first place! ;-p
> 
> [0] This can happen when another python package using setuptools is
>     built before nftables. However, with PPD, this never happens 
> because
>     host-python-setuptools is never in the dependency chain of 
> nftables.
> 
> Reported-by: Julien Olivain <ju.o@free.fr>

Tested-by: Julien Olivain <ju.o@free.fr>

> Suggested-by: James Hilliard <james.hilliard1@gmail.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Adam Duskett <aduskett@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  package/nftables/Config.in                    |  5 +++++
>  package/nftables/nftables-python/Config.in    |  4 ++++
>  .../nftables-python/nftables-python.hash      |  1 +
>  .../nftables-python/nftables-python.mk        | 22 +++++++++++++++++++
>  package/nftables/nftables.mk                  | 10 +++------
>  5 files changed, 35 insertions(+), 7 deletions(-)
>  create mode 100644 package/nftables/nftables-python/Config.in
>  create mode 120000 
> package/nftables/nftables-python/nftables-python.hash
>  create mode 100644 package/nftables/nftables-python/nftables-python.mk
> 
> diff --git a/package/nftables/Config.in b/package/nftables/Config.in
> index 8d172b7256..833a3a38a2 100644
> --- a/package/nftables/Config.in
> +++ b/package/nftables/Config.in
> @@ -13,5 +13,10 @@ config BR2_PACKAGE_NFTABLES
> 
>  	  http://www.netfilter.org/projects/nftables/index.html
> 
> +# Legacy: this used to be handled in nftables.mk
> +if BR2_PACKAGE_NFTABLES
> +source "package/nftables/nftables-python/Config.in"
> +endif
> +
>  comment "nftables needs a toolchain w/ wchar, headers >= 3.12"
>  	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_12
> diff --git a/package/nftables/nftables-python/Config.in 
> b/package/nftables/nftables-python/Config.in
> new file mode 100644
> index 0000000000..b16e2d0ad1
> --- /dev/null
> +++ b/package/nftables/nftables-python/Config.in
> @@ -0,0 +1,4 @@
> +config BR2_PACKAGE_NFTABLES_PYTHON
> +	bool "python bindings"
> +	default y # legacy
> +	depends on BR2_PACKAGE_PYTHON3
> diff --git a/package/nftables/nftables-python/nftables-python.hash 
> b/package/nftables/nftables-python/nftables-python.hash
> new file mode 120000
> index 0000000000..9ac74580e7
> --- /dev/null
> +++ b/package/nftables/nftables-python/nftables-python.hash
> @@ -0,0 +1 @@
> +../nftables.hash
> \ No newline at end of file
> diff --git a/package/nftables/nftables-python/nftables-python.mk 
> b/package/nftables/nftables-python/nftables-python.mk
> new file mode 100644
> index 0000000000..908bacd99a
> --- /dev/null
> +++ b/package/nftables/nftables-python/nftables-python.mk
> @@ -0,0 +1,22 @@
> +################################################################################
> +#
> +# nftables-python
> +#
> +################################################################################
> +
> +# The following assignments work only because nftables.mk is included 
> before
> +# this file is.
> +NFTABLES_PYTHON_VERSION = $(NFTABLES_VERSION)
> +NFTABLES_PYTHON_SOURCE = $(NFTABLES_SOURCE)
> +NFTABLES_PYTHON_SITE = $(NFTABLES_SITE)
> +NFTABLES_PYTHON_LICENSE = $(NFTABLES_LICENSE)
> +NFTABLES_PYTHON_LICENSE_FILES = $(NFTABLES_LICENSE_FILES)
> +
> +# We share the same source code as nftables
> +NFTABLES_PYTHON_DL_SUBDIR = nftables
> +
> +NFTABLES_PYTHON_SUBDIR = py
> +
> +NFTABLES_PYTHON_SETUP_TYPE = setuptools
> +
> +$(eval $(python-package))
> diff --git a/package/nftables/nftables.mk 
> b/package/nftables/nftables.mk
> index b0a14bd429..aa88203ab5 100644
> --- a/package/nftables/nftables.mk
> +++ b/package/nftables/nftables.mk
> @@ -42,13 +42,6 @@ else
>  NFTABLES_CONF_OPTS += --without-json
>  endif
> 
> -ifeq ($(BR2_PACKAGE_PYTHON3),y)
> -NFTABLES_CONF_OPTS += --enable-python
> -NFTABLES_DEPENDENCIES += python3
> -else
> -NFTABLES_CONF_OPTS += --disable-python
> -endif
> -
>  NFTABLES_CONF_ENV = LIBS="$(NFTABLES_LIBS)"
> 
>  define NFTABLES_LINUX_CONFIG_FIXUPS
> @@ -58,3 +51,6 @@ define NFTABLES_LINUX_CONFIG_FIXUPS
>  endef
> 
>  $(eval $(autotools-package))
> +
> +# Legacy: we used to handle it in this .mk
> +include package/nftables/nftables-python/nftables-python.mk

Best regards,

Julien.
diff mbox series

Patch

diff --git a/package/nftables/Config.in b/package/nftables/Config.in
index 8d172b7256..833a3a38a2 100644
--- a/package/nftables/Config.in
+++ b/package/nftables/Config.in
@@ -13,5 +13,10 @@  config BR2_PACKAGE_NFTABLES
 
 	  http://www.netfilter.org/projects/nftables/index.html
 
+# Legacy: this used to be handled in nftables.mk
+if BR2_PACKAGE_NFTABLES
+source "package/nftables/nftables-python/Config.in"
+endif
+
 comment "nftables needs a toolchain w/ wchar, headers >= 3.12"
 	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_12
diff --git a/package/nftables/nftables-python/Config.in b/package/nftables/nftables-python/Config.in
new file mode 100644
index 0000000000..b16e2d0ad1
--- /dev/null
+++ b/package/nftables/nftables-python/Config.in
@@ -0,0 +1,4 @@ 
+config BR2_PACKAGE_NFTABLES_PYTHON
+	bool "python bindings"
+	default y # legacy
+	depends on BR2_PACKAGE_PYTHON3
diff --git a/package/nftables/nftables-python/nftables-python.hash b/package/nftables/nftables-python/nftables-python.hash
new file mode 120000
index 0000000000..9ac74580e7
--- /dev/null
+++ b/package/nftables/nftables-python/nftables-python.hash
@@ -0,0 +1 @@ 
+../nftables.hash
\ No newline at end of file
diff --git a/package/nftables/nftables-python/nftables-python.mk b/package/nftables/nftables-python/nftables-python.mk
new file mode 100644
index 0000000000..908bacd99a
--- /dev/null
+++ b/package/nftables/nftables-python/nftables-python.mk
@@ -0,0 +1,22 @@ 
+################################################################################
+#
+# nftables-python
+#
+################################################################################
+
+# The following assignments work only because nftables.mk is included before
+# this file is.
+NFTABLES_PYTHON_VERSION = $(NFTABLES_VERSION)
+NFTABLES_PYTHON_SOURCE = $(NFTABLES_SOURCE)
+NFTABLES_PYTHON_SITE = $(NFTABLES_SITE)
+NFTABLES_PYTHON_LICENSE = $(NFTABLES_LICENSE)
+NFTABLES_PYTHON_LICENSE_FILES = $(NFTABLES_LICENSE_FILES)
+
+# We share the same source code as nftables
+NFTABLES_PYTHON_DL_SUBDIR = nftables
+
+NFTABLES_PYTHON_SUBDIR = py
+
+NFTABLES_PYTHON_SETUP_TYPE = setuptools
+
+$(eval $(python-package))
diff --git a/package/nftables/nftables.mk b/package/nftables/nftables.mk
index b0a14bd429..aa88203ab5 100644
--- a/package/nftables/nftables.mk
+++ b/package/nftables/nftables.mk
@@ -42,13 +42,6 @@  else
 NFTABLES_CONF_OPTS += --without-json
 endif
 
-ifeq ($(BR2_PACKAGE_PYTHON3),y)
-NFTABLES_CONF_OPTS += --enable-python
-NFTABLES_DEPENDENCIES += python3
-else
-NFTABLES_CONF_OPTS += --disable-python
-endif
-
 NFTABLES_CONF_ENV = LIBS="$(NFTABLES_LIBS)"
 
 define NFTABLES_LINUX_CONFIG_FIXUPS
@@ -58,3 +51,6 @@  define NFTABLES_LINUX_CONFIG_FIXUPS
 endef
 
 $(eval $(autotools-package))
+
+# Legacy: we used to handle it in this .mk
+include package/nftables/nftables-python/nftables-python.mk