Decouple symbol versioning from distribution packaging mode - #3426
Open
justsmth wants to merge 1 commit into
Open
Decouple symbol versioning from distribution packaging mode#3426justsmth wants to merge 1 commit into
justsmth wants to merge 1 commit into
Conversation
ENABLE_SYMBOL_VERSIONING was gated solely on ENABLE_DIST_PKG, which also
forces COHABITANT_HEADERS and COHABITANT_BINARIES. Consumers could not
take versioned symbols without also relocating headers into
include/aws-lc and renaming the bssl tool.
ENABLE_SYMBOL_VERSIONING is now its own option, defaulting to the value
of ENABLE_DIST_PKG so existing configurations are unchanged. Requesting
it on a static, Apple, or Windows build is a configure error; when merely
inherited it stays quietly off, so a static dist-pkg build still
configures.
Also adds -DSYMBOL_VERSION_NAMESPACE=<prefix> to replace the AWS_LC
prefix in version node names, for consumers who ship a private
libcrypto. Only node names are rewritten; the checked-in .map files are
untouched and the renamed script is written into the build tree. A
non-default namespace is not ABI-compatible with a stock AWS-LC build, so
it is documented as private-distribution only and is refused outright
when versioning is disabled.
Two things that are not obvious from the diff:
- ENABLE_SYMBOL_VERSIONING is deliberately not declared with option().
option() would freeze the first configure's default into the cache,
so a later -DENABLE_DIST_PKG=ON would leave versioning stuck off.
Testing DEFINED instead lets an explicit -D win while an unset value
keeps tracking ENABLE_DIST_PKG.
- The namespace rewrite is duplicated in CMake rather than shelling
out to util/generate_version_script, because the checked-in .map
files exist precisely so that a build never runs the generator
(notably with -DDISABLE_GO=ON). run_symbol_version_test.sh pins the
two implementations together: it renames a multi-node fixture with
each and diffs the results, so a change to the generator's output
format cannot silently leave the CMake copy behind.
The default path links the checked-in .map verbatim and the generator
still emits byte-identical output, so the CI map drift check is
unaffected.
Drive-by: converts a trailing "[[ ... ]] && cmd" in check_dropped_symbols
to an explicit if, so a false condition can never become the function's
exit status under set -e.
Contributor
|
🔒 Security Review — View Report Please review before merging. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3426 +/- ##
==========================================
+ Coverage 78.24% 78.46% +0.22%
==========================================
Files 696 696
Lines 124511 124514 +3
Branches 17279 17281 +2
==========================================
+ Hits 97418 97701 +283
+ Misses 26168 25886 -282
- Partials 925 927 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
WillChilds-Klein
self-requested a review
August 14, 2026 17:03
justsmth
requested review from
geedo0 and
prasden
and removed request for
prasden
August 14, 2026 17:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
Symbol versioning was previously enabled only through
ENABLE_DIST_PKG, which also relocates headers and renames binaries. This change adds an independentENABLE_SYMBOL_VERSIONINGsetting that defaults toENABLE_DIST_PKG, preserving existing behavior while allowing consumers to adopt versioned ELF symbols without the other distribution packaging changes. Explicit requests are rejected for unsupported static, Apple, and Windows builds, while inherited settings remain disabled without breaking existing static distribution package configurations.This also adds
SYMBOL_VERSION_NAMESPACEfor private distributions that need version nodes other thanAWS_LC_*. Custom version scripts are generated in the build tree without modifying the checked-in.mapfiles, and the standalone version script generator supports the same namespace override.Documentation is updated to describe the independent configuration, platform restrictions, and ABI implications of custom namespaces.
Call-outs:
ENABLE_SYMBOL_VERSIONINGintentionally does not use CMake'soption()because doing so would cache its initial default and prevent an unset value from continuing to trackENABLE_DIST_PKGacross reconfiguration.SYMBOL_VERSION_NAMESPACEchanges the ABI contract and is only appropriate for privately distributed libraries. It is rejected when symbol versioning is disabled.DISABLE_GO=ON. The tests verify that both implementations produce identical output.Testing:
run_symbol_version_test.shto verify that symbol versioning can be enabled without distribution packaging, does not enable header or binary cohabitation, writes namespaced scripts only into the build tree, rejects namespaces when versioning is disabled, and keeps the CMake and Go rewrite implementations equivalent.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.