mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
ci: test windows cross build (#25000)
We cross build when using bazel for windows. This causes a couple hiccups in that v8 does a mksnapshot step that is expecting to snapshot on the host arch which wasn't matching when we were doing the crossbuild. This was causing segfault failiures when starting up codemode from a cross built artifact. This changes things such that we cross build the library and then run and link a snapshot on the host machine/arch which is windows. This gives us a functional snapshot and library that can start code-mode on windows. This fixes the build and then fixes two test regressions we had.
This commit is contained in:
Channing Conger
committed by
GitHub
Unverified
GitHub
parent
72d0bfb6ba
commit
4be1a168fc
@@ -105,9 +105,9 @@ common:ci --disk_cache=
|
||||
# Shared config for the main Bazel CI workflow.
|
||||
common:ci-bazel --config=ci
|
||||
common:ci-bazel --build_metadata=TAG_workflow=bazel
|
||||
# Bazel CI cross-compiles in several legs, and the V8-backed code-mode tests
|
||||
# are not stable in that setup yet. Keep running the rest of the Rust
|
||||
# integration suites through the workspace-root launcher.
|
||||
# Keep code-mode integration cases out of ordinary Bazel legs. The
|
||||
# Windows-cross config below re-enables them after generating its Windows V8
|
||||
# snapshot on the Windows runner.
|
||||
common:ci-bazel --test_env=CODEX_BAZEL_TEST_SKIP_FILTERS=suite::code_mode::
|
||||
|
||||
# Shared config for Bazel-backed Rust linting.
|
||||
@@ -186,12 +186,16 @@ common:ci-windows-cross --config=ci-windows
|
||||
common:ci-windows-cross --build_metadata=TAG_windows_cross_compile=true
|
||||
common:ci-windows-cross --host_platform=//:rbe
|
||||
common:ci-windows-cross --strategy=TestRunner=local
|
||||
# V8 embeds IsolateData offsets in snapshot builtins; Windows snapshots must be
|
||||
# generated by a Windows mksnapshot binary rather than the Linux RBE host tool.
|
||||
common:ci-windows-cross --strategy=V8Mksnapshot=local
|
||||
common:ci-windows-cross --local_test_jobs=4
|
||||
common:ci-windows-cross --test_env=RUST_TEST_THREADS=1
|
||||
# Native Windows CI still covers the PowerShell tests. The cross-built gnullvm
|
||||
# binaries currently 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
|
||||
# run on the Windows runner. Keep V8-backed code-mode tests enabled except for
|
||||
# the hidden dynamic-tool callback test, which currently times out on Windows.
|
||||
common:ci-windows-cross --test_env=CODEX_BAZEL_TEST_SKIP_FILTERS=powershell,suite::code_mode::code_mode_can_call_hidden_dynamic_tools
|
||||
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
|
||||
|
||||
+12
-12
@@ -20,8 +20,9 @@ jobs:
|
||||
test:
|
||||
# PRs use the sharded Windows cross-compiled test jobs below. Post-merge
|
||||
# pushes to main also run the native Windows test job for broader Windows
|
||||
# signal without putting PR latency back on the critical path. Cargo CI
|
||||
# owns V8/code-mode test coverage for now.
|
||||
# signal without putting PR latency back on the critical path. When
|
||||
# authenticated RBE is available, the Windows-cross shards exercise the
|
||||
# source-built V8/code-mode targets.
|
||||
timeout-minutes: 30
|
||||
strategy:
|
||||
fail-fast: false
|
||||
@@ -91,9 +92,9 @@ jobs:
|
||||
# path. V8 consumers under `//codex-rs/...` still participate
|
||||
# transitively through `//...`.
|
||||
-//third_party/v8:all
|
||||
# V8-backed code-mode tests are covered by Cargo CI. Bazel CI
|
||||
# cross-compiles in several legs, and those tests are not stable in
|
||||
# that setup yet.
|
||||
# Keep V8-backed code-mode tests out of the ordinary macOS/Linux
|
||||
# legs; authenticated Windows-cross shards below exercise the
|
||||
# source-built gnullvm V8 path.
|
||||
-//codex-rs/code-mode:code-mode-unit-tests
|
||||
-//codex-rs/v8-poc:v8-poc-unit-tests
|
||||
)
|
||||
@@ -135,9 +136,9 @@ jobs:
|
||||
key: ${{ steps.prepare_bazel.outputs.repository-cache-key }}
|
||||
|
||||
test-windows-shard:
|
||||
# Split the Windows Bazel test leg across separate Windows
|
||||
# hosts. Each shard still uses Linux RBE for build actions, but the test
|
||||
# execution itself happens on its own Windows runner.
|
||||
# Split the Windows Bazel test leg across separate Windows hosts. Jobs with
|
||||
# BuildBuddy credentials use Linux RBE for build actions; test execution
|
||||
# remains on a Windows runner.
|
||||
timeout-minutes: 30
|
||||
strategy:
|
||||
fail-fast: false
|
||||
@@ -183,7 +184,7 @@ jobs:
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
bazel_test_query='tests(//...) except tests(//third_party/v8:all) except //codex-rs/code-mode:code-mode-unit-tests except //codex-rs/v8-poc:v8-poc-unit-tests except attr(tags, "manual", tests(//...))'
|
||||
bazel_test_query='tests(//...) except tests(//third_party/v8:all) except attr(tags, "manual", tests(//...))'
|
||||
mapfile -t bazel_targets < <(
|
||||
MSYS2_ARG_CONV_EXCL='*' bazel query --output=label "${bazel_test_query}" \
|
||||
| LC_ALL=C sort
|
||||
@@ -289,9 +290,8 @@ jobs:
|
||||
# path. V8 consumers under `//codex-rs/...` still participate
|
||||
# transitively through `//...`.
|
||||
-//third_party/v8:all
|
||||
# Keep this aligned with the main Bazel job. The native Windows
|
||||
# job preserves broad post-merge coverage, but code-mode/V8 tests
|
||||
# are covered by Cargo CI rather than Bazel for now.
|
||||
# Keep this job broad and cheap; authenticated Windows-cross jobs
|
||||
# add source-built V8-backed code-mode coverage.
|
||||
-//codex-rs/code-mode:code-mode-unit-tests
|
||||
-//codex-rs/v8-poc:v8-poc-unit-tests
|
||||
)
|
||||
|
||||
@@ -375,7 +375,11 @@ async fn code_mode_only_restricts_prompt_tools() -> Result<()> {
|
||||
let first_body = resp_mock.single_request().body_json();
|
||||
assert_eq!(
|
||||
tool_names(&first_body),
|
||||
vec!["exec".to_string(), "wait".to_string()]
|
||||
vec![
|
||||
"exec".to_string(),
|
||||
"wait".to_string(),
|
||||
"web_search".to_string()
|
||||
]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
@@ -457,7 +461,12 @@ if (!tool) {
|
||||
let first_body = resp_mock.single_request().body_json();
|
||||
assert_eq!(
|
||||
tool_names(&first_body),
|
||||
vec!["exec".to_string(), "wait".to_string()]
|
||||
vec![
|
||||
"exec".to_string(),
|
||||
"wait".to_string(),
|
||||
"web_search".to_string(),
|
||||
"image_generation".to_string()
|
||||
]
|
||||
);
|
||||
|
||||
let exec_description = first_body
|
||||
@@ -2898,6 +2907,7 @@ text(JSON.stringify(Object.getOwnPropertyNames(globalThis).sort()));
|
||||
"escape",
|
||||
"exit",
|
||||
"eval",
|
||||
"generatedImage",
|
||||
"globalThis",
|
||||
"image",
|
||||
"isFinite",
|
||||
@@ -2953,7 +2963,7 @@ text(JSON.stringify(tool));
|
||||
parsed,
|
||||
serde_json::json!({
|
||||
"name": "view_image",
|
||||
"description": "View a local image file from the filesystem when visual inspection is needed. Use this for images already available on disk.\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file\n path: string;\n}): Promise<{\n // Image detail hint returned by view_image. Returns `high` for default resized behavior or `original` when original resolution is preserved.\n detail: \"high\" | \"original\";\n // Data URL for the loaded image.\n image_url: string;\n}>; };\n```",
|
||||
"description": "View a local image file from the filesystem when visual inspection is needed. Use this for images already available on disk.\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file.\n path: string;\n}): Promise<{\n // Image detail hint returned by view_image. Returns `high` for default resized behavior or `original` when original resolution is preserved.\n detail: \"high\" | \"original\";\n // Data URL for the loaded image.\n image_url: string;\n}>; };\n```",
|
||||
})
|
||||
);
|
||||
|
||||
|
||||
@@ -106,6 +106,55 @@ index 9648e4a..88efd41 100644
|
||||
}) + select({
|
||||
":should_add_rdynamic": ["-rdynamic"],
|
||||
"//conditions:default": [],
|
||||
@@ -459,6 +488,11 @@
|
||||
def _mksnapshot(ctx):
|
||||
prefix = ctx.attr.prefix
|
||||
suffix = ctx.attr.suffix
|
||||
+ # Windows cross-builds use Linux for the exec configuration, but the
|
||||
+ # snapshot generator must match the target ABI and run on the Windows
|
||||
+ # runner. Action strategies only choose where an action runs; they cannot
|
||||
+ # change this executable from the exec to the target configuration.
|
||||
+ tool = ctx.executable.target_tool if ctx.attr.target_os == "win" else ctx.executable.tool
|
||||
outs = [
|
||||
ctx.actions.declare_file(prefix + "/snapshot" + suffix + ".cc"),
|
||||
ctx.actions.declare_file(prefix + "/embedded" + suffix + ".S"),
|
||||
@@ -476,7 +510,7 @@
|
||||
outs[1].path,
|
||||
] + ctx.attr.args,
|
||||
- executable = ctx.executable.tool,
|
||||
+ executable = tool,
|
||||
progress_message = "Running mksnapshot",
|
||||
)
|
||||
return [DefaultInfo(files = depset(outs))]
|
||||
@@ -491,6 +525,12 @@
|
||||
executable = True,
|
||||
cfg = "exec",
|
||||
),
|
||||
+ "target_tool": attr.label(
|
||||
+ mandatory = True,
|
||||
+ allow_files = True,
|
||||
+ executable = True,
|
||||
+ cfg = "target",
|
||||
+ ),
|
||||
"target_os": attr.string(mandatory = True),
|
||||
"prefix": attr.string(mandatory = True),
|
||||
"suffix": attr.string(mandatory = True),
|
||||
@@ -504,6 +544,7 @@
|
||||
args = args,
|
||||
prefix = "noicu",
|
||||
tool = ":noicu/mksnapshot" + suffix,
|
||||
+ target_tool = ":noicu/mksnapshot" + suffix,
|
||||
suffix = suffix,
|
||||
target_os = select({
|
||||
"@v8//bazel/config:is_macos": "mac",
|
||||
@@ -516,6 +557,7 @@
|
||||
args = args,
|
||||
prefix = "icu",
|
||||
tool = ":icu/mksnapshot" + suffix,
|
||||
+ target_tool = ":icu/mksnapshot" + suffix,
|
||||
suffix = suffix,
|
||||
target_os = select({
|
||||
"@v8//bazel/config:is_macos": "mac",
|
||||
diff --git a/orig/v8-14.6.202.11/BUILD.bazel b/mod/v8-14.6.202.11/BUILD.bazel
|
||||
index 421ebcd..52283ea 100644
|
||||
--- a/orig/v8-14.6.202.11/BUILD.bazel
|
||||
@@ -309,7 +358,7 @@ index 421ebcd..52283ea 100644
|
||||
],
|
||||
)
|
||||
|
||||
@@ -4772,9 +4784,15 @@ v8_binary(
|
||||
@@ -4772,9 +4784,20 @@ v8_binary(
|
||||
":icu/generated_torque_initializers",
|
||||
":icu/v8_initializers_files",
|
||||
],
|
||||
@@ -317,11 +366,16 @@ index 421ebcd..52283ea 100644
|
||||
+ # external-reference helpers together while producing the snapshot, the
|
||||
+ # final embedder binary may not fold the same pair and startup
|
||||
+ # deserialization will reject the snapshot.
|
||||
+ # GN also increases the x64 Windows stack reserve because constructing the
|
||||
+ # snapshot overflows the linker's 1 MiB default.
|
||||
linkopts = select({
|
||||
"@v8//bazel/config:is_android": ["-llog"],
|
||||
- "//conditions:default": [],
|
||||
+ "@v8//bazel/config:is_macos": ["-Wl,-no_deduplicate"],
|
||||
+ "@v8//bazel/config:is_windows": ["/OPT:NOICF"],
|
||||
+ "@v8//bazel/config:is_windows": [
|
||||
+ "-Wl,/OPT:NOICF",
|
||||
+ "-Wl,/STACK:2097152",
|
||||
+ ],
|
||||
+ "//conditions:default": ["-Wl,--icf=none"],
|
||||
}),
|
||||
noicu_deps = [":v8_libshared_noicu"],
|
||||
|
||||
Reference in New Issue
Block a user