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

Makefile.am: sort lists #398

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Makefile.am: sort lists #398

wants to merge 2 commits into from

Conversation

gperciva
Copy link
Member

No description provided.

@gperciva
Copy link
Member Author

This depends on #396 and #397.

@gperciva gperciva marked this pull request as draft January 31, 2025 18:09
@gperciva
Copy link
Member Author

When we added libscrypt_crypto_aes in #386, it worked almost by accident. That is: since scrypt uses libtool, it's a lot more forgiving of the order of libraries. Also, clang's linker is forgiving.

If we try to apply this method of lib...crypto_aes to tarsnap with gcc's linker, it fails unless we add libtarsnap.a to _LDADD twice. Once for the general "here's the symbols we need", and once at the end for "here's the cpusupport_x86_aesni_detect_1 etc symbols".

I only got that working 10 minutes ago, so I'm still reconsidering how best to approach it in scrypt. (But the PRs you already merged are still good.)

libcpusupport_crypto_aes.la needs to resolve symbols such as
cpusupport_x86_aesni_detect_1.  In scrypt, that was previously provided
by cpusupport_x86_aesni.o, which was listed explicitly on the
command-line (via Makefile.am's ${crypto_scrypt_files}).

However, in the tarsnap repository, cpusupport_x86_aesni.o was linked
into libtarsnap.a.  When we tried to add libcperciva_crypto_aes.a, that
created a circular dependency:
- libtarsnap.a needed crypto_aes symbols (which were in
  libcperciva_crypto_aes)
- libcperciva_crypto_aes needed cpusupport_x86_aesni_detect_1 (which
  were in libtarsnap.a)

To avoid the circular dependency, we're moving the cpusupport detection
functionality into its own library.  We're also clarifying the order of
libraries in the scrypt_LDADD list.
Also, remove memlimit.h from libscrypt_memlimit.la; it's not needed.
@gperciva gperciva marked this pull request as ready for review February 2, 2025 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant