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

Always build "internal" library as static #1725

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Conversation

SWilson4
Copy link
Member

Since liboqs-internal (see #1667) is never actually installed on the user's system, it doesn't make sense to build it as a shared library and then link the test programs against it. This PR forces liboqs-internal to be built as a static library regardless of the BUILD_SHARED_LIBS variable.

Fixes #1723.

  • Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
  • Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider will also need to be ready for review and merge by the time this is merged.)

@SWilson4 SWilson4 requested a review from baentsch March 11, 2024 14:13
@SWilson4 SWilson4 changed the title Always build "internal" library as statuc Always build "internal" library as static Mar 11, 2024
@baentsch
Copy link
Member

This PR forces liboqs-internal to be built as a static library regardless of the BUILD_SHARED_LIBS variable.

Hmm -- does this also guarantee that the (test) executables then are statically linked (to the symbols in the internal lib) while they're still dynamically linked to the symbols in the public (shared) lib?

@vt-alt
Copy link
Contributor

vt-alt commented Mar 11, 2024

I tested the patch and it looked good. Test binaries still linked to liboqs.so.5 (and there is no liboqs-internal.so anymore):

builder@x86_64-p10:~/tmp/liboqs-buildroot$ ldd usr/bin/oqs-test-sha3
        linux-vdso.so.1 (0x00007ffe185ec000)
        liboqs.so.5 => not found
        libcrypto.so.1.1 => /lib64/libcrypto.so.1.1 (0x00007f0fcdc42000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f0fcdc21000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f0fcda48000)
        libz.so.1 => /lib64/libz.so.1 (0x00007f0fcda2a000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00007f0fcda24000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f0fcdf5a000)

It says not found because it's inside of buildroot (temporary dir where liboqs is installed).

@SWilson4
Copy link
Member Author

Hmm -- does this also guarantee that the (test) executables then are statically linked (to the symbols in the internal lib) while they're still dynamically linked to the symbols in the public (shared) lib?

Yes, I believe so. (Based on the test executables now running if liboqs-internal.a is removed but not if liboqs.so.5 is removed.)

Signed-off-by: Spencer Wilson <[email protected]>
@SWilson4 SWilson4 force-pushed the sw-internal-lib-static branch from ff7164c to fe7f7ca Compare March 11, 2024 16:27
@SWilson4 SWilson4 marked this pull request as ready for review March 11, 2024 16:27
@SWilson4 SWilson4 requested a review from dstebila as a code owner March 11, 2024 16:27
@SWilson4 SWilson4 added the bug Something isn't working; high priority to fix label Mar 11, 2024
Copy link
Contributor

@jgoertzen-sb jgoertzen-sb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Edit: I always forget to use my personal account... sorry

Copy link
Member

@Martyrshot Martyrshot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (But from this account)

@baentsch baentsch merged commit 1bc6d11 into main Mar 13, 2024
77 checks passed
@baentsch baentsch deleted the sw-internal-lib-static branch March 13, 2024 09:56
@dstebila
Copy link
Member

Thanks @baentsch! The discussion in the OQS status call yesterday was that we would tag an -rc2 after this landed, which I will do tonight unless I hear otherwise.

Eddy-M-K pushed a commit to Eddy-M-K/liboqs that referenced this pull request Apr 5, 2024
* Always build oqs-internal library as static

Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Eddy Kim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working; high priority to fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.10.0-rc1: kat-kem: liboqs-internal.so => not found
6 participants