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: add libcperciva_cpusupport_detect.la #399

Closed
wants to merge 1 commit into from

Conversation

gperciva
Copy link
Member

No description provided.

@gperciva
Copy link
Member Author

I'm reasonably confident that the diff is correct.

I'm not confident that the commit message does an adequate job of explaining it; particularly since this is the scrypt repository, not the tarsnap one.

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.

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

Wait, I should check that this actually does fix the problem for tarsnap.

@gperciva gperciva force-pushed the lib-cpusupport-detect branch from 714bd16 to e094f8a Compare February 2, 2025 01:15
@gperciva
Copy link
Member Author

gperciva commented Feb 2, 2025

Yep, it fixes the tarsnap problem.

@gperciva gperciva marked this pull request as ready for review February 2, 2025 01:16
@@ -101,15 +91,20 @@ AM_CPPFLAGS= \
-D_XOPEN_SOURCE=700 \
${CFLAGS_POSIX}

# Each "paragraph" denotes a rank of dependencies. Libraries without
Copy link
Member Author

Choose a reason for hiding this comment

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

This is wordier than I'd like, but I decided to err on the side of clarity.

@gperciva gperciva marked this pull request as draft February 2, 2025 03:51
@gperciva
Copy link
Member Author

gperciva commented Feb 2, 2025

Nope, I forgot to check libscrypt-kdf. (and it's broken)

I'm beginning to have some mild dislike of autotools.

@gperciva gperciva force-pushed the lib-cpusupport-detect branch from e094f8a to 12726b6 Compare February 2, 2025 04:00
@gperciva gperciva marked this pull request as ready for review February 2, 2025 04:00
@gperciva
Copy link
Member Author

gperciva commented Feb 2, 2025

Fortunately it was an easy fix, and everything appears to be working now.

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

gperciva commented Feb 4, 2025

This was merged as part of #398.

@gperciva gperciva closed this Feb 4, 2025
@gperciva gperciva deleted the lib-cpusupport-detect branch February 4, 2025 17:33
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