2 Commits

• ci: run Windows argument-comment-lint via native Bazel (#16120)

  ## Why
  
  Follow-up to #16106.
  
  `argument-comment-lint` already runs as a native Bazel aspect on Linux
  and macOS, but Windows is still the long pole in `rust-ci`. To move
  Windows onto the same native Bazel lane, the toolchain split has to let
  exec-side helper binaries build in an MSVC environment while still
  linting repo crates as `windows-gnullvm`.
  
  Pushing the Windows lane onto the native Bazel path exposed a second
  round of Windows-only issues in the mixed exec-toolchain plumbing after
  the initial wrapper/target fixes landed.
  
  ## What Changed
  
  - keep the Windows lint lanes on the native Bazel/aspect path in
  `rust-ci.yml` and `rust-ci-full.yml`
  - add a dedicated `local_windows_msvc` platform for exec-side helper
  binaries while keeping `local_windows` as the `windows-gnullvm` target
  platform
  - patch `rules_rust` so `repository_set(...)` preserves explicit
  exec-platform constraints for the generated toolchains, keep the
  Windows-specific bootstrap/direct-link fixes needed for the nightly lint
  driver, and expose exec-side `rustc-dev` `.rlib`s to the MSVC sysroot
  - register the custom Windows nightly toolchain set with MSVC exec
  constraints while still exposing both `x86_64-pc-windows-msvc` and
  `x86_64-pc-windows-gnullvm` targets
  - enable `dev_components` on the custom Windows nightly repository set
  so the MSVC exec helper toolchain actually downloads the
  compiler-internal crates that `clippy_utils` needs
  - teach `run-argument-comment-lint-bazel.sh` to enumerate concrete
  Windows Rust rules, normalize the resulting labels, and skip explicitly
  requested incompatible targets instead of failing before the lint run
  starts
  - patch `rules_rust` build-script env propagation so exec-side
  `windows-msvc` helper crates drop forwarded MinGW include and linker
  search paths as whole flag/path pairs instead of emitting malformed
  `CFLAGS`, `CXXFLAGS`, and `LDFLAGS`
  - export the Windows VS/MSVC SDK environment in `setup-bazel-ci` and
  pass the relevant variables through `run-bazel-ci.sh` via `--action_env`
  / `--host_action_env` so Bazel build scripts can see the MSVC and UCRT
  headers on native Windows runs
  - add inline comments to the Windows `setup-bazel-ci` MSVC environment
  export step so it is easier to audit how `vswhere`, `VsDevCmd.bat`, and
  the filtered `GITHUB_ENV` export fit together
  - patch `aws-lc-sys` to skip its standalone `memcmp` probe under Bazel
  `windows-msvc` build-script environments, which avoids a Windows-native
  toolchain mismatch that blocked the lint lane before it reached the
  aspect execution
  - patch `aws-lc-sys` to prefer its bundled `prebuilt-nasm` objects for
  Bazel `windows-msvc` build-script runs, which avoids missing
  `generated-src/win-x86_64/*.asm` runfiles in the exec-side helper
  toolchain
  - annotate the Linux test-only callsites in `codex-rs/linux-sandbox` and
  `codex-rs/core` that the wider native lint coverage surfaced
  
  ## Patches
  
  This PR introduces a large patch stack because the Windows Bazel lint
  lane currently depends on behavior that upstream dependencies do not
  provide out of the box in the mixed `windows-gnullvm` target /
  `windows-msvc` exec-toolchain setup.
  
  - Most of the `rules_rust` patches look like upstream candidates rather
  than OpenAI-only policy. Preserving explicit exec-platform constraints,
  forwarding the right MSVC/UCRT environment into exec-side build scripts,
  exposing exec-side `rustc-dev` artifacts, and keeping the Windows
  bootstrap/linker behavior coherent all look like fixes to the Bazel/Rust
  integration layer itself.
  - The two `aws-lc-sys` patches are more tactical. They special-case
  Bazel `windows-msvc` build-script environments to avoid a `memcmp` probe
  mismatch and missing NASM runfiles. Those may be harder to upstream
  as-is because they rely on Bazel-specific detection instead of a general
  Cargo/build-script contract.
  - Short term, carrying these patches in-tree is reasonable because they
  unblock a real CI lane and are still narrow enough to audit. Long term,
  the goal should not be to keep growing a permanent local fork of either
  dependency.
  - My current expectation is that the `rules_rust` patches are less
  controversial and should be broken out into focused upstream proposals,
  while the `aws-lc-sys` patches are more likely to be temporary escape
  hatches unless that crate wants a more general hook for hermetic build
  systems.
  
  Suggested follow-up plan:
  
  1. Split the `rules_rust` deltas into upstream-sized PRs or issues with
  minimized repros.
  2. Revisit the `aws-lc-sys` patches during the next dependency bump and
  see whether they can be replaced by an upstream fix, a crate upgrade, or
  a cleaner opt-in mechanism.
  3. Treat each dependency update as a chance to delete patches one by one
  so the local patch set only contains still-needed deltas.
  
  ## Verification
  
  - `./.github/scripts/run-argument-comment-lint-bazel.sh
  --config=argument-comment-lint --keep_going`
  - `RUNNER_OS=Windows
  ./.github/scripts/run-argument-comment-lint-bazel.sh --nobuild
  --config=argument-comment-lint --platforms=//:local_windows
  --keep_going`
  - `cargo test -p codex-linux-sandbox`
  - `cargo test -p codex-core shell_snapshot_tests`
  - `just argument-comment-lint`
  
  ## References
  
  - #16106

  Michael Bolin · 2026-03-30 15:32:04 -07:00
  19f0d196d1
• bazel: enable the full Windows gnullvm CI path (#15952)

  ## Why
  
  This PR is the current, consolidated follow-up to the earlier Windows
  Bazel attempt in #11229. The goal is no longer just to get a tiny
  Windows smoke job limping along: it is to make the ordinary Bazel CI
  path usable on `windows-latest` for `x86_64-pc-windows-gnullvm`, with
  the same broad `//...` test shape that macOS and Linux already use.
  
  The earlier smoke-list version of this work was useful as a foothold,
  but it was not a good long-term landing point. Windows Bazel kept
  surfacing real issues outside that allowlist:
  
  - GitHub's Windows runner exposed runfiles-manifest bugs such as
  `FINDSTR: Cannot open D:MANIFEST`, which broke Bazel test launchers even
  when the manifest file existed.
  - `rules_rs`, `rules_rust`, LLVM extraction, and Abseil still needed
  `windows-gnullvm`-specific fixes for our hermetic toolchain.
  - the V8 path needed more work than just turning the Windows matrix
  entry back on: `rusty_v8` does not ship Windows GNU artifacts in the
  same shape we need, and Bazel's in-tree V8 build needed a set of Windows
  GNU portability fixes.
  
  Windows performance pressure also pushed this toward a full solution
  instead of a permanent smoke suite. During this investigation we hit
  targets such as `//codex-rs/shell-command:shell-command-unit-tests` that
  were much more expensive on Windows because they repeatedly spawn real
  PowerShell parsers (see #16057 for one concrete example of that
  pressure). That made it much more valuable to get the real Windows Bazel
  path working than to keep iterating on a narrowly curated subset.
  
  The net result is that this PR now aims for the same CI contract on
  Windows that we already expect elsewhere: keep standalone
  `//third_party/v8:all` out of the ordinary Bazel lane, but allow V8
  consumers under `//codex-rs/...` to build and test transitively through
  `//...`.
  
  ## What Changed
  
  ### CI and workflow wiring
  
  - re-enable the `windows-latest` / `x86_64-pc-windows-gnullvm` Bazel
  matrix entry in `.github/workflows/bazel.yml`
  - move the Windows Bazel output root to `D:\b` and enable `git config
  --global core.longpaths true` in
  `.github/actions/setup-bazel-ci/action.yml`
  - keep the ordinary Bazel target set on Windows aligned with macOS and
  Linux by running `//...` while excluding only standalone
  `//third_party/v8:all` targets from the normal lane
  
  ### Toolchain and module support for `windows-gnullvm`
  
  - patch `rules_rs` so `windows-gnullvm` is modeled as a distinct Windows
  exec/toolchain platform instead of collapsing into the generic Windows
  shape
  - patch `rules_rust` build-script environment handling so llvm-mingw
  build-script probes do not inherit unsupported `-fstack-protector*`
  flags
  - patch the LLVM module archive so it extracts cleanly on Windows and
  provides the MinGW libraries this toolchain needs
  - patch Abseil so its thread-local identity path matches the hermetic
  `windows-gnullvm` toolchain instead of taking an incompatible MinGW
  pthread path
  - keep both MSVC and GNU Windows targets in the generated Cargo metadata
  because the current V8 release-asset story still uses MSVC-shaped names
  in some places while the Bazel build targets the GNU ABI
  
  ### Windows test-launch and binary-behavior fixes
  
  - update `workspace_root_test_launcher.bat.tpl` to read the runfiles
  manifest directly instead of shelling out to `findstr`, which was the
  source of the `D:MANIFEST` failures on the GitHub Windows runner
  - thread a larger Windows GNU stack reserve through `defs.bzl` so
  Bazel-built binaries that pull in V8 behave correctly both under normal
  builds and under `bazel test`
  - remove the no-longer-needed Windows bootstrap sh-toolchain override
  from `.bazelrc`
  
  ### V8 / `rusty_v8` Windows GNU support
  
  - export and apply the new Windows GNU patch set from
  `patches/BUILD.bazel` / `MODULE.bazel`
  - patch the V8 module/rules/source layers so the in-tree V8 build can
  produce Windows GNU archives under Bazel
  - teach `third_party/v8/BUILD.bazel` to build Windows GNU static
  archives in-tree instead of aliasing them to the MSVC prebuilts
  - reuse the Linux release binding for the experimental Windows GNU path
  where `rusty_v8` does not currently publish a Windows GNU binding
  artifact
  
  ## Testing
  
  - the primary end-to-end validation for this work is the `Bazel`
  workflow plus `v8-canary`, since the hard parts are Windows-specific and
  depend on real GitHub runner behavior
  - before consolidation back onto this PR, the same net change passed the
  full Bazel matrix in [run
  23675590471](https://github.com/openai/codex/actions/runs/23675590471)
  and passed `v8-canary` in [run
  23675590453](https://github.com/openai/codex/actions/runs/23675590453)
  - those successful runs included the `windows-latest` /
  `x86_64-pc-windows-gnullvm` Bazel job with the ordinary `//...` path,
  not the earlier Windows smoke allowlist
  
  ---
  [//]: # (BEGIN SAPLING FOOTER)
  Stack created with [Sapling](https://sapling-scm.com). Best reviewed
  with [ReviewStack](https://reviewstack.dev/openai/codex/pull/15952).
  * #16067
  * __->__ #15952

  Michael Bolin · 2026-03-27 20:37:03 -07:00
  343d1af3da