Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please add option to build as a shared library #32

Closed
remicollet opened this issue May 4, 2023 · 8 comments · Fixed by #34
Closed

Please add option to build as a shared library #32

remicollet opened this issue May 4, 2023 · 8 comments · Fixed by #34
Assignees
Labels
enhancement New feature or request

Comments

@remicollet
Copy link
Contributor

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

@dkg
Copy link
Contributor

dkg commented May 31, 2023

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
Copy link

ni4 commented May 31, 2023

@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
Copy link
Member

maxirmx commented Jun 15, 2023

@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
Copy link

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
Copy link
Member

maxirmx commented Jun 15, 2023

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 rnpgp/rnp#2076
it is fixed)

@andreasstieger
Copy link

andreasstieger commented Jun 15, 2023

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

@ronaldtse
Copy link
Contributor

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!

@ronaldtse ronaldtse added the enhancement New feature or request label Jun 16, 2023
@maxirmx maxirmx linked a pull request Jun 17, 2023 that will close this issue
@antonsviridenko
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants