Guard and document the macOS/Linux native-linker override and ThinLTO by zzcgumn · Pull Request #418 · dds-bridge/dds

zzcgumn · 2026-10-08T22:28:05Z

Summary

Adds //python:ci_macos_native_linker_test, a regression guard for the extra_link_flags/extra_link_libs/supports_start_end_lib mechanism that routes native Darwin/Linux links through the host linker (deb44608, 20d97b4d, 85770509) — this had no test coverage, unlike the apple_support ASAN patches (ci_macos_asan_test).
Verifies and documents that swapping macOS's link driver from ld64.lld to Apple's native /usr/bin/ld does not silently break the -flto=thin ThinLTO invariant: built and linked a real target on Xcode 27.0 / macOS 26.6.2 with -v, confirmed Apple's linker (ld-27037.1) reports built-in LTO support for LLVM 21 bitcode without needing the classic -lto_library plugin flag. Added one new test assertion plus a doc paragraph (docs/BUILD_SYSTEM.md) so a future regression here doesn't go unnoticed.
Brings specs/build-system.md's "Toolchains are hermetic" invariant up to date: it already documented the Linux linker exception but not the macOS one, and didn't mention the two apple_support patches under "Key entry points".

Why this is separate from the v3.1.1 release PR

This is pure test/doc polish discovered while backporting the macOS 27 fix to a release/v3.1.1 branch for main (see #417) — it doesn't change any build behavior, so it goes straight to develop rather than through the release branch.

Test plan

bazelisk test //library/... //python/... — 87/87 pass
New test (ci_macos_native_linker_test) passes against develop's existing MODULE.bazel/.bazelrc/CPPVARIABLES.bzl as-is (pure regression guard, nothing to fix)

🤖 Generated with Claude Code

Adds a regression test for the extra_link_flags/extra_link_libs/ supports_start_end_lib mechanism (no prior guard existed for it, unlike the apple_support ASAN patches), verifies and documents that Apple's current linker still performs ThinLTO without an explicit -lto_library plugin flag, and brings specs/build-system.md's hermeticity invariant up to date with the macOS/Linux linker exceptions already shipped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

zzcgumn · 2026-10-09T07:06:00Z

@tameware , Claude suggests we do this to reduce the risk of similar breaks in the future.

Copilot

🟡 Changes recommended

The ThinLTO test cannot detect the claimed silent regression, and parts of the new documentation are inaccurate.

3 open findings

Add integration coverage proving ThinLTO cross-TU optimization · New
Forward linker verbosity flags to reliably show LTO support · New
Scope macOS hermeticity claim to toolchain and SDK exceptions · New

What changed in this PR

Adds regression checks and documentation for native linker overrides and macOS ThinLTO.

Changes:

Adds configuration assertions for macOS/Linux linker behavior.
Documents linker exceptions and ThinLTO verification.
Registers the new Python test and its data.

File	Description
`.bazelrc`	Clarifies MSAN feature impact.
`BUILD.bazel`	Exposes linker configuration files to tests.
`docs/BUILD_SYSTEM.md`	Documents native linking and ThinLTO.
`python/BUILD.bazel`	Registers the regression test.
`python/tests/ci_macos_native_linker_test.py`	Checks linker and LTO configuration.
`specs/build-system.md`	Updates toolchain invariants and patch references.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

- Narrow the new ThinLTO test's name/docstring: it only guards that -flto=thin stays requested, not that cross-TU optimization actually happens — a future linker could keep accepting the flag while silently dropping the optimization, and this test can't catch that. - Fix the LTO verification command in docs/BUILD_SYSTEM.md: plain `bazel build -s`/`-v` never reach the linker's own stderr banner; `--linkopt=-Wl,-v` is needed to forward -v through clang to the linker. - Scope the macOS hermeticity bullet in specs/build-system.md to the default toolchain and call out that --config=asan switches compilation itself (not just the link) to the installed Xcode toolchain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

zzcgumn · 2026-10-09T08:27:53Z

Addressed in e2cc9e1:

Renamed/clarified test_macos_keeps_thin_lto_at_compile_and_link → test_macos_requests_thin_lto_flags_at_compile_and_link; docstring now states explicitly it only guards that -flto=thin stays requested, not that cross-TU optimization actually occurs.
docs/BUILD_SYSTEM.md: fixed the verification command to bazel build --linkopt=-Wl,-v //path:target, since plain -s/-v never reach the linker's own banner.
specs/build-system.md: scoped the hermeticity bullet to the default toolchain and called out that --config=asan switches macOS compilation itself (not just the link) to the installed Xcode toolchain.

Didn't add a full cross-TU LTO integration fixture — building a fixture that reliably detects whether the linker actually performed cross-TU inlining (vs. just accepted the flag) is a meaningfully bigger, more fragile piece of infrastructure than this polish PR's scope, and the doc already documents the manual verification procedure (now corrected) as the mitigation for that gap.

104/104 tests pass.

🤖 Generated with Claude Code

Copilot

🟡 Changes recommended

The build-system specification inaccurately describes macOS SDK hermeticity and introduces documentation that can become stale.

1 open finding

Use bazelisk for verification commands · New

3 resolved since last review

Add integration coverage proving ThinLTO cross-TU optimization
Scope macOS hermeticity claim to toolchain and SDK exceptions
Forward linker verbosity flags to reliably show LTO support

Previously missed (2)

In code that hasn't changed since last review

Document Xcode SDK and linker as host inputs

specs/build-system.md:83

This still overstates hermeticity: the compiler binary comes from toolchains_llvm, but it compiles against the installed Xcode SDK, whose .tbd inputs are the reason this override is needed (docs/BUILD_SYSTEM.md:56-76). Describe both the SDK and linker as host inputs so the reproducibility invariant is accurate.

Remove hard-coded dependency version

specs/build-system.md:114

This hard-codes an exact dependency version immediately after the spec says exact versions live only in MODULE.bazel and drift (lines 71-72). Remove the version here so this entry does not become stale on the next apple_support bump.

🧠 Review effort: Balanced

- Use bazelisk (not bare bazel) in the LTO verification commands, matching the rest of docs/BUILD_SYSTEM.md. - specs/build-system.md: the macOS hermeticity bullet now names the installed SDK, not just the linker, as a host input — compilation reads the host SDK's headers/.tbd stubs even on the default toolchain. - specs/build-system.md: drop the hard-coded apple_support 1.24.2 version from Key entry points; exact versions live only in MODULE.bazel per this spec's own dependency-graph bullet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

zzcgumn · 2026-10-09T16:13:47Z

Addressed in 1d705cd:

Switched the LTO verification commands to `bazelisk` (was bare `bazel`), matching the rest of the doc.
`specs/build-system.md`: the hermeticity bullet now calls out the installed macOS SDK itself as a host input (compilation reads its headers/`.tbd` stubs), not just the linker.
`specs/build-system.md`: removed the hard-coded `apple_support 1.24.2` version from Key entry points — versions live only in `MODULE.bazel` per this spec's own rule.

24/24 `//python/...` tests pass (doc/spec-only change, no behavior touched).

🤖 Generated with Claude Code

Copilot

🟢 Approval recommended

The focused tests and documentation accurately reflect the existing linker configuration without changing build behavior.

0 open findings

1 resolved since last review

Use bazelisk for verification commands

🧠 Review effort: Balanced

tameware · 2026-10-09T17:57:08Z

Approved, with one unresolved comment.

"macOS keeps requesting ThinLTO" read as an assertion of fact rather than a description of what the test checks. Spell out that this only guards flag presence in CPPVARIABLES.bzl, not that the linker actually performs cross-TU optimization — matching the test method's own docstring. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

zzcgumn requested review from tameware and a balanced review from Copilot October 9, 2026 07:05

Copilot started reviewing on behalf of zzcgumn October 9, 2026 07:05 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread python/tests/ci_macos_native_linker_test.py

Comment thread docs/BUILD_SYSTEM.md Outdated

Comment thread specs/build-system.md Outdated

tameware requested a balanced review from Copilot October 9, 2026 12:54

Copilot started reviewing on behalf of tameware October 9, 2026 12:55 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread docs/BUILD_SYSTEM.md Outdated

zzcgumn self-assigned this Oct 9, 2026

zzcgumn requested a balanced review from Copilot October 9, 2026 16:24

Copilot started reviewing on behalf of zzcgumn October 9, 2026 16:24 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

zzcgumn added the Clean Copilot review Copilot reviewed and had neither new comments nor new suppressed comments. label Oct 9, 2026

tameware approved these changes Oct 9, 2026

View reviewed changes

Comment thread python/BUILD.bazel Outdated

zzcgumn merged commit 2271747 into develop Oct 10, 2026
14 checks passed

zzcgumn mentioned this pull request Oct 10, 2026

Bump toolchains_llvm to 1.11.2, drop #791 archive_override #423

Open

4 of 5 tasks

Navigation Menu

Repository navigation

Guard and document the macOS/Linux native-linker override and ThinLTO - #418

Guard and document the macOS/Linux native-linker override and ThinLTO#418
zzcgumn merged 4 commits into
developfrom
chore/macos_native_linker_test_and_docs

zzcgumn commented Oct 8, 2026

Uh oh!

zzcgumn commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zzcgumn commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

zzcgumn commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

tameware commented Oct 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

zzcgumn commented Oct 8, 2026

Summary

Why this is separate from the v3.1.1 release PR

Test plan

Uh oh!

zzcgumn commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zzcgumn commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

zzcgumn commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

tameware commented Oct 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants