Skip to content

Decouple symbol versioning from distribution packaging mode - #3426

Open
justsmth wants to merge 1 commit into
aws:mainfrom
justsmth:decouple-symbol-versioning
Open

Decouple symbol versioning from distribution packaging mode#3426
justsmth wants to merge 1 commit into
aws:mainfrom
justsmth:decouple-symbol-versioning

Conversation

@justsmth

Copy link
Copy Markdown
Collaborator

Description of changes:

Symbol versioning was previously enabled only through ENABLE_DIST_PKG, which also relocates headers and renames binaries. This change adds an independent ENABLE_SYMBOL_VERSIONING setting that defaults to ENABLE_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_NAMESPACE for private distributions that need version nodes other than AWS_LC_*. Custom version scripts are generated in the build tree without modifying the checked-in .map files, 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_VERSIONING intentionally does not use CMake's option() because doing so would cache its initial default and prevent an unset value from continuing to track ENABLE_DIST_PKG across reconfiguration.
  • A custom SYMBOL_VERSION_NAMESPACE changes the ABI contract and is only appropriate for privately distributed libraries. It is rejected when symbol versioning is disabled.
  • Namespace rewriting exists in both CMake and the Go generator so builds can continue using checked-in version scripts with DISABLE_GO=ON. The tests verify that both implementations produce identical output.

Testing:

  • Added Go unit coverage for valid and invalid namespaces, single-node and multi-node rewrites, namespace collisions, inheritance preservation, and end-to-end version script generation.
  • Extended run_symbol_version_test.sh to 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.
  • Existing integration coverage continues to verify version definitions, versioned exports, linking, and detection of unversioned or silently dropped symbols.

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.

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.
@github-actions

Copy link
Copy Markdown
Contributor

🔒 Security ReviewView Report

Please review before merging.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.46%. Comparing base (1b0a9fe) to head (8479745).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@WillChilds-Klein
WillChilds-Klein self-requested a review August 14, 2026 17:03
@justsmth
justsmth requested review from geedo0 and prasden and removed request for prasden August 14, 2026 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants