mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
## 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
134 lines
5.3 KiB
YAML
134 lines
5.3 KiB
YAML
name: setup-bazel-ci
|
|
description: Prepare a Bazel CI runner with shared caches and optional test prerequisites.
|
|
inputs:
|
|
target:
|
|
description: Target triple used for cache namespacing.
|
|
required: true
|
|
install-test-prereqs:
|
|
description: Install Node.js and DotSlash for Bazel-backed test jobs.
|
|
required: false
|
|
default: "false"
|
|
outputs:
|
|
cache-hit:
|
|
description: Whether the Bazel repository cache key was restored exactly.
|
|
value: ${{ steps.cache_bazel_repository_restore.outputs.cache-hit }}
|
|
|
|
runs:
|
|
using: composite
|
|
steps:
|
|
- name: Set up Node.js for js_repl tests
|
|
if: inputs.install-test-prereqs == 'true'
|
|
uses: actions/setup-node@v6
|
|
with:
|
|
node-version-file: codex-rs/node-version.txt
|
|
|
|
# Some integration tests rely on DotSlash being installed.
|
|
# See https://github.com/openai/codex/pull/7617.
|
|
- name: Install DotSlash
|
|
if: inputs.install-test-prereqs == 'true'
|
|
uses: facebook/install-dotslash@v2
|
|
|
|
- name: Make DotSlash available in PATH (Unix)
|
|
if: inputs.install-test-prereqs == 'true' && runner.os != 'Windows'
|
|
shell: bash
|
|
run: cp "$(which dotslash)" /usr/local/bin
|
|
|
|
- name: Make DotSlash available in PATH (Windows)
|
|
if: inputs.install-test-prereqs == 'true' && runner.os == 'Windows'
|
|
shell: pwsh
|
|
run: Copy-Item (Get-Command dotslash).Source -Destination "$env:LOCALAPPDATA\Microsoft\WindowsApps\dotslash.exe"
|
|
|
|
- name: Set up Bazel
|
|
uses: bazelbuild/setup-bazelisk@v3
|
|
|
|
# Restore bazel repository cache so we don't have to redownload all the external dependencies
|
|
# on every CI run.
|
|
- name: Restore bazel repository cache
|
|
id: cache_bazel_repository_restore
|
|
uses: actions/cache/restore@v5
|
|
with:
|
|
path: |
|
|
~/.cache/bazel-repo-cache
|
|
key: bazel-cache-${{ inputs.target }}-${{ hashFiles('MODULE.bazel', 'codex-rs/Cargo.lock', 'codex-rs/Cargo.toml') }}
|
|
restore-keys: |
|
|
bazel-cache-${{ inputs.target }}
|
|
|
|
- name: Configure Bazel output root (Windows)
|
|
if: runner.os == 'Windows'
|
|
shell: pwsh
|
|
run: |
|
|
# Use the shortest available drive to reduce argv/path length issues,
|
|
# but avoid the drive root because some Windows test launchers mis-handle
|
|
# MANIFEST paths there.
|
|
$hasDDrive = Test-Path 'D:\'
|
|
$bazelOutputUserRoot = if ($hasDDrive) { 'D:\b' } else { 'C:\b' }
|
|
$repoContentsCache = Join-Path $env:RUNNER_TEMP "bazel-repo-contents-cache-$env:GITHUB_RUN_ID-$env:GITHUB_JOB"
|
|
"BAZEL_OUTPUT_USER_ROOT=$bazelOutputUserRoot" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append
|
|
"BAZEL_REPO_CONTENTS_CACHE=$repoContentsCache" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append
|
|
if (-not $hasDDrive) {
|
|
$repositoryCache = Join-Path $env:USERPROFILE '.cache\bazel-repo-cache'
|
|
"BAZEL_REPOSITORY_CACHE=$repositoryCache" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append
|
|
}
|
|
|
|
- name: Expose MSVC SDK environment (Windows)
|
|
if: runner.os == 'Windows'
|
|
shell: pwsh
|
|
run: |
|
|
# Bazel exec-side Rust build scripts do not reliably inherit the MSVC developer
|
|
# shell on GitHub-hosted Windows runners, so discover the latest VS install and
|
|
# ask `VsDevCmd.bat` to materialize the x64/x64 compiler + SDK environment.
|
|
$vswhere = "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe"
|
|
if (-not (Test-Path $vswhere)) {
|
|
throw "vswhere.exe not found"
|
|
}
|
|
|
|
$installPath = & $vswhere -latest -products * -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath 2>$null
|
|
if (-not $installPath) {
|
|
throw "Could not locate a Visual Studio installation with VC tools"
|
|
}
|
|
|
|
$vsDevCmd = Join-Path $installPath 'Common7\Tools\VsDevCmd.bat'
|
|
if (-not (Test-Path $vsDevCmd)) {
|
|
throw "VsDevCmd.bat not found at $vsDevCmd"
|
|
}
|
|
|
|
# Keep the export surface explicit: these are the paths and SDK roots that the
|
|
# MSVC toolchain probes need later when Bazel runs Windows exec-platform build
|
|
# scripts such as `aws-lc-sys`.
|
|
$varsToExport = @(
|
|
'INCLUDE',
|
|
'LIB',
|
|
'LIBPATH',
|
|
'PATH',
|
|
'UCRTVersion',
|
|
'UniversalCRTSdkDir',
|
|
'VCINSTALLDIR',
|
|
'VCToolsInstallDir',
|
|
'WindowsLibPath',
|
|
'WindowsSdkBinPath',
|
|
'WindowsSdkDir',
|
|
'WindowsSDKLibVersion',
|
|
'WindowsSDKVersion'
|
|
)
|
|
|
|
# `VsDevCmd.bat` is a batch file, so invoke it under `cmd.exe`, suppress its
|
|
# banner, then dump the resulting environment with `set`. Re-export only the
|
|
# approved keys into `GITHUB_ENV` so later steps inherit the same MSVC context.
|
|
$envLines = & cmd.exe /c ('"{0}" -no_logo -arch=x64 -host_arch=x64 >nul && set' -f $vsDevCmd)
|
|
foreach ($line in $envLines) {
|
|
if ($line -notmatch '^(.*?)=(.*)$') {
|
|
continue
|
|
}
|
|
|
|
$name = $matches[1]
|
|
$value = $matches[2]
|
|
if ($varsToExport -contains $name) {
|
|
"$name=$value" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append
|
|
}
|
|
}
|
|
|
|
- name: Enable Git long paths (Windows)
|
|
if: runner.os == 'Windows'
|
|
shell: pwsh
|
|
run: git config --global core.longpaths true
|