Please add option to build as a shared library

rnpgp / sexpp

C++ Library for SEXP (S-expressions)

Other

7 stars 4 forks source link

Please add option to build as a shared library #32

Closed remicollet closed 1 year ago

remicollet commented 1 year ago

So can later be packaged separately and used by rnp, instead of using bundled copy

dkg commented 1 year ago

Alternately, just merge it into librnp directly as source, rather than keeping it separate.

Using git submodules makes it significantly more challenging to have a reliable packaging workflow.

ni4 commented 1 year ago

@dkg latest release (v0.17.0) includes sexp sources in non-snapshot packages ( rnp-v0.17.0.tar.gz / rnp-v0.17.0.zip ), or does this bring some inconvenienses as well?

maxirmx commented 1 year ago

@andreasstieger , @remicollet, @dkg thank you for your notes

Generally speaking we do not want to expose sexp as a separate entity So:

When rnp is built/used as a shared library sexp is "linked into it". There is no need to install libsexp or expose it in any other way
When rnp is built/used as a static library, sexp must be linked as a second static library even though it doesn't provide any public API.
If rnp is built from a source package, it includes a correct snapshot of the sexp sources as @ni4

I can readily accept that any or all of the items above are wrong but I am afraid I need some help to understand the issue.

andreasstieger commented 1 year ago

Speaking for the openSUSE packagers, based on our shared library packaging policy, which is more or less the same in most major distributions:

We avoid packaging static libraries at any cost. We literally patch rnp until it is a shared library. We further unbundle sexp into a shared library by itself as a standalone source package, building a lib, a -devel and a cli package. We will proceed to do the same in Thunderbird next, to use the system Botan or OpenSSL.

Our patch: https://build.opensuse.org/package/view_file/security:privacy/sexp/sexp-0.8.4-shared-lib.patch?expand=1

Could be trivially be made into a build-time configurable option.

maxirmx commented 1 year ago

Thank you, @andreasstieger

sexp static library is an intermmediate build step. There are other static libraries created during rnp build that are intermmediate build steps as well.

I suggest that you do not patch it; do not package libsexp.a; just deleted it, if it is not deleted by rnp build script. (we had an issue where static libraries were packaged in shared libraries bundle https://github.com/rnpgp/rnp/pull/2076 it is fixed)

andreasstieger commented 1 year ago

Actually we patch rnp to not build and statically link it's bundled sexp but use the distribution sexp shared library.

ronaldtse commented 1 year ago

I think we have sufficiently good reasons to execute this ticket as is. Thank you all for raising this!

@maxirmx it's all yours, thank you!

antonsviridenko commented 1 year ago

Gentoo Linux also packages sexp as a separate library package

https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-util/librnp/files/librnp-0.17.0-dont-bundle-sexp.patch https://packages.gentoo.org/packages/dev-libs/sexp