Skip to content

cmake: Amend bitcoin_crypto static library#172

Merged
hebasto merged 1 commit into
cmake-stagingfrom
240426-cmake-EF
May 1, 2024
Merged

cmake: Amend bitcoin_crypto static library#172
hebasto merged 1 commit into
cmake-stagingfrom
240426-cmake-EF

Conversation

@hebasto
Copy link
Copy Markdown
Owner

@hebasto hebasto commented Apr 26, 2024

Avoid using source-specific compile options. They hard to reason about and error-prone when combining with target-specific compile options.

The resulted build logic effectively mirrors the master branch's one.

Required for #157.

Similar to #171.

@hebasto hebasto added the blocker Required for other steps label Apr 27, 2024
Avoid using source-specific compile options. They hard to reason about
and error-prone when combining with target-specific compile options.

The resulted build logic effectively mirrors the master branch's one.
Comment thread src/crypto/CMakeLists.txt
target_compile_definitions(bitcoin_crypto
PRIVATE
ENABLE_SSE41
add_library(bitcoin_crypto_sse41 STATIC EXCLUDE_FROM_ALL
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same comment as #171. Would OBJECT libraries be better here?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Same as #171 (comment) :)

Yes, it could. But CMake's developers recommend to use STATIC libraries if there is no compelling reasons for OBJECT ones. For example, from Professional CMake: A Practical Guide 18th Edition

Prefer defining static libraries over object libraries. Static libraries are simpler, have more
complete and robust support from earlier CMake versions and they are well understood by most
developers. Object libraries have their uses, but they are also less flexible than static libraries.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Object libraries have their uses, but they are also less flexible than static libraries.

This is not clear to me. In which ways are they less flexible?

Copy link
Copy Markdown
Owner Author

@hebasto hebasto May 1, 2024

Choose a reason for hiding this comment

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

Object libraries have their uses, but they are also less flexible than static libraries.

This is not clear to me. In which ways are they less flexible?

I think, this part of the quote is not related to us as we use CMake >= 3.22.

Related to https://cmake.org/cmake/help/latest/command/target_link_libraries.html#linking-object-libraries

Copy link
Copy Markdown
Owner Author

@hebasto hebasto May 1, 2024

Choose a reason for hiding this comment

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

The internal CMake code that handles STATIC libraries is older and more robust than code for OBJECT ones. Sticking to the former type only allows us to have predictable behaviour across the whole codebase.

Copy link
Copy Markdown

@sedited sedited left a comment

Choose a reason for hiding this comment

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

ACK 5ca0799

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocker Required for other steps

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants