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
116 lines
3.3 KiB
Bash
Executable File
116 lines
3.3 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
|
|
set -euo pipefail
|
|
|
|
ci_config=ci-linux
|
|
case "${RUNNER_OS:-}" in
|
|
macOS)
|
|
ci_config=ci-macos
|
|
;;
|
|
Windows)
|
|
ci_config=ci-windows
|
|
;;
|
|
esac
|
|
|
|
bazel_lint_args=("$@")
|
|
if [[ "${RUNNER_OS:-}" == "Windows" ]]; then
|
|
has_host_platform_override=0
|
|
for arg in "${bazel_lint_args[@]}"; do
|
|
if [[ "$arg" == --host_platform=* ]]; then
|
|
has_host_platform_override=1
|
|
break
|
|
fi
|
|
done
|
|
|
|
if [[ $has_host_platform_override -eq 0 ]]; then
|
|
# The nightly Windows lint toolchain is registered with an MSVC exec
|
|
# platform even though the lint target platform stays on `windows-gnullvm`.
|
|
# Override the host platform here so the exec-side helper binaries actually
|
|
# match the registered toolchain set.
|
|
bazel_lint_args+=("--host_platform=//:local_windows_msvc")
|
|
fi
|
|
|
|
# Native Windows lint runs need exec-side Rust helper binaries and proc-macros
|
|
# to use rust-lld instead of the C++ linker path. The default `none`
|
|
# preference resolves to `cc` when a cc_toolchain is present, which currently
|
|
# routes these exec actions through clang++ with an argument shape it cannot
|
|
# consume.
|
|
bazel_lint_args+=("--@rules_rust//rust/settings:toolchain_linker_preference=rust")
|
|
|
|
# Some Rust top-level targets are still intentionally incompatible with the
|
|
# local Windows MSVC exec platform. Skip those explicit targets so the native
|
|
# lint aspect can run across the compatible crate graph instead of failing the
|
|
# whole build after analysis.
|
|
bazel_lint_args+=("--skip_incompatible_explicit_targets")
|
|
fi
|
|
|
|
bazel_startup_args=()
|
|
if [[ -n "${BAZEL_OUTPUT_USER_ROOT:-}" ]]; then
|
|
bazel_startup_args+=("--output_user_root=${BAZEL_OUTPUT_USER_ROOT}")
|
|
fi
|
|
|
|
run_bazel() {
|
|
if [[ "${RUNNER_OS:-}" == "Windows" ]]; then
|
|
MSYS2_ARG_CONV_EXCL='*' bazel "$@"
|
|
return
|
|
fi
|
|
|
|
bazel "$@"
|
|
}
|
|
|
|
run_bazel_with_startup_args() {
|
|
if [[ ${#bazel_startup_args[@]} -gt 0 ]]; then
|
|
run_bazel "${bazel_startup_args[@]}" "$@"
|
|
return
|
|
fi
|
|
|
|
run_bazel "$@"
|
|
}
|
|
|
|
read_query_labels() {
|
|
local query="$1"
|
|
local query_stdout
|
|
local query_stderr
|
|
query_stdout="$(mktemp)"
|
|
query_stderr="$(mktemp)"
|
|
|
|
if ! run_bazel_with_startup_args \
|
|
--noexperimental_remote_repo_contents_cache \
|
|
query \
|
|
--keep_going \
|
|
--output=label \
|
|
"$query" >"$query_stdout" 2>"$query_stderr"; then
|
|
cat "$query_stderr" >&2
|
|
rm -f "$query_stdout" "$query_stderr"
|
|
exit 1
|
|
fi
|
|
|
|
cat "$query_stdout"
|
|
rm -f "$query_stdout" "$query_stderr"
|
|
}
|
|
|
|
final_build_targets=(//codex-rs/...)
|
|
if [[ "${RUNNER_OS:-}" == "Windows" ]]; then
|
|
# Bazel's local Windows platform currently lacks a default test toolchain for
|
|
# `rust_test`, so target the concrete Rust crate rules directly. The lint
|
|
# aspect still walks their crate graph, which preserves incremental reuse for
|
|
# non-test code while avoiding non-Rust wrapper targets such as platform_data.
|
|
final_build_targets=()
|
|
while IFS= read -r label; do
|
|
[[ -n "$label" ]] || continue
|
|
final_build_targets+=("$label")
|
|
done < <(read_query_labels 'kind("rust_(library|binary|proc_macro) rule", //codex-rs/...)')
|
|
|
|
if [[ ${#final_build_targets[@]} -eq 0 ]]; then
|
|
echo "Failed to discover Windows Bazel lint targets." >&2
|
|
exit 1
|
|
fi
|
|
fi
|
|
|
|
./.github/scripts/run-bazel-ci.sh \
|
|
-- \
|
|
build \
|
|
"${bazel_lint_args[@]}" \
|
|
-- \
|
|
"${final_build_targets[@]}"
|