Skip to content

bazel: switch C++ toolchain to hermetic-llvm; add etc/bazel-hermetic#10812

Merged
maliberty merged 35 commits into
The-OpenROAD-Project:masterfrom
oharboe:hermetic-llvm-toolchain
Jul 8, 2026
Merged

bazel: switch C++ toolchain to hermetic-llvm; add etc/bazel-hermetic#10812
maliberty merged 35 commits into
The-OpenROAD-Project:masterfrom
oharboe:hermetic-llvm-toolchain

Conversation

@oharboe

@oharboe oharboe commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

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.lld against libxml2.so.2. libxml2 2.14 changed its soname to libxml2.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 prints no version information available (llvm/llvm-project#138225; fixed in LLVM release binaries from 23.1.0, scheduled 2026-08-25).

Change

  • Switch the C++ toolchain to hermetic-llvm (BCR module llvm, v0.8.11): statically linked LLVM 22.1.8 binaries and a zero-sysroot cc_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-tidy now comes from @llvm//tools:clang-tidy.
  • tcl_lang 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. Dropped when a fixed tcl_lang lands 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: every py_binary needed a host python3 via rules_python's legacy stub shebang; bootstrap_impl=script removes that.

Verification

Ubuntu 26.04, no libxml2.so.2 on the host (the old sudo ln workaround deleted), gcc/cc/clang/ld/python3/perl/make/cmake all absent from the pruned PATH:

$ etc/bazel-hermetic build //:openroad
INFO: Build completed successfully, 6404 total actions
$ grep -c "libxml2\|/usr/include" build.log
0
$ ldd bazel-bin/openroad | head -5
        linux-vdso.so.1
        libpthread.so.0 => /usr/lib/x86_64-linux-gnu/libpthread.so.0
        libdl.so.2 => /usr/lib/x86_64-linux-gnu/libdl.so.2
        libm.so.6 => /usr/lib/x86_64-linux-gnu/libm.so.6
        libc.so.6 => /usr/lib/x86_64-linux-gnu/libc.so.6

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:

  • Revert the bazel: installer for ubuntu26 #10778 workaround in etc/DependencyInstaller.sh::_install_bazel: the Ubuntu 26.04 libxml2-dev install and the libxml2.so.16 → libxml2.so.2 symlink written into /usr/lib/<arch>-linux-gnu/; drop libxml2 from the apt and yum -bazel package lists. Audit whether libc6-dev/glibc-devel and libtinfo6 are still needed under a zero-sysroot toolchain.
  • Comment on closed Bazel build fails on Ubuntu 26.04: downloaded LLVM ld.lld depends on missing libxml2.so.2 #10761: the sudo ln workaround and installer symlink are obsolete and can be removed from affected systems.
  • Verify Bazel build fails on glibc 2.43 #9786 (BCR 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.
  • Switch OpenMP to @llvm-project//openmp:libomp (from source, version-locked to the compiler, per review suggestion). Verified working end to end — builds, links statically, set_thread_count engages — 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.
  • Drop the tcl_lang crypt.h patch when a fixed release lands in BCR.
  • Update docs that still describe toolchains_llvm workflows, and remove the NixOS "comment out the toolchain" advice (NixOS now works unmodified, per review feedback on this PR).
  • Add a CI job that builds through tools/bazel on a runner without libxml2.so.2/compilers so host-tool leaks cannot regress.
  • Make the test harness hermetic: test/regression_test.sh invokes host python3 for standalone_python tests and //:dup_id_test shells out to bare python3; once fixed, re-tighten tools/bazel with per-action PATH pruning (backed out for now — it also fragmented action cache keys via the per-run toolbox path).
  • Note new QoR/perf baselines for Bazel- and CMake-built openroad from the same source produce different QoR #10336 (compiler moves 20.1.8 → 22.1.8) before any bisection work.

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. Once
merged 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): the tclZipfs.c crypt.h include fix carried here as bazel/tcl-patches/.
  • hermetic-llvm: propose the libc include-precedence change carried here as bazel/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 as bazel/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 .tbd symlinks when an umbrella sub-framework's private-framework target is not in the SDK subset (FontServices case), and the undocumented PrintCoreusr/include/cups coupling.

oharboe added 2 commits July 4, 2026 15:00
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>
@oharboe oharboe requested a review from a team as a code owner July 4, 2026 13:01
@oharboe oharboe requested a review from osamahammad21 July 4, 2026 13:01
@github-actions github-actions Bot added the size/M label Jul 4, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread etc/bazel-hermetic
Comment thread etc/bazel-hermetic Outdated
Comment thread etc/bazel-hermetic Outdated
Comment thread etc/bazel-hermetic Outdated
- 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>
@oharboe

oharboe commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

@hzeller Look what the cat dragged in! Hermetic LLVM or halucination?

oharboe added 2 commits July 4, 2026 15:27
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>
@oharboe

oharboe commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

@maliberty Look at what the cat(Claude) dragged in :-)

@dzbarsky dzbarsky left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool!

Comment thread tools/bazel Outdated
Comment thread etc/bazel-hermetic Outdated
Comment thread etc/bazel-hermetic
Comment thread MODULE.bazel
oharboe added 2 commits July 4, 2026 15:47
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>
@hzeller

hzeller commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

This looks very promising!
As I said before, the only way to get an actual hermetic toolchain is to boostrap it.

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!

oharboe added 5 commits July 4, 2026 16:19
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>
@maliberty maliberty requested review from maliberty and removed request for osamahammad21 July 4, 2026 16:03
Comment thread etc/bazel-hermetic
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>
@github-actions github-actions Bot added size/XL and removed size/M labels Jul 4, 2026
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>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread etc/bazel-hermetic Outdated
Comment thread etc/bazel-hermetic Outdated
oharboe and others added 2 commits July 7, 2026 22:54
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>
@oharboe

oharboe commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread etc/bazel-hermetic Outdated
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>
@oharboe

oharboe commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread etc/bazel-hermetic
# ${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[@]}"})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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[@]}"}")

Comment thread etc/bazel-hermetic
# 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[@]}"} "$@")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The array expansion ${extra_flags[@]+"${extra_flags[@]}"} is unquoted on the outside, which can lead to word splitting if any of the extra flags contain spaces. Double-quote the entire expansion to ensure safe expansion.

  bazel_argv+=("${command}" "${extra_flags[@]+"${extra_flags[@]}"}" "$@")

Comment thread etc/bazel-hermetic
# 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[@]}"})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The array expansion ${startup_flags[@]+"${startup_flags[@]}"} is unquoted on the outside, which can lead to word splitting if any of the startup flags contain spaces. Double-quote the entire expansion to ensure safe expansion.

  bazel_argv=("${startup_flags[@]+"${startup_flags[@]}"}")

Comment thread etc/bazel-hermetic
TERM="${TERM:-dumb}" \
LANG="${lang}" \
PATH="${TOOLBOX}" \
${preserved_env[@]+"${preserved_env[@]}"} \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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[@]}"}" \

Comment thread etc/bazel-hermetic
LANG="${lang}" \
PATH="${TOOLBOX}" \
${preserved_env[@]+"${preserved_env[@]}"} \
"${TOOLBOX}/bazel" ${bazel_argv[@]+"${bazel_argv[@]}"}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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[@]}"}"

Comment thread etc/bazel-hermetic
# 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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)

@oharboe

oharboe commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

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

@oharboe

oharboe commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@maliberty @hzeller Merge now and a free for all to refine/upstream afterwards as the case may be?

@maliberty

Copy link
Copy Markdown
Member

@codex review

@maliberty

Copy link
Copy Markdown
Member

we can ignore gemini and give codex one more pass

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread test/downstream/MODULE.bazel
maliberty pushed a commit to oharboe/OpenROAD that referenced this pull request Jul 7, 2026
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>
@maliberty

Copy link
Copy Markdown
Member

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
@oharboe

oharboe commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

@maliberty Thanks for merging!

Anything else from me?

@maliberty maliberty enabled auto-merge July 8, 2026 05:05
@maliberty

Copy link
Copy Markdown
Member

Anything else from me?

Are there upstream PRs for the patches in this PR?

@oharboe

oharboe commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

@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 maliberty merged commit e823004 into The-OpenROAD-Project:master Jul 8, 2026
15 of 16 checks passed
@oharboe

oharboe commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

@hzeller @maliberty 🥳

@oharboe

oharboe commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

@maliberty — here's the inventory of local patches this PR left carried in MODULE.bazel, with where each one is headed and how it will be retired.

Guiding approach

  • Where a dep is a repo we control whose Bazel build files are in-tree, we retarget MODULE.bazel at a pinned fork commit (idiomatic Bazel remote-resource tracking) and drop the in-tree patch.
  • Where the dep's Bazel build comes from the BCR overlay (the upstream project ships no MODULE.bazel/BUILD.bazel), we stay a contributor: send the fix to the existing BCR module / source project and keep the local patch until it ships in a real BCR release. We are deliberately not standing up our own BCR fork-registry (that's a maintainer role we don't want).

Carried patches

Patch Module Where the fix goes Status / plan
bazel/bazel-orfs-patches/0035-…non-root-overrides orfs The-OpenROAD-Project/OpenROAD-flow-scripts ORFS is in-tree-Bazel → retarget archive_override to a pinned fork commit + open PR; drop patch
bazel/bison-patches/0001-gnulib-wrapper-headers-use-I… bison bazelbuild/bazel-central-registry (bison@3.8.2.bcr.8) overlay-only fix; PR to BCR (mirrors the sed@4.9.bcr.5 #7915 and gawk@5.3.2.bcr.7 #7989 fixes). Keep patch until released
bazel/boost_context_disable_parse_headers.patch boost.context bazelbuild/bazel-central-registry overlay-only (features=["parse_headers"] breaks under the zero-sysroot toolchain; still present in 1.90.0.bcr.1). PR to BCR; keep patch until released
bazel/tcl-patches/0001-tclZipfs…crypt.h… tcl_lang Tcl (fossil/SourceForge — no GitHub PR) no clean contributor path; likely carried until a tcl_lang BCR release picks it up. Suggestions welcome
bazel/abc-patches/0001-mainUtils-flush-prompt… abc YosysHQ/abc see ask below — keep patch meanwhile

Already upstream / no local patch — just drop the override when BCR catches up

  • scip (scip_tinycthread_tny_prefix.patch) → scipopt/scip@cc917e07 is merged upstream and is already in the v10 source; keeping 9.2.3 + patch for now rather than a 9→10 major bump. Drop when a 9.2.x BCR release carries it.
  • boost.icl (…icl-54) → boostorg/icl#54 merged; landed after boost 1.90, so not yet in a BCR boost.icl release. Drop when it is.
  • sed / gawk / m4 → already bumped to fixed BCR versions in this PR; no patch carried.

The one ask

@maliberty — the abc mainUtils flush/echo/EOF patch is yours, and the natural home is YosysHQ/abc (which is what our BCR abc module tracks). You've upstreamed to abc before — could you open that PR upstream? We'll keep carrying the patch until it lands in a BCR abc release, then drop it. (Worth flagging to berkeley-abc/abc too so it survives the next YosysHQ rebase.)

Not part of this inventory

  • bazel/scip-patches/0001-…once-flag… is an untracked local experiment, not referenced in MODULE.bazel — ignore.
  • The hermetic-llvm -isystem → -idirafter change is not carried as a local patch here (there's no llvm override in MODULE.bazel); it lives in the llvm 0.8.11 release, not in this tree.

@oharboe

oharboe commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Correction on the orfs row above: I'd implied a clean "retarget archive_override to a newer commit + drop the patch." On checking, that doesn't work — the patch stays for now.

Bumping orfs past the current pin (tried bdab1d08f5 and origin/master HEAD) fails module resolution:

No repository visible as '@bazel-orfs' from repository '@@orfs+'
  ... @@orfs+//:MODULE.bazel: use_repo_rule("@bazel-orfs//private:designs.bzl", ...)

Current ORFS MODULE.bazel calls use_repo_rule("@bazel-orfs//private:designs.bzl") at top level, which needs @bazel-orfs visible from @orfs+ — but bazel-orfs is declared dev_dependency (root-only), so it's invisible when orfs is consumed as a non-root dep. The old remove-non-root-overrides patch worked only because the pinned 10a2baea had no @bazel-orfs reference left after its deletions.

So retiring this patch needs a real upstream fix in OpenROAD-flow-scripts to make orfs cleanly consumable as a non-root module (e.g. bazel-orfs as a regular bazel_dep with the version supplied downstream, while git_override/orfs.default stay root-only) — with its own tests in that repo. Keeping patch 0035 here until then. The rest of the inventory is unaffected.

@oharboe

oharboe commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Upstream PRs created from this inventory:

Not submitted, with reasons:

  • boost.context — the parse_headers feature is templated across all 113 modular-boost overlays, not a boost.context bug, so a one-off version bump would be inconsistent and clobbered by the next boost release. Keeping the local patch.
  • abc@maliberty, still yours to open upstream (YosysHQ/abc); patch stays here until it lands in a BCR abc release.
  • orfs — patch stays; needs the upstream ORFS non-root-consumption fix described above.
  • tcl_lang — no GitHub PR path (fossil/SourceForge); carried until a BCR release picks it up.
  • scip / boost.icl — already fixed upstream; just awaiting a BCR release to drop the overrides.

@maliberty

Copy link
Copy Markdown
Member

I can't PR YosysHQ/abc as PRs are not accepted there. The changes are in the upstream abc. I've asked YosysHQ to update but still waiting.

@oharboe

oharboe commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

I can't PR YosysHQ/abc as PRs are not accepted there. The changes are in the upstream abc. I've asked YosysHQ to update but still waiting.

All good. The patch in in OpenROAD is a record of that todo item... All in good time...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bazel build fails on Ubuntu 26.04: downloaded LLVM ld.lld depends on missing libxml2.so.2

5 participants