Add SONAME Support to AWS-LC #2546
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2546 +/- ##
==========================================
+ Coverage 78.71% 78.73% +0.01%
==========================================
Files 645 645
Lines 110641 110642 +1
Branches 15648 15649 +1
==========================================
+ Hits 87094 87109 +15
+ Misses 22846 22834 -12
+ Partials 701 699 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
skmcgrail
force-pushed
the
soname
branch
12 times, most recently
from
6c99a49 to
08ffcc4
Compare
skmcgrail
changed the title
skmcgrail
requested review from
samuel40791765
and removed request for
andrewhop
There was a problem hiding this comment.
Awesome change, can we remove this file now or are we still required to keep it?
There was a problem hiding this comment.
Same question here, can we remove include/openssl/base.h entirely and replace it with this?
| @@ -4,6 +4,6 @@ includedir=@includedir_for_pc_file@ | |||
|
|
|||
| Name: AWS-LC-libcrypto | |||
| Description: AWS-LC cryptography library | |||
| Version: @VERSION@ | |||
| Version: 1.1.1 | |||
There was a problem hiding this comment.
Could we still abstract this into CMakeLists to make the pkgconfig files easier to change?
| if(PERFORM_SONAME_BUILD) | ||
| set(CRYPTO_LIB_NAME "${CRYPTO_LIB_NAME}-${SOFTWARE_NAME}") | ||
| set(SSL_LIB_NAME "${SSL_LIB_NAME}-${SOFTWARE_NAME}") | ||
| endif() |
There was a problem hiding this comment.
Should we fold this into the same if condition on L77?
| install(CODE " | ||
| set(TGT lib${CRYPTO_LIB_NAME}.so.${ABI_VERSION}) | ||
| set(LNK \$ENV{DESTDIR}${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/libcrypto.so) | ||
| message(STATUS \"Creating symlink: \${LNK} → \${TGT}\") | ||
| execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink \${TGT} \${LNK}) | ||
| " | ||
| COMPONENT Development) |
There was a problem hiding this comment.
Minor nit: Indentation
| if(PERFORM_SONAME_BUILD) | ||
| install(CODE " | ||
| set(TGT lib${SSL_LIB_NAME}.so.${ABI_VERSION}) | ||
| set(LNK \$ENV{DESTDIR}${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/libssl.so) | ||
| message(STATUS \"Creating symlink: \${LNK} → \${TGT}\") | ||
| execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink \${TGT} \${LNK}) | ||
| " | ||
| COMPONENT Development) | ||
| endif() |
There was a problem hiding this comment.
Minor nit: indentation
Description of changes:
The following would be the naming structure for the different flavors of the AWS-LC project for the adoption of SONAME on UNIX platforms (sans macOS/Apple and Windows devices).
Similarly libssl would follow a matching naming scheme to that shown in the table above. This scheme allows for possible ABI changes amongst each variant of the library we offer.
Our default CMake installation commands will continue to provide the
libcrypto.soandlibssl.sosymlinks to the SONAME symlink file, as to not require changes to existing applications linking to AWS-LC dynamically. No changes will be made to the static build of AWS-LC, and the static archive files will continue to be namedlibcrypto.aandlibssl.aas expected.Here is an example install directory of what this would look like:
Call-outs:
This enables the SONAME build by default, but we might wish to either flip the default state of the
ENABLE_PRE_SONAME_BUILDCMake build option to be ON by default rather then OFF if we don't wish to start using the immediately. Right now it is off by default as I wanted to validate CI passes correctly with the change without having to plumb yet another dimension that I don't believe is fully necessary for every build variation we have.The runbook for releases will need to be updated to indicated that the software version for release is now fully controlled via the top-level
CMakeLists.txtvariable calledSOFTWARE_VERSION. In addition two headers are now templated with static information from the CMake configuration and generated back into theincludefolder (for backwards compatibility for some one-off builds, though directly depending on the source include directory is discouraged, and the installation include directory is treated as the supported methodology).Lastly a number of CI jobs has historically had logic for explicitly checking the linkage of a compiled integration or program to verify it linked the built AWS-LC libcrypto or libssl from the installation directory to ensure the test correctly used AWS-LC as expected. To facilitate the long-term ability to remain agnostic to whether a build is or is not using an SONAME a
check-linage.shshell script is provided in the AWS-LC CMake build directory output that can be used to validate that a provided file dynamically links AWS-LC from the installation prefix directory that was configured for the build. This script should be used going forward rather then usinglddandgrepwith a hard-coded library file name, as this script is templated with the necessary information to perform the check correctly.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.