diff --git a/.bazelrc b/.bazelrc index 76f81ade40..a068b44840 100644 --- a/.bazelrc +++ b/.bazelrc @@ -153,6 +153,25 @@ common:ci-macos --config=remote common:ci-macos --strategy=remote common:ci-macos --strategy=TestRunner=darwin-sandbox,local +# On Windows, use Linux remote execution for build actions but keep test actions +# on the Windows runner so Bazel's normal test sharding and flaky-test retries +# still run against Windows binaries. +common:ci-windows-cross --config=ci-windows +common:ci-windows-cross --build_metadata=TAG_windows_cross_compile=true +common:ci-windows-cross --config=remote +common:ci-windows-cross --host_platform=//:rbe +common:ci-windows-cross --strategy=remote +common:ci-windows-cross --strategy=TestRunner=local +common:ci-windows-cross --local_test_jobs=4 +common:ci-windows-cross --test_env=RUST_TEST_THREADS=1 +# Native Windows CI still covers these tests. The cross-built gnullvm binaries +# currently crash in V8-backed code-mode tests and hang in PowerShell AST parser +# tests when those binaries are run on the Windows runner. +common:ci-windows-cross --test_env=CODEX_BAZEL_TEST_SKIP_FILTERS=suite::code_mode::,powershell +common:ci-windows-cross --platforms=//:windows_x86_64_gnullvm +common:ci-windows-cross --extra_execution_platforms=//:rbe,//:windows_x86_64_msvc +common:ci-windows-cross --extra_toolchains=//:windows_gnullvm_tests_on_msvc_host_toolchain + # Linux-only V8 CI config. common:ci-v8 --config=ci common:ci-v8 --build_metadata=TAG_workflow=v8 diff --git a/.github/scripts/compute-bazel-windows-path.ps1 b/.github/scripts/compute-bazel-windows-path.ps1 index 6b6bbe0462..81fd668c8b 100644 --- a/.github/scripts/compute-bazel-windows-path.ps1 +++ b/.github/scripts/compute-bazel-windows-path.ps1 @@ -5,9 +5,9 @@ tool entries, such as Maven, that can change independently of this repo and cause avoidable cache misses. This script derives a smaller, cache-stable PATH that keeps the Windows -toolchain entries Bazel-backed CI tasks need: MSVC and Windows SDK paths, Git, -PowerShell, Node, Python, DotSlash, and the standard Windows system -directories. +toolchain entries Bazel-backed CI tasks need: MSVC and Windows SDK paths, +MinGW runtime DLL paths for gnullvm-built tests, Git, PowerShell, Node, Python, +DotSlash, and the standard Windows system directories. `setup-bazel-ci` runs this after exporting the MSVC environment, and the script publishes the result via `GITHUB_ENV` as `CODEX_BAZEL_WINDOWS_PATH` so later steps can pass that explicit PATH to Bazel. @@ -49,6 +49,8 @@ foreach ($pathEntry in ($env:PATH -split ';')) { $pathEntry -like '*Microsoft Visual Studio*' -or $pathEntry -like '*Windows Kits*' -or $pathEntry -like '*Microsoft SDKs*' -or + $pathEntry -eq 'C:\mingw64\bin' -or + $pathEntry -like 'C:\msys64\*\bin' -or $pathEntry -like 'C:\Program Files\Git\*' -or $pathEntry -like 'C:\Program Files\PowerShell\*' -or $pathEntry -like 'C:\hostedtoolcache\windows\node\*' -or @@ -85,6 +87,12 @@ if ($pwshCommand) { Add-StablePathEntry (Split-Path $pwshCommand.Source -Parent) } +foreach ($mingwPath in @('C:\mingw64\bin', 'C:\msys64\mingw64\bin', 'C:\msys64\ucrt64\bin')) { + if (Test-Path $mingwPath) { + Add-StablePathEntry $mingwPath + } +} + if ($windowsAppsPath) { Add-StablePathEntry $windowsAppsPath } diff --git a/.github/scripts/run-bazel-ci.sh b/.github/scripts/run-bazel-ci.sh index b81e0a4d57..f98e4d8cb9 100755 --- a/.github/scripts/run-bazel-ci.sh +++ b/.github/scripts/run-bazel-ci.sh @@ -6,6 +6,7 @@ print_failed_bazel_test_logs=0 print_failed_bazel_action_summary=0 remote_download_toplevel=0 windows_msvc_host_platform=0 +windows_cross_compile=0 while [[ $# -gt 0 ]]; do case "$1" in @@ -25,6 +26,10 @@ while [[ $# -gt 0 ]]; do windows_msvc_host_platform=1 shift ;; + --windows-cross-compile) + windows_cross_compile=1 + shift + ;; --) shift break @@ -37,7 +42,7 @@ while [[ $# -gt 0 ]]; do done if [[ $# -eq 0 ]]; then - echo "Usage: $0 [--print-failed-test-logs] [--print-failed-action-summary] [--remote-download-toplevel] [--windows-msvc-host-platform] -- -- " >&2 + echo "Usage: $0 [--print-failed-test-logs] [--print-failed-action-summary] [--remote-download-toplevel] [--windows-msvc-host-platform] [--windows-cross-compile] -- -- " >&2 exit 1 fi @@ -61,7 +66,11 @@ case "${RUNNER_OS:-}" in ci_config=ci-macos ;; Windows) - ci_config=ci-windows + if [[ $windows_cross_compile -eq 1 ]]; then + ci_config=ci-windows-cross + else + ci_config=ci-windows + fi ;; esac @@ -105,8 +114,8 @@ print_bazel_test_log_tails() { while IFS= read -r target; do failed_targets+=("$target") done < <( - grep -E '^FAIL: //' "$console_log" \ - | sed -E 's#^FAIL: (//[^ ]+).*#\1#' \ + grep -E '^(FAIL: //|ERROR: .* Testing //)' "$console_log" \ + | sed -E 's#^FAIL: (//[^ ]+).*#\1#; s#^ERROR: .* Testing (//[^ ]+) failed:.*#\1#' \ | sort -u ) @@ -244,6 +253,12 @@ if [[ ${#bazel_args[@]} -eq 0 || ${#bazel_targets[@]} -eq 0 ]]; then exit 1 fi +if [[ "${RUNNER_OS:-}" == "Windows" && $windows_cross_compile -eq 1 && -z "${BUILDBUDDY_API_KEY:-}" ]]; then + # Fork PRs do not receive the BuildBuddy secret needed for the remote + # cross-compile config. Preserve the previous local Windows build shape. + windows_msvc_host_platform=1 +fi + post_config_bazel_args=() if [[ "${RUNNER_OS:-}" == "Windows" && $windows_msvc_host_platform -eq 1 ]]; then has_host_platform_override=0 @@ -269,6 +284,25 @@ if [[ $remote_download_toplevel -eq 1 ]]; then post_config_bazel_args+=(--remote_download_toplevel) fi +if [[ "${RUNNER_OS:-}" == "Windows" && $windows_cross_compile -eq 1 && -n "${BUILDBUDDY_API_KEY:-}" ]]; then + # `--enable_platform_specific_config` expands `common:windows` on Windows + # hosts after ordinary rc configs, which can override `ci-windows-cross`'s + # RBE host platform. Repeat the host platform on the command line so V8 and + # other genrules execute on Linux RBE workers instead of Git Bash locally. + # + # Bazel also derives the default genrule shell from the client host. Without + # an explicit shell executable, remote Linux actions can be asked to run + # `C:\Program Files\Git\usr\bin\bash.exe`. + post_config_bazel_args+=(--host_platform=//:rbe --shell_executable=/bin/bash) +fi + +if [[ "${RUNNER_OS:-}" == "Windows" && $windows_cross_compile -eq 1 && -z "${BUILDBUDDY_API_KEY:-}" ]]; then + # The Windows cross-compile config depends on remote execution. Fork PRs do + # not receive the BuildBuddy secret, so fall back to the existing local build + # shape and keep its lower concurrency cap. + post_config_bazel_args+=(--jobs=8) +fi + if [[ -n "${BAZEL_REPO_CONTENTS_CACHE:-}" ]]; then # Windows self-hosted runners can run multiple Bazel jobs concurrently. Give # each job its own repo contents cache so they do not fight over the shared @@ -287,37 +321,57 @@ if [[ -n "${CODEX_BAZEL_EXECUTION_LOG_COMPACT_DIR:-}" ]]; then fi if [[ "${RUNNER_OS:-}" == "Windows" ]]; then - windows_action_env_vars=( - INCLUDE - LIB - LIBPATH - UCRTVersion - UniversalCRTSdkDir - VCINSTALLDIR - VCToolsInstallDir - WindowsLibPath - WindowsSdkBinPath - WindowsSdkDir - WindowsSDKLibVersion - WindowsSDKVersion - ) + pass_windows_build_env=1 + if [[ $windows_cross_compile -eq 1 && -n "${BUILDBUDDY_API_KEY:-}" ]]; then + # Remote build actions execute on Linux RBE workers. Passing the Windows + # runner's build environment there makes Bazel genrules try to execute + # C:\Program Files\Git\usr\bin\bash.exe on Linux. + pass_windows_build_env=0 + fi - for env_var in "${windows_action_env_vars[@]}"; do - if [[ -n "${!env_var:-}" ]]; then - post_config_bazel_args+=("--action_env=${env_var}" "--host_action_env=${env_var}") - fi - done + if [[ $pass_windows_build_env -eq 1 ]]; then + windows_action_env_vars=( + INCLUDE + LIB + LIBPATH + UCRTVersion + UniversalCRTSdkDir + VCINSTALLDIR + VCToolsInstallDir + WindowsLibPath + WindowsSdkBinPath + WindowsSdkDir + WindowsSDKLibVersion + WindowsSDKVersion + ) + + for env_var in "${windows_action_env_vars[@]}"; do + if [[ -n "${!env_var:-}" ]]; then + post_config_bazel_args+=("--action_env=${env_var}" "--host_action_env=${env_var}") + fi + done + fi if [[ -z "${CODEX_BAZEL_WINDOWS_PATH:-}" ]]; then echo "CODEX_BAZEL_WINDOWS_PATH must be set for Windows Bazel CI." >&2 exit 1 fi - post_config_bazel_args+=( - "--action_env=PATH=${CODEX_BAZEL_WINDOWS_PATH}" - "--host_action_env=PATH=${CODEX_BAZEL_WINDOWS_PATH}" - "--test_env=PATH=${CODEX_BAZEL_WINDOWS_PATH}" - ) + if [[ $pass_windows_build_env -eq 1 ]]; then + post_config_bazel_args+=( + "--action_env=PATH=${CODEX_BAZEL_WINDOWS_PATH}" + "--host_action_env=PATH=${CODEX_BAZEL_WINDOWS_PATH}" + ) + elif [[ $windows_cross_compile -eq 1 ]]; then + # Remote build actions run on Linux RBE workers. Give their shell snippets + # a Linux PATH while preserving CODEX_BAZEL_WINDOWS_PATH below for local + # Windows test execution. + post_config_bazel_args+=( + "--action_env=PATH=/usr/bin:/bin" + "--host_action_env=PATH=/usr/bin:/bin" + ) + fi + post_config_bazel_args+=("--test_env=PATH=${CODEX_BAZEL_WINDOWS_PATH}") fi bazel_console_log="$(mktemp)" diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index ef41330c46..ef7520523a 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -17,13 +17,10 @@ concurrency: cancel-in-progress: ${{ github.ref_name != 'main' }} jobs: test: - # Even though a no-cache-hit Windows build seems to exceed the 30-minute - # limit on occasion, the more common reason for exceeding the limit is a - # true test failure in a rust_test() marked "flaky" that gets run 3x. - # In that case, extra time generally does not give us more signal. - # - # Ultimately we need true distributed builds (e.g., - # https://www.buildbuddy.io/docs/rbe-setup/) to speed things up. + # PRs use a fast Windows cross-compiled test leg for pre-merge signal. + # Post-merge pushes to main also run the native Windows test job below, + # which keeps V8/code-mode coverage without putting PR latency back on the + # critical path. timeout-minutes: 30 strategy: fail-fast: false @@ -47,13 +44,16 @@ jobs: # - os: ubuntu-24.04-arm # target: aarch64-unknown-linux-gnu - # Windows + # Windows fast path: build the windows-gnullvm binaries with Linux + # RBE, then run the resulting Windows tests on the Windows runner. + # The main-only native Windows job below preserves full V8/code-mode + # coverage post-merge. - os: windows-latest target: x86_64-pc-windows-gnullvm runs-on: ${{ matrix.os }} # Configure a human readable name for each job - name: Local Bazel build on ${{ matrix.os }} for ${{ matrix.target }} + name: Bazel test on ${{ matrix.os }} for ${{ matrix.target }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -91,6 +91,7 @@ jobs: ) bazel_wrapper_args=( + --print-failed-action-summary --print-failed-test-logs ) bazel_test_args=( @@ -100,8 +101,19 @@ jobs: --build_metadata=COMMIT_SHA=${GITHUB_SHA} ) if [[ "${RUNNER_OS}" == "Windows" ]]; then - bazel_wrapper_args+=(--windows-msvc-host-platform) - bazel_test_args+=(--jobs=8) + bazel_wrapper_args+=( + --windows-cross-compile + --remote-download-toplevel + ) + # Tradeoff: the Linux-RBE-built windows-gnullvm V8 archive + # currently crashes during direct V8/code-mode smoke tests on the + # Windows runner. Keep the broader fast Windows suite in PR CI and + # rely on the main-only native Windows job below for full + # V8/code-mode signal while we investigate the cross-built archive. + bazel_targets+=( + -//codex-rs/code-mode:code-mode-unit-tests + -//codex-rs/v8-poc:v8-poc-unit-tests + ) fi ./.github/scripts/run-bazel-ci.sh \ @@ -130,6 +142,75 @@ jobs: path: ${{ steps.prepare_bazel.outputs.repository-cache-path }} key: ${{ steps.prepare_bazel.outputs.repository-cache-key }} + test-windows-native-main: + # Native Windows Bazel tests are slower and frequently approach the + # 30-minute PR budget, but they provide the full V8/code-mode signal that + # the fast cross-compiled PR leg intentionally trades away. Run this only + # for post-merge commits to main and give it a larger timeout. + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + timeout-minutes: 40 + runs-on: windows-latest + name: Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) + + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + + - name: Prepare Bazel CI + id: prepare_bazel + uses: ./.github/actions/prepare-bazel-ci + with: + target: x86_64-pc-windows-gnullvm + cache-scope: bazel-${{ github.job }} + install-test-prereqs: "true" + + - name: bazel test //... + env: + BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }} + shell: bash + run: | + bazel_targets=( + //... + # Keep standalone V8 library targets out of the ordinary Bazel CI + # path. V8 consumers under `//codex-rs/...` still participate + # transitively through `//...`. + -//third_party/v8:all + ) + + bazel_test_args=( + test + --test_tag_filters=-argument-comment-lint + --test_verbose_timeout_warnings + --build_metadata=COMMIT_SHA=${GITHUB_SHA} + --build_metadata=TAG_windows_native_main=true + ) + + ./.github/scripts/run-bazel-ci.sh \ + --print-failed-action-summary \ + --print-failed-test-logs \ + -- \ + "${bazel_test_args[@]}" \ + -- \ + "${bazel_targets[@]}" + + - name: Upload Bazel execution logs + if: always() && !cancelled() + continue-on-error: true + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7 + with: + name: bazel-execution-logs-test-windows-native-x86_64-pc-windows-gnullvm + path: ${{ runner.temp }}/bazel-execution-logs + if-no-files-found: ignore + + # Save the job-scoped Bazel repository cache after cache misses. Keep the + # upload non-fatal so cache service issues never fail the job itself. + - name: Save bazel repository cache + if: always() && !cancelled() && steps.prepare_bazel.outputs.repository-cache-hit != 'true' + continue-on-error: true + uses: actions/cache/save@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5 + with: + path: ${{ steps.prepare_bazel.outputs.repository-cache-path }} + key: ${{ steps.prepare_bazel.outputs.repository-cache-key }} + clippy: timeout-minutes: 30 strategy: diff --git a/BUILD.bazel b/BUILD.bazel index 3f59ff1160..a82126e6f1 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -30,6 +30,40 @@ platform( parents = ["@platforms//host"], ) +platform( + name = "windows_x86_64_gnullvm", + constraint_values = [ + "@platforms//cpu:x86_64", + "@platforms//os:windows", + "@rules_rs//rs/experimental/platforms/constraints:windows_gnullvm", + ], +) + +platform( + name = "windows_x86_64_msvc", + constraint_values = [ + "@platforms//cpu:x86_64", + "@platforms//os:windows", + "@rules_rs//rs/experimental/platforms/constraints:windows_msvc", + ], +) + +toolchain( + name = "windows_gnullvm_tests_on_msvc_host_toolchain", + exec_compatible_with = [ + "@platforms//cpu:x86_64", + "@platforms//os:windows", + "@rules_rs//rs/experimental/platforms/constraints:windows_msvc", + ], + target_compatible_with = [ + "@platforms//cpu:x86_64", + "@platforms//os:windows", + "@rules_rs//rs/experimental/platforms/constraints:windows_gnullvm", + ], + toolchain = "@bazel_tools//tools/test:empty_toolchain", + toolchain_type = "@bazel_tools//tools/test:default_test_toolchain_type", +) + alias( name = "rbe", actual = "@rbe_platform", diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs index b0530a4fb4..66f21807ba 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/tests.rs @@ -50,6 +50,10 @@ fn pwsh_path() -> Option { } fn sandbox_cwd() -> PathBuf { + if let Ok(workspace_root) = std::env::var("INSTA_WORKSPACE_ROOT") { + return PathBuf::from(workspace_root); + } + PathBuf::from(env!("CARGO_MANIFEST_DIR")) .parent() .expect("repo root") diff --git a/defs.bzl b/defs.bzl index 71a3138418..043dec3c36 100644 --- a/defs.bzl +++ b/defs.bzl @@ -1,8 +1,8 @@ load("@crates//:data.bzl", "DEP_DATA") load("@crates//:defs.bzl", "all_crate_deps") load("@rules_platform//platform_data:defs.bzl", "platform_data") -load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_proc_macro", "rust_test") load("@rules_rust//cargo/private:cargo_build_script_wrapper.bzl", "cargo_build_script") +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_proc_macro", "rust_test") PLATFORMS = [ "linux_arm64_musl", @@ -31,6 +31,16 @@ WINDOWS_RUSTC_LINK_FLAGS = select({ "//conditions:default": [], }) +WINDOWS_GNULLVM_INCOMPATIBLE = select({ + "@rules_rs//rs/experimental/platforms/constraints:windows_gnullvm": ["@platforms//:incompatible"], + "//conditions:default": [], +}) + +WINDOWS_GNULLVM_ONLY = select({ + "@rules_rs//rs/experimental/platforms/constraints:windows_gnullvm": [], + "//conditions:default": ["@platforms//:incompatible"], +}) + # libwebrtc uses Objective-C categories from native archives. Any Bazel-linked # macOS binary/test that can pull it in must keep category symbols alive. MACOS_WEBRTC_RUSTC_LINK_FLAGS = select({ @@ -64,12 +74,16 @@ def _workspace_root_test_impl(ctx): test_bin = ctx.executable.test_bin workspace_root_marker = ctx.file.workspace_root_marker launcher_template = ctx.file._windows_launcher_template if is_windows else ctx.file._bash_launcher_template + runfile_env_exports = _windows_runfile_env_exports(ctx) if is_windows else _bash_runfile_env_exports(ctx) + workspace_root_setup = _windows_workspace_root_setup(ctx) if is_windows else _bash_workspace_root_setup(ctx) ctx.actions.expand_template( template = launcher_template, output = launcher, is_executable = True, substitutions = { + "__RUNFILE_ENV_EXPORTS__": runfile_env_exports, "__TEST_BIN__": test_bin.short_path, + "__WORKSPACE_ROOT_SETUP__": workspace_root_setup, "__WORKSPACE_ROOT_MARKER__": workspace_root_marker.short_path, }, ) @@ -78,6 +92,22 @@ def _workspace_root_test_impl(ctx): for data_dep in ctx.attr.data: runfiles = runfiles.merge(ctx.runfiles(files = data_dep[DefaultInfo].files.to_list())) runfiles = runfiles.merge(data_dep[DefaultInfo].default_runfiles) + for runfile_dep in ctx.attr.runfile_env: + executable = runfile_dep[DefaultInfo].files_to_run.executable + if executable == None: + fail("{} does not provide an executable for runfile_env".format(runfile_dep.label)) + runfiles = runfiles.merge(ctx.runfiles(files = [executable])) + runfiles = runfiles.merge(runfile_dep[DefaultInfo].default_runfiles) + + location_targets = ( + ctx.attr.data + + [ctx.attr.test_bin, ctx.attr.workspace_root_marker] + + ctx.attr.runfile_env.keys() + ) + env = { + key: ctx.expand_location(value, targets = location_targets) + for key, value in ctx.attr.env.items() + } return [ DefaultInfo( @@ -86,18 +116,55 @@ def _workspace_root_test_impl(ctx): runfiles = runfiles, ), RunEnvironmentInfo( - environment = ctx.attr.env, + environment = env, ), ] +def _bash_runfile_env_exports(ctx): + lines = [] + for runfile_dep, env_var in ctx.attr.runfile_env.items(): + executable = runfile_dep[DefaultInfo].files_to_run.executable + if executable == None: + fail("{} does not provide an executable for runfile_env".format(runfile_dep.label)) + lines.append('RUNFILE_ENV_ARGS+=("{}=$(resolve_runfile "{}")")'.format(env_var, executable.short_path)) + return "\n".join(lines) + +def _windows_runfile_env_exports(ctx): + lines = [] + for runfile_dep, env_var in ctx.attr.runfile_env.items(): + executable = runfile_dep[DefaultInfo].files_to_run.executable + if executable == None: + fail("{} does not provide an executable for runfile_env".format(runfile_dep.label)) + lines.append('call :resolve_runfile {} "{}"'.format(env_var, executable.short_path)) + lines.append("if errorlevel 1 exit /b 1") + return "\n".join(lines) + +def _bash_workspace_root_setup(ctx): + if not ctx.attr.chdir_workspace_root: + return "" + return 'export INSTA_WORKSPACE_ROOT="${workspace_root}"\ncd "${workspace_root}"' + +def _windows_workspace_root_setup(ctx): + if not ctx.attr.chdir_workspace_root: + return "" + return """set "INSTA_WORKSPACE_ROOT=%workspace_root%" +cd /d "%workspace_root%" || exit /b 1""" + workspace_root_test = rule( implementation = _workspace_root_test_impl, test = True, + toolchains = ["@bazel_tools//tools/test:default_test_toolchain_type"], attrs = { + "chdir_workspace_root": attr.bool( + default = True, + ), "data": attr.label_list( allow_files = True, ), "env": attr.string_dict(), + "runfile_env": attr.label_keyed_string_dict( + cfg = "target", + ), "test_bin": attr.label( cfg = "target", executable = True, @@ -255,6 +322,7 @@ def codex_rust_crate( unit_test_name = name + "-unit-tests" unit_test_binary = name + "-unit-tests-bin" unit_test_shard_count = _test_shard_count(test_shard_counts, unit_test_name) + # Shard at the workspace_root_test layer. rules_rust's sharding wrapper # expects to run from its own runfiles cwd, while workspace_root_test # deliberately changes cwd so Insta sees Cargo-like snapshot paths. @@ -298,9 +366,11 @@ def codex_rust_crate( sanitized_binaries = [] cargo_env = {} + cargo_env_runfiles = {} for binary, main in binaries.items(): #binary = binary.replace("-", "_") sanitized_binaries.append(binary) + cargo_env_runfiles[":" + binary] = "CARGO_BIN_EXE_" + binary cargo_env["CARGO_BIN_EXE_" + binary] = "$(rlocationpath :%s)" % binary rust_binary( @@ -317,6 +387,7 @@ def codex_rust_crate( for binary_label in extra_binaries: sanitized_binaries.append(binary_label) binary = Label(binary_label).name + cargo_env_runfiles[binary_label] = "CARGO_BIN_EXE_" + binary cargo_env["CARGO_BIN_EXE_" + binary] = "$(rlocationpath %s)" % binary_label integration_test_kwargs = {} @@ -331,6 +402,7 @@ def codex_rust_crate( test_name = name + "-" + test_file_stem.replace("/", "-") if not test_name.endswith("-test"): test_name += "-test" + windows_cross_test_binary = test_name + "-windows-cross-bin" test_kwargs = {} test_kwargs.update(integration_test_kwargs) @@ -340,6 +412,9 @@ def codex_rust_crate( test_kwargs["shard_count"] = test_shard_count test_kwargs["flaky"] = True + # Keep the existing integration test shape on non-gnullvm platforms. + # Windows cross tests need workspace_root_test so runfile env vars + # resolve to Windows-native absolute paths before the test starts. rust_test( name = test_name, crate_name = test_crate_name, @@ -356,14 +431,48 @@ def codex_rust_crate( "--remap-path-prefix=codex-rs=", ], rustc_env = rustc_env, - # Important: do not merge `test_env` here. Its unit-test-only - # `INSTA_WORKSPACE_ROOT="codex-rs"` is tuned for unit tests that - # execute from the repo root and can misplace integration snapshots. env = cargo_env, + target_compatible_with = WINDOWS_GNULLVM_INCOMPATIBLE, tags = test_tags, **test_kwargs ) + windows_cross_test_kwargs = {} + windows_cross_test_kwargs.update(integration_test_kwargs) + if test_shard_count: + windows_cross_test_kwargs["shard_count"] = test_shard_count + windows_cross_test_kwargs["flaky"] = True + + rust_test( + name = windows_cross_test_binary, + crate_name = test_crate_name, + crate_root = test, + srcs = [test], + data = native.glob(["tests/**"], allow_empty = True) + sanitized_binaries + test_data_extra, + compile_data = native.glob(["tests/**"], allow_empty = True) + integration_compile_data_extra, + deps = all_crate_deps(normal = True, normal_dev = True) + maybe_deps + deps_extra, + rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS + [ + "--remap-path-prefix=../codex-rs=", + "--remap-path-prefix=codex-rs=", + ], + rustc_env = rustc_env, + env = cargo_env, + target_compatible_with = WINDOWS_GNULLVM_ONLY, + tags = test_tags + ["manual"], + ) + + workspace_root_test( + name = test_name + "-windows-cross", + chdir_workspace_root = False, + env = cargo_env, + runfile_env = cargo_env_runfiles, + test_bin = ":" + windows_cross_test_binary, + workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker", + target_compatible_with = WINDOWS_GNULLVM_ONLY, + tags = test_tags, + **windows_cross_test_kwargs + ) + def _test_shard_count(test_shard_counts, test_name): shard_count = test_shard_counts.get(test_name) if shard_count == None: diff --git a/patches/v8_bazel_rules.patch b/patches/v8_bazel_rules.patch index 10e1a57679..df845939d0 100644 --- a/patches/v8_bazel_rules.patch +++ b/patches/v8_bazel_rules.patch @@ -133,7 +133,7 @@ index 85f31b7..7314584 100644 ], outs = [ "include/inspector/Debugger.h", -@@ -4426,15 +4426,18 @@ genrule( +@@ -4426,15 +4426,19 @@ genrule( "src/inspector/protocol/Schema.cpp", "src/inspector/protocol/Schema.h", ], @@ -145,6 +145,7 @@ index 85f31b7..7314584 100644 + --inspector_protocol_dir $$INSPECTOR_PROTOCOL_DIR \ --config $(location :src/inspector/inspector_protocol_config.json) \ --config_value protocol.path=$(location :include/js_protocol.pdl) \ ++ --config_value crdtp.dir=third_party/inspector_protocol/crdtp \ --output_base $(@D)/src/inspector", - local = 1, message = "Generating inspector files", diff --git a/patches/v8_source_portability.patch b/patches/v8_source_portability.patch index 4f5f46005f..d480e11c1a 100644 --- a/patches/v8_source_portability.patch +++ b/patches/v8_source_portability.patch @@ -83,9 +83,21 @@ index 420df0b..6f47969 100644 return __libc_stack_end; } diff --git a/orig/v8-14.6.202.11/src/base/platform/platform-win32.cc b/mod/v8-14.6.202.11/src/base/platform/platform-win32.cc -index f5d9ddc..542ea1a 100644 +index f5d9ddc..1c08b0f 100644 --- a/orig/v8-14.6.202.11/src/base/platform/platform-win32.cc +++ b/mod/v8-14.6.202.11/src/base/platform/platform-win32.cc +@@ -20,7 +20,11 @@ + #include + + // This has to come after windows.h. ++#ifdef __MINGW32__ ++#include ++#else + #include ++#endif + #include // For SymLoadModule64 and al. + #include // For _msize() + #include // For timeGetTime(). @@ -69,9 +69,7 @@ static_assert(offsetof(V8_CRITICAL_SECTION, SpinCount) == // Extra functions for MinGW. Most of these are the _s functions which are in // the Microsoft Visual Studio C++ CRT. @@ -171,7 +183,7 @@ diff --git a/orig/v8-14.6.202.11/src/heap/base/asm/x64/push_registers_masm.asm b index d0d0563..72e230b 100644 --- a/orig/v8-14.6.202.11/src/heap/base/asm/x64/push_registers_masm.asm +++ b/mod/v8-14.6.202.11/src/heap/base/asm/x64/push_registers_masm.asm -@@ -1,70 +1,30 @@ +@@ -1,70 +1,47 @@ -;; Copyright 2020 the V8 project authors. All rights reserved. -;; Use of this source code is governed by a BSD-style license that can be -;; found in the LICENSE file. diff --git a/scripts/list-bazel-clippy-targets.sh b/scripts/list-bazel-clippy-targets.sh index 73c0777e26..141d0cf48d 100755 --- a/scripts/list-bazel-clippy-targets.sh +++ b/scripts/list-bazel-clippy-targets.sh @@ -14,6 +14,9 @@ manual_rust_test_targets="$( --output=label \ -- 'kind("rust_test rule", attr(tags, "manual", //codex-rs/... except //codex-rs/v8-poc/...))' )" +if [[ "${RUNNER_OS:-}" != "Windows" ]]; then + manual_rust_test_targets="$(printf '%s\n' "${manual_rust_test_targets}" | grep -v -- '-windows-cross-bin$' || true)" +fi printf '%s\n' \ "//codex-rs/..." \ diff --git a/third_party/v8/BUILD.bazel b/third_party/v8/BUILD.bazel index 27e9fa3ffe..94bb3b32c6 100644 --- a/third_party/v8/BUILD.bazel +++ b/third_party/v8/BUILD.bazel @@ -174,18 +174,23 @@ genrule( name = "binding_cc", srcs = ["@v8_crate_146_4_0//:binding_cc"], outs = ["binding.cc"], - # Keep this as a literal shell snippet. The string-concatenated form looked - # cleaner in Starlark but produced a broken `sed` invocation in CI. - cmd = """ - sed \ - -e '/#include "v8\\/src\\/flags\\/flags.h"/d' \ - -e 's|"v8/src/libplatform/default-platform.h"|"src/libplatform/default-platform.h"|' \ - -e 's| namespace i = v8::internal;| (void)usage;|' \ - -e '/using HelpOptions = i::FlagList::HelpOptions;/d' \ - -e '/HelpOptions help_options = HelpOptions(HelpOptions::kExit, usage);/d' \ - -e 's| i::FlagList::SetFlagsFromCommandLine(argc, argv, true, help_options);| v8::V8::SetFlagsFromCommandLine(argc, argv, true);|' \ - $(location @v8_crate_146_4_0//:binding_cc) > "$@" - """, + # Keep this as one physical shell line. In Windows cross CI, this genrule + # runs on Linux RBE from a Windows Bazel client; multiline command text can + # carry CRLF into `/bin/bash` as a standalone `$'\r'` command. Use an + # explicit argv-style join so separators stay visible without shell + # newlines. + cmd = " ".join([ + "sed", + "-e '/#include \"v8\\/src\\/flags\\/flags.h\"/d'", + "-e 's|\"v8/src/libplatform/default-platform.h\"|\"src/libplatform/default-platform.h\"|'", + "-e 's| namespace i = v8::internal;| (void)usage;|'", + "-e '/using HelpOptions = i::FlagList::HelpOptions;/d'", + "-e '/HelpOptions help_options = HelpOptions(HelpOptions::kExit, usage);/d'", + "-e 's| i::FlagList::SetFlagsFromCommandLine(argc, argv, true, help_options);| v8::V8::SetFlagsFromCommandLine(argc, argv, true);|'", + "$(location @v8_crate_146_4_0//:binding_cc)", + ">", + '"$@"', + ]), ) copy_file( diff --git a/tools/argument-comment-lint/list-bazel-targets.sh b/tools/argument-comment-lint/list-bazel-targets.sh index 1874a65f3c..f8cb4f5e20 100755 --- a/tools/argument-comment-lint/list-bazel-targets.sh +++ b/tools/argument-comment-lint/list-bazel-targets.sh @@ -9,7 +9,14 @@ cd "${repo_root}" # `*-unit-tests-bin` rust_test targets generated by `codex_rust_crate()`. # Add only those manual rust_test targets explicitly so inline `#[cfg(test)]` # call sites are linted without pulling in unrelated manual release targets. +manual_rust_test_targets="$( + ./.github/scripts/run-bazel-query-ci.sh \ + --output=label \ + -- 'kind("rust_test rule", attr(tags, "manual", //codex-rs/...))' +)" +if [[ "${RUNNER_OS:-}" != "Windows" ]]; then + manual_rust_test_targets="$(printf '%s\n' "${manual_rust_test_targets}" | grep -v -- '-windows-cross-bin$' || true)" +fi + printf '%s\n' "//codex-rs/..." -./.github/scripts/run-bazel-query-ci.sh \ - --output=label \ - -- 'kind("rust_test rule", attr(tags, "manual", //codex-rs/...))' +printf '%s\n' "${manual_rust_test_targets}" diff --git a/workspace_root_test_launcher.bat.tpl b/workspace_root_test_launcher.bat.tpl index af82e5ecff..3613b91d76 100644 --- a/workspace_root_test_launcher.bat.tpl +++ b/workspace_root_test_launcher.bat.tpl @@ -10,20 +10,29 @@ for %%I in ("%workspace_root_marker_dir%..\..") do set "workspace_root=%%~fI" call :resolve_runfile test_bin "__TEST_BIN__" if errorlevel 1 exit /b 1 -set "INSTA_WORKSPACE_ROOT=%workspace_root%" -cd /d "%workspace_root%" || exit /b 1 +__RUNFILE_ENV_EXPORTS__ + +__WORKSPACE_ROOT_SETUP__ set "TOTAL_SHARDS=%RULES_RUST_TEST_TOTAL_SHARDS%" if not defined TOTAL_SHARDS set "TOTAL_SHARDS=%TEST_TOTAL_SHARDS%" +if defined TESTBRIDGE_TEST_ONLY if "%~1"=="" ( + "%test_bin%" "%TESTBRIDGE_TEST_ONLY%" + exit /b !ERRORLEVEL! +) +if defined CODEX_BAZEL_TEST_SKIP_FILTERS ( + call :run_selected_libtest %* + exit /b !ERRORLEVEL! +) if defined TOTAL_SHARDS if not "%TOTAL_SHARDS%"=="0" ( - call :run_sharded_libtest %* + call :run_selected_libtest %* exit /b !ERRORLEVEL! ) "%test_bin%" %* exit /b %ERRORLEVEL% -:run_sharded_libtest +:run_selected_libtest if defined TEST_SHARD_STATUS_FILE if defined TEST_TOTAL_SHARDS if not "%TEST_TOTAL_SHARDS%"=="0" ( type nul > "%TEST_SHARD_STATUS_FILE%" ) @@ -35,7 +44,9 @@ if not "%~1"=="" ( set "SHARD_INDEX=%RULES_RUST_TEST_SHARD_INDEX%" if not defined SHARD_INDEX set "SHARD_INDEX=%TEST_SHARD_INDEX%" -if not defined SHARD_INDEX ( +set "HAS_SHARDS=" +if defined TOTAL_SHARDS if not "%TOTAL_SHARDS%"=="0" set "HAS_SHARDS=1" +if defined HAS_SHARDS if not defined SHARD_INDEX ( >&2 echo TEST_SHARD_INDEX or RULES_RUST_TEST_SHARD_INDEX must be set when sharding is enabled exit /b 1 ) @@ -60,9 +71,12 @@ powershell.exe -NoProfile -ExecutionPolicy Bypass -Command ^ "$ErrorActionPreference = 'Stop';" ^ "$tests = @(Get-Content -LiteralPath $env:TEMP_LIST | Where-Object { $_.EndsWith(': test') } | ForEach-Object { $_.Substring(0, $_.Length - 6) });" ^ "[Array]::Sort($tests, [StringComparer]::Ordinal);" ^ - "$totalShards = [uint32]$env:TOTAL_SHARDS; $shardIndex = [uint32]$env:SHARD_INDEX;" ^ + "$hasShards = -not [string]::IsNullOrEmpty($env:HAS_SHARDS);" ^ + "$skipFilters = @();" ^ + "if (-not [string]::IsNullOrEmpty($env:CODEX_BAZEL_TEST_SKIP_FILTERS)) { $skipFilters = @($env:CODEX_BAZEL_TEST_SKIP_FILTERS -split ',' | Where-Object { $_ -ne '' }) };" ^ + "if ($hasShards) { $totalShards = [uint32]$env:TOTAL_SHARDS; $shardIndex = [uint32]$env:SHARD_INDEX };" ^ "$fnvPrime = [uint64]16777619; $u32Mask = [uint64]4294967295;" ^ - "foreach ($test in $tests) { $hash = [uint32]2166136261; foreach ($byte in [Text.Encoding]::UTF8.GetBytes($test)) { $hash = [uint32](([uint64]($hash -bxor $byte) * $fnvPrime) -band $u32Mask) }; if (($hash %% $totalShards) -eq $shardIndex) { $test } }" ^ + "foreach ($test in $tests) { $skip = $false; foreach ($filter in $skipFilters) { if ($test.Contains($filter)) { $skip = $true; break } }; if ($skip) { continue }; if ($hasShards) { $hash = [uint32]2166136261; foreach ($byte in [Text.Encoding]::UTF8.GetBytes($test)) { $hash = [uint32](([uint64]($hash -bxor $byte) * $fnvPrime) -band $u32Mask) }; if (($hash %% $totalShards) -eq $shardIndex) { $test } } else { $test } }" ^ > "!TEMP_SHARD_LIST!" if errorlevel 1 ( rmdir /s /q "!TEMP_DIR!" 2>nul diff --git a/workspace_root_test_launcher.sh.tpl b/workspace_root_test_launcher.sh.tpl index 1ba752506b..4606fd8b15 100644 --- a/workspace_root_test_launcher.sh.tpl +++ b/workspace_root_test_launcher.sh.tpl @@ -47,6 +47,30 @@ resolve_runfile() { workspace_root_marker="$(resolve_runfile "__WORKSPACE_ROOT_MARKER__")" workspace_root="$(dirname "$(dirname "$(dirname "${workspace_root_marker}")")")" test_bin="$(resolve_runfile "__TEST_BIN__")" +RUNFILE_ENV_ARGS=() + +__RUNFILE_ENV_EXPORTS__ + +run_test_bin() { + if (( ${#RUNFILE_ENV_ARGS[@]} > 0 )); then + env "${RUNFILE_ENV_ARGS[@]}" "${test_bin}" "$@" + else + "${test_bin}" "$@" + fi +} + +exec_test_bin() { + if (( ${#RUNFILE_ENV_ARGS[@]} > 0 )); then + exec env "${RUNFILE_ENV_ARGS[@]}" "${test_bin}" "$@" + else + exec "${test_bin}" "$@" + fi +} + +libtest_args=("$@") +if [[ ${#libtest_args[@]} -eq 0 && -n "${TESTBRIDGE_TEST_ONLY:-}" ]]; then + libtest_args+=("${TESTBRIDGE_TEST_ONLY}") +fi test_shard_index() { local test_name="$1" @@ -67,35 +91,58 @@ test_shard_index() { echo $(( hash % TOTAL_SHARDS )) } -run_sharded_libtest() { +run_selected_libtest() { if [[ -n "${TEST_SHARD_STATUS_FILE:-}" && "${TEST_TOTAL_SHARDS:-0}" != "0" ]]; then touch "${TEST_SHARD_STATUS_FILE}" fi # Extra libtest args are usually ad-hoc local filters. Preserve those exactly # rather than combining them with generated exact filters. - if [[ $# -gt 0 ]]; then - exec "${test_bin}" "$@" + if [[ ${#libtest_args[@]} -gt 0 ]]; then + exec_test_bin "${libtest_args[@]}" fi - if [[ -z "${SHARD_INDEX}" ]]; then + local has_shards=0 + if [[ -n "${TOTAL_SHARDS}" && "${TOTAL_SHARDS}" != "0" ]]; then + has_shards=1 + fi + + if [[ "${has_shards}" == "1" && -z "${SHARD_INDEX}" ]]; then echo "TEST_SHARD_INDEX or RULES_RUST_TEST_SHARD_INDEX must be set when sharding is enabled" >&2 exit 1 fi local list_output local test_list - list_output="$("${test_bin}" --list --format terse)" + list_output="$(run_test_bin --list --format terse)" test_list="$(printf '%s\n' "${list_output}" | grep ': test$' | sed 's/: test$//' | LC_ALL=C sort || true)" if [[ -z "${test_list}" ]]; then exit 0 fi + local skip_filters="${CODEX_BAZEL_TEST_SKIP_FILTERS:-}" + local shard_tests=() local test_name while IFS= read -r test_name; do - if (( $(test_shard_index "${test_name}") == SHARD_INDEX )); then + local skip=0 + if [[ -n "${skip_filters}" ]]; then + local filter + local old_ifs="${IFS}" + IFS=',' + for filter in ${skip_filters}; do + if [[ -n "${filter}" && "${test_name}" == *"${filter}"* ]]; then + skip=1 + break + fi + done + IFS="${old_ifs}" + fi + if [[ "${skip}" == "1" ]]; then + continue + fi + if [[ "${has_shards}" == "0" || $(test_shard_index "${test_name}") == "${SHARD_INDEX}" ]]; then shard_tests+=("${test_name}") fi done <<< "${test_list}" @@ -104,16 +151,19 @@ run_sharded_libtest() { exit 0 fi - exec "${test_bin}" "${shard_tests[@]}" --exact + exec_test_bin "${shard_tests[@]}" --exact } -export INSTA_WORKSPACE_ROOT="${workspace_root}" -cd "${workspace_root}" +__WORKSPACE_ROOT_SETUP__ TOTAL_SHARDS="${RULES_RUST_TEST_TOTAL_SHARDS:-${TEST_TOTAL_SHARDS:-}}" SHARD_INDEX="${RULES_RUST_TEST_SHARD_INDEX:-${TEST_SHARD_INDEX:-}}" -if [[ -n "${TOTAL_SHARDS}" && "${TOTAL_SHARDS}" != "0" ]]; then - run_sharded_libtest "$@" +if [[ -n "${CODEX_BAZEL_TEST_SKIP_FILTERS:-}" || ( -n "${TOTAL_SHARDS}" && "${TOTAL_SHARDS}" != "0" ) ]]; then + run_selected_libtest fi -exec "${test_bin}" "$@" +if [[ ${#libtest_args[@]} -gt 0 ]]; then + exec_test_bin "${libtest_args[@]}" +else + exec_test_bin +fi