CI: Customize v8 building#22086
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cf09e4acf
ℹ️ 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".
| # Keep V8 artifact releases out of Codex's normal "latest release" channel. | ||
| prerelease: true | ||
|
|
||
| - name: Amend existing GitHub Release |
There was a problem hiding this comment.
This was helpful for the iteration cycle. Its feels like a hack that we maybe shouldn't support long term.
| } | ||
|
|
||
| #[test] | ||
| fn parses_crdtp_dispatchable_messages() { |
There was a problem hiding this comment.
This is a new test just to flex the linked crdtp libraries.
bolinfest
left a comment
There was a problem hiding this comment.
Off the top of my head:
- I feel like there should be a Python script or something that someone should expect to be able to run to update to a new version of v8 and that this should be called out in
third_party/v8/README.md. - The PR body says that we build the v8 part of the
x86_64-w64-windows-gnubuild from source now, with a compatible ABI. Previously, that part was not runnable, which led to carve-outs in CI like this:
codex/.github/workflows/bazel.yml
Lines 88 to 99 in e3f481d
Can those go away after this PR? (If so, I'm fine seeing that done in a separate PR since this one is already a lot to reason about.)
|
I updated the documentation and added a codex skill for updating the version. I'm going to try running the skill with the un-released version |
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Produce explicitly named sandbox release pairs alongside the current compatibility artifacts, and validate staged sandbox outputs before publication across the supported artifact targets. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Keep non-Windows rusty-v8 consumers on the resolved 147.4.0 source-built graph. Avoid mixing LLVM's default libc++ headers into rusty-v8's custom libc++ build, propagate that configuration to external C++ deps, and export the musl-specific libc++ define through the shared header target. Also make checksum validation follow the remaining prebuilt asset version while the source-build migration is in progress. Co-authored-by: Codex <noreply@openai.com>
Add the `crdtp_binding.cc` source and CRDTP headers to the 147 Bazel V8 binding target so Windows GNU builds provide the symbols required by the `v8::crdtp` Rust APIs. Add a regression test that exercises CRDTP JSON conversion and dispatchable message parsing.
Fetch the published per-target checksum asset alongside the musl archive and binding so Cargo musl jobs keep working without re-expanding the MODULE.bazel checksum manifest contract.
b0f35b1 to
b489b06
Compare
Summary
Move the rusty_v8 artifact production into hermetic Bazel path and bump the
v8crate to147.4.0The new flow builds V8 release artifacts from source for Darwin and Linux targets, publishes both the current release-compatible artifacts and sandbox-enabled variants, and keeps Cargo consumers on prebuilt binaries by continuing to feed the
v8crate the archive and generated binding files it already expects.Why
We need control over V8 build-time features without giving up prebuilt artifacts for downstream Cargo builds.
Upstream
rusty_v8already supports source-only features such asv8_enable_sandbox, but its normal prebuilt release assets do not cover every feature combination we need. Building the artifacts ourselves lets us enable settings such as the V8 sandbox and pointer compression at artifact build time, then publish those outputs so ordinary Cargo builds can still consume prebuilts instead of compiling V8 locally.This keeps the fast consumer experience of prebuilt
rusty_v8archives while giving us a reproducible path to ship featureful variants that upstream does not currently publish for us.Implementation Notes
The Bazel graph in this PR is not copied wholesale from
rusty_v8;rusty_v8's normal source build is still GN/Ninja-based.Instead, this change starts from upstream V8's Bazel rules and adapts them to Codex's hermetic toolchains and dependency layout. Where we intentionally follow
rusty_v8, we mirror its existing artifact contract:v8crate version and generated binding expectationsuse_custom_libcxxfeaturesrc_bindingoutputs consumed by thev8crateTo preserve that contract, the Bazel release path pins the libc++, libc++abi, and llvm-libc revisions used by
rusty_v8 v147.4.0, builds release artifacts with--config=rusty-v8-upstream-libcxx, and folds the matching runtime objects into the final static archive.Windows
Windows is annoyingly handled differently.
Codex's current hermetic Bazel Windows C++ platform is
windows-gnullvm/x86_64-w64-windows-gnu, while upstreamrusty_v8publishes Windows prebuilts for*-pc-windows-msvc. Those are different ABIs, so the Bazel graph cannot truthfully reproduce the upstream MSVC artifacts until we add a real MSVC-targeting C++ toolchain.For now:
rusty_v8release archives.rusty_v8source builds for MSVC sandbox artifacts, but MSVC is not yet part of the Bazel-produced release matrix.Validation
This PR is technically self validating through CI. I have already published it as a release tag so the artifacts from this branch are published to https://github.com/openai/codex/releases/tag/rusty-v8-v147.4.0 CI for this PR should therefore consume our own release targets. I have also locally tested for linux and darwin.