mirror of
https://github.com/openai/codex.git
synced 2026-04-26 15:45:02 +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
182 lines
8.3 KiB
Diff
182 lines
8.3 KiB
Diff
# What: expose an exec-side Rust standard library alongside the target stdlib.
|
|
# Why: mixed Windows toolchains compile repo crates for `windows-gnullvm`, but
|
|
# exec-side helper binaries (build.rs, runners) may need the host MSVC stdlib.
|
|
# The toolchain sysroot must therefore carry both stdlib trees so rustc can
|
|
# resolve the correct one for each `--target`.
|
|
|
|
diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl
|
|
--- a/rust/toolchain.bzl
|
|
+++ b/rust/toolchain.bzl
|
|
@@ -209,6 +209,7 @@ def _generate_sysroot(
|
|
clippy = None,
|
|
cargo_clippy = None,
|
|
llvm_tools = None,
|
|
+ exec_rust_std = None,
|
|
rust_std = None,
|
|
rustfmt = None,
|
|
linker = None):
|
|
@@ -312,7 +313,15 @@ def _generate_sysroot(
|
|
|
|
# Made available to support $(location) expansion in stdlib_linkflags and extra_rustc_flags.
|
|
transitive_file_sets.append(depset(ctx.files.rust_std))
|
|
+
|
|
+ sysroot_exec_rust_std = None
|
|
+ if exec_rust_std:
|
|
+ sysroot_exec_rust_std = _symlink_sysroot_tree(ctx, name, exec_rust_std)
|
|
+ transitive_file_sets.extend([sysroot_exec_rust_std])
|
|
|
|
+ # Made available to support $(location) expansion in extra_exec_rustc_flags.
|
|
+ transitive_file_sets.append(depset(ctx.files.exec_rust_std))
|
|
+
|
|
# Declare a file in the root of the sysroot to make locating the sysroot easy
|
|
sysroot_anchor = ctx.actions.declare_file("{}/rust.sysroot".format(name))
|
|
ctx.actions.write(
|
|
@@ -323,6 +332,7 @@ def _generate_sysroot(
|
|
"cargo-clippy: {}".format(cargo_clippy),
|
|
"linker: {}".format(linker),
|
|
"llvm_tools: {}".format(llvm_tools),
|
|
+ "exec_rust_std: {}".format(exec_rust_std),
|
|
"rust_std: {}".format(rust_std),
|
|
"rustc_lib: {}".format(rustc_lib),
|
|
"rustc: {}".format(rustc),
|
|
@@ -340,6 +350,7 @@ def _generate_sysroot(
|
|
cargo_clippy = sysroot_cargo_clippy,
|
|
clippy = sysroot_clippy,
|
|
linker = sysroot_linker,
|
|
+ exec_rust_std = sysroot_exec_rust_std,
|
|
rust_std = sysroot_rust_std,
|
|
rustc = sysroot_rustc,
|
|
rustc_lib = sysroot_rustc_lib,
|
|
@@ -410,12 +421,14 @@ def _rust_toolchain_impl(ctx):
|
|
)
|
|
|
|
rust_std = ctx.attr.rust_std
|
|
+ exec_rust_std = ctx.attr.exec_rust_std if ctx.attr.exec_rust_std else rust_std
|
|
|
|
sysroot = _generate_sysroot(
|
|
ctx = ctx,
|
|
rustc = ctx.file.rustc,
|
|
rustdoc = ctx.file.rust_doc,
|
|
rustc_lib = ctx.attr.rustc_lib,
|
|
+ exec_rust_std = exec_rust_std,
|
|
rust_std = rust_std,
|
|
rustfmt = ctx.file.rustfmt,
|
|
clippy = ctx.file.clippy_driver,
|
|
@@ -452,7 +465,7 @@ def _rust_toolchain_impl(ctx):
|
|
|
|
expanded_stdlib_linkflags = _expand_flags(ctx, "stdlib_linkflags", rust_std[rust_common.stdlib_info].srcs, make_variables)
|
|
expanded_extra_rustc_flags = _expand_flags(ctx, "extra_rustc_flags", rust_std[rust_common.stdlib_info].srcs, make_variables)
|
|
- expanded_extra_exec_rustc_flags = _expand_flags(ctx, "extra_exec_rustc_flags", rust_std[rust_common.stdlib_info].srcs, make_variables)
|
|
+ expanded_extra_exec_rustc_flags = _expand_flags(ctx, "extra_exec_rustc_flags", exec_rust_std[rust_common.stdlib_info].srcs, make_variables)
|
|
|
|
linking_context = cc_common.create_linking_context(
|
|
linker_inputs = depset([
|
|
@@ -793,6 +806,10 @@ rust_toolchain = rule(
|
|
doc = "The Rust standard library.",
|
|
mandatory = True,
|
|
),
|
|
+ "exec_rust_std": attr.label(
|
|
+ doc = "Optional Rust standard library for exec-configuration Rust tools. Defaults to rust_std.",
|
|
+ mandatory = False,
|
|
+ ),
|
|
"rustc": attr.label(
|
|
doc = "The location of the `rustc` binary. Can be a direct source or a filegroup containing one item.",
|
|
allow_single_file = True,
|
|
diff --git a/rust/private/repository_utils.bzl b/rust/private/repository_utils.bzl
|
|
--- a/rust/private/repository_utils.bzl
|
|
+++ b/rust/private/repository_utils.bzl
|
|
@@ -341,6 +341,7 @@ rust_toolchain(
|
|
name = "{toolchain_name}",
|
|
rust_doc = "//:rustdoc",
|
|
rust_std = "//:rust_std-{target_triple}",
|
|
+ exec_rust_std = {exec_rust_std_label},
|
|
rustc = "//:rustc",
|
|
linker = {linker_label},
|
|
linker_type = {linker_type},
|
|
@@ -384,6 +385,7 @@ def BUILD_for_rust_toolchain(
|
|
include_llvm_tools,
|
|
include_linker,
|
|
include_objcopy = False,
|
|
+ exec_rust_std_label = None,
|
|
stdlib_linkflags = None,
|
|
extra_rustc_flags = None,
|
|
extra_exec_rustc_flags = None,
|
|
@@ -405,6 +407,7 @@ def BUILD_for_rust_toolchain(
|
|
include_llvm_tools (bool): Whether llvm-tools are present in the toolchain.
|
|
include_linker (bool): Whether a linker is available in the toolchain.
|
|
include_objcopy (bool): Whether rust-objcopy is available in the toolchain.
|
|
+ exec_rust_std_label (str, optional): Label for an exec-side stdlib when it differs from rust_std.
|
|
stdlib_linkflags (list, optional): Overridden flags needed for linking to rust
|
|
stdlib, akin to BAZEL_LINKLIBS. Defaults to
|
|
None.
|
|
@@ -453,6 +456,7 @@ def BUILD_for_rust_toolchain(
|
|
staticlib_ext = system_to_staticlib_ext(target_triple.system),
|
|
dylib_ext = system_to_dylib_ext(target_triple.system),
|
|
allocator_library = repr(allocator_library_label),
|
|
+ exec_rust_std_label = repr(exec_rust_std_label),
|
|
global_allocator_library = repr(global_allocator_library_label),
|
|
stdlib_linkflags = stdlib_linkflags,
|
|
default_edition = default_edition,
|
|
diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
|
|
--- a/rust/private/rustc.bzl
|
|
+++ b/rust/private/rustc.bzl
|
|
@@ -1011,7 +1011,10 @@ def construct_arguments(
|
|
if build_metadata and not use_json_output:
|
|
fail("build_metadata requires parse_json_output")
|
|
|
|
- use_exec_target = is_exec_configuration(ctx) and crate_info.type == "bin"
|
|
+ # Exec-configuration crates (build scripts, proc-macros, and their
|
|
+ # dependencies) must all target the exec triple so they can link against
|
|
+ # each other and the exec-side standard library.
|
|
+ use_exec_target = is_exec_configuration(ctx)
|
|
|
|
output_dir = getattr(crate_info.output, "dirname", None)
|
|
linker_script = getattr(file, "linker_script", None)
|
|
diff --git a/rust/repositories.bzl b/rust/repositories.bzl
|
|
--- a/rust/repositories.bzl
|
|
+++ b/rust/repositories.bzl
|
|
@@ -536,6 +536,18 @@ def _rust_toolchain_tools_repository_impl(ctx):
|
|
build_components.append(rust_stdlib_content)
|
|
sha256s.update(rust_stdlib_sha256)
|
|
|
|
+ exec_rust_std_label = None
|
|
+ if exec_triple.str != target_triple.str:
|
|
+ exec_rust_stdlib_content, exec_rust_stdlib_sha256 = load_rust_stdlib(
|
|
+ ctx = ctx,
|
|
+ target_triple = exec_triple,
|
|
+ version = version,
|
|
+ iso_date = iso_date,
|
|
+ )
|
|
+ build_components.append(exec_rust_stdlib_content)
|
|
+ sha256s.update(exec_rust_stdlib_sha256)
|
|
+ exec_rust_std_label = "//:rust_std-{}".format(exec_triple.str)
|
|
+
|
|
stdlib_linkflags = None
|
|
if "BAZEL_RUST_STDLIB_LINKFLAGS" in ctx.os.environ:
|
|
stdlib_linkflags = ctx.os.environ["BAZEL_RUST_STDLIB_LINKFLAGS"].split(":")
|
|
@@ -552,6 +564,7 @@ def _rust_toolchain_tools_repository_impl(ctx):
|
|
include_llvm_tools = include_llvm_tools,
|
|
include_linker = include_linker,
|
|
include_objcopy = include_objcopy,
|
|
+ exec_rust_std_label = exec_rust_std_label,
|
|
extra_rustc_flags = ctx.attr.extra_rustc_flags,
|
|
extra_exec_rustc_flags = ctx.attr.extra_exec_rustc_flags,
|
|
opt_level = ctx.attr.opt_level if ctx.attr.opt_level else None,
|
|
@@ -575,8 +588,16 @@ def _rust_toolchain_tools_repository_impl(ctx):
|
|
if ctx.attr.dev_components:
|
|
rustc_dev_sha256 = load_rustc_dev_nightly(
|
|
ctx = ctx,
|
|
target_triple = target_triple,
|
|
version = version,
|
|
iso_date = iso_date,
|
|
)
|
|
sha256s.update(rustc_dev_sha256)
|
|
+ if exec_triple.str != target_triple.str:
|
|
+ exec_rustc_dev_sha256 = load_rustc_dev_nightly(
|
|
+ ctx = ctx,
|
|
+ target_triple = exec_triple,
|
|
+ version = version,
|
|
+ iso_date = iso_date,
|
|
+ )
|
|
+ sha256s.update(exec_rustc_dev_sha256)
|