RFE: link python against shared library

[zippy2]

Honestly, I have no idea why libseccomp is linked into the python module statically. With dynamic linking we can decrease overall memory consumption (as the shared object is loaded in the memory just once) - just like with regular binaries that link to libseccomp.

[coveralls]

Coverage: 89.71%. Remained the same when pulling 8ce946556a8db8fe5507a0cfca3163114215dbac on zippy2:python_dynamic_link into 791a252af94fa67932463919c674e77185780f8c on seccomp:main.

[zippy2]

Alright. After some fixing of configure.ac I'm able to reproduce the issue. I'll be updating this pull request shortly.

[pcmoore]

Hi @zippy2, thanks for the PR.

While looking at the individual commits, aside from the static/dynamic linking change commit, the other three are bugfixes that would be good to have regardless, does that sound right or am I missing some Python subtlety? If so, I'd like to merge those, assuming @drakenclimber is okay with them, while we sort out the linking.

FWIW, I don't recall the exact reasoning for originally doing a static link, but I tend to think there may be some value in keeping it that way so we don't have to worry too much about a disconnect between the Python bindings and the native library. Of course if we separate out the bindings, which I think is still a good goal, we'll need to do this ...

[zippy2]

Hi @zippy2, thanks for the PR.

While looking at the individual commits, aside from the static/dynamic linking change commit, the other three are bugfixes that would be good to have regardless, does that sound right or am I missing some Python subtlety? If so, I'd like to merge those, assuming @drakenclimber is okay with them, while we sort out the linking.

Correct. Those three commits are basically independent of the dynamic-linking one. Feel free to merge any subset of them.

FWIW, I don't recall the exact reasoning for originally doing a static link, but I tend to think there may be some value in keeping it that way so we don't have to worry too much about a disconnect between the Python bindings and the native library. Of course if we separate out the bindings, which I think is still a good goal, we'll need to do this ...

Yeah, I suspected that this is just a historic baggage. The only scenario in which static linking would be beneficial is when the API/ABI would change (esp. since libseccomp.so doesn't export versioned symbols). But as long as that assumption is incorrect, we can start linking dynamically.

[pcmoore]

Thanks for the prompt response @zippy2.

I'm inclined to take the three bugfixes now, and also backport those into the release-2.5 branch, but I'm thinking it would be best to play it safe and leave the dynamic linking until we split the bindings out into their own repo (see issue #61).

What do you think @drakenclimber?

[drakenclimber]

Thanks for the prompt response @zippy2.

I'm inclined to take the three bugfixes now, and also backport those into the release-2.5 branch, but I'm thinking it would be best to play it safe and leave the dynamic linking until we split the bindings out into their own repo (see issue #61).

What do you think @drakenclimber?

I think that's very reasonable. Safety is a primary goal of seccomp/libseccomp, and I think erring on the side of caution is the right choice.

Are there any other negative side effects of a static library other than wasted memory? I can't think of anything.

Perhaps this raises the priority of splitting out the python code into its own repo. We need to figure out testing, but other than that I don't see a lot of snags to making the split.

[zippy2]

Thanks for the prompt response @zippy2. I'm inclined to take the three bugfixes now, and also backport those into the release-2.5 branch, but I'm thinking it would be best to play it safe and leave the dynamic linking until we split the bindings out into their own repo (see issue #61). What do you think @drakenclimber?

I think that's very reasonable. Safety is a primary goal of seccomp/libseccomp, and I think erring on the side of caution is the right choice.

Fair enough. Thank you both!

Are there any other negative side effects of a static library other than wasted memory? I can't think of anything.

Dynamic linking has many advantages over static one, in general, but since it is a python module that's statically linked, it's effectively dynamically linked. It's not a binary that's statically linked.

Perhaps this raises the priority of splitting out the python code into its own repo. We need to figure out testing, but other than that I don't see a lot of snags to making the split.

You could still go with static linking, even if it were two separate repos. I mean, plenty of distros even ship the static library. But I think it's great opportunity to start linking dynamically.

[pcmoore]

@drakenclimber I'm okay with merging the three commits below, leaving out the dynamic linking commit; do they look good to you?

https://github.com/seccomp/libseccomp/pull/392/commits/8ce946556a8db8fe5507a0cfca3163114215dbac https://github.com/seccomp/libseccomp/pull/392/commits/34b18816a63372bff0fdb4e79fadc9fadbfe4036 https://github.com/seccomp/libseccomp/pull/392/commits/44f0910a0d804bc769b54e9c24c8d0a23f9905db

Acked-by: Paul Moore <paul@paul-moore.com>

[drakenclimber]

@drakenclimber I'm okay with merging the three commits below, leaving out the dynamic linking commit; do they look good to you?

8ce9465 34b1881 44f0910

Acked-by: Paul Moore <paul@paul-moore.com>

Yes, they look good to me.

Acked-by: Tom Hromatka <tom.hromatka@oracle.com>

[pcmoore]

Merged at 744c9a897b74ad66d065791593e25a05e4b6f6a1, thanks @zippy2! I'm going to close this out as I think the best way to handle the static vs. shared library argument is with the other GH issues where we split out the Python bindings into their own repo. Of course if you disagree with that, please comment and/or re-open this issue.