fix(bazel): repair v8-canary config (llvm flag target + lockfile drift) - #625
Closed
KooshaPari wants to merge 27 commits into
Closed
fix(bazel): repair v8-canary config (llvm flag target + lockfile drift)#625KooshaPari wants to merge 27 commits into
KooshaPari wants to merge 27 commits into
Conversation
…ULE.bazel.lock drift
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:XS
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| # graph (rules_cc 0.2.16 vs 0.2.17) differs between runs; both rewrite the | ||
| # committed MODULE.bazel.lock and trip check-clean-worktree. Ignore the lockfile | ||
| # for builds; the repository cache keeps module resolution cheap. | ||
| build --lockfile_mode=off |
There was a problem hiding this comment.
Suggestion: Disabling the Bzlmod lockfile for every build and test invocation allows Bazel to resolve a different module graph than the committed MODULE.bazel.lock, so builds can silently consume changed dependency or toolchain versions and no longer satisfy the repository's documented reproducibility invariant. Keep the lockfile enforced for normal builds and restrict any lockfile bypass to the dedicated lock-update workflow or an explicit developer option. [possible bug]
Severity Level: Major ⚠️
- ⚠️ V8 canary builds may use differing dependency graphs.
- ⚠️ Ordinary Bazel builds ignore committed dependency resolution.
- ⚠️ Local and CI results can diverge after registry changes.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** .bazelrc
**Line:** 29:29
**Comment:**
*Possible Bug: Disabling the Bzlmod lockfile for every build and test invocation allows Bazel to resolve a different module graph than the committed `MODULE.bazel.lock`, so builds can silently consume changed dependency or toolchain versions and no longer satisfy the repository's documented reproducibility invariant. Keep the lockfile enforced for normal builds and restrict any lockfile bypass to the dedicated lock-update workflow or an explicit developer option.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous Review Summaries (17 snapshots, latest commit a0f54ba)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a0f54ba)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit eeaab31)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit 95c8076)Status: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous review (commit 2e88ead)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit 8921419)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit bbd7caf)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit fbbc39c)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit 7b7f331)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit ff34e24)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 6b916a9)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous review (commit 1ff45b1)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous Review Summaries (6 snapshots, latest commit bbd7caf)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit bbd7caf)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit fbbc39c)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 7b7f331)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit ff34e24)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 6b916a9)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous review (commit 1ff45b1)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Reviewed by step-3.7-flash · Input: 76.3K · Output: 26.7K · Cached: 1.1M |
MODULE.bazel never carried the v8 section after the codex import (#603): the v8-canary build failed at option load with 'No repository visible as '@v8''. Restore the full consumer wiring from the pre-import layout with the currently pinned versions (crate 149.2.0, V8 source 14.9.207.2): - v8 bazel_dep + archive_override (integrity computed from upstream tarball) - v8_crate_149_2_0 / v8_crate_146_4_0 http_archive (shas verified) - rusty_v8_libcxx / rusty_v8_libcxxabi (LLVM 21.1.8 release tarballs) - rusty_v8_llvm_libc git_repository (chromium mirror pin) - Windows MSVC http_file archives (shas match the checksum manifest) - v8_targets new_local_repository + crate.annotation/inject_repo Rename stale @llvm labels in third_party/v8 to the module's canonical toolchains_llvm_bootstrapped name, including the @@llvm++glibc/+musl/ +kernel_headers extension-repo labels (the 0.5.6 module generates those repos under its own canonical name). check-module-bazel and the rusty_v8_bazel unit tests pass.
third_party/v8/BUILD.bazel loads @bazel_skylib//rules:copy_file.bzl but MODULE.bazel never declared the module after the codex import. Pin the fork's previous version (1.8.2, per pre-import MODULE.bazel).
…trapped The root BUILD.bazel, bazel/platforms/release_binaries.bzl, bazel/rules/testing/wine/wine_runtime.bzl, and rbe.bzl still referenced @llvm//constraints/libc and @llvm//platforms, which are not visible from the main repo map (the module is pinned as toolchains_llvm_bootstrapped). Rename them to the canonical name so analysis of //third_party/v8 pair targets no longer aborts on '//:local_linux'.
V8's ICU torque compile fails against the hermetic glibc headers because libc++'s __locale_dir/support/linux.h uses iswspace_l/iswprint_l/iswcntrl_l, which glibc only declares under __USE_GNU. Chromium builds libc++ with _GNU_SOURCE; mirror that on the custom-libcxx header library and the libc++ runtime build.
v8_libbase compiles with -nostdinc++ against the custom libc++ headers,
but the toolchain's builtin include paths put its own libc++ headers
(runtimes/libcxx) before the glibc/musl headers. libc++'s C-compat
wrappers (e.g. <string.h>) rely on #include_next to reach the C library,
so the search dead-ends on the toolchain's libc++ copy and C symbols like
::memcpy never resolve ('reference to unresolved using declaration' in
__functional/hash.h).
Wire the extension-generated C library headers into _default_args deps,
mirroring the libcxx overlay selects (glibc for gnu platforms, musl for
musl), so include_next from the custom libc++ wrappers lands on the real
headers. Also add --verbose_failures to the canary build step so future
failures print the full action command.
The v8+ external repo cannot see @toolchains_llvm_bootstrapped by name (its patched MODULE.bazel does not declare the module). Reference the gnu/musl config settings via their canonical @@ names, matching how the libcxx overlays reference the extension repos.
The v8+ external module cannot reference @toolchains_llvm_bootstrapped at
all - its patched MODULE.bazel does not declare the module, so even
canonical labels fail to resolve ('Repository ... is not defined').
Move the gnu/musl platform selection into a root-repo wrapper target
(rusty_v8_c_library_headers) that v8 reaches through the same
@@//third_party/v8:... pattern as the libcxx overlays, and select the
glibc/musl header sets inside the root repo where the toolchain module
and its extension repos are visible.
The patched bazel/BUILD.icu builds icuuc/icui18n with -nostdinc++ and the custom libc++ headers, so their include_next C-compat wrappers hit the same dead-end as the v8 targets: no glibc headers in the search path, C symbols like memcpy/memset unresolved (cmemory.h errors). Add rusty_v8_c_library_headers to CUSTOM_LIBCXX_DEPS alongside the headers/runtime wrappers.
With -nostdinc++ the custom libc++ headers (used for the V8 build) and the toolchain's own libc++ headers both sit before glibc/musl in the include search path. libc++'s C-compat wrappers reach the C library via #include_next, so the chain dead-ends on the toolchain's libc++ copy (identical _LIBCPP_* guards skip its body) and C symbols like ::memcpy never resolve, even with glibc on the path after it. Patch the toolchains_llvm_bootstrapped module so linux_toolchain_args emits the kernel + glibc/musl -isystem paths before the libc++ ones, keeping linux_target_headers (module map) in sync. The root-repo rusty_v8_c_library_headers wrapper stays as belt-and-suspenders.
…hain" This reverts commit ff34e24.
The custom libc++ (rusty_v8_libcxx) and the toolchain's own libc++ are both libc++ 21.1.8 with identical _LIBCPP_*_H include guards. When a custom header does #include_next <string.h> (or <ctype.h>, <math.h>, ...), the search lands on the toolchain's copy, whose guard is already defined, so its body - including its own include_next - is skipped and glibc/musl is never reached: C symbols like ::memcpy stay undeclared. Restructure the custom copy's C-compat wrappers so the include_next runs before the include guard (the pattern <wchar.h>/<stdio.h>/<stddef.h> already use): the toolchain's copy then runs, defines the standard _LIBCPP_*_H guard (satisfying the <cctype>-style checks), and forwards to the C library. The remaining wrappers get their guards renamed (_LIBCPP_CR_*_H) so the chain flows through instead of dead-ending; their post-include_next bodies are undefs/macros only, so running twice is harmless. math.h keeps its guard but drops the MSVC-only _USE_MATH_DEFINES #else branch, which never triggers on our targets. Verified with a local compile harness replicating the CI include layout (custom -I, toolchain -isystem, real C library): the patched set compiles cleanly, the unpatched set reproduces the unresolved-symbol failures.
The previous approach renamed the custom copy's include guards (_LIBCPP_*_H -> _LIBCPP_CR_*_H). That fixed the v8/ICU compile layout, where the toolchain's own libc++ 21.1.8 copy follows in the include search path and still defines the standard guards - but it broke the libcxx runtime build itself: that build has no toolchain libc++ copy, so <cctype>/<cwctype> from src/system_error.cpp never found libc++'s <ctype.h>/<wctype.h> and the self-check #error fired. Restructure all 13 C-compat wrappers uniformly to libc++'s own include_next-before-guard pattern (as <wchar.h>/<stdio.h>/<stddef.h> already use): the include_next runs first, then the wrapper defines the STANDARD-named guard. In the v8/ICU layout the toolchain copy runs and defines the guard (or the custom copy does if it lands there); in the runtime layout the custom copy defines it. Both layouts satisfy the <cctype>-style checks and the chain always reaches glibc/musl. Files: complex.h ctype.h errno.h fenv.h float.h inttypes.h math.h stdatomic.h stdbool.h string.h tgmath.h uchar.h wctype.h. math.h drops its MSVC-only _USE_MATH_DEFINES #else branch (never reached on our targets); uchar.h keeps its has_include_next fallback and defines the guard after the whole block. Validated with a local g++ harness replicating both CI include layouts: patched headers compile in the with-toolchain-copy and without it, and the unpatched control reproduces the original unresolved-symbol errors. The patch was applied in a git-initialized checkout mirroring bazel's patch step (a stray .git in a parent directory previously made git apply skip silently).
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XL
…uild With the custom libc++ runtime now compiling, v8's host-tool links pulled BOTH the custom libcxx/libcxxabi objects and the toolchain's own liblibcxx.static.a / liblibcxxabi.static.a (wired in via the cc_toolchain static_runtime_lib), so every std::* definition in stdexcept.o was duplicated and ld.lld aborted. Suppress the toolchain's libc++/libc++abi/libunwind runtime archives when the use_rusty_v8_custom_libcxx config is active: the custom libcxx runtime provides libc++ and libc++abi, and a new static_runtime_lib_no_libcxx filegroup keeps only libunwind.static since the custom libc++abi calls _Unwind_* without defining it. dynamic_runtime_lib is switched to :none in that config as well. Other configs are unchanged. The select key references the main repo's use_rusty_v8_custom_libcxx config_setting (@@//third_party/v8:...), which mirrors the @v8// flag, so the toolchain module can react to the flag without depending on v8. The patch is applied via the existing single_version_override with patch_strip = 1. Verified against the BCR 0.5.6 source archive (sha256 matches source.json integrity).
The custom-libcxx config_setting matched for the bootstrap runtime builds too (runtime_stage stage0/stage1), where the toolchain's runtimes_none / runtimes_stage1 keys also match - an illegal ambiguous select. Gate the suppression on a combined config_setting that additionally requires runtime_stage=complete, so only user targets drop the toolchain's libc++.
The toolchain module's select on the main-repo config_setting requires visibility from the module context; private (default) visibility failed analysis with a Visibility error.
The custom-libcxx config_setting gates on the toolchain's runtime_stage flag, which was package-private; analysis failed with a Visibility error. Make the string_setting public so the main repo can reference it.
aarch64 linux-gnu now fails at link time: abseil's FormatArgImpl::Dispatch<std::basic_string> is explicitly instantiated in a .cc compiled against the toolchain's libc++, while v8 (custom libc++, ABI namespace __Cr) references std::__Cr::basic_string - undefined symbol. Wire the existing rules_cc patch (previously written but never applied) into the rules_cc single_version_override: when the use_rusty_v8_custom_libcxx config is active, cc_library targets in the external C++ dependency repos (abseil-cpp+, v8's highway and simdutf) compile with -nostdinc++ and the custom libc++ headers, so their std types match v8's __Cr ABI namespace. Other configs and repos are unaffected.
The v1.2.2 plugin definitions break with the pinned tool versions:
- taplo: the >=0.8.0 download is a bare .gz single binary; v1.2.2 lacks
rename_single_file, so the decompressed file name never matches the
`taplo` binary trunk looks up ("Unable to find binary in PATH").
- ruff: the lint command still passes `--format json`, which was removed
from ruff in v0.1.0 (now `--output-format`), causing exit_code=2.
v1.11.0 (the ref trunk-action pins) fixes both and requires trunk
>=1.22.2-beta.5, compatible with the pinned cli 1.22.2.
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Owner
Author
This was referenced Aug 13, 2026
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.
User description
Two config bugs were breaking every v8-canary/release-matrix job since the bzlmod migration (#603, 2026-07-02):
.bazelrc referenced --@llvm//config:experimental_stub_libgcc_s, but MODULE.bazel imports the hermetic LLVM module as \ oolchains_llvm_bootstrapped\ (and its llvm extension repo as \llvm-project) — nothing declares @llvm\ in the repo map. Bazel failed at option load: 'No repository visible as '@llvm' from main repository' (exit 2). The flag exists at @toolchains_llvm_bootstrapped//config:experimental_stub_libgcc_s\ (verified in cerisier/toolchains_llvm_bootstrapped 0.5.6 config/defs.bzl).
Bazel 9.2.0 (bazelisk latest, no .bazelversion) rewrites the committed MODULE.bazel.lock on every run (rules_cc 0.2.16 vs 0.2.17 graph mismatch), and the check-clean-worktree gate then fails with 'M MODULE.bazel.lock'. Added \�uild --lockfile_mode=off\ — repository cache keeps resolution cheap; the committed lock stays untouched.
Known remaining requirement (separate from this PR): v8-canary builds need \BUILDBUDDY_API_KEY\ (repo or 'bazel' environment secret) — without it the wrapper runs builds fully locally and V8 source builds exceed hosted-runner limits. That is a credentials action, not a code change.
CodeAnt-AI Description
Restore reliable V8 canary and release builds across supported platforms
What Changed
Impact
✅ V8 canary builds run across supported platforms✅ Fewer Bazel configuration and repository-resolution failures✅ Stable clean worktrees after Bazel builds💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.