diff mbox series

[3/3] package/python-iptables: use installed iptables by default

Message ID 20200226142617.4170-3-frank.vanbever@essensium.com
State Superseded
Headers show
Series [1/3] package/python-iptables: add explicit dependency on dynamic libs | expand

Commit Message

Frank Vanbever Feb. 26, 2020, 2:26 p.m. UTC
python-iptables depends on ctypes.util.find_library() which does not work due to
the absence of gcc and friends on target. The location of the xtables library
and the iptables modules can be configured through environment variables. Within
the scope of buildroot we can determine what these should be at build time and
replace the calls to os.getenv() with the correct value.

Fixes: https://bugs.busybox.net/show_bug.cgi?id=12271

Signed-off-by: Frank Vanbever <frank.vanbever@essensium.com>
---
 package/python-iptables/Config.in          | 2 +-
 package/python-iptables/python-iptables.mk | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

Comments

Thomas Petazzoni Feb. 26, 2020, 3:33 p.m. UTC | #1
Hello Frank,

On Wed, 26 Feb 2020 15:26:17 +0100
Frank Vanbever <frank.vanbever@essensium.com> wrote:

> diff --git a/package/python-iptables/python-iptables.mk b/package/python-iptables/python-iptables.mk
> index 66e478a89a..086d5b2457 100644
> --- a/package/python-iptables/python-iptables.mk
> +++ b/package/python-iptables/python-iptables.mk
> @@ -9,5 +9,14 @@ PYTHON_IPTABLES_SITE = https://files.pythonhosted.org/packages/08/5e/16a5ca35c42
>  PYTHON_IPTABLES_SETUP_TYPE = setuptools
>  PYTHON_IPTABLES_LICENSE = Apache-2.0
>  PYTHON_IPTABLES_LICENSE_FILES = NOTICE
> +PYTHON_IPTABLES_DEPENDENCIES = iptables
> +
> +define PYTHON_IPTABLES_SET_XTABLES_ENV_VARS
> +	XTABLES_VERSION=`awk '/XTABLES_VERSION_CODE/ {print $$NF}' $(STAGING_DIR)/usr/include/xtables-version.h`; \
> +	sed -i "s/os.getenv(\"PYTHON_IPTABLES_XTABLES_VERSION\")/$$XTABLES_VERSION/" $(@D)/iptc/xtables.py; \
> +	sed -i "s/os.getenv(\"XTABLES_LIBDIR\")/\"\/usr\/lib\/xtables\"/" $(@D)/iptc/xtables.py

This last line is not using the XTABLES_VERSION variable, so it does
not need to be a continuation line from the previous commands.

Also, use % as a sed separator instead of / so that you don't have to
escape all the slashes.

But overall, is there a better way ? Like some official way to pass
these values at build time as setup.py options, with the environment
variable taking precedence if available ? That would make this
hopefully acceptable by upstream. Perhaps the setup.py logic could even
check the xtables-version.h by itself ?

Best regards,

Thomas
Frank Vanbever Feb. 27, 2020, 2:56 p.m. UTC | #2
Hi Thomas,

Thank you for the comments.

 > But overall, is there a better way ? Like some official way to pass
> these values at build time as setup.py options, with the environment
> variable taking precedence if available ? That would make this
> hopefully acceptable by upstream. Perhaps the setup.py logic could even
> check the xtables-version.h by itself ?

For installations from source this would work. [1] However Python also has the 
option of distributing Wheel binary packages, where this logic would not be 
executed. [2] This would mean that we put the burden of handling the difference 
between installations on the upstream.

I took a second look and actually I think I can propose a workaround to the 
upstream. There's 2 problems that need to be tackled.

The first one is loading libxtables.so. iptables installs a libxtables.so 
symlink without any version. CDLL has no issues with this so it can just try 
and load that if all the other attempts fail.

The second problem is the location of the XTABLES_LIBDIR. Right now it tries 
all paths that are mentioned in the output of ldconfig -N -v. I guess it 
wouldn't be that much of a stretch to test some sensible locations (/usr/
local/lib/xtables, /usr/lib/xtables etc) if that should fail, in the same vein 
as the libc solution. It's not clean but it gets the job done.

I'll see if I can whip something up and submit it upstream.

Best regards,
Frank

[1] https://stackoverflow.com/questions/20288711/post-install-script-with-python-setuptools
[2] https://github.com/pypa/setuptools/issues/1782
Frank Vanbever Feb. 28, 2020, 4:17 p.m. UTC | #3
Hi Thomas,

> I'll see if I can whip something up and submit it upstream.

I gave it another try but it seems that I was a bit too optimistic yesterday. 
The xtables module requires the ABI  version to cast the return values for 
matches and targets to something it can access.

I have a final last ditch theory about how this could be fixed, but to be honest 
it's really starting to look like more trouble than it's worth.

The ABI version information is embedded into struct xtables_match and 
 struct xtables_target. You should be able to get to these just from a handle 
to the libxtables.so and the location of the iptables extensions (i.e. 
xtables_libdir), for which you could use the same solution as for libc and 
test a number of sensible locations if ldconfig is unavailable.
The xtables_{match,target} structs always have the version field as the first 
field in the returned struct so I think you could do an 
xtables_find_{target,match} and do a ctypes.cast to a dummy ctypes.struct class 
with  just the version field.

The way the module is written though requires the ABI  version global variable 
to instantiate the xtables class which contains the logic that would allow you 
to find a match/target. So you get into this chicken or the egg type situation 
where you need to know the ABI version to get the workaround way to get the 
ABI version to work.


Best regards,
Frank
diff mbox series

Patch

diff --git a/package/python-iptables/Config.in b/package/python-iptables/Config.in
index a35577bad3..0e24283d06 100644
--- a/package/python-iptables/Config.in
+++ b/package/python-iptables/Config.in
@@ -1,7 +1,7 @@ 
 config BR2_PACKAGE_PYTHON_IPTABLES
 	bool "python-iptables"
 	depends on !BR2_STATIC_LIBS
-	select BR2_PACKAGE_IPTABLES # runtime dependency
+	select BR2_PACKAGE_IPTABLES
 	help
 	  Python bindings for iptables.
 
diff --git a/package/python-iptables/python-iptables.mk b/package/python-iptables/python-iptables.mk
index 66e478a89a..086d5b2457 100644
--- a/package/python-iptables/python-iptables.mk
+++ b/package/python-iptables/python-iptables.mk
@@ -9,5 +9,14 @@  PYTHON_IPTABLES_SITE = https://files.pythonhosted.org/packages/08/5e/16a5ca35c42
 PYTHON_IPTABLES_SETUP_TYPE = setuptools
 PYTHON_IPTABLES_LICENSE = Apache-2.0
 PYTHON_IPTABLES_LICENSE_FILES = NOTICE
+PYTHON_IPTABLES_DEPENDENCIES = iptables
+
+define PYTHON_IPTABLES_SET_XTABLES_ENV_VARS
+	XTABLES_VERSION=`awk '/XTABLES_VERSION_CODE/ {print $$NF}' $(STAGING_DIR)/usr/include/xtables-version.h`; \
+	sed -i "s/os.getenv(\"PYTHON_IPTABLES_XTABLES_VERSION\")/$$XTABLES_VERSION/" $(@D)/iptc/xtables.py; \
+	sed -i "s/os.getenv(\"XTABLES_LIBDIR\")/\"\/usr\/lib\/xtables\"/" $(@D)/iptc/xtables.py
+endef
+
+PYTHON_IPTABLES_PRE_BUILD_HOOKS += PYTHON_IPTABLES_SET_XTABLES_ENV_VARS
 
 $(eval $(python-package))