bazel: switch C++ toolchain to hermetic-llvm; add etc/bazel-hermetic#10812
Conversation
Replace toolchains_llvm + prebuilt LLVM release binaries with hermetic-llvm (BCR module 'llvm'): statically linked LLVM 22.1.8 binaries and a zero-sysroot cc_toolchain. No host compiler, linker, headers or libraries are involved; the toolchain runs unmodified on hosts without libxml2.so.2 (Ubuntu 25.10+, Arch, Fedora 41+, where prebuilt ld.lld could not start) and on non-FHS distros such as NixOS. clang-tidy now comes from @llvm//tools:clang-tidy. tcl_lang needs a one-line patch: tclZipfs.c's '#include "crypt.h"' resolved to glibc's crypt.h instead of the vendored minizip header once glibc headers became explicit -isystem directories. Drop the override when a fixed tcl_lang lands in BCR. Tip of the hat to @dzbarsky for pointing us at hermetic-llvm: bazel-contrib/toolchains_llvm#795 (comment) Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Launches bazel with an allowlisted PATH (no host compilers, linkers or
interpreters), BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1, and only the
workspace bazelrc, so builds fail instead of silently reaching for host
tools the build should provide. Proxy and cache environment variables
pass through; the bazelisk-managed bazel binary is resolved portably.
First finding: rules_python's legacy py_binary stub needs a host
python3 ('#!/usr/bin/env python3') before the hermetic interpreter
takes over. bootstrap_impl=script uses a shell stub instead.
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
Code Review
This pull request transitions the C++ toolchain from toolchains_llvm to hermetic-llvm (@llvm), updates associated configurations, documentation, and scripts, and introduces a new etc/bazel-hermetic script to run Bazel in a pruned environment. Feedback on the new hermeticity script highlights several critical issues: command -v can create circular symlinks for shell builtins, the ELF binary check breaks on macOS, essential environment variables for SSH and SSL are not preserved, and the use of exec bypasses the cleanup trap, leading to leaked temporary directories.
- type -P instead of command -v: builtins (pwd, test, true, printf) returned bare names and produced self-referential toolbox symlinks. - Detect wrappers by bazelisk name or shebang instead of requiring ELF, so native macOS (Mach-O) bazel binaries are symlinked directly. - Preserve SSH_AUTH_SOCK, SSL_CERT_FILE/DIR and TMPDIR alongside proxy and cache variables. - Drop exec so the EXIT trap runs and the toolbox directory is not leaked on every invocation. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@hzeller Look what the cat dragged in! Hermetic LLVM or halucination? |
std::filesystem (src/tcl_readline_setup.cc) requires macOS 10.15. The previous 10.13 floor was never enforced: toolchains_llvm ignored --macos_minimum_os and compiled against the host SDK default. hermetic-llvm honors it, failing the Mac build with "'path' is unavailable: introduced in macOS 10.15". Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
With bootstrap_impl=script, a py_binary (tclint) invoked via realpath cannot locate its runfiles after the lint scripts cd to the workspace. Export the runfiles root so nested tools resolve it from the environment. No-op under 'bazel test', where the runner sets it. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@maliberty Look at what the cat(Claude) dragged in :-) |
hermetic-llvm ships a deliberately minimal macOS SDK subset; frameworks beyond its six defaults are opt-in via the osx.frameworks extension tag. Qt's bootstrap failed on the first missing one (ApplicationServices/ApplicationServices.h). List the desktop frameworks qtbase links against. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
ATS (inside the ApplicationServices umbrella) ships .tbd symlinks into
the FontServices private framework. Without it in the subset they
dangle, and bazel rejects the whole sysroot as an action input
('The file type ... is not supported'). Zero dangling symlinks remain
in the extracted SDK after this.
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
This looks very promising! Still waiting for the build to complete but it looks like this is the first time in 2 years that I don't have to comment out the llvm toolchain on nixos, as it properly works! |
hermetic-llvm's SDK extraction excludes usr/include/cups unless PrintCore is listed explicitly; naming only its ApplicationServices umbrella left PDEPluginInterface.h's '#import <cups/ppd.h>' dangling when Qt compiles Objective-C++. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
AppKit's NSColor.h includes CoreImage/CIColor.h unconditionally. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Suggestions from review: - tools/bazel is bazelisk's wrapper convention, so the pruned environment is applied automatically to every bazelisk invocation (Go bazelisk; the npm package does not implement the convention). $BAZEL_REAL is used when bazelisk provides it; direct invocation and BAZELISK_SKIP_WRAPPER=1 remain available. - BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1 moves to .bazelrc so it applies to unwrapped invocations too. - --host_action_env=PATH complements --action_env for exec-config actions. DISPLAY/WAYLAND_DISPLAY/XAUTHORITY pass through for 'bazel run' of GUI targets. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
AppKit reaches all three on macOS: NSImageView.h includes Symbols/NSSymbolEffect.h unconditionally, the NSText headers take the non-UIKit __has_include branch into the UIFoundation private framework, and NSSharingService pulls CloudKit. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Jenkins surfaced three problems with the first wrapper cut: - --action_env=PATH broke test actions: the harness uses tee/file and standalone_python tests invoke host python3 (bazel's own test-setup.sh also uses file). Actions now keep bazel's strict default PATH; per-action pruning returns when the harness stops using host tools (see the PR cleanup list). - An --action_env PATH naming a per-run mktemp directory would also fragment action cache keys across runs and machines. - The npm bazelisk implements the tools/bazel wrapper convention too, so the wrapper's own 'bazelisk --version' probe re-entered the wrapper; probe with BAZELISK_SKIP_WRAPPER=1 and pass flags-only invocations (e.g. --version) through verbatim. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
boost::icl's exclusive_less_than comparator violates strict weak ordering (boostorg/icl#51). libstdc++ tolerates it; libc++'s std::map does not: interval_set::operator-= subtracts only the first overlapping interval and leaves phantom coverage (boostorg/icl#55). With the hermetic toolchain's libc++, pad placed IO filler cells on top of pads (seven src/pad tests) and grt's overlapping_edges golden diverged. Carry upstream boostorg/icl@7de3f55655 ('Refactored interval lookup operations', The-OpenROAD-Project#54, merged after boost 1.90), trimmed to include/, as a single_version_override patch. Verified: an isolated interval_set subtraction repro is wrong with libc++ headers and correct with the patch at -O0 through -O3; all 61 //src/pad tests and //src/grt/test:overlapping_edges pass. Drop the patch when a boost.icl release containing the fix lands in BCR. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
hermetic-llvm injects Linux kernel and glibc headers as the first -isystem entries, outranking -isystem/-I directories of libraries that deliberately shadow libc headers. BCR sed (gnulib's stdio.h replacement) fails to compile (SETLOCALE_NULL_MAX, _GL_ATTRIBUTE_* undeclared) — breaking test/orfs/gcd targets on Jenkins — and tcl's vendored minizip crypt.h loses to glibc's crypt.h. Host sysroots provide libc via the default search path, searched after all user includes; carry a patch switching the two cc_args to -idirafter to restore that precedence. Verified: unpatched BCR sed 4.9 builds; //:openroad builds; 327 pad/grt/odb tests pass. Propose upstream after this stabilizes; drop the patch when released. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
Code Review
This pull request transitions the build system from toolchains_llvm to hermetic-llvm (llvm), providing a zero-sysroot C++ toolchain. It introduces several single-version overrides and patches for dependencies like bison, boost.icl, gawk, m4, sed, and tcl_lang to resolve header shadowing and compilation issues under the new toolchain. Additionally, it updates macOS minimum OS requirements, configures macOS frameworks, and adds a developer diagnostic script etc/bazel-hermetic to detect host toolchain leaks. The review feedback suggests avoiding hardcoded canonical repository names in the bison patch by using native.repo_name(), and optimizing the bazel-hermetic script by replacing eval with indirect variable expansion and using the Bash builtin read instead of spawning a subshell with head.
Gemini flagged the hardcoded `external/bison+/lib` canonical repo name as non-portable (Bazel 7 uses `~`, Bazel 8 `+`). Correct in principle — this repo pins Bazel 8.6.0 so `+` is right for what ships, but the patch is headed upstream (bison@3.8.2.bcr.8) where it must survive both. Gemini's suggested `native.repo_name()` does NOT work here: `native` is undefined at BUILD-file top level (verified: "name 'native' is not defined"), and BISON_COPTS is a top-level assignment. The bare builtin `repo_name()` is the one callable there — verified in a scratch Bazel 8.6.0 module that it returns the canonical name with the separator (`depb+`), so bison resolves to `bison+`, byte-identical to the previous hardcoding on Bazel 8 and correct on Bazel 7.1+. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Two style/robustness findings from the latest Gemini pass:
- Read the bazel signature with `read -r -n 2` instead of a `head`
subprocess; the redirection still follows the symlink via open(), so
no head/realpath/readlink is involved. Verified it reads the ELF magic
through a symlink and matches "#!" for a shebang wrapper.
- Preserve environment via pure-bash indirect expansion: `${!var+x}`
tests whether the named variable is set (set -u-safe) and `${!var}`
reads it — no eval, no subprocess.
Full stub-bazel harness still green: build/repo_env, bug-case exit,
info passthrough, and the non-bazelisk-wrapper fallback.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
/gemini review |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Code Review
This pull request transitions the build system from toolchains_llvm to hermetic-llvm (the llvm BCR module) to enforce a zero-sysroot, statically linked LLVM toolchain. It introduces several patches for transitive dependencies (such as bison, boost.icl, gawk, m4, sed, and tcl_lang) to resolve header shadowing and compilation issues under the hermetic toolchain. Additionally, it adds an opt-in developer diagnostic script, etc/bazel-hermetic, to run Bazel in a pruned environment and catch host-toolchain leaks. Feedback on this script suggests preserving cloud credential environment variables (e.g., Google Cloud and AWS) to prevent remote cache authentication failures.
Gemini: env -i strips the credentials bazel needs to authenticate to cloud remote-cache / remote-execution backends (GCS, S3), so a build that relies on the remote cache would fail to authenticate. Preserve GOOGLE_APPLICATION_CREDENTIALS and the common AWS_* variables alongside the proxy/TLS/bazelisk config already passed through — same rationale: auth/cache configuration is not toolchain leakage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions the project's C++ toolchain from toolchains_llvm to hermetic-llvm (BCR module llvm) to ensure a statically linked, zero-sysroot build environment. The changes include updates to Bazel configuration files, documentation, and dependency patches for bison, boost.icl, and tcl_lang. Additionally, a new developer diagnostic script etc/bazel-hermetic is introduced. Feedback on this script highlights several critical shell scripting issues, specifically the need to double-quote array expansions to prevent word splitting when arguments contain spaces, and the recommendation to resolve symbolic links portably when determining the script's root directory.
| # ${arr[@]+"${arr[@]}"} expands to nothing when the array is empty: a bare | ||
| # "${arr[@]}" is an unbound-variable error under set -u on bash < 4.4 (the | ||
| # 3.2 that macOS ships as /bin/bash). | ||
| bazel_argv=(--nosystem_rc --nohome_rc ${startup_flags[@]+"${startup_flags[@]}"}) |
There was a problem hiding this comment.
The array expansion ${startup_flags[@]+"${startup_flags[@]}"} is unquoted on the outside. This causes the shell to perform word splitting and globbing on the expanded elements. If any startup flag contains spaces (e.g., --host_jvm_args="-Xmx4g -Xms2g"), it will be incorrectly split into multiple arguments.
To preserve elements with spaces and prevent word splitting, the entire expansion should be double-quoted: "${startup_flags[@]+"${startup_flags[@]}"}".
bazel_argv=(--nosystem_rc --nohome_rc "${startup_flags[@]+"${startup_flags[@]}"}")
| # 3.2 that macOS ships as /bin/bash). | ||
| bazel_argv=(--nosystem_rc --nohome_rc ${startup_flags[@]+"${startup_flags[@]}"}) | ||
| if [[ -n ${command} ]]; then | ||
| bazel_argv+=("${command}" ${extra_flags[@]+"${extra_flags[@]}"} "$@") |
There was a problem hiding this comment.
| # Flags-only invocations (e.g. `bazel --version`) go to the binary verbatim; | ||
| # rc-file options would be rejected there. | ||
| if [[ -z ${command} ]]; then | ||
| bazel_argv=(${startup_flags[@]+"${startup_flags[@]}"}) |
There was a problem hiding this comment.
| TERM="${TERM:-dumb}" \ | ||
| LANG="${lang}" \ | ||
| PATH="${TOOLBOX}" \ | ||
| ${preserved_env[@]+"${preserved_env[@]}"} \ |
There was a problem hiding this comment.
The array expansion ${preserved_env[@]+"${preserved_env[@]}"} is unquoted on the outside. If any preserved environment variable contains spaces (e.g., SSL_CERT_FILE="/path with spaces/cert.pem"), it will be split into multiple arguments. This will cause env to fail or misinterpret the split parts as the command to execute.
Double-quote the entire expansion to ensure that each environment variable is passed as a single, intact argument to env.
"${preserved_env[@]+"${preserved_env[@]}"}" \
| LANG="${lang}" \ | ||
| PATH="${TOOLBOX}" \ | ||
| ${preserved_env[@]+"${preserved_env[@]}"} \ | ||
| "${TOOLBOX}/bazel" ${bazel_argv[@]+"${bazel_argv[@]}"} |
There was a problem hiding this comment.
The array expansion ${bazel_argv[@]+"${bazel_argv[@]}"} is unquoted on the outside. This will cause any arguments in bazel_argv that contain spaces (such as target names, options, or query expressions with spaces) to be split.
Double-quote the entire expansion to ensure that all arguments are passed to bazel exactly as they were received.
"${TOOLBOX}/bazel" "${bazel_argv[@]+"${bazel_argv[@]}"}"
| # the workspace tmp/ (gitignored): per-user and per-checkout by | ||
| # construction (no /tmp cross-user collisions) and not on a RAM-backed | ||
| # tmpfs as /tmp commonly is. | ||
| ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) |
There was a problem hiding this comment.
If the script is executed via a symbolic link (e.g., symlinked to a directory on the user's PATH like ~/bin/bazel-hermetic), ${BASH_SOURCE[0]} will point to the symlink itself rather than the actual script location. This causes ROOT to resolve incorrectly, breaking the workspace path resolution.
We can resolve the symlink portably in Bash using a simple loop.
TARGET="${BASH_SOURCE[0]}"
while [[ -L ${TARGET} ]]; do
TARGET=$(readlink "${TARGET}")
done
ROOT=$(cd "$(dirname "${TARGET}")/.." && pwd)
|
@maliberty Gemini is trickle feeding me review feedback, I change one thing, then it complains about another in the etc/bazel-hermetic, only used for local testing. |
|
@maliberty @hzeller Merge now and a free for all to refine/upstream afterwards as the case may be? |
|
@codex review |
|
we can ignore gemini and give codex one more pass |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74eca7d537
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
m4's vendored gnulib headers deliberately shadow libc headers; under toolchains that inject libc headers as leading -isystem entries (hermetic-llvm, The-OpenROAD-Project#10812) m4@1.4.21 fails to compile (O_SEARCH, BCR The-OpenROAD-Project#7642 class). 1.4.21.bcr.4 packages the gnulib wrappers so they keep precedence. Behavior-neutral on the current toolchain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
I've merged your three other hermeticity PRs. Please update this one correspondingly. |
…hain Signed-off-by: Matt Liberty <mliberty@precisioninno.com> # Conflicts: # MODULE.bazel # MODULE.bazel.lock
|
@maliberty Thanks for merging! Anything else from me? |
Are there upstream PRs for the patches in this PR? |
|
@maliberty Once this merges, I'll do an inventory of patches in OpenROAD and see which we can create upstream patches for. I have patches to upstream in ORFS + bazel-orfs + locally in our repo too. I'll have Claude do a full inventory and create upstream PRs for those that are eligble and where we're done with churn and testing. |
|
@maliberty — here's the inventory of local patches this PR left carried in Guiding approach
Carried patches
Already upstream / no local patch — just drop the override when BCR catches up
The one ask@maliberty — the abc Not part of this inventory
|
|
Correction on the orfs row above: I'd implied a clean "retarget Bumping Current ORFS So retiring this patch needs a real upstream fix in OpenROAD-flow-scripts to make |
|
Upstream PRs created from this inventory:
Not submitted, with reasons:
|
|
I can't PR |
All good. The patch in in OpenROAD is a record of that todo item... All in good time... |
Replaces #10809 (and the earlier #9809).
Scope: this PR does one thing — swap the C++ toolchain — plus the minimum fallout fixes for CI to pass. It unblocks a list of cleanups (below) that are deliberately deferred; review this on the toolchain swap alone.
Tip of the hat to @dzbarsky for pointing us at hermetic-llvm: bazel-contrib/toolchains_llvm#795 (comment)
Problem
Prebuilt LLVM release binaries dynamically link
ld.lldagainstlibxml2.so.2. libxml2 2.14 changed its soname tolibxml2.so.16, so on Ubuntu 25.10+/Arch/Fedora 41+ the linker cannot start without host workarounds (libxml2-dev+sudo ln), and with a compatibility symlink every link action printsno version information available(llvm/llvm-project#138225; fixed in LLVM release binaries from 23.1.0, scheduled 2026-08-25).Change
llvm, v0.8.11): statically linked LLVM 22.1.8 binaries and a zero-sysrootcc_toolchain. No host compiler, linker, headers or libraries are involved — the libxml2 problem ceases to exist rather than being worked around, and the toolchain also runs unmodified on non-FHS distros (NixOS).clang-tidynow comes from@llvm//tools:clang-tidy.tcl_langone-line patch:tclZipfs.c's#include "crypt.h"resolved to glibc'scrypt.hinstead of the vendored minizip header once glibc headers became explicit-isystemdirectories. Dropped when a fixedtcl_langlands in BCR.etc/bazel-hermetic: launches bazel with an allowlisted PATH (no host compilers/linkers/interpreters),BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1, and only the workspace bazelrc — builds fail instead of silently reaching for host tools the build should provide. Proxy/cache env passes through; the bazelisk-managed bazel binary is resolved portably (incorporates the review feedback from bazel: stub ld.lld's libxml2.so.2 via toolchains_llvm; add etc/bazel-hermetic #10809). First finding, fixed here: everypy_binaryneeded a hostpython3via rules_python's legacy stub shebang;bootstrap_impl=scriptremoves that.Verification
Ubuntu 26.04, no
libxml2.so.2on the host (the oldsudo lnworkaround deleted),gcc/cc/clang/ld/python3/perl/make/cmakeall absent from the pruned PATH:The binary targets glibc 2.28+, so it remains portable across distros.
Type of Change
Bug fix (build infrastructure)
I have verified that the local build succeeds
My code follows the repository's formatting guidelines
I have signed my commits (DCO).
Out of scope — follow-up cleanup once this merges
Unblocked by this PR, deliberately not in it. Each is its own concern and its own PR:
etc/DependencyInstaller.sh::_install_bazel: the Ubuntu 26.04libxml2-devinstall and thelibxml2.so.16 → libxml2.so.2symlink written into/usr/lib/<arch>-linux-gnu/; droplibxml2from the apt and yum-bazelpackage lists. Audit whetherlibc6-dev/glibc-develandlibtinfo6are still needed under a zero-sysroot toolchain.sudo lnworkaround and installer symlink are obsolete and can be removed from affected systems.sed/ncurses vs glibc 2.43 host headers) no longer reproduces — zero-sysroot removes the failure class — and close it; abandoned PR bazel: fix latent C++ compiler errors surfaced on Ubuntu 26.04 LTS #10523 becomes moot.@llvm-project//openmp:libomp(from source, version-locked to the compiler, per review suggestion). Verified working end to end — builds, links statically,set_thread_countengages — including the one-line dep change it needs in the OpenSTA submodule (src/sta/BUILD), which is why it is a follow-up PR rather than part of this one.tcl_langcrypt.hpatch when a fixed release lands in BCR.toolchains_llvmworkflows, and remove the NixOS "comment out the toolchain" advice (NixOS now works unmodified, per review feedback on this PR).tools/bazelon a runner withoutlibxml2.so.2/compilers so host-tool leaks cannot regress.test/regression_test.shinvokes hostpython3for standalone_python tests and//:dup_id_testshells out to barepython3; once fixed, re-tightentools/bazelwith per-action PATH pruning (backed out for now — it also fragmented action cache keys via the per-run toolbox path).Upstream PRs — after this merges
This PR carries two small local patches instead of waiting on upstream review
cycles. That ordering is deliberate. Master today does not build on hosts
without
libxml2.so.2(Ubuntu 25.10+, Arch, Fedora 41+) except by hand-editing/usr/lib; that is worth days of review, not weeks of upstream latency. Oncemerged and proven stable in daily use, the patches go upstream in the exact
shape usage validated — and if upstream review reshapes them, the follow-up
here adopts what actually lands and drops the local copy.
tcl_lang(BCR): thetclZipfs.ccrypt.h include fix carried here asbazel/tcl-patches/.hermetic-llvm: propose the libc include-precedence change carried here asbazel/hermetic-llvm-patches/(-isystem → -idirafter for kernel/glibc headers; fixes the gnulib/BCR-sed and vendored-header-shadowing class); drop the local patch when released.bazel-orfs: gnumake's repository rule runs GNU Make's configure, which probes PATH for ld; plumb the linker like CC is plumbed so the rule survives pruned-PATH environments (etc/bazel).boost.icl(BCR): the strict-weak-ordering fix carried here asbazel/boost-icl-patches/is already merged upstream (Refactored interval lookup operations boostorg/icl#54); drop the patch when a boost.icl release containing it reaches BCR. Worth a comment on interval_map::operator-= leaves phantom covering intervals boostorg/icl#55 that Macro halo option, unit tests for tapcell #54 resolves the interval_set case under libc++.hermetic-llvm: field report + fixes from this adoption — dangling.tbdsymlinks when an umbrella sub-framework's private-framework target is not in the SDK subset (FontServices case), and the undocumentedPrintCore↔usr/include/cupscoupling.