mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +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
230 lines
9.5 KiB
YAML
230 lines
9.5 KiB
YAML
name: rust-ci
|
|
on:
|
|
pull_request: {}
|
|
workflow_dispatch:
|
|
|
|
jobs:
|
|
# --- Detect what changed so the fast PR workflow only runs relevant jobs ----
|
|
changed:
|
|
name: Detect changed areas
|
|
runs-on: ubuntu-24.04
|
|
outputs:
|
|
argument_comment_lint: ${{ steps.detect.outputs.argument_comment_lint }}
|
|
argument_comment_lint_package: ${{ steps.detect.outputs.argument_comment_lint_package }}
|
|
codex: ${{ steps.detect.outputs.codex }}
|
|
workflows: ${{ steps.detect.outputs.workflows }}
|
|
steps:
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
|
|
with:
|
|
fetch-depth: 0
|
|
- name: Detect changed paths (no external action)
|
|
id: detect
|
|
shell: bash
|
|
run: |
|
|
set -euo pipefail
|
|
|
|
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
|
|
BASE_SHA='${{ github.event.pull_request.base.sha }}'
|
|
HEAD_SHA='${{ github.event.pull_request.head.sha }}'
|
|
echo "Base SHA: $BASE_SHA"
|
|
echo "Head SHA: $HEAD_SHA"
|
|
mapfile -t files < <(git diff --name-only --no-renames "$BASE_SHA" "$HEAD_SHA")
|
|
else
|
|
# On manual runs, default to the full fast-PR bundle.
|
|
files=("codex-rs/force" "tools/argument-comment-lint/force" ".github/force")
|
|
fi
|
|
|
|
codex=false
|
|
argument_comment_lint=false
|
|
argument_comment_lint_package=false
|
|
workflows=false
|
|
for f in "${files[@]}"; do
|
|
[[ $f == codex-rs/* ]] && codex=true
|
|
[[ $f == codex-rs/* || $f == tools/argument-comment-lint/* || $f == justfile ]] && argument_comment_lint=true
|
|
[[ $f == tools/argument-comment-lint/* || $f == .github/workflows/rust-ci.yml || $f == .github/workflows/rust-ci-full.yml ]] && argument_comment_lint_package=true
|
|
[[ $f == .github/* ]] && workflows=true
|
|
done
|
|
|
|
echo "argument_comment_lint=$argument_comment_lint" >> "$GITHUB_OUTPUT"
|
|
echo "argument_comment_lint_package=$argument_comment_lint_package" >> "$GITHUB_OUTPUT"
|
|
echo "codex=$codex" >> "$GITHUB_OUTPUT"
|
|
echo "workflows=$workflows" >> "$GITHUB_OUTPUT"
|
|
|
|
# --- Fast Cargo-native PR checks -------------------------------------------
|
|
general:
|
|
name: Format / etc
|
|
runs-on: ubuntu-24.04
|
|
needs: changed
|
|
if: ${{ needs.changed.outputs.codex == 'true' || needs.changed.outputs.workflows == 'true' }}
|
|
defaults:
|
|
run:
|
|
working-directory: codex-rs
|
|
steps:
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
|
|
- uses: dtolnay/rust-toolchain@a0b273b48ed29de4470960879e8381ff45632f26 # 1.93.0
|
|
with:
|
|
components: rustfmt
|
|
- name: cargo fmt
|
|
run: cargo fmt -- --config imports_granularity=Item --check
|
|
|
|
cargo_shear:
|
|
name: cargo shear
|
|
runs-on: ubuntu-24.04
|
|
needs: changed
|
|
if: ${{ needs.changed.outputs.codex == 'true' || needs.changed.outputs.workflows == 'true' }}
|
|
defaults:
|
|
run:
|
|
working-directory: codex-rs
|
|
steps:
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
|
|
- uses: dtolnay/rust-toolchain@a0b273b48ed29de4470960879e8381ff45632f26 # 1.93.0
|
|
- uses: taiki-e/install-action@44c6d64aa62cd779e873306675c7a58e86d6d532 # v2
|
|
with:
|
|
tool: cargo-shear
|
|
version: 1.5.1
|
|
- name: cargo shear
|
|
run: cargo shear
|
|
|
|
argument_comment_lint_package:
|
|
name: Argument comment lint package
|
|
runs-on: ubuntu-24.04
|
|
needs: changed
|
|
if: ${{ needs.changed.outputs.argument_comment_lint_package == 'true' }}
|
|
steps:
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
|
|
- uses: dtolnay/rust-toolchain@a0b273b48ed29de4470960879e8381ff45632f26 # 1.93.0
|
|
- name: Install nightly argument-comment-lint toolchain
|
|
shell: bash
|
|
run: |
|
|
rustup toolchain install nightly-2025-09-18 \
|
|
--profile minimal \
|
|
--component llvm-tools-preview \
|
|
--component rustc-dev \
|
|
--component rust-src \
|
|
--no-self-update
|
|
rustup default nightly-2025-09-18
|
|
- name: Cache cargo-dylint tooling
|
|
id: cargo_dylint_cache
|
|
uses: actions/cache@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5
|
|
with:
|
|
path: |
|
|
~/.cargo/bin/cargo-dylint
|
|
~/.cargo/bin/dylint-link
|
|
~/.cargo/registry/index
|
|
~/.cargo/registry/cache
|
|
~/.cargo/git/db
|
|
key: argument-comment-lint-${{ runner.os }}-${{ hashFiles('tools/argument-comment-lint/Cargo.lock', 'tools/argument-comment-lint/rust-toolchain', '.github/workflows/rust-ci.yml', '.github/workflows/rust-ci-full.yml') }}
|
|
- name: Install cargo-dylint tooling
|
|
if: ${{ steps.cargo_dylint_cache.outputs.cache-hit != 'true' }}
|
|
run: cargo install --locked cargo-dylint dylint-link
|
|
- name: Check Python wrapper syntax
|
|
run: python3 -m py_compile tools/argument-comment-lint/wrapper_common.py tools/argument-comment-lint/run.py tools/argument-comment-lint/run-prebuilt-linter.py tools/argument-comment-lint/test_wrapper_common.py
|
|
- name: Test Python wrapper helpers
|
|
run: python3 -m unittest discover -s tools/argument-comment-lint -p 'test_*.py'
|
|
- name: Test argument comment lint package
|
|
working-directory: tools/argument-comment-lint
|
|
run: cargo test
|
|
|
|
argument_comment_lint_prebuilt:
|
|
name: Argument comment lint - ${{ matrix.name }}
|
|
runs-on: ${{ matrix.runs_on || matrix.runner }}
|
|
timeout-minutes: ${{ matrix.timeout_minutes }}
|
|
needs: changed
|
|
if: ${{ needs.changed.outputs.argument_comment_lint == 'true' || needs.changed.outputs.workflows == 'true' }}
|
|
strategy:
|
|
fail-fast: false
|
|
matrix:
|
|
include:
|
|
- name: Linux
|
|
runner: ubuntu-24.04
|
|
timeout_minutes: 30
|
|
- name: macOS
|
|
runner: macos-15-xlarge
|
|
timeout_minutes: 30
|
|
- name: Windows
|
|
runner: windows-x64
|
|
timeout_minutes: 30
|
|
runs_on:
|
|
group: codex-runners
|
|
labels: codex-windows-x64
|
|
steps:
|
|
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
|
|
- uses: ./.github/actions/setup-bazel-ci
|
|
with:
|
|
target: ${{ runner.os }}
|
|
install-test-prereqs: true
|
|
- name: Install Linux sandbox build dependencies
|
|
if: ${{ runner.os == 'Linux' }}
|
|
shell: bash
|
|
run: |
|
|
sudo DEBIAN_FRONTEND=noninteractive apt-get update
|
|
sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends pkg-config libcap-dev
|
|
- name: Run argument comment lint on codex-rs via Bazel
|
|
if: ${{ runner.os != 'Windows' }}
|
|
env:
|
|
BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }}
|
|
shell: bash
|
|
run: |
|
|
bazel_targets="$(./tools/argument-comment-lint/list-bazel-targets.sh)"
|
|
./.github/scripts/run-bazel-ci.sh \
|
|
-- \
|
|
build \
|
|
--config=argument-comment-lint \
|
|
--keep_going \
|
|
--build_metadata=COMMIT_SHA=${GITHUB_SHA} \
|
|
-- \
|
|
${bazel_targets}
|
|
- name: Run argument comment lint on codex-rs via Bazel
|
|
if: ${{ runner.os == 'Windows' }}
|
|
env:
|
|
BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }}
|
|
shell: bash
|
|
run: |
|
|
./.github/scripts/run-argument-comment-lint-bazel.sh \
|
|
--config=argument-comment-lint \
|
|
--platforms=//:local_windows \
|
|
--keep_going \
|
|
--build_metadata=COMMIT_SHA=${GITHUB_SHA}
|
|
|
|
# --- Gatherer job that you mark as the ONLY required status -----------------
|
|
results:
|
|
name: CI results (required)
|
|
needs:
|
|
[
|
|
changed,
|
|
general,
|
|
cargo_shear,
|
|
argument_comment_lint_package,
|
|
argument_comment_lint_prebuilt,
|
|
]
|
|
if: always()
|
|
runs-on: ubuntu-24.04
|
|
steps:
|
|
- name: Summarize
|
|
shell: bash
|
|
run: |
|
|
echo "argpkg : ${{ needs.argument_comment_lint_package.result }}"
|
|
echo "arglint: ${{ needs.argument_comment_lint_prebuilt.result }}"
|
|
echo "general: ${{ needs.general.result }}"
|
|
echo "shear : ${{ needs.cargo_shear.result }}"
|
|
|
|
# If nothing relevant changed (PR touching only root README, etc.),
|
|
# declare success regardless of other jobs.
|
|
if [[ '${{ needs.changed.outputs.argument_comment_lint }}' != 'true' && '${{ needs.changed.outputs.codex }}' != 'true' && '${{ needs.changed.outputs.workflows }}' != 'true' ]]; then
|
|
echo 'No relevant changes -> CI not required.'
|
|
exit 0
|
|
fi
|
|
|
|
if [[ '${{ needs.changed.outputs.argument_comment_lint_package }}' == 'true' ]]; then
|
|
[[ '${{ needs.argument_comment_lint_package.result }}' == 'success' ]] || { echo 'argument_comment_lint_package failed'; exit 1; }
|
|
fi
|
|
|
|
if [[ '${{ needs.changed.outputs.argument_comment_lint }}' == 'true' || '${{ needs.changed.outputs.workflows }}' == 'true' ]]; then
|
|
[[ '${{ needs.argument_comment_lint_prebuilt.result }}' == 'success' ]] || { echo 'argument_comment_lint_prebuilt failed'; exit 1; }
|
|
fi
|
|
|
|
if [[ '${{ needs.changed.outputs.codex }}' == 'true' || '${{ needs.changed.outputs.workflows }}' == 'true' ]]; then
|
|
[[ '${{ needs.general.result }}' == 'success' ]] || { echo 'general failed'; exit 1; }
|
|
[[ '${{ needs.cargo_shear.result }}' == 'success' ]] || { echo 'cargo_shear failed'; exit 1; }
|
|
fi
|