agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Codex/windows bazel rust test coverage no rs (#16528)

Browse Source 
# Why this PR exists

This PR is trying to fix a coverage gap in the Windows Bazel Rust test
lane.

Before this change, the Windows `bazel test //...` job was nominally
part of PR CI, but a non-trivial set of `//codex-rs/...` Rust test
targets did not actually contribute test signal on Windows. In
particular, targets such as `//codex-rs/core:core-unit-tests`,
`//codex-rs/core:core-all-test`, and `//codex-rs/login:login-unit-tests`
were incompatible during Bazel analysis on the Windows gnullvm platform,
so they never reached test execution there. That is why the
Cargo-powered Windows CI job could surface Windows-only failures that
the Bazel-powered job did not report: Cargo was executing those tests,
while Bazel was silently dropping them from the runnable target set.

The main goal of this PR is to make the Windows Bazel test lane execute
those Rust test targets instead of skipping them during analysis, while
still preserving `windows-gnullvm` as the target configuration for the
code under test. In other words: use an MSVC host/exec toolchain where
Bazel helper binaries and build scripts need it, but continue compiling
the actual crate targets with the Windows gnullvm cfgs that our current
Bazel matrix is supposed to exercise.

# Important scope note

This branch intentionally removes the non-resource-loading `.rs` test
and production-code changes from the earlier
`codex/windows-bazel-rust-test-coverage` branch. The only Rust source
changes kept here are runfiles/resource-loading fixes in TUI tests:

- `codex-rs/tui/src/chatwidget/tests.rs`
- `codex-rs/tui/tests/manager_dependency_regression.rs`

That is deliberate. Since the corresponding tests already pass under
Cargo, this PR is meant to test whether Bazel infrastructure/toolchain
fixes alone are enough to get a healthy Windows Bazel test signal,
without changing test behavior for Windows timing, shell output, or
SQLite file-locking.

# How this PR changes the Windows Bazel setup

## 1. Split Windows host/exec and target concerns in the Bazel test lane

The core change is that the Windows Bazel test job now opts into an MSVC
host platform for Bazel execution-time tools, but only for `bazel test`,
not for the Bazel clippy build.

Files:

- `.github/workflows/bazel.yml`
- `.github/scripts/run-bazel-ci.sh`
- `MODULE.bazel`

What changed:

- `run-bazel-ci.sh` now accepts `--windows-msvc-host-platform`.
- When that flag is present on Windows, the wrapper appends
`--host_platform=//:local_windows_msvc` unless the caller already
provided an explicit `--host_platform`.
- `bazel.yml` passes that wrapper flag only for the Windows `bazel test
//...` job.
- The Bazel clippy job intentionally does **not** pass that flag, so
clippy stays on the default Windows gnullvm host/exec path and continues
linting against the target cfgs we care about.
- `run-bazel-ci.sh` also now forwards `CODEX_JS_REPL_NODE_PATH` on
Windows and normalizes the `node` executable path with `cygpath -w`, so
tests that need Node resolve the runner's Node installation correctly
under the Windows Bazel test environment.

Why this helps:

- The original incompatibility chain was mostly on the **exec/tool**
side of the graph, not in the Rust test code itself. Moving host tools
to MSVC lets Bazel resolve helper binaries and generators that were not
viable on the gnullvm exec platform.
- Keeping the target platform on gnullvm preserves cfg coverage for the
crates under test, which is important because some Windows behavior
differs between `msvc` and `gnullvm`.

## 2. Teach the repo's Bazel Rust macro about Windows link flags and
integration-test knobs

Files:

- `defs.bzl`
- `codex-rs/core/BUILD.bazel`
- `codex-rs/otel/BUILD.bazel`
- `codex-rs/tui/BUILD.bazel`

What changed:

- Replaced the old gnullvm-only linker flag block with
`WINDOWS_RUSTC_LINK_FLAGS`, which now handles both Windows ABIs:
  - gnullvm gets `-C link-arg=-Wl,--stack,8388608`
- MSVC gets `-C link-arg=/STACK:8388608`, `-C
link-arg=/NODEFAULTLIB:libucrt.lib`, and `-C link-arg=ucrt.lib`
- Threaded those Windows link flags into generated `rust_binary`,
unit-test binaries, and integration-test binaries.
- Extended `codex_rust_crate(...)` with:
  - `integration_test_args`
  - `integration_test_timeout`
- Used those new knobs to:
- mark `//codex-rs/core:core-all-test` as a long-running integration
test
  - serialize `//codex-rs/otel:otel-all-test` with `--test-threads=1`
- Added `src/**/*.rs` to `codex-rs/tui` test runfiles, because one
regression test scans source files at runtime and Bazel does not expose
source-tree directories unless they are declared as data.

Why this helps:

- Once host-side MSVC tools are available, we still need the generated
Rust test binaries to link correctly on Windows. The MSVC-side
stack/UCRT flags make those binaries behave more like their Cargo-built
equivalents.
- The integration-test macro knobs avoid hardcoding one-off test
behavior in ad hoc BUILD rules and make the generated test targets more
expressive where Bazel and Cargo have different runtime defaults.

## 3. Patch `rules_rs` / `rules_rust` so Windows MSVC exec-side Rust and
build scripts are actually usable

Files:

- `MODULE.bazel`
- `patches/rules_rs_windows_exec_linker.patch`
- `patches/rules_rust_windows_bootstrap_process_wrapper_linker.patch`
- `patches/rules_rust_windows_build_script_runner_paths.patch`
- `patches/rules_rust_windows_exec_msvc_build_script_env.patch`
- `patches/rules_rust_windows_msvc_direct_link_args.patch`
- `patches/rules_rust_windows_process_wrapper_skip_temp_outputs.patch`
- `patches/BUILD.bazel`

What these patches do:

- `rules_rs_windows_exec_linker.patch`
- Adds a `rust-lld` filegroup for Windows Rust toolchain repos,
symlinked to `lld-link.exe` from `PATH`.
  - Marks Windows toolchains as using a direct linker driver.
  - Supplies Windows stdlib link flags for both gnullvm and MSVC.
- `rules_rust_windows_bootstrap_process_wrapper_linker.patch`
- For Windows MSVC Rust targets, prefers the Rust toolchain linker over
an inherited C++ linker path like `clang++`.
- This specifically avoids the broken mixed-mode command line where
rustc emits MSVC-style `/NOLOGO` / `/LIBPATH:` / `/OUT:` arguments but
Bazel still invokes `clang++.exe`.
- `rules_rust_windows_build_script_runner_paths.patch`
- Normalizes forward-slash execroot-relative paths into Windows path
separators before joining them on Windows.
- Uses short Windows paths for `RUSTC`, `OUT_DIR`, and the build-script
working directory to avoid path-length and quoting issues in third-party
build scripts.
- Exposes `RULES_RUST_BAZEL_BUILD_SCRIPT_RUNNER=1` to build scripts so
crate-local patches can detect "this is running under Bazel's
build-script runner".
- Fixes the Windows runfiles cleanup filter so generated files with
retained suffixes are actually retained.
- `rules_rust_windows_exec_msvc_build_script_env.patch`
- For exec-side Windows MSVC build scripts, stops force-injecting
Bazel's `CC`, `CXX`, `LD`, `CFLAGS`, and `CXXFLAGS` when that would send
GNU-flavored tool paths/flags into MSVC-oriented Cargo build scripts.
- Rewrites or strips GNU-only `--sysroot`, MinGW include/library paths,
stack-protector, and `_FORTIFY_SOURCE` flags on the MSVC exec path.
- The practical effect is that build scripts can fall back to the Visual
Studio toolchain environment already exported by CI instead of crashing
inside Bazel's hermetic `clang.exe` setup.
- `rules_rust_windows_msvc_direct_link_args.patch`
- When using a direct linker on Windows, stops forwarding GNU driver
flags such as `-L...` and `--sysroot=...` that `lld-link.exe` does not
understand.
- Passes non-`.lib` native artifacts as explicit `-Clink-arg=<path>`
entries when needed.
- Filters C++ runtime libraries to `.lib` artifacts on the Windows
direct-driver path.
- `rules_rust_windows_process_wrapper_skip_temp_outputs.patch`
- Excludes transient `*.tmp*` and `*.rcgu.o` files from process-wrapper
dependency search-path consolidation, so unstable compiler outputs do
not get treated as real link search-path inputs.

Why this helps:

- The host-platform split alone was not enough. Once Bazel started
analyzing/running previously incompatible Rust tests on Windows, the
next failures were in toolchain plumbing:
- MSVC-targeted Rust tests were being linked through `clang++` with
MSVC-style arguments.
- Cargo build scripts running under Bazel's Windows MSVC exec platform
were handed Unix/GNU-flavored path and flag shapes.
- Some generated paths were too long or had path-separator forms that
third-party Windows build scripts did not tolerate.
- These patches make that mixed Bazel/Cargo/Rust/MSVC path workable
enough for the test lane to actually build and run the affected crates.

## 4. Patch third-party crate build scripts that were not robust under
Bazel's Windows MSVC build-script path

Files:

- `MODULE.bazel`
- `patches/aws-lc-sys_windows_msvc_prebuilt_nasm.patch`
- `patches/ring_windows_msvc_include_dirs.patch`
- `patches/zstd-sys_windows_msvc_include_dirs.patch`

What changed:

- `aws-lc-sys`
- Detects Bazel's Windows MSVC build-script runner via
`RULES_RUST_BAZEL_BUILD_SCRIPT_RUNNER` or a `bazel-out` manifest-dir
path.
- Uses `clang-cl` for Bazel Windows MSVC builds when no explicit
`CC`/`CXX` is set.
- Allows prebuilt NASM on the Bazel Windows MSVC path even when `nasm`
is not available directly in the runner environment.
- Avoids canonicalizing `CARGO_MANIFEST_DIR` in the Bazel Windows MSVC
case, because that path may point into Bazel output/runfiles state where
preserving the given path is more reliable than forcing a local
filesystem canonicalization.
- `ring`
- Under the Bazel Windows MSVC build-script runner, copies the
pregenerated source tree into `OUT_DIR` and uses that as the
generated-source root.
- Adds include paths needed by MSVC compilation for
Fiat/curve25519/P-256 generated headers.
- Rewrites a few relative includes in C sources so the added include
directories are sufficient.
- `zstd-sys`
- Adds MSVC-only include directories for `compress`, `decompress`, and
feature-gated dictionary/legacy/seekable sources.
- Skips `-fvisibility=hidden` on MSVC targets, where that
GCC/Clang-style flag is not the right mechanism.

Why this helps:

- After the `rules_rust` plumbing started running build scripts on the
Windows MSVC exec path, some third-party crates still failed for
crate-local reasons: wrong compiler choice, missing include directories,
build-script assumptions about manifest paths, or Unix-only C compiler
flags.
- These crate patches address those crate-local assumptions so the
larger toolchain change can actually reach first-party Rust test
execution.

## 5. Keep the only `.rs` test changes to Bazel/Cargo runfiles parity

Files:

- `codex-rs/tui/src/chatwidget/tests.rs`
- `codex-rs/tui/tests/manager_dependency_regression.rs`

What changed:

- Instead of asking `find_resource!` for a directory runfile like
`src/chatwidget/snapshots` or `src`, these tests now resolve one known
file runfile first and then walk to its parent directory.

Why this helps:

- Bazel runfiles are more reliable for explicitly declared files than
for source-tree directories that happen to exist in a Cargo checkout.
- This keeps the tests working under both Cargo and Bazel without
changing their actual assertions.

# What we tried before landing on this shape, and why those attempts did
not work

## Attempt 1: Force `--host_platform=//:local_windows_msvc` for all
Windows Bazel jobs

This did make the previously incompatible test targets show up during
analysis, but it also pushed the Bazel clippy job and some unrelated
build actions onto the MSVC exec path.

Why that was bad:

- Windows clippy started running third-party Cargo build scripts with
Bazel's MSVC exec settings and crashed in crates such as `tree-sitter`
and `libsqlite3-sys`.
- That was a regression in a job that was previously giving useful
gnullvm-targeted lint signal.

What this PR does instead:

- The wrapper flag is opt-in, and `bazel.yml` uses it only for the
Windows `bazel test` lane.
- The clippy lane stays on the default Windows gnullvm host/exec
configuration.

## Attempt 2: Broaden the `rules_rust` linker override to all Windows
Rust actions

This fixed the MSVC test-lane failure where normal `rust_test` targets
were linked through `clang++` with MSVC-style arguments, but it broke
the default gnullvm path.

Why that was bad:

-
`@@rules_rs++rules_rust+rules_rust//util/process_wrapper:process_wrapper`
on the gnullvm exec platform started linking with `lld-link.exe` and
then failed to resolve MinGW-style libraries such as `-lkernel32`,
`-luser32`, and `-lmingw32`.

What this PR does instead:

- The linker override is restricted to Windows MSVC targets only.
- The gnullvm path keeps its original linker behavior, while MSVC uses
the direct Windows linker.

## Attempt 3: Keep everything on pure Windows gnullvm and patch the V8 /
Python incompatibility chain instead

This would have preserved a single Windows ABI everywhere, but it is a
much larger project than this PR.

Why that was not the practical first step:

- The original incompatibility chain ran through exec-side generators
and helper tools, not only through crate code.
- `third_party/v8` is already special-cased on Windows gnullvm because
`rusty_v8` only publishes Windows prebuilts under MSVC names.
- Fixing that path likely means deeper changes in
V8/rules_python/rules_rust toolchain resolution and generator execution,
not just one local CI flag.

What this PR does instead:

- Keep gnullvm for the target cfgs we want to exercise.
- Move only the Windows test lane's host/exec platform to MSVC, then
patch the build-script/linker boundary enough for that split
configuration to work.

## Attempt 4: Validate compatibility with `bazel test --nobuild ...`

This turned out to be a misleading local validation command.

Why:

- `bazel test --nobuild ...` can successfully analyze targets and then
still exit 1 with "Couldn't start the build. Unable to run tests"
because there are no runnable test actions after `--nobuild`.

Better local check:

```powershell
bazel build --nobuild --keep_going --host_platform=//:local_windows_msvc //codex-rs/login:login-unit-tests //codex-rs/core:core-unit-tests //codex-rs/core:core-all-test
```

# Which patches probably deserve upstream follow-up

My rough take is that the `rules_rs` / `rules_rust` patches are the
highest-value upstream candidates, because they are fixing generic
Windows host/exec + MSVC direct-linker behavior rather than
Codex-specific test logic.

Strong upstream candidates:

- `patches/rules_rs_windows_exec_linker.patch`
- `patches/rules_rust_windows_bootstrap_process_wrapper_linker.patch`
- `patches/rules_rust_windows_build_script_runner_paths.patch`
- `patches/rules_rust_windows_exec_msvc_build_script_env.patch`
- `patches/rules_rust_windows_msvc_direct_link_args.patch`
- `patches/rules_rust_windows_process_wrapper_skip_temp_outputs.patch`

Why these seem upstreamable:

- They address general-purpose problems in the Windows MSVC exec path:
  - missing direct-linker exposure for Rust toolchains
  - wrong linker selection when rustc emits MSVC-style args
- Windows path normalization/short-path issues in the build-script
runner
  - forwarding GNU-flavored CC/link flags into MSVC Cargo build scripts
  - unstable temp outputs polluting process-wrapper search-path state

Potentially upstreamable crate patches, but likely with more care:

- `patches/zstd-sys_windows_msvc_include_dirs.patch`
- `patches/ring_windows_msvc_include_dirs.patch`
- `patches/aws-lc-sys_windows_msvc_prebuilt_nasm.patch`

Notes on those:

- The `zstd-sys` and `ring` include-path fixes look fairly generic for
MSVC/Bazel build-script environments and may be straightforward to
propose upstream after we confirm CI stability.
- The `aws-lc-sys` patch is useful, but it includes a Bazel-specific
environment probe and CI-specific compiler fallback behavior. That
probably needs a cleaner upstream-facing shape before sending it out, so
upstream maintainers are not forced to adopt Codex's exact CI
assumptions.

Probably not worth upstreaming as-is:

- The repo-local Starlark/test target changes in `defs.bzl`,
`codex-rs/*/BUILD.bazel`, and `.github/scripts/run-bazel-ci.sh` are
mostly Codex-specific policy and CI wiring, not generic rules changes.

# Validation notes for reviewers

On this branch, I ran the following local checks after dropping the
non-resource-loading Rust edits:

```powershell
cargo test -p codex-tui
just --shell 'C:\Program Files\Git\bin\bash.exe' --shell-arg -lc -- fix -p codex-tui
python .\tools\argument-comment-lint\run-prebuilt-linter.py -p codex-tui
just --shell 'C:\Program Files\Git\bin\bash.exe' --shell-arg -lc fmt
```

One local caveat:

- `just argument-comment-lint` still fails on this Windows machine for
an unrelated Bazel toolchain-resolution issue in
`//codex-rs/exec:exec-all-test`, so I used the direct prebuilt linter
for `codex-tui` as the local fallback.

# Expected reviewer takeaway

If this PR goes green, the important conclusion is that the Windows
Bazel test coverage gap was primarily a Bazel host/exec toolchain
problem, not a need to make the Rust tests themselves Windows-specific.
That would be a strong signal that the deleted non-resource-loading Rust
test edits from the earlier branch should stay out, and that future work
should focus on upstreaming the generic `rules_rs` / `rules_rust`
Windows fixes and reducing the crate-local patch surface.
This commit is contained in:
Michael Bolin
2026-04-03 15:34:03 -07:00
committed by GitHub
Unverified
parent 4b8bab6ad3
commit eaf12beacf
22 changed files with 796 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/scripts/run-bazel-ci.sh
+28 -2
View File
@@ -5,6 +5,7 @@ set -euo pipefail
print_failed_bazel_test_logs=0
use_node_test_env=0
remote_download_toplevel=0
windows_msvc_host_platform=0
while [[ $# -gt 0 ]]; do
case "$1" in
@@ -20,6 +21,10 @@ while [[ $# -gt 0 ]]; do
remote_download_toplevel=1
shift
;;
--windows-msvc-host-platform)
windows_msvc_host_platform=1
shift
;;
--)
shift
break
@@ -32,7 +37,7 @@ while [[ $# -gt 0 ]]; do
done
if [[ $# -eq 0 ]]; then
echo "Usage: $0 [--print-failed-test-logs] [--use-node-test-env] [--remote-download-toplevel] -- <bazel args> -- <targets>" >&2
echo "Usage: $0 [--print-failed-test-logs] [--use-node-test-env] [--remote-download-toplevel] [--windows-msvc-host-platform] -- <bazel args> -- <targets>" >&2
exit 1
fi
@@ -121,14 +126,35 @@ if [[ ${#bazel_args[@]} -eq 0 || ${#bazel_targets[@]} -eq 0 ]]; then
exit 1
fi
if [[ $use_node_test_env -eq 1 && "${RUNNER_OS:-}" != "Windows" ]]; then
if [[ $use_node_test_env -eq 1 ]]; then
# Bazel test sandboxes on macOS may resolve an older Homebrew `node`
# before the `actions/setup-node` runtime on PATH.
node_bin="$(which node)"
if [[ "${RUNNER_OS:-}" == "Windows" ]]; then
node_bin="$(cygpath -w "${node_bin}")"
fi
bazel_args+=("--test_env=CODEX_JS_REPL_NODE_PATH=${node_bin}")
fi
post_config_bazel_args=()
if [[ "${RUNNER_OS:-}" == "Windows" && $windows_msvc_host_platform -eq 1 ]]; then
has_host_platform_override=0
for arg in "${bazel_args[@]}"; do
if [[ "$arg" == --host_platform=* ]]; then
has_host_platform_override=1
break
fi
done
if [[ $has_host_platform_override -eq 0 ]]; then
# Keep Windows Bazel targets on `windows-gnullvm` for cfg coverage, but opt
# specific jobs into an MSVC exec platform when they need helper binaries
# like Rust test wrappers and V8 generators to resolve a compatible host
# toolchain.
post_config_bazel_args+=("--host_platform=//:local_windows_msvc")
fi
fi
if [[ $remote_download_toplevel -eq 1 ]]; then
# Override the CI config's remote_download_minimal setting when callers need
# the built artifact to exist on disk after the command completes.
.github/workflows/bazel.yml
+9 -2
View File
@@ -82,9 +82,16 @@ jobs:
-//third_party/v8:all
)
bazel_wrapper_args=(
--print-failed-test-logs
--use-node-test-env
)
if [[ "${RUNNER_OS}" == "Windows" ]]; then
bazel_wrapper_args+=(--windows-msvc-host-platform)
fi
./.github/scripts/run-bazel-ci.sh \
--print-failed-test-logs \
--use-node-test-env \
"${bazel_wrapper_args[@]}" \
-- \
test \
--test_tag_filters=-argument-comment-lint \
MODULE.bazel
+19 -12
View File
@@ -72,6 +72,7 @@ single_version_override(
patches = [
"//patches:rules_rs_windows_gnullvm_exec.patch",
"//patches:rules_rs_delete_git_worktree_pointer.patch",
"//patches:rules_rs_windows_exec_linker.patch",
],
version = "0.0.43",
)
@@ -85,7 +86,9 @@ rules_rust.patch(
"//patches:rules_rust_windows_gnullvm_build_script.patch",
"//patches:rules_rust_windows_exec_msvc_build_script_env.patch",
"//patches:rules_rust_windows_bootstrap_process_wrapper_linker.patch",
"//patches:rules_rust_windows_build_script_runner_paths.patch",
"//patches:rules_rust_windows_msvc_direct_link_args.patch",
"//patches:rules_rust_windows_process_wrapper_skip_temp_outputs.patch",
"//patches:rules_rust_windows_exec_bin_target.patch",
"//patches:rules_rust_windows_exec_std.patch",
"//patches:rules_rust_windows_exec_rustc_dev_rlib.patch",
@@ -189,8 +192,18 @@ bazel_dep(name = "zstd", version = "1.5.7")
crate.annotation(
crate = "zstd-sys",
gen_build_script = "off",
deps = ["@zstd"],
gen_build_script = "on",
patch_args = ["-p1"],
patches = [
"//patches:zstd-sys_windows_msvc_include_dirs.patch",
],
)
crate.annotation(
crate = "ring",
patch_args = ["-p1"],
patches = [
"//patches:ring_windows_msvc_include_dirs.patch",
],
)
crate.annotation(
build_script_env = {
@@ -218,8 +231,7 @@ bazel_dep(name = "bzip2", version = "1.0.8.bcr.3")
crate.annotation(
crate = "bzip2-sys",
gen_build_script = "off",
deps = ["@bzip2//:bz2"],
gen_build_script = "on",
)
inject_repo(crate, "bzip2")
@@ -228,24 +240,19 @@ bazel_dep(name = "zlib", version = "1.3.1.bcr.8")
crate.annotation(
crate = "libz-sys",
gen_build_script = "off",
deps = ["@zlib"],
gen_build_script = "on",
)
inject_repo(crate, "zlib")
bazel_dep(name = "xz", version = "5.4.5.bcr.8")
# TODO(zbarsky): Enable annotation after fixing windows arm64 builds.
crate.annotation(
crate = "lzma-sys",
gen_build_script = "off",
deps = ["@xz//:lzma"],
gen_build_script = "on",
)
bazel_dep(name = "openssl", version = "3.5.4.bcr.0")
inject_repo(crate, "xz")
crate.annotation(
build_script_data = [
"@openssl//:gen_dir",
MODULE.bazel.lock
Generated
-2
View File
@@ -228,8 +228,6 @@
"https://bcr.bazel.build/modules/upb/0.0.0-20220923-a547704/MODULE.bazel": "7298990c00040a0e2f121f6c32544bab27d4452f80d9ce51349b1a28f3005c43",
"https://bcr.bazel.build/modules/with_cfg.bzl/0.12.0/MODULE.bazel": "b573395fe63aef4299ba095173e2f62ccfee5ad9bbf7acaa95dba73af9fc2b38",
"https://bcr.bazel.build/modules/with_cfg.bzl/0.12.0/source.json": "3f3fbaeafecaf629877ad152a2c9def21f8d330d91aa94c5dc75bbb98c10b8b8",
"https://bcr.bazel.build/modules/xz/5.4.5.bcr.8/MODULE.bazel": "e48a69bd54053c2ec5fffc2a29fb70122afd3e83ab6c07068f63bc6553fa57cc",
"https://bcr.bazel.build/modules/xz/5.4.5.bcr.8/source.json": "bd7e928ccd63505b44f4784f7bbf12cc11f9ff23bf3ca12ff2c91cd74846099e",
"https://bcr.bazel.build/modules/zlib/1.2.11/MODULE.bazel": "07b389abc85fdbca459b69e2ec656ae5622873af3f845e1c9d80fe179f3effa0",
"https://bcr.bazel.build/modules/zlib/1.3.1.bcr.5/MODULE.bazel": "eec517b5bbe5492629466e11dae908d043364302283de25581e3eb944326c4ca",
"https://bcr.bazel.build/modules/zlib/1.3.1.bcr.8/MODULE.bazel": "772c674bb78a0342b8caf32ab5c25085c493ca4ff08398208dcbe4375fe9f776",
codex-rs/app-server/BUILD.bazel
+1
View File
@@ -3,5 +3,6 @@ load("//:defs.bzl", "codex_rust_crate")
codex_rust_crate(
name = "app-server",
crate_name = "codex_app_server",
integration_test_timeout = "long",
test_tags = ["no-sandbox"],
)
codex-rs/core/BUILD.bazel
+2
View File
@@ -30,6 +30,7 @@ codex_rust_crate(
integration_compile_data_extra = [
"//codex-rs/apply-patch:apply_patch_tool_instructions.md",
],
integration_test_timeout = "long",
test_data_extra = [
"config.schema.json",
] + glob([
@@ -46,6 +47,7 @@ codex_rust_crate(
"//:AGENTS.md",
],
test_tags = ["no-sandbox"],
unit_test_timeout = "long",
extra_binaries = [
"//codex-rs/linux-sandbox:codex-linux-sandbox",
"//codex-rs/rmcp-client:test_stdio_server",
codex-rs/otel/BUILD.bazel
+1
View File
@@ -3,4 +3,5 @@ load("//:defs.bzl", "codex_rust_crate")
codex_rust_crate(
name = "otel",
crate_name = "codex_otel",
integration_test_args = ["--test-threads=1"],
)
codex-rs/tui/BUILD.bazel
+4 -1
View File
@@ -15,7 +15,10 @@ codex_rust_crate(
"//codex-rs/collaboration-mode-templates:templates/default.md",
"//codex-rs/collaboration-mode-templates:templates/plan.md",
],
test_data_extra = glob(["src/**/snapshots/**"]) + ["//codex-rs/core:model_availability_nux_fixtures"],
test_data_extra = glob([
"src/**/*.rs",
"src/**/snapshots/**",
]) + ["//codex-rs/core:model_availability_nux_fixtures"],
integration_compile_data_extra = ["src/test_backend.rs"],
extra_binaries = [
"//codex-rs/cli:codex",
codex-rs/tui/src/chatwidget/tests.rs
+8 -1
View File
@@ -210,7 +210,14 @@ pub(super) use tokio::sync::mpsc::unbounded_channel;
pub(super) use toml::Value as TomlValue;
pub(super) fn chatwidget_snapshot_dir() -> PathBuf {
codex_utils_cargo_bin::find_resource!("src/chatwidget/snapshots").expect("snapshot dir")
let snapshot_file = codex_utils_cargo_bin::find_resource!(
"src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_tall.snap"
)
.expect("snapshot file");
snapshot_file
.parent()
.unwrap_or_else(|| panic!("snapshot file has no parent: {}", snapshot_file.display()))
.to_path_buf()
}
macro_rules! assert_chatwidget_snapshot {
codex-rs/tui/tests/manager_dependency_regression.rs
+5 -2
View File
@@ -21,9 +21,12 @@ fn rust_sources_under(dir: &Path) -> Vec<PathBuf> {
#[test]
fn tui_runtime_source_does_not_depend_on_manager_escape_hatches() {
let src_dir = codex_utils_cargo_bin::find_resource!("src")
let src_file = codex_utils_cargo_bin::find_resource!("src/chatwidget.rs")
.unwrap_or_else(|err| panic!("failed to resolve src runfile: {err}"));
let sources = rust_sources_under(&src_dir);
let src_dir = src_file
.parent()
.unwrap_or_else(|| panic!("source file has no parent: {}", src_file.display()));
let sources = rust_sources_under(src_dir);
let forbidden = [
"AuthManager",
"ThreadManager",
codex-rs/windows-sandbox-rs/build.rs
+10
View File
@@ -1,4 +1,14 @@
fn main() {
if std::env::var_os("RULES_RUST_BAZEL_BUILD_SCRIPT_RUNNER").is_some()
&& matches!(std::env::var("CARGO_CFG_TARGET_ENV").as_deref(), Ok("gnu"))
{
// The Windows Bazel lint/test lane targets `windows-gnullvm`, where
// `winres` can emit a `resource` link directive without a usable
// archive in `OUT_DIR`. Skip embedding the manifest there; Cargo's
// normal MSVC builds still compile it.
return;
}
let mut res = winres::WindowsResource::new();
res.set_manifest_file("codex-windows-sandbox-setup.manifest");
let _ = res.compile();
defs.bzl
+34 -8
View File
@@ -13,15 +13,21 @@ PLATFORMS = [
"windows_arm64",
]
# The Bazel-built windows-gnullvm binaries that pull in V8 need a larger PE
# stack reserve than the default linker setting. Thread the flag through the
# executable and test entry points so the final linked artifacts behave the same
# in normal builds and under `bazel test`.
WINDOWS_GNULLVM_RUSTC_STACK_FLAGS = select({
# Match Cargo's Windows linker behavior so Bazel-built binaries and tests use
# the same stack reserve on both Windows ABIs and resolve UCRT imports on MSVC.
WINDOWS_RUSTC_LINK_FLAGS = select({
"@rules_rs//rs/experimental/platforms/constraints:windows_gnullvm": [
"-C",
"link-arg=-Wl,--stack,8388608",
],
"@rules_rs//rs/experimental/platforms/constraints:windows_msvc": [
"-C",
"link-arg=/STACK:8388608",
"-C",
"link-arg=/NODEFAULTLIB:libucrt.lib",
"-C",
"link-arg=ucrt.lib",
],
"//conditions:default": [],
})
@@ -119,8 +125,11 @@ def codex_rust_crate(
rustc_env = {},
deps_extra = [],
integration_compile_data_extra = [],
integration_test_args = [],
integration_test_timeout = None,
test_data_extra = [],
test_tags = [],
unit_test_timeout = None,
extra_binaries = []):
"""Defines a Rust crate with library, binaries, and tests wired for Bazel + Cargo parity.
@@ -149,9 +158,14 @@ def codex_rust_crate(
deps_extra: Extra normal deps beyond @crates resolution.
Typically only needed when features add additional deps.
integration_compile_data_extra: Extra compile_data for integration tests.
integration_test_args: Optional args for integration test binaries.
integration_test_timeout: Optional Bazel timeout for integration test
targets generated from `tests/*.rs`.
test_data_extra: Extra runtime data for tests.
test_tags: Tags applied to unit + integration test targets.
Typically used to disable the sandbox, but see https://bazel.build/reference/be/common-definitions#common.tags
unit_test_timeout: Optional Bazel timeout for the unit-test target
generated from `src/**/*.rs`.
extra_binaries: Additional binary labels to surface as test data and
`CARGO_BIN_EXE_*` environment variables. These are only needed for binaries from a different crate.
"""
@@ -232,7 +246,7 @@ def codex_rust_crate(
# `../codex-rs/<crate>/...` paths for `file!()`. Strip either
# prefix so the workspace-root launcher sees Cargo-like metadata
# such as `tui/src/...`.
rustc_flags = rustc_flags_extra + WINDOWS_GNULLVM_RUSTC_STACK_FLAGS + [
rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS + [
"--remap-path-prefix=../codex-rs=",
"--remap-path-prefix=codex-rs=",
],
@@ -241,12 +255,17 @@ def codex_rust_crate(
tags = test_tags + ["manual"],
)
unit_test_kwargs = {}
if unit_test_timeout:
unit_test_kwargs["timeout"] = unit_test_timeout
workspace_root_test(
name = name + "-unit-tests",
env = test_env,
test_bin = ":" + unit_test_binary,
workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker",
tags = test_tags,
**unit_test_kwargs
)
maybe_deps += [name]
@@ -264,7 +283,7 @@ def codex_rust_crate(
crate_root = main,
deps = all_crate_deps() + maybe_deps + deps_extra,
edition = crate_edition,
rustc_flags = rustc_flags_extra + WINDOWS_GNULLVM_RUSTC_STACK_FLAGS,
rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS,
srcs = native.glob(["src/**/*.rs"]),
visibility = ["//visibility:public"],
)
@@ -274,6 +293,12 @@ def codex_rust_crate(
binary = Label(binary_label).name
cargo_env["CARGO_BIN_EXE_" + binary] = "$(rlocationpath %s)" % binary_label
integration_test_kwargs = {}
if integration_test_args:
integration_test_kwargs["args"] = integration_test_args
if integration_test_timeout:
integration_test_kwargs["timeout"] = integration_test_timeout
for test in native.glob(["tests/*.rs"], allow_empty = True):
test_file_stem = test.removeprefix("tests/").removesuffix(".rs")
test_crate_name = test_file_stem.replace("-", "_")
@@ -292,7 +317,7 @@ def codex_rust_crate(
# Bazel has emitted both `codex-rs/<crate>/...` and
# `../codex-rs/<crate>/...` paths for `file!()`. Strip either
# prefix so Insta records Cargo-like metadata such as `core/tests/...`.
rustc_flags = rustc_flags_extra + WINDOWS_GNULLVM_RUSTC_STACK_FLAGS + [
rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS + [
"--remap-path-prefix=../codex-rs=",
"--remap-path-prefix=codex-rs=",
],
@@ -302,4 +327,5 @@ def codex_rust_crate(
# execute from the repo root and can misplace integration snapshots.
env = cargo_env,
tags = test_tags,
**integration_test_kwargs
)
patches/BUILD.bazel
+5
View File
@@ -5,15 +5,20 @@ exports_files([
"aws-lc-sys_windows_msvc_memcmp_probe.patch",
"llvm_windows_symlink_extract.patch",
"rules_rust_windows_bootstrap_process_wrapper_linker.patch",
"rules_rust_windows_build_script_runner_paths.patch",
"rules_rust_windows_exec_bin_target.patch",
"rules_rust_windows_exec_std.patch",
"rules_rust_windows_process_wrapper_skip_temp_outputs.patch",
"rules_rust_repository_set_exec_constraints.patch",
"rules_rust_windows_msvc_direct_link_args.patch",
"rules_rust_windows_gnullvm_build_script.patch",
"rules_rs_windows_gnullvm_exec.patch",
"rules_rs_windows_exec_linker.patch",
"rusty_v8_prebuilt_out_dir.patch",
"ring_windows_msvc_include_dirs.patch",
"v8_bazel_rules.patch",
"v8_module_deps.patch",
"v8_source_portability.patch",
"windows-link.patch",
"zstd-sys_windows_msvc_include_dirs.patch",
])
patches/aws-lc-sys_windows_msvc_prebuilt_nasm.patch
+102 -13
View File
@@ -1,24 +1,98 @@
diff --git a/builder/main.rs b/builder/main.rs
--- a/builder/main.rs
+++ b/builder/main.rs
@@ -721,16 +721,29 @@
fn get_crate_cflags() -> Option<String> {
optional_env_optional_crate_target("TARGET_CFLAGS")
.or(optional_env_optional_crate_target("CFLAGS"))
diff --git a/builder/cc_builder.rs b/builder/cc_builder.rs
index 95cacb0..d5d814b 100644
--- a/builder/cc_builder.rs
+++ b/builder/cc_builder.rs
@@ -20,9 +20,10 @@ mod win_x86_64;
use crate::nasm_builder::NasmBuilder;
use crate::{
cargo_env, disable_jitter_entropy, emit_warning, env_name_for_target, env_var_to_bool,
- execute_command, get_crate_cc, get_crate_cflags, get_crate_cxx, is_no_asm, out_dir,
- requested_c_std, set_env_for_target, target, target_arch, target_env, target_os, target_vendor,
- test_clang_cl_command, CStdRequested, EnvGuard, OutputLibType,
+ execute_command, get_crate_cc, get_crate_cflags, get_crate_cxx,
+ is_bazel_windows_msvc_build_script, is_no_asm, out_dir, requested_c_std, set_env_for_target,
+ target, target_arch, target_env, target_os, target_vendor, test_clang_cl_command,
+ CStdRequested, EnvGuard, OutputLibType,
};
use std::cell::Cell;
use std::collections::HashMap;
@@ -769,6 +770,30 @@ impl CcBuilder {
}
}
+fn configure_windows_msvc_clang_cl() {
+ let should_use_clang_cl = target_os() == "windows"
+ && target_env() == "msvc"
+ && (target_arch() == "aarch64" || is_bazel_windows_msvc_build_script());
+ if !should_use_clang_cl {
+ return;
+ }
+
+fn is_bazel_windows_msvc_build_script() -> bool {
+ let clang_cl = if test_clang_cl_command() {
+ Some(String::from("clang-cl"))
+ } else {
+ let system_clang_cl = PathBuf::from(r"C:\Program Files\LLVM\bin\clang-cl.exe");
+ system_clang_cl.is_file().then(|| system_clang_cl.display().to_string())
+ };
+ if let Some(clang_cl) = clang_cl {
+ if get_crate_cc().is_none() {
+ set_env_for_target("CC", &clang_cl);
+ }
+ if get_crate_cxx().is_none() {
+ set_env_for_target("CXX", &clang_cl);
+ }
+ }
+}
+
impl crate::Builder for CcBuilder {
fn check_dependencies(&self) -> Result<(), String> {
if OutputLibType::Dynamic == self.output_lib_type {
@@ -784,18 +809,12 @@ impl crate::Builder for CcBuilder {
return Err("cc_builder for libssl not supported".to_string());
}
+ configure_windows_msvc_clang_cl();
Ok(())
}
fn build(&self) -> Result<(), String> {
- if target_os() == "windows"
- && target_arch() == "aarch64"
- && target_env() == "msvc"
- && get_crate_cc().is_none()
- && test_clang_cl_command()
- {
- set_env_for_target("CC", "clang-cl");
- }
+ configure_windows_msvc_clang_cl();
println!("cargo:root={}", self.out_dir.display());
let sources = crate::cc_builder::identify_sources();
diff --git a/builder/main.rs b/builder/main.rs
index 51a9bc1..e714ba4 100644
--- a/builder/main.rs
+++ b/builder/main.rs
@@ -723,14 +723,31 @@ fn get_crate_cflags() -> Option<String> {
.or(optional_env_optional_crate_target("CFLAGS"))
}
+pub(crate) fn is_bazel_windows_msvc_build_script() -> bool {
+ if !target().ends_with("windows-msvc") {
+ return false;
+ }
+
+ if optional_env("RULES_RUST_BAZEL_BUILD_SCRIPT_RUNNER").is_some() {
+ return true;
+ }
+
+ let cargo_manifest_dir = cargo_env("CARGO_MANIFEST_DIR");
+ let manifest_dir = Path::new(&cargo_manifest_dir);
+ manifest_dir
+ .components()
+ .any(|component| component.as_os_str() == "bazel-out")
}
+}
+
fn use_prebuilt_nasm() -> bool {
+ let use_prebuilt_for_bazel_windows_msvc = is_bazel_windows_msvc_build_script();
target_os() == "windows"
@@ -32,12 +106,27 @@ diff --git a/builder/main.rs b/builder/main.rs
&& (Some(true) == allow_prebuilt_nasm() || is_prebuilt_nasm())
}
fn allow_prebuilt_nasm() -> Option<bool> {
@@ -817,8 +834,12 @@ fn main() {
initialize();
prepare_cargo_cfg();
- let manifest_dir = current_dir();
- let manifest_dir = dunce::canonicalize(Path::new(&manifest_dir)).unwrap();
+ let manifest_dir = if is_bazel_windows_msvc_build_script() {
+ PathBuf::from(cargo_env("CARGO_MANIFEST_DIR"))
+ } else {
+ let manifest_dir = current_dir();
+ dunce::canonicalize(Path::new(&manifest_dir)).unwrap()
+ };
let prefix_str = prefix_string();
let prefix = if is_no_prefix() {
None
diff --git a/builder/nasm_builder.rs b/builder/nasm_builder.rs
index cf1f5c8..f683274 100644
--- a/builder/nasm_builder.rs
+++ b/builder/nasm_builder.rs
@@ -57,7 +57,7 @@
if self.files.is_empty() {
@@ -57,7 +57,7 @@ impl NasmBuilder {
return vec![];
}
patches/ring_windows_msvc_include_dirs.patch
+100
View File
@@ -0,0 +1,100 @@
diff --git a/build.rs b/build.rs
index 9843ad8aa..573075489 100644
--- a/build.rs
+++ b/build.rs
@@ -346,7 +346,29 @@ fn ring_build_rs_main(c_root_dir: &Path, core_name_and_version: &str) {
// we want to optimize for minimizing the build tools required: No Perl,
// no nasm, etc.
let generated_dir = if !is_git {
- c_root_dir.join(PREGENERATED)
+ let pregenerated_dir = c_root_dir.join(PREGENERATED);
+ if target.os == WINDOWS
+ && target.env == "msvc"
+ && env::var_os("RULES_RUST_BAZEL_BUILD_SCRIPT_RUNNER").is_some()
+ {
+ let mut pending = vec![(pregenerated_dir.clone(), out_dir.clone())];
+ while let Some((src_dir, dst_dir)) = pending.pop() {
+ fs::create_dir_all(&dst_dir).unwrap();
+ for entry in fs::read_dir(&src_dir).unwrap() {
+ let entry = entry.unwrap();
+ let src_path = entry.path();
+ let dst_path = dst_dir.join(entry.file_name());
+ if entry.file_type().unwrap().is_dir() {
+ pending.push((src_path, dst_path));
+ } else {
+ fs::copy(&src_path, &dst_path).unwrap();
+ }
+ }
+ }
+ out_dir.clone()
+ } else {
+ pregenerated_dir
+ }
} else {
generate_sources_and_preassemble(
&out_dir,
@@ -569,6 +591,15 @@ fn configure_cc(c: &mut cc::Build, target: &Target, c_root_dir: &Path, include_d
let _ = c.include(c_root_dir.join("include"));
let _ = c.include(include_dir);
+ let _ = c.include(c_root_dir.join("third_party").join("fiat"));
+ if compiler.is_like_msvc() {
+ let _ = c.include(c_root_dir.join("crypto").join("curve25519"));
+ let _ = c.include(c_root_dir.join("crypto").join("limbs"));
+ let _ = c.include(c_root_dir.join("crypto").join("fipsmodule").join("aes"));
+ let _ = c.include(c_root_dir.join("crypto").join("fipsmodule").join("bn"));
+ let _ = c.include(c_root_dir.join("crypto").join("fipsmodule").join("ec"));
+ let _ = c.include(c_root_dir.join("crypto").join("poly1305"));
+ }
for f in cpp_flags(&compiler) {
let _ = c.flag(f);
}
diff --git a/crypto/curve25519/curve25519.c b/crypto/curve25519/curve25519.c
index 99d7d7fbb..2f69ad560 100644
--- a/crypto/curve25519/curve25519.c
+++ b/crypto/curve25519/curve25519.c
@@ -47,11 +47,11 @@
#if defined(__GNUC__)
#pragma GCC diagnostic ignored "-Wpedantic"
#endif
-#include "../../third_party/fiat/curve25519_64.h"
+#include "curve25519_64.h"
#elif defined(OPENSSL_64_BIT)
-#include "../../third_party/fiat/curve25519_64_msvc.h"
+#include "curve25519_64_msvc.h"
#else
-#include "../../third_party/fiat/curve25519_32.h"
+#include "curve25519_32.h"
#endif
diff --git a/crypto/curve25519/curve25519_64_adx.c b/crypto/curve25519/curve25519_64_adx.c
index 88964a9dd..b660f55f4 100644
--- a/crypto/curve25519/curve25519_64_adx.c
+++ b/crypto/curve25519/curve25519_64_adx.c
@@ -19,5 +19,5 @@
#pragma GCC diagnostic ignored "-Wpedantic"
#pragma GCC diagnostic ignored "-Wsign-conversion"
-#include "../../third_party/fiat/curve25519_64_adx.h"
+#include "curve25519_64_adx.h"
#endif
diff --git a/crypto/fipsmodule/ec/p256.c b/crypto/fipsmodule/ec/p256.c
index 0117916da..4224d1192 100644
--- a/crypto/fipsmodule/ec/p256.c
+++ b/crypto/fipsmodule/ec/p256.c
@@ -50,11 +50,11 @@
#if defined(__GNUC__)
#pragma GCC diagnostic ignored "-Wpedantic"
#endif
-#include "../../../third_party/fiat/p256_64.h"
+#include "p256_64.h"
#elif defined(OPENSSL_64_BIT)
-#include "../../../third_party/fiat/p256_64_msvc.h"
+#include "p256_64_msvc.h"
#else
-#include "../../../third_party/fiat/p256_32.h"
+#include "p256_32.h"
#endif
patches/rules_rs_windows_exec_linker.patch
+73
View File
@@ -0,0 +1,73 @@
# What: use a working Windows direct linker for `rules_rs` exec toolchains and
# preserve the Windows stdlib link flags the stable wrapper was dropping.
# Scope: Windows-only linker metadata for the generated `rules_rs` toolchains.
diff --git a/rs/experimental/toolchains/declare_rustc_toolchains.bzl b/rs/experimental/toolchains/declare_rustc_toolchains.bzl
index 67e491c..3f1cff5 100644
--- a/rs/experimental/toolchains/declare_rustc_toolchains.bzl
+++ b/rs/experimental/toolchains/declare_rustc_toolchains.bzl
@@ -50,6 +50,8 @@ def declare_rustc_toolchains(
rust_toolchain(
name = rust_toolchain_name,
rust_doc = "{}rustdoc".format(rustc_repo_label),
+ linker = "{}rust-lld".format(rustc_repo_label) if exec_triple.system == "windows" else None,
+ linker_type = "direct" if exec_triple.system == "windows" else None,
rust_std = select(rust_std_select),
rustc = "{}rustc".format(rustc_repo_label),
cargo = "{}cargo".format(cargo_repo_label),
@@ -82,7 +84,20 @@ def declare_rustc_toolchains(
stdlib_linkflags = select({
"@platforms//os:freebsd": ["-lexecinfo", "-lpthread"],
"@platforms//os:macos": ["-lSystem", "-lresolv"],
- # TODO: windows
+ "@rules_rs//rs/experimental/platforms/constraints:windows_gnullvm": [
+ "advapi32.lib",
+ "ws2_32.lib",
+ "userenv.lib",
+ "Bcrypt.lib",
+ ],
+ "@rules_rs//rs/experimental/platforms/constraints:windows_msvc": [
+ "advapi32.lib",
+ "ws2_32.lib",
+ "userenv.lib",
+ "Bcrypt.lib",
+ "ucrt.lib",
+ ],
+ # TODO: other platforms
"//conditions:default": [],
}),
default_edition = edition,
diff --git a/rs/private/rustc_repository.bzl b/rs/private/rustc_repository.bzl
index f4f0286..6558bb2 100644
--- a/rs/private/rustc_repository.bzl
+++ b/rs/private/rustc_repository.bzl
@@ -1,13 +1,28 @@
load("@rules_rust//rust/platform:triple.bzl", "triple")
load(
"@rules_rust//rust/private:repository_utils.bzl",
"BUILD_for_compiler",
)
load(":rust_repository_utils.bzl", "download_and_extract", "RUST_REPOSITORY_COMMON_ATTR")
+_WINDOWS_EXEC_LINKER_BUILD = """
+filegroup(
+ name = "rust-lld",
+ srcs = ["bin/lld-link.exe"],
+ visibility = ["//visibility:public"],
+)
+"""
+
def _rustc_repository_impl(rctx):
exec_triple = triple(rctx.attr.triple)
download_and_extract(rctx, "rustc", "rustc", exec_triple)
- rctx.file("BUILD.bazel", BUILD_for_compiler(exec_triple))
+ build_file = BUILD_for_compiler(exec_triple)
+ if exec_triple.system == "windows":
+ lld_link = rctx.which("lld-link.exe")
+ if lld_link == None:
+ fail("lld-link.exe not found on PATH")
+ rctx.symlink(lld_link, "bin/lld-link.exe")
+ build_file += _WINDOWS_EXEC_LINKER_BUILD
+ rctx.file("BUILD.bazel", build_file)
return rctx.repo_metadata(reproducible = True)
patches/rules_rust_windows_bootstrap_process_wrapper_linker.patch
+9 -11
View File
@@ -1,23 +1,21 @@
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -472,7 +472,19 @@
@@ -472,7 +472,17 @@
)
ld_is_direct_driver = False
- if not ld or toolchain.linker_preference == "rust":
+ # The bootstrap process wrapper is built without the normal rules_rust
+ # process wrapper. On Windows nightly toolchains that expose rust-lld, the
+ # C++ toolchain path currently resolves to clang++ while still emitting
+ # MSVC-style arguments, so prefer rust-lld for this one bootstrap binary
+ # instead of switching all Rust actions over.
+ use_bootstrap_rust_linker = (
+ # Windows MSVC Rust targets can inherit a GNU-flavored C++ linker path
+ # (`clang++`) alongside MSVC-style arguments. Prefer the toolchain linker
+ # there so both exec helpers and ordinary MSVC Rust targets link through
+ # the Windows direct linker instead.
+ use_windows_rust_linker = (
+ toolchain.target_os.startswith("windows") and
+ toolchain.linker != None and
+ hasattr(ctx.executable, "_bootstrap_process_wrapper") and
+ not ctx.executable._process_wrapper
+ toolchain.target_abi == "msvc" and
+ toolchain.linker != None
+ )
+
+ if not ld or toolchain.linker_preference == "rust" or use_bootstrap_rust_linker:
+ if not ld or toolchain.linker_preference == "rust" or use_windows_rust_linker:
ld = toolchain.linker.path
ld_is_direct_driver = toolchain.linker_type == "direct"
patches/rules_rust_windows_build_script_runner_paths.patch
+171
View File
@@ -0,0 +1,171 @@
--- a/cargo/private/cargo_build_script_runner/bin.rs
+++ b/cargo/private/cargo_build_script_runner/bin.rs
@@ -24,6 +24,85 @@
use cargo_build_script_runner::cargo_manifest_dir::{remove_symlink, symlink, RunfilesMaker};
use cargo_build_script_runner::{BuildScriptOutput, CompileAndLinkFlags};
+#[cfg(windows)]
+fn normalize_windows_relative_path(path: &str) -> String {
+ path.replace('/', "\\")
+}
+
+#[cfg(not(windows))]
+fn normalize_windows_relative_path(path: &str) -> String {
+ path.to_owned()
+}
+
+fn exec_root_join(exec_root: &Path, relative: &str) -> PathBuf {
+ exec_root.join(normalize_windows_relative_path(relative))
+}
+
+#[cfg(windows)]
+#[link(name = "Kernel32")]
+unsafe extern "system" {
+ fn GetShortPathNameW(
+ lpszLongPath: *const u16,
+ lpszShortPath: *mut u16,
+ cchBuffer: u32,
+ ) -> u32;
+}
+
+#[cfg(windows)]
+fn windows_extended_length_path(path: &Path) -> PathBuf {
+ let path = path.as_os_str().to_string_lossy();
+ if path.starts_with(r"\\?\") {
+ PathBuf::from(path.to_string())
+ } else if let Some(stripped) = path.strip_prefix(r"\\") {
+ PathBuf::from(format!(r"\\?\UNC\{stripped}"))
+ } else {
+ PathBuf::from(format!(r"\\?\{path}"))
+ }
+}
+
+#[cfg(windows)]
+fn shorten_windows_path(path: &Path) -> PathBuf {
+ use std::os::windows::ffi::OsStrExt;
+
+ let long_path = windows_extended_length_path(path);
+ let long_path = long_path
+ .as_os_str()
+ .encode_wide()
+ .chain(Some(0))
+ .collect::<Vec<u16>>();
+
+ unsafe {
+ let required = GetShortPathNameW(long_path.as_ptr(), std::ptr::null_mut(), 0);
+ if required == 0 {
+ return path.to_owned();
+ }
+
+ let mut short_path = vec![0u16; required as usize];
+ let written = GetShortPathNameW(long_path.as_ptr(), short_path.as_mut_ptr(), required);
+ if written == 0 {
+ path.to_owned()
+ } else {
+ let short_path = PathBuf::from(
+ <std::ffi::OsString as std::os::windows::ffi::OsStringExt>::from_wide(
+ &short_path[..written as usize],
+ ),
+ );
+ PathBuf::from(
+ short_path
+ .as_os_str()
+ .to_string_lossy()
+ .trim_start_matches(r"\\?\")
+ .to_string(),
+ )
+ }
+ }
+}
+
+#[cfg(not(windows))]
+fn shorten_windows_path(path: &Path) -> PathBuf {
+ path.to_owned()
+}
+
fn run_buildrs() -> Result<(), String> {
// We use exec_root.join rather than std::fs::canonicalize, to avoid resolving symlinks, as
// some execution strategies and remote execution environments may use symlinks in ways which
@@ -31,10 +110,11 @@ fn run_buildrs() -> Result<(), String> {
// directory - resolving these may cause tools which inspect $0, or try to resolve files
// relative to themselves, to fail.
let exec_root = env::current_dir().expect("Failed to get current directory");
- let manifest_dir_env = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR was not set");
- let rustc_env = env::var("RUSTC").expect("RUSTC was not set");
- let manifest_dir = exec_root.join(manifest_dir_env);
- let rustc = exec_root.join(&rustc_env);
+ let manifest_dir_env =
+ normalize_windows_relative_path(&env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR was not set"));
+ let rustc_env = normalize_windows_relative_path(&env::var("RUSTC").expect("RUSTC was not set"));
+ let manifest_dir = exec_root_join(&exec_root, &manifest_dir_env);
+ let rustc = shorten_windows_path(&exec_root_join(&exec_root, &rustc_env));
let Args {
progname,
crate_links,
@@ -55,7 +135,8 @@ fn run_buildrs() -> Result<(), String> {
cargo_manifest_maker.create_runfiles_dir().unwrap()
}
- let out_dir_abs = exec_root.join(out_dir);
+ let out_dir_abs = shorten_windows_path(&exec_root_join(&exec_root, &out_dir));
// For some reason Google's RBE does not create the output directory, force create it.
create_dir_all(&out_dir_abs)
.unwrap_or_else(|_| panic!("Failed to make output directory: {:?}", out_dir_abs));
@@ -89,11 +170,12 @@ fn run_buildrs() -> Result<(), String> {
let working_directory = resolve_rundir(&rundir, &exec_root, &manifest_dir)?;
- let mut command = Command::new(exec_root.join(progname));
+ let mut command = Command::new(exec_root_join(&exec_root, &progname));
command
- .current_dir(&working_directory)
+ .current_dir(shorten_windows_path(&working_directory))
.envs(target_env_vars)
.env("OUT_DIR", &out_dir_abs)
+ .env("RULES_RUST_BAZEL_BUILD_SCRIPT_RUNNER", "1")
.env("CARGO_MANIFEST_DIR", manifest_dir)
.env("RUSTC", rustc)
.env("RUST_BACKTRACE", "full");
@@ -123,7 +205,7 @@ fn run_buildrs() -> Result<(), String> {
for tool_env_var in &["CC", "CXX", "LD"] {
if let Some(tool_path) = env::var_os(tool_env_var) {
- command.env(tool_env_var, exec_root.join(tool_path));
+ command.env(tool_env_var, exec_root_join(&exec_root, &tool_path.to_string_lossy()));
}
}
@@ -139,7 +221,7 @@ fn run_buildrs() -> Result<(), String> {
command.env_remove("AR");
command.env_remove("ARFLAGS");
} else {
- command.env("AR", exec_root.join(ar_path));
+ command.env("AR", exec_root_join(&exec_root, &ar_path.to_string_lossy()));
}
}
@@ -257,7 +339,8 @@ fn resolve_rundir(rundir: &str, exec_root: &Path, manifest_dir: &Path) -> Result
if rundir.is_empty() {
return Ok(manifest_dir.to_owned());
}
- let rundir_path = Path::new(rundir);
+ let rundir = normalize_windows_relative_path(rundir);
+ let rundir_path = Path::new(&rundir);
if rundir_path.is_absolute() {
return Err(format!("rundir must be empty (to run in manifest path) or relative path (relative to exec root), but was {:?}", rundir));
}
diff --git a/cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs b/cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs
--- a/cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs
+++ b/cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs
@@ -282,7 +282,7 @@
/// been copied into the runfiles directory.
fn drain_runfiles_dir_windows(&self) -> Result<(), String> {
for dest in self.runfiles.values() {
- if !self
+ if self
.filename_suffixes_to_retain
.iter()
.any(|suffix| dest.ends_with(suffix))
{
continue;
}
patches/rules_rust_windows_exec_msvc_build_script_env.patch
+119 -46
View File
@@ -1,62 +1,39 @@
diff --git a/cargo/private/cargo_build_script.bzl b/cargo/private/cargo_build_script.bzl
--- a/cargo/private/cargo_build_script.bzl
+++ b/cargo/private/cargo_build_script.bzl
@@ -142,40 +142,82 @@ def _strip_stack_protector_for_windows_llvm_mingw(toolchain, args):
def _rewrite_windows_exec_msvc_cc_args(toolchain, args):
"""Translate GNU-flavored cc args when exec-side build scripts target Windows MSVC."""
@@ -144,9 +144,14 @@ def _rewrite_windows_exec_msvc_cc_args(toolchain, args):
if toolchain.target_flag_value != toolchain.exec_triple.str or not toolchain.exec_triple.str.endswith("-pc-windows-msvc"):
return args
- rewritten = []
- skip_next = False
- for arg in args:
- if skip_next:
- skip_next = False
- continue
+ rewritten = [
+ "-target",
+ toolchain.target_flag_value,
+ ]
+ skip_next = False
skip_next = False
- for arg in args:
+ for index in range(len(args)):
+ arg = args[index]
+
+ if skip_next:
+ skip_next = False
+ continue
if arg == "-target":
- skip_next = True
+ skip_next = True
if skip_next:
skip_next = False
continue
if arg.startswith("-target=") or arg.startswith("--target="):
continue
@@ -161,21 +166,58 @@ def _rewrite_windows_exec_msvc_cc_args(toolchain, args):
if arg == "-nostdlibinc" or arg.startswith("--sysroot"):
continue
- if "mingw-w64-" in arg or "mingw_import_libraries_directory" in arg or "mingw_crt_library_search_directory" in arg:
+ if arg.startswith("-fstack-protector") or arg.startswith("-D_FORTIFY_SOURCE="):
continue
- if arg.startswith("-fstack-protector"):
- continue
-
- if arg.startswith("-D_FORTIFY_SOURCE="):
- continue
+ continue
+
+ if arg == "-isystem" and index + 1 < len(args):
+ path = args[index + 1]
+ if "mingw-w64-" in path or "mingw_import_libraries_directory" in path or "mingw_crt_library_search_directory" in path:
+ skip_next = True
+ continue
rewritten.append(arg)
- return [
- "-target",
- toolchain.target_flag_value,
- ] + rewritten
+
+ rewritten.append(arg)
+
+ return rewritten
+
+def _rewrite_windows_exec_msvc_link_args(toolchain, args):
@@ -78,8 +55,9 @@ diff --git a/cargo/private/cargo_build_script.bzl b/cargo/private/cargo_build_sc
+ continue
+
+ if arg.startswith("--sysroot="):
+ continue
+
continue
- if arg.startswith("-fstack-protector"):
+ if arg == "-L" and index + 1 < len(args):
+ path = args[index + 1]
+ if "mingw_import_libraries_directory" in path or "mingw_crt_library_search_directory" in path:
@@ -87,25 +65,120 @@ diff --git a/cargo/private/cargo_build_script.bzl b/cargo/private/cargo_build_sc
+ continue
+ rewritten.extend([arg, path])
+ skip_next = True
+ continue
+
continue
- if arg.startswith("-D_FORTIFY_SOURCE="):
+ if arg.startswith("-L") and (
+ "mingw_import_libraries_directory" in arg or
+ "mingw_crt_library_search_directory" in arg
+ ):
+ continue
+
+ rewritten.append(arg)
+
continue
rewritten.append(arg)
- return [
- "-target",
- toolchain.target_flag_value,
- ] + rewritten
+ return rewritten
def get_cc_compile_args_and_env(cc_toolchain, feature_configuration):
"""Gather cc environment variables from the given `cc_toolchain`
@@ -509,6 +550,7 @@ def _construct_build_script_env(
@@ -508,15 +550,23 @@ def _cargo_build_script_impl(ctx):
cc_toolchain, feature_configuration = find_cc_toolchain(ctx)
linker, _, link_args, linker_env = get_linker_and_args(ctx, "bin", toolchain, cc_toolchain, feature_configuration, None)
env.update(**linker_env)
env["LD"] = linker
- env["LD"] = linker
- env["LDFLAGS"] = " ".join(_pwd_flags(link_args))
+ use_windows_exec_msvc_path_tools = (
+ toolchain.target_flag_value == toolchain.exec_triple.str and
+ toolchain.exec_triple.str.endswith("-pc-windows-msvc")
+ )
+
+ if not use_windows_exec_msvc_path_tools:
+ env["LD"] = linker
+ link_args = _rewrite_windows_exec_msvc_link_args(toolchain, link_args)
env["LDFLAGS"] = " ".join(_pwd_flags(link_args))
+ env["LDFLAGS"] = " ".join(_pwd_flags(link_args))
+
+ # Defaults for cxx flags.
+ env["CFLAGS"] = ""
+ env["CXXFLAGS"] = ""
# Defaults for cxx flags.
- # Defaults for cxx flags.
env["ARFLAGS"] = ""
- env["CFLAGS"] = ""
- env["CXXFLAGS"] = ""
fallback_tools = []
- if not cc_toolchain:
+ if not cc_toolchain and not use_windows_exec_msvc_path_tools:
fallbacks = {
"AR": "_fallback_ar",
"CC": "_fallback_cc",
@@ -542,36 +592,37 @@ def _cargo_build_script_impl(ctx):
toolchain_tools.append(cc_toolchain.all_files)
- env["CC"] = cc_common.get_tool_for_action(
- feature_configuration = feature_configuration,
- action_name = ACTION_NAMES.c_compile,
- )
- env["CXX"] = cc_common.get_tool_for_action(
- feature_configuration = feature_configuration,
- action_name = ACTION_NAMES.cpp_compile,
- )
- env["AR"] = cc_common.get_tool_for_action(
- feature_configuration = feature_configuration,
- action_name = ACTION_NAMES.cpp_link_static_library,
- )
-
- # Many C/C++ toolchains are missing an action_config for AR because
- # one was never included in the unix_cc_toolchain_config.
- if not env["AR"]:
- env["AR"] = cc_toolchain.ar_executable
-
- cc_c_args = _strip_stack_protector_for_windows_llvm_mingw(toolchain, cc_c_args)
- cc_cxx_args = _strip_stack_protector_for_windows_llvm_mingw(toolchain, cc_cxx_args)
- cc_c_args = _rewrite_windows_exec_msvc_cc_args(toolchain, cc_c_args)
- cc_cxx_args = _rewrite_windows_exec_msvc_cc_args(toolchain, cc_cxx_args)
- # Populate CFLAGS and CXXFLAGS that cc-rs relies on when building from source, in particular
- # to determine the deployment target when building for apple platforms (`macosx-version-min`
- # for example, itself derived from the `macos_minimum_os` Bazel argument).
- env["CFLAGS"] = " ".join(_pwd_flags(cc_c_args))
- env["CXXFLAGS"] = " ".join(_pwd_flags(cc_cxx_args))
- # It may be tempting to forward ARFLAGS, but cc-rs is opinionated enough
- # that doing so is more likely to hurt than help. If you need to change
- # ARFLAGS, make changes to cc-rs.
+ if not use_windows_exec_msvc_path_tools:
+ env["CC"] = cc_common.get_tool_for_action(
+ feature_configuration = feature_configuration,
+ action_name = ACTION_NAMES.c_compile,
+ )
+ env["CXX"] = cc_common.get_tool_for_action(
+ feature_configuration = feature_configuration,
+ action_name = ACTION_NAMES.cpp_compile,
+ )
+ env["AR"] = cc_common.get_tool_for_action(
+ feature_configuration = feature_configuration,
+ action_name = ACTION_NAMES.cpp_link_static_library,
+ )
+
+ # Many C/C++ toolchains are missing an action_config for AR because
+ # one was never included in the unix_cc_toolchain_config.
+ if not env["AR"]:
+ env["AR"] = cc_toolchain.ar_executable
+
+ cc_c_args = _strip_stack_protector_for_windows_llvm_mingw(toolchain, cc_c_args)
+ cc_cxx_args = _strip_stack_protector_for_windows_llvm_mingw(toolchain, cc_cxx_args)
+ cc_c_args = _rewrite_windows_exec_msvc_cc_args(toolchain, cc_c_args)
+ cc_cxx_args = _rewrite_windows_exec_msvc_cc_args(toolchain, cc_cxx_args)
+ # Populate CFLAGS and CXXFLAGS that cc-rs relies on when building from source, in particular
+ # to determine the deployment target when building for apple platforms (`macosx-version-min`
+ # for example, itself derived from the `macos_minimum_os` Bazel argument).
+ env["CFLAGS"] = " ".join(_pwd_flags(cc_c_args))
+ env["CXXFLAGS"] = " ".join(_pwd_flags(cc_cxx_args))
+ # It may be tempting to forward ARFLAGS, but cc-rs is opinionated enough
+ # that doing so is more likely to hurt than help. If you need to change
+ # ARFLAGS, make changes to cc-rs.
# Inform build scripts of rustc flags
# https://github.com/rust-lang/cargo/issues/9600
patches/rules_rust_windows_msvc_direct_link_args.patch
+34 -9
View File
@@ -1,6 +1,32 @@
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -2305,7 +2305,7 @@
@@ -501,11 +501,41 @@
filtered_args.append(version)
# Keep library search path flags
+ elif processed_arg == "-L" and i + 1 < len(link_args):
+ path = link_args[i + 1]
+ if ld_is_direct_driver and toolchain.target_os == "windows":
+ skip_next = True
+ continue
+ filtered_args.extend([processed_arg, path])
+ skip_next = True
+
elif processed_arg.startswith("-L"):
+ if ld_is_direct_driver and toolchain.target_os == "windows":
+ continue
filtered_args.append(processed_arg)
# Keep sysroot flags (as single or two-part arguments)
elif processed_arg == "--sysroot" or processed_arg.startswith("--sysroot="):
+ if ld_is_direct_driver and toolchain.target_os == "windows":
+ if processed_arg == "--sysroot" and i + 1 < len(link_args):
+ skip_next = True
+ continue
filtered_args.append(processed_arg)
if processed_arg == "--sysroot" and i + 1 < len(link_args):
# Two-part argument, keep the next arg too
@@ -2305,7 +2335,7 @@
return crate.metadata.dirname
return crate.output.dirname
@@ -30,11 +56,17 @@
# Windows toolchains can inherit POSIX defaults like -pthread from C deps,
# which fails to link with the MinGW/LLD toolchain. Drop them here.
@@ -2558,17 +2563,25 @@
@@ -2453,14 +2483,21 @@
else:
# For all other crate types we want to link C++ runtime library statically
# (for example libstdc++.a or libc++.a).
+ runtime_libs = cc_toolchain.static_runtime_lib(feature_configuration = feature_configuration)
+ if toolchain.target_os == "windows" and use_direct_link_driver:
+ runtime_libs = depset([
+ runtime_lib
+ for runtime_lib in runtime_libs.to_list()
+ if runtime_lib.basename.endswith(".lib")
+ ])
args.add_all(
- cc_toolchain.static_runtime_lib(feature_configuration = feature_configuration),
+ runtime_libs,
@@ -47,13 +79,6 @@
- map_each = get_lib_name,
- format_each = "-lstatic=%s",
- )
+ if toolchain.target_os == "windows" and use_direct_link_driver:
+ for runtime_lib in runtime_libs.to_list():
+ if runtime_lib.basename.endswith(".lib"):
+ args.add(get_lib_name(runtime_lib), format = "-lstatic=%s")
+ else:
+ args.add(runtime_lib.path, format = "--codegen=link-arg=%s")
+ else:
+ args.add_all(
+ runtime_libs,
+ map_each = get_lib_name,
patches/rules_rust_windows_process_wrapper_skip_temp_outputs.patch
+15
View File
@@ -0,0 +1,15 @@
--- a/util/process_wrapper/main.rs
+++ b/util/process_wrapper/main.rs
@@ -213,6 +213,12 @@ fn consolidate_dependency_search_paths(
let file_name_lower = file_name
.to_string_lossy()
.to_ascii_lowercase();
+ if file_name_lower.contains(".tmp") || file_name_lower.ends_with(".rcgu.o") {
+ // MSVC link actions can leave transient `*.exe.tmp*` and
+ // per-codegen-unit `*.rcgu.o` outputs next to the final binary.
+ // Those are not stable linker search-path inputs.
+ continue;
+ }
if !seen.insert(file_name_lower) {
continue;
}
patches/zstd-sys_windows_msvc_include_dirs.patch
+41
View File
@@ -0,0 +1,41 @@
diff --git a/build.rs b/build.rs
index 912a3bf..e078f0e 100644
--- a/build.rs
+++ b/build.rs
@@ -85,6 +85,7 @@ fn flag_if_supported_with_fallbacks(config: &mut cc::Build, flags: &[&str]) {
}
fn compile_zstd() {
+ let target = env::var("TARGET").unwrap_or_default();
let mut config = cc::Build::new();
// Search the following directories for C files to add to the compilation.
@@ -164,6 +165,16 @@ fn compile_zstd() {
// Some extra parameters
config.include("zstd/lib/");
config.include("zstd/lib/common");
+ if target.contains("msvc") {
+ config.include("zstd/lib/compress");
+ config.include("zstd/lib/decompress");
+ #[cfg(feature = "zdict_builder")]
+ config.include("zstd/lib/dictBuilder");
+ #[cfg(feature = "legacy")]
+ config.include("zstd/lib/legacy");
+ #[cfg(feature = "seekable")]
+ config.include("zstd/contrib/seekable_format");
+ }
config.warnings(false);
config.define("ZSTD_LIB_DEPRECATED", Some("0"));
@@ -210,7 +221,9 @@ fn compile_zstd() {
// Hide symbols from resulting library,
// so we can be used with another zstd-linking lib.
- // See https://github.com/gyscos/zstd-rs/issues/58
- config.flag("-fvisibility=hidden");
+ // See https://github.com/gyscos/zstd-rs/issues/58.
+ if !target.contains("msvc") {
+ config.flag("-fvisibility=hidden");
+ }
config.define("XXH_PRIVATE_API", Some(""));
config.define("ZSTDLIB_VISIBILITY", Some(""));