From 5037a2d19924f2e49490453ab2a913da938afbe5 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 27 Mar 2026 19:42:30 -0700 Subject: [PATCH 1/2] refactor: rewrite argument-comment lint wrappers in Python (#16063) ## Why The `argument-comment-lint` entrypoints had grown into two shell wrappers with duplicated parsing, environment setup, and Cargo forwarding logic. The recent `--` separator regression was a good example of the problem: the behavior was subtle, easy to break, and hard to verify. This change rewrites those wrappers in Python so the control flow is easier to follow, the shared behavior lives in one place, and the tricky argument/defaulting paths have direct test coverage. ## What changed - replaced `tools/argument-comment-lint/run.sh` and `tools/argument-comment-lint/run-prebuilt-linter.sh` with Python entrypoints: `run.py` and `run-prebuilt-linter.py` - moved shared wrapper behavior into `tools/argument-comment-lint/wrapper_common.py`, including: - splitting lint args from forwarded Cargo args after `--` - defaulting repo runs to `--manifest-path codex-rs/Cargo.toml --workspace --no-deps` - defaulting non-`--fix` runs to `--all-targets` unless the caller explicitly narrows the target set - setting repo defaults for `DYLINT_RUSTFLAGS` and `CARGO_INCREMENTAL` - kept the prebuilt wrapper thin: it still just resolves the packaged DotSlash entrypoint, keeps `rustup` shims first on `PATH`, infers `RUSTUP_HOME` when needed, and then launches the packaged `cargo-dylint` path - updated `justfile`, `rust-ci.yml`, and `tools/argument-comment-lint/README.md` to use the Python entrypoints - updated `rust-ci` so the package job runs Python syntax checks plus the new wrapper unit tests, and the OS-specific lint jobs invoke the wrappers through an explicit Python interpreter This is a follow-up to #16054: it keeps the current lint semantics while making the wrapper logic maintainable enough to iterate on safely. ## Validation - `python3 -m py_compile tools/argument-comment-lint/wrapper_common.py tools/argument-comment-lint/run.py tools/argument-comment-lint/run-prebuilt-linter.py tools/argument-comment-lint/test_wrapper_common.py` - `python3 -m unittest discover -s tools/argument-comment-lint -p 'test_*.py'` - `python3 ./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-terminal-detection -- --lib` - `python3 ./tools/argument-comment-lint/run.py -p codex-terminal-detection -- --lib` --- .github/workflows/rust-ci.yml | 16 +- justfile | 4 +- tools/argument-comment-lint/README.md | 18 +- .../run-prebuilt-linter.py | 45 +++ .../run-prebuilt-linter.sh | 202 ------------- tools/argument-comment-lint/run.py | 35 +++ tools/argument-comment-lint/run.sh | 161 ----------- .../test_wrapper_common.py | 88 ++++++ tools/argument-comment-lint/wrapper_common.py | 271 ++++++++++++++++++ 9 files changed, 461 insertions(+), 379 deletions(-) create mode 100755 tools/argument-comment-lint/run-prebuilt-linter.py delete mode 100755 tools/argument-comment-lint/run-prebuilt-linter.sh create mode 100755 tools/argument-comment-lint/run.py delete mode 100755 tools/argument-comment-lint/run.sh create mode 100644 tools/argument-comment-lint/test_wrapper_common.py create mode 100644 tools/argument-comment-lint/wrapper_common.py diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index ee2db79083..6c93a27006 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -116,8 +116,10 @@ jobs: - name: Install cargo-dylint tooling if: ${{ steps.cargo_dylint_cache.outputs.cache-hit != 'true' }} run: cargo install --locked cargo-dylint dylint-link - - name: Check source wrapper syntax - run: bash -n tools/argument-comment-lint/run.sh + - name: Check Python wrapper syntax + run: python3 -m py_compile tools/argument-comment-lint/wrapper_common.py tools/argument-comment-lint/run.py tools/argument-comment-lint/run-prebuilt-linter.py tools/argument-comment-lint/test_wrapper_common.py + - name: Test Python wrapper helpers + run: python3 -m unittest discover -s tools/argument-comment-lint -p 'test_*.py' - name: Test argument comment lint package working-directory: tools/argument-comment-lint run: cargo test @@ -156,11 +158,15 @@ jobs: - name: Run argument comment lint on codex-rs if: ${{ runner.os == 'macOS' }} shell: bash - run: ./tools/argument-comment-lint/run-prebuilt-linter.sh + run: python3 ./tools/argument-comment-lint/run-prebuilt-linter.py - name: Run argument comment lint on codex-rs (default targets only) - if: ${{ runner.os != 'macOS' }} + if: ${{ runner.os == 'Linux' }} shell: bash - run: ./tools/argument-comment-lint/run-prebuilt-linter.sh -- --lib --bins + run: python3 ./tools/argument-comment-lint/run-prebuilt-linter.py -- --lib --bins + - name: Run argument comment lint on codex-rs (default targets only) + if: ${{ runner.os == 'Windows' }} + shell: bash + run: python ./tools/argument-comment-lint/run-prebuilt-linter.py -- --lib --bins # --- CI to validate on different os/targets -------------------------------- lint_build: diff --git a/justfile b/justfile index 87ef6bb280..39a65df46d 100644 --- a/justfile +++ b/justfile @@ -97,11 +97,11 @@ write-hooks-schema: # Run the argument-comment Dylint checks across codex-rs. [no-cd] argument-comment-lint *args: - ./tools/argument-comment-lint/run-prebuilt-linter.sh "$@" + ./tools/argument-comment-lint/run-prebuilt-linter.py "$@" [no-cd] argument-comment-lint-from-source *args: - ./tools/argument-comment-lint/run.sh "$@" + ./tools/argument-comment-lint/run.py "$@" # Tail logs from the state SQLite database log *args: diff --git a/tools/argument-comment-lint/README.md b/tools/argument-comment-lint/README.md index 5e145aa39c..92b0739601 100644 --- a/tools/argument-comment-lint/README.md +++ b/tools/argument-comment-lint/README.md @@ -84,9 +84,9 @@ rustup toolchain install nightly-2025-09-18 \ ``` The checked-in DotSlash file lives at `tools/argument-comment-lint/argument-comment-lint`. -`run-prebuilt-linter.sh` resolves that file via `dotslash` and is the path used by +`run-prebuilt-linter.py` resolves that file via `dotslash` and is the path used by `just clippy`, `just argument-comment-lint`, and the Rust CI job. The -source-build path remains available in `run.sh` for people +source-build path remains available in `run.py` for people iterating on the lint crate itself. The Unix archive layout is: @@ -110,7 +110,7 @@ host-qualified nightly filename to the plain `nightly-2025-09-18` channel when needed, and then invokes `cargo-dylint dylint --lib-path ` with the repo's default `DYLINT_RUSTFLAGS` and `CARGO_INCREMENTAL=0` settings. -The checked-in `run-prebuilt-linter.sh` wrapper uses the fetched package +The checked-in `run-prebuilt-linter.py` wrapper uses the fetched package contents directly so the current checked-in alpha artifact works the same way. It also makes sure the `rustup` shims stay ahead of any direct toolchain `cargo` binary on `PATH`, and sets `RUSTUP_HOME` from `rustup show home` when @@ -120,17 +120,17 @@ required for the current Windows Dylint driver path. If you are changing the lint crate itself, use the source-build wrapper: ```bash -./tools/argument-comment-lint/run.sh -p codex-core +./tools/argument-comment-lint/run.py -p codex-core ``` Run the lint against `codex-rs` from the repo root: ```bash -./tools/argument-comment-lint/run-prebuilt-linter.sh -p codex-core +./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-core just argument-comment-lint -p codex-core ``` -If no package selection is provided, `run-prebuilt-linter.sh` defaults to checking the +If no package selection is provided, `run-prebuilt-linter.py` defaults to checking the `codex-rs` workspace with `--workspace --no-deps`. For non-`--fix` runs, both wrappers also default the underlying Cargo invocation to `--all-targets` unless you explicitly narrow the target set, so @@ -140,7 +140,7 @@ Repo runs also promote `uncommented_anonymous_literal_argument` to an error by default: ```bash -./tools/argument-comment-lint/run-prebuilt-linter.sh -p codex-core +./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-core ``` The wrapper does that by setting `DYLINT_RUSTFLAGS`, and it leaves an explicit @@ -152,11 +152,11 @@ hoc run: ```bash DYLINT_RUSTFLAGS="-A uncommented-anonymous-literal-argument" \ CARGO_INCREMENTAL=1 \ - ./tools/argument-comment-lint/run.sh -p codex-core + ./tools/argument-comment-lint/run.py -p codex-core ``` To override an explicitly narrow target selection, or to be explicit in scripts: ```bash -./tools/argument-comment-lint/run-prebuilt-linter.sh -p codex-core -- --all-targets +./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-core -- --all-targets ``` diff --git a/tools/argument-comment-lint/run-prebuilt-linter.py b/tools/argument-comment-lint/run-prebuilt-linter.py new file mode 100755 index 0000000000..6f1e4c8854 --- /dev/null +++ b/tools/argument-comment-lint/run-prebuilt-linter.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python3 + +from __future__ import annotations + +import os +import sys + +from wrapper_common import ( + build_final_args, + exec_command, + fetch_packaged_entrypoint, + find_packaged_cargo_dylint, + normalize_packaged_library, + parse_wrapper_args, + prefer_rustup_shims, + repo_root, + set_default_lint_env, +) + + +def main() -> "Never": + root = repo_root() + parsed = parse_wrapper_args(sys.argv[1:]) + final_args = build_final_args(parsed, root / "codex-rs" / "Cargo.toml") + + env = os.environ.copy() + prefer_rustup_shims(env) + set_default_lint_env(env) + + package_entrypoint = fetch_packaged_entrypoint( + root / "tools" / "argument-comment-lint" / "argument-comment-lint", + env, + ) + cargo_dylint = find_packaged_cargo_dylint(package_entrypoint) + library_path = normalize_packaged_library(package_entrypoint) + + command = [str(cargo_dylint), "dylint", "--lib-path", str(library_path)] + if not parsed.has_library_selection: + command.append("--all") + command.extend(final_args) + exec_command(command, env) + + +if __name__ == "__main__": + main() diff --git a/tools/argument-comment-lint/run-prebuilt-linter.sh b/tools/argument-comment-lint/run-prebuilt-linter.sh deleted file mode 100755 index bc095ed555..0000000000 --- a/tools/argument-comment-lint/run-prebuilt-linter.sh +++ /dev/null @@ -1,202 +0,0 @@ -#!/usr/bin/env bash - -set -euo pipefail - -repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" -manifest_path="$repo_root/codex-rs/Cargo.toml" -dotslash_manifest="$repo_root/tools/argument-comment-lint/argument-comment-lint" - -has_manifest_path=false -has_package_selection=false -has_library_selection=false -has_no_deps=false -has_cargo_target_selection=false -has_fix=false -after_separator=false -expect_value="" -lint_args=() -cargo_args=() - -for arg in "$@"; do - if [[ "$after_separator" == true ]]; then - cargo_args+=("$arg") - case "$arg" in - --all-targets|--lib|--bins|--tests|--examples|--benches|--doc) - has_cargo_target_selection=true - ;; - --bin|--test|--example|--bench) - has_cargo_target_selection=true - ;; - --bin=*|--test=*|--example=*|--bench=*) - has_cargo_target_selection=true - ;; - esac - continue - fi - - case "$arg" in - --) - after_separator=true - continue - ;; - esac - - lint_args+=("$arg") - - if [[ -n "$expect_value" ]]; then - case "$expect_value" in - manifest_path) - has_manifest_path=true - ;; - package_selection) - has_package_selection=true - ;; - library_selection) - has_library_selection=true - ;; - esac - expect_value="" - continue - fi - - case "$arg" in - --manifest-path) - expect_value="manifest_path" - ;; - --manifest-path=*) - has_manifest_path=true - ;; - -p|--package) - expect_value="package_selection" - ;; - --package=*) - has_package_selection=true - ;; - --fix) - has_fix=true - ;; - --lib|--lib-path) - expect_value="library_selection" - ;; - --lib=*|--lib-path=*) - has_library_selection=true - ;; - --workspace) - has_package_selection=true - ;; - --no-deps) - has_no_deps=true - ;; - esac -done - -final_args=() -if [[ "$has_manifest_path" == false ]]; then - final_args+=(--manifest-path "$manifest_path") -fi -if [[ "$has_package_selection" == false ]]; then - final_args+=(--workspace) -fi -if [[ "$has_no_deps" == false ]]; then - final_args+=(--no-deps) -fi -if [[ "$has_fix" == false && "$has_cargo_target_selection" == false ]]; then - cargo_args+=(--all-targets) -fi -if [[ ${#lint_args[@]} -gt 0 ]]; then - final_args+=("${lint_args[@]}") -fi -if [[ ${#cargo_args[@]} -gt 0 ]]; then - final_args+=(-- "${cargo_args[@]}") -fi - -if ! command -v dotslash >/dev/null 2>&1; then - cat >&2 </dev/null 2>&1; then - rustup_bin_dir="$(dirname "$(command -v rustup)")" - path_entries=() - while IFS= read -r entry; do - [[ -n "$entry" && "$entry" != "$rustup_bin_dir" ]] && path_entries+=("$entry") - done < <(printf '%s\n' "${PATH//:/$'\n'}") - PATH="$rustup_bin_dir" - if ((${#path_entries[@]} > 0)); then - PATH+=":$(IFS=:; echo "${path_entries[*]}")" - fi - export PATH - - if [[ -z "${RUSTUP_HOME:-}" ]]; then - rustup_home="$(rustup show home 2>/dev/null || true)" - if [[ -n "$rustup_home" ]]; then - export RUSTUP_HOME="$rustup_home" - fi - fi -fi - -package_entrypoint="$(dotslash -- fetch "$dotslash_manifest")" -bin_dir="$(cd "$(dirname "$package_entrypoint")" && pwd)" -package_root="$(cd "$bin_dir/.." && pwd)" -library_dir="$package_root/lib" - -cargo_dylint="$bin_dir/cargo-dylint" -if [[ ! -x "$cargo_dylint" ]]; then - cargo_dylint="$bin_dir/cargo-dylint.exe" -fi -if [[ ! -x "$cargo_dylint" ]]; then - echo "bundled cargo-dylint executable not found under $bin_dir" >&2 - exit 1 -fi - -shopt -s nullglob -libraries=("$library_dir"/*@*) -shopt -u nullglob -if [[ ${#libraries[@]} -eq 0 ]]; then - echo "no packaged Dylint library found in $library_dir" >&2 - exit 1 -fi -if [[ ${#libraries[@]} -ne 1 ]]; then - echo "expected exactly one packaged Dylint library in $library_dir" >&2 - exit 1 -fi - -library_path="${libraries[0]}" -library_filename="$(basename "$library_path")" -normalized_library_path="$library_path" -library_ext=".${library_filename##*.}" -library_stem="${library_filename%.*}" -if [[ "$library_stem" =~ ^(.+@nightly-[0-9]{4}-[0-9]{2}-[0-9]{2})-.+$ ]]; then - normalized_library_filename="${BASH_REMATCH[1]}$library_ext" - temp_dir="$(mktemp -d "${TMPDIR:-/tmp}/argument-comment-lint.XXXXXX")" - normalized_library_path="$temp_dir/$normalized_library_filename" - cp "$library_path" "$normalized_library_path" -fi - -if [[ -n "${DYLINT_RUSTFLAGS:-}" ]]; then - if [[ "$DYLINT_RUSTFLAGS" != *"-D uncommented-anonymous-literal-argument"* ]]; then - DYLINT_RUSTFLAGS+=" -D uncommented-anonymous-literal-argument" - fi - if [[ "$DYLINT_RUSTFLAGS" != *"-A unknown_lints"* ]]; then - DYLINT_RUSTFLAGS+=" -A unknown_lints" - fi -else - DYLINT_RUSTFLAGS="-D uncommented-anonymous-literal-argument -A unknown_lints" -fi -export DYLINT_RUSTFLAGS - -if [[ -z "${CARGO_INCREMENTAL:-}" ]]; then - export CARGO_INCREMENTAL=0 -fi - -command=("$cargo_dylint" dylint --lib-path "$normalized_library_path") -if [[ "$has_library_selection" == false ]]; then - command+=(--all) -fi -command+=("${final_args[@]}") - -exec "${command[@]}" diff --git a/tools/argument-comment-lint/run.py b/tools/argument-comment-lint/run.py new file mode 100755 index 0000000000..b4be29a26d --- /dev/null +++ b/tools/argument-comment-lint/run.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 + +from __future__ import annotations + +import os +import sys + +from wrapper_common import ( + build_final_args, + ensure_source_prerequisites, + exec_command, + parse_wrapper_args, + repo_root, + set_default_lint_env, +) + + +def main() -> "Never": + root = repo_root() + parsed = parse_wrapper_args(sys.argv[1:]) + final_args = build_final_args(parsed, root / "codex-rs" / "Cargo.toml") + + env = os.environ.copy() + ensure_source_prerequisites(env) + set_default_lint_env(env) + + command = ["cargo", "dylint", "--path", str(root / "tools" / "argument-comment-lint")] + if not parsed.has_library_selection: + command.append("--all") + command.extend(final_args) + exec_command(command, env) + + +if __name__ == "__main__": + main() diff --git a/tools/argument-comment-lint/run.sh b/tools/argument-comment-lint/run.sh deleted file mode 100755 index 882e4d90c0..0000000000 --- a/tools/argument-comment-lint/run.sh +++ /dev/null @@ -1,161 +0,0 @@ -#!/usr/bin/env bash - -set -euo pipefail - -repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" -lint_path="$repo_root/tools/argument-comment-lint" -manifest_path="$repo_root/codex-rs/Cargo.toml" -toolchain_channel="nightly-2025-09-18" -strict_lint="uncommented-anonymous-literal-argument" -noise_lint="unknown_lints" - -has_manifest_path=false -has_package_selection=false -has_no_deps=false -has_library_selection=false -has_cargo_target_selection=false -has_fix=false -after_separator=false -expect_value="" -lint_args=() -cargo_args=() - -ensure_local_prerequisites() { - if ! command -v cargo-dylint >/dev/null 2>&1 || ! command -v dylint-link >/dev/null 2>&1; then - cat >&2 <&2 < None: + parsed = wrapper_common.parse_wrapper_args([]) + final_args = wrapper_common.build_final_args(parsed, Path("/repo/codex-rs/Cargo.toml")) + + self.assertEqual( + final_args, + [ + "--manifest-path", + "/repo/codex-rs/Cargo.toml", + "--workspace", + "--no-deps", + "--", + "--all-targets", + ], + ) + + def test_forwarded_cargo_args_keep_single_separator(self) -> None: + parsed = wrapper_common.parse_wrapper_args(["-p", "codex-core", "--", "--tests"]) + final_args = wrapper_common.build_final_args(parsed, Path("/repo/codex-rs/Cargo.toml")) + + self.assertEqual( + final_args, + [ + "--manifest-path", + "/repo/codex-rs/Cargo.toml", + "--no-deps", + "-p", + "codex-core", + "--", + "--tests", + ], + ) + + def test_fix_does_not_add_all_targets(self) -> None: + parsed = wrapper_common.parse_wrapper_args(["--fix", "-p", "codex-core"]) + final_args = wrapper_common.build_final_args(parsed, Path("/repo/codex-rs/Cargo.toml")) + + self.assertEqual( + final_args, + [ + "--manifest-path", + "/repo/codex-rs/Cargo.toml", + "--no-deps", + "--fix", + "-p", + "codex-core", + ], + ) + + def test_explicit_manifest_and_workspace_are_preserved(self) -> None: + parsed = wrapper_common.parse_wrapper_args( + [ + "--manifest-path", + "/tmp/custom/Cargo.toml", + "--workspace", + "--no-deps", + "--", + "--bins", + ] + ) + final_args = wrapper_common.build_final_args(parsed, Path("/repo/codex-rs/Cargo.toml")) + + self.assertEqual( + final_args, + [ + "--manifest-path", + "/tmp/custom/Cargo.toml", + "--workspace", + "--no-deps", + "--", + "--bins", + ], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/argument-comment-lint/wrapper_common.py b/tools/argument-comment-lint/wrapper_common.py new file mode 100644 index 0000000000..3202c22381 --- /dev/null +++ b/tools/argument-comment-lint/wrapper_common.py @@ -0,0 +1,271 @@ +#!/usr/bin/env python3 + +from __future__ import annotations + +from dataclasses import dataclass +import os +from pathlib import Path +import re +import shlex +import shutil +import subprocess +import sys +import tempfile +from typing import MutableMapping, Sequence + +STRICT_LINT = "uncommented-anonymous-literal-argument" +NOISE_LINT = "unknown_lints" +TOOLCHAIN_CHANNEL = "nightly-2025-09-18" + +_TARGET_SELECTION_ARGS = { + "--all-targets", + "--lib", + "--bins", + "--tests", + "--examples", + "--benches", + "--doc", +} +_TARGET_SELECTION_PREFIXES = ("--bin=", "--test=", "--example=", "--bench=") +_TARGET_SELECTION_WITH_VALUE = {"--bin", "--test", "--example", "--bench"} +_NIGHTLY_LIBRARY_PATTERN = re.compile( + r"^(.+@nightly-[0-9]{4}-[0-9]{2}-[0-9]{2})-.+$" +) + + +@dataclass +class ParsedWrapperArgs: + lint_args: list[str] + cargo_args: list[str] + has_manifest_path: bool = False + has_package_selection: bool = False + has_no_deps: bool = False + has_library_selection: bool = False + has_cargo_target_selection: bool = False + has_fix: bool = False + + +def repo_root() -> Path: + return Path(__file__).resolve().parents[2] + + +def parse_wrapper_args(argv: Sequence[str]) -> ParsedWrapperArgs: + parsed = ParsedWrapperArgs(lint_args=[], cargo_args=[]) + after_separator = False + expect_value: str | None = None + + for arg in argv: + if after_separator: + parsed.cargo_args.append(arg) + if arg in _TARGET_SELECTION_ARGS or arg in _TARGET_SELECTION_WITH_VALUE: + parsed.has_cargo_target_selection = True + elif arg.startswith(_TARGET_SELECTION_PREFIXES): + parsed.has_cargo_target_selection = True + continue + + if arg == "--": + after_separator = True + continue + + parsed.lint_args.append(arg) + + if expect_value is not None: + if expect_value == "manifest_path": + parsed.has_manifest_path = True + elif expect_value == "package_selection": + parsed.has_package_selection = True + elif expect_value == "library_selection": + parsed.has_library_selection = True + expect_value = None + continue + + if arg == "--manifest-path": + expect_value = "manifest_path" + elif arg.startswith("--manifest-path="): + parsed.has_manifest_path = True + elif arg in {"-p", "--package"}: + expect_value = "package_selection" + elif arg.startswith("--package="): + parsed.has_package_selection = True + elif arg == "--fix": + parsed.has_fix = True + elif arg == "--workspace": + parsed.has_package_selection = True + elif arg == "--no-deps": + parsed.has_no_deps = True + elif arg in {"--lib", "--lib-path"}: + expect_value = "library_selection" + elif arg.startswith("--lib=") or arg.startswith("--lib-path="): + parsed.has_library_selection = True + + return parsed + + +def build_final_args(parsed: ParsedWrapperArgs, manifest_path: Path) -> list[str]: + final_args: list[str] = [] + cargo_args = list(parsed.cargo_args) + + if not parsed.has_manifest_path: + final_args.extend(["--manifest-path", str(manifest_path)]) + if not parsed.has_package_selection: + final_args.append("--workspace") + if not parsed.has_no_deps: + final_args.append("--no-deps") + if not parsed.has_fix and not parsed.has_cargo_target_selection: + cargo_args.append("--all-targets") + final_args.extend(parsed.lint_args) + if cargo_args: + final_args.extend(["--", *cargo_args]) + return final_args + + +def append_env_flag(env: MutableMapping[str, str], key: str, flag: str) -> None: + value = env.get(key) + if value is None or value == "": + env[key] = flag + return + if flag not in value: + env[key] = f"{value} {flag}" + + +def set_default_lint_env(env: MutableMapping[str, str]) -> None: + append_env_flag(env, "DYLINT_RUSTFLAGS", f"-D {STRICT_LINT}") + append_env_flag(env, "DYLINT_RUSTFLAGS", f"-A {NOISE_LINT}") + if not env.get("CARGO_INCREMENTAL"): + env["CARGO_INCREMENTAL"] = "0" + + +def die(message: str) -> "Never": + print(message, file=sys.stderr) + raise SystemExit(1) + + +def require_command(name: str, install_message: str | None = None) -> str: + executable = shutil.which(name) + if executable is None: + if install_message is None: + die(f"{name} is required but was not found on PATH.") + die(install_message) + return executable + + +def run_capture(args: Sequence[str], env: MutableMapping[str, str] | None = None) -> str: + try: + completed = subprocess.run( + list(args), + capture_output=True, + check=True, + env=None if env is None else dict(env), + text=True, + ) + except subprocess.CalledProcessError as error: + command = shlex.join(str(part) for part in error.cmd) + stderr = error.stderr.strip() + stdout = error.stdout.strip() + output = stderr or stdout + if output: + die(f"{command} failed:\n{output}") + die(f"{command} failed with exit code {error.returncode}") + return completed.stdout.strip() + + +def ensure_source_prerequisites(env: MutableMapping[str, str]) -> None: + require_command( + "cargo-dylint", + "argument-comment-lint source wrapper requires cargo-dylint and dylint-link.\n" + "Install them with:\n" + " cargo install --locked cargo-dylint dylint-link", + ) + require_command( + "dylint-link", + "argument-comment-lint source wrapper requires cargo-dylint and dylint-link.\n" + "Install them with:\n" + " cargo install --locked cargo-dylint dylint-link", + ) + require_command( + "rustup", + "argument-comment-lint source wrapper requires rustup.\n" + f"Install the {TOOLCHAIN_CHANNEL} toolchain with:\n" + f" rustup toolchain install {TOOLCHAIN_CHANNEL} \\\n" + " --component llvm-tools-preview \\\n" + " --component rustc-dev \\\n" + " --component rust-src", + ) + toolchains = run_capture(["rustup", "toolchain", "list"], env=env) + if not any(line.startswith(TOOLCHAIN_CHANNEL) for line in toolchains.splitlines()): + die( + "argument-comment-lint source wrapper requires the " + f"{TOOLCHAIN_CHANNEL} toolchain with rustc-dev support.\n" + "Install it with:\n" + f" rustup toolchain install {TOOLCHAIN_CHANNEL} \\\n" + " --component llvm-tools-preview \\\n" + " --component rustc-dev \\\n" + " --component rust-src" + ) + + +def prefer_rustup_shims(env: MutableMapping[str, str]) -> None: + rustup = shutil.which("rustup", path=env.get("PATH")) + if rustup is None: + return + + rustup_bin_dir = str(Path(rustup).resolve().parent) + path_entries = [ + entry + for entry in env.get("PATH", "").split(os.pathsep) + if entry and entry != rustup_bin_dir + ] + env["PATH"] = os.pathsep.join([rustup_bin_dir, *path_entries]) + + if not env.get("RUSTUP_HOME"): + rustup_home = run_capture(["rustup", "show", "home"], env=env) + if rustup_home: + env["RUSTUP_HOME"] = rustup_home + + +def fetch_packaged_entrypoint(dotslash_manifest: Path, env: MutableMapping[str, str]) -> Path: + require_command( + "dotslash", + "argument-comment-lint prebuilt wrapper requires dotslash.\n" + "Install dotslash, or use:\n" + " ./tools/argument-comment-lint/run.py ...", + ) + entrypoint = run_capture(["dotslash", "--", "fetch", str(dotslash_manifest)], env=env) + return Path(entrypoint).resolve() + + +def find_packaged_cargo_dylint(package_entrypoint: Path) -> Path: + bin_dir = package_entrypoint.parent + cargo_dylint = bin_dir / "cargo-dylint" + if not cargo_dylint.is_file(): + cargo_dylint = bin_dir / "cargo-dylint.exe" + if not cargo_dylint.is_file(): + die(f"bundled cargo-dylint executable not found under {bin_dir}") + return cargo_dylint + + +def normalize_packaged_library(package_entrypoint: Path) -> Path: + library_dir = package_entrypoint.parent.parent / "lib" + libraries = sorted(path for path in library_dir.glob("*@*") if path.is_file()) + if not libraries: + die(f"no packaged Dylint library found in {library_dir}") + if len(libraries) != 1: + die(f"expected exactly one packaged Dylint library in {library_dir}") + + library_path = libraries[0] + match = _NIGHTLY_LIBRARY_PATTERN.match(library_path.stem) + if match is None: + return library_path + + temp_dir = Path(tempfile.mkdtemp(prefix="argument-comment-lint.")) + normalized_library_path = temp_dir / f"{match.group(1)}{library_path.suffix}" + shutil.copy2(library_path, normalized_library_path) + return normalized_library_path + + +def exec_command(command: Sequence[str], env: MutableMapping[str, str]) -> "Never": + try: + completed = subprocess.run(list(command), env=dict(env), check=False) + except FileNotFoundError: + die(f"{command[0]} is required but was not found on PATH.") + raise SystemExit(completed.returncode) From a55041425f1d500e8b9ab1cd63760bd36d6904d4 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 27 Mar 2026 19:50:18 -0700 Subject: [PATCH 2/2] ci: enable the Windows Bazel workflow path --- .bazelrc | 3 - .github/actions/setup-bazel-ci/action.yml | 10 +- .github/workflows/bazel.yml | 20 +- MODULE.bazel | 44 +++ MODULE.bazel.lock | 8 + defs.bzl | 16 +- patches/BUILD.bazel | 4 + ...seil_windows_gnullvm_thread_identity.patch | 16 + patches/llvm_windows_symlink_extract.patch | 64 ++++ patches/rules_rs_windows_gnullvm_exec.patch | 151 +++++++++ ...es_rust_windows_gnullvm_build_script.patch | 38 +++ patches/rusty_v8_prebuilt_out_dir.patch | 22 ++ patches/v8_bazel_rules.patch | 60 +++- patches/v8_module_deps.patch | 21 +- patches/v8_source_portability.patch | 295 ++++++++++++++++-- third_party/v8/BUILD.bazel | 29 +- workspace_root_test_launcher.bat.tpl | 16 +- 17 files changed, 761 insertions(+), 56 deletions(-) create mode 100644 patches/abseil_windows_gnullvm_thread_identity.patch create mode 100644 patches/llvm_windows_symlink_extract.patch create mode 100644 patches/rules_rs_windows_gnullvm_exec.patch create mode 100644 patches/rules_rust_windows_gnullvm_build_script.patch diff --git a/.bazelrc b/.bazelrc index 331f8634d2..340871831a 100644 --- a/.bazelrc +++ b/.bazelrc @@ -20,9 +20,6 @@ common:windows --host_platform=//:local_windows common --@rules_cc//cc/toolchains/args/archiver_flags:use_libtool_on_macos=False common --@llvm//config:experimental_stub_libgcc_s -# We need to use the sh toolchain on windows so we don't send host bash paths to the linux executor. -common:windows --@rules_rust//rust/settings:experimental_use_sh_toolchain_for_bootstrap_process_wrapper - # TODO(zbarsky): rules_rust doesn't implement this flag properly with remote exec... # common --@rules_rust//rust/settings:pipelined_compilation diff --git a/.github/actions/setup-bazel-ci/action.yml b/.github/actions/setup-bazel-ci/action.yml index 0a6b3aa241..0775f928a5 100644 --- a/.github/actions/setup-bazel-ci/action.yml +++ b/.github/actions/setup-bazel-ci/action.yml @@ -57,5 +57,11 @@ runs: if: runner.os == 'Windows' shell: pwsh run: | - # Use a very short path to reduce argv/path length issues. - "BAZEL_OUTPUT_USER_ROOT=C:\" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append + # Use a very short path to reduce argv/path length issues, but avoid the + # drive root because some Windows test launchers mis-handle MANIFEST paths there. + "BAZEL_OUTPUT_USER_ROOT=D:\b" | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append + + - name: Enable Git long paths (Windows) + if: runner.os == 'Windows' + shell: pwsh + run: git config --global core.longpaths true diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index 501b7076a2..f10e94d084 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -17,6 +17,7 @@ concurrency: cancel-in-progress: ${{ github.ref_name != 'main' }} jobs: test: + timeout-minutes: 120 strategy: fail-fast: false matrix: @@ -39,9 +40,9 @@ jobs: # - os: ubuntu-24.04-arm # target: aarch64-unknown-linux-gnu - # TODO: Enable Windows once we fix the toolchain issues there. - #- os: windows-latest - # target: x86_64-pc-windows-gnullvm + # Windows + - os: windows-latest + target: x86_64-pc-windows-gnullvm runs-on: ${{ matrix.os }} # Configure a human readable name for each job @@ -67,8 +68,14 @@ jobs: BUILDBUDDY_API_KEY: ${{ secrets.BUILDBUDDY_API_KEY }} shell: bash run: | - # Keep V8 out of the ordinary Bazel CI path. Only the dedicated - # canary and release workflows should build `third_party/v8`. + 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 + ) + ./.github/scripts/run-bazel-ci.sh \ --print-failed-test-logs \ --use-node-test-env \ @@ -77,8 +84,7 @@ jobs: --test_verbose_timeout_warnings \ --build_metadata=COMMIT_SHA=${GITHUB_SHA} \ -- \ - //... \ - -//third_party/v8:all + "${bazel_targets[@]}" # Save bazel repository cache explicitly; make non-fatal so cache uploading # never fails the overall job. Only save when key wasn't hit. diff --git a/MODULE.bazel b/MODULE.bazel index 8564db701a..63a86bf830 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -3,6 +3,25 @@ module(name = "codex") bazel_dep(name = "bazel_skylib", version = "1.8.2") bazel_dep(name = "platforms", version = "1.0.0") bazel_dep(name = "llvm", version = "0.6.8") +# The upstream LLVM archive contains a few unix-only symlink entries and is +# missing a couple of MinGW compatibility archives that windows-gnullvm needs +# during extraction and linking, so patch it until upstream grows native support. +single_version_override( + module_name = "llvm", + patch_strip = 1, + patches = [ + "//patches:llvm_windows_symlink_extract.patch", + ], +) +# Abseil picks a MinGW pthread TLS path that does not match our hermetic +# windows-gnullvm toolchain; force it onto the portable C++11 thread-local path. +single_version_override( + module_name = "abseil-cpp", + patch_strip = 1, + patches = [ + "//patches:abseil_windows_gnullvm_thread_identity.patch", + ], +) register_toolchains("@llvm//toolchain:all") @@ -44,8 +63,28 @@ bazel_dep(name = "apple_support", version = "2.1.0") bazel_dep(name = "rules_cc", version = "0.2.16") bazel_dep(name = "rules_platform", version = "0.1.0") bazel_dep(name = "rules_rs", version = "0.0.43") +# `rules_rs` 0.0.43 does not model `windows-gnullvm` as a distinct Windows exec +# platform, so patch it until upstream grows that support for both x86_64 and +# aarch64. +single_version_override( + module_name = "rules_rs", + patch_strip = 1, + patches = [ + "//patches:rules_rs_windows_gnullvm_exec.patch", + ], + version = "0.0.43", +) rules_rust = use_extension("@rules_rs//rs/experimental:rules_rust.bzl", "rules_rust") +# Build-script probe binaries inherit CFLAGS/CXXFLAGS from Bazel's C++ +# toolchain. On `windows-gnullvm`, llvm-mingw does not ship +# `libssp_nonshared`, so strip the forwarded stack-protector flags there. +rules_rust.patch( + patches = [ + "//patches:rules_rust_windows_gnullvm_build_script.patch", + ], + strip = 1, +) use_repo(rules_rust, "rules_rust") toolchains = use_extension("@rules_rs//rs/experimental/toolchains:module_extension.bzl", "toolchains") @@ -65,10 +104,15 @@ crate.from_cargo( "aarch64-unknown-linux-gnu", "aarch64-unknown-linux-musl", "aarch64-apple-darwin", + # Keep both Windows ABIs in the generated Cargo metadata: the V8 + # experiment still consumes release assets that only exist under the + # MSVC names while targeting the GNU toolchain. + "aarch64-pc-windows-msvc", "aarch64-pc-windows-gnullvm", "x86_64-unknown-linux-gnu", "x86_64-unknown-linux-musl", "x86_64-apple-darwin", + "x86_64-pc-windows-msvc", "x86_64-pc-windows-gnullvm", ], use_experimental_platforms = True, diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 2333f48504..9e7ff69455 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -1576,15 +1576,19 @@ }, "@@rules_rs+//rs/experimental/toolchains:module_extension.bzl%toolchains": { "cargo-1.93.0-aarch64-apple-darwin.tar.xz": "6443909350322ad07f09bb5edfd9ff29268e6fe88c7d78bfba7a5e254248dc25", + "cargo-1.93.0-aarch64-pc-windows-gnullvm.tar.xz": "387832b989c8eb96c9ebd66402a87962167633bc6d91a49ffb8c7903c45f3476", "cargo-1.93.0-aarch64-pc-windows-msvc.tar.xz": "155bff7a16aa7054e7ed7c3a82e362d4b302b3882d751b823e06ff63ae3f103d", "cargo-1.93.0-aarch64-unknown-linux-gnu.tar.xz": "5998940b8b97286bb67facb1a85535eeb3d4d7a61e36a85e386e5c0c5cfe5266", "cargo-1.93.0-x86_64-apple-darwin.tar.xz": "95a47c5ed797c35419908f04188d8b7de09946e71073c4b72632b16f5b10dfae", + "cargo-1.93.0-x86_64-pc-windows-gnullvm.tar.xz": "f19766837559f90476508140cb95cc708220012ec00a854fa9f99187b1f246b6", "cargo-1.93.0-x86_64-pc-windows-msvc.tar.xz": "e59c5e2baa9ec17261f2cda6676ebf7b68b21a860e3f7451c4d964728951da75", "cargo-1.93.0-x86_64-unknown-linux-gnu.tar.xz": "c23de3ae709ff33eed5e4ae59d1f9bcd75fa4dbaa9fb92f7b06bfb534b8db880", "clippy-1.93.0-aarch64-apple-darwin.tar.xz": "0b6e943a8d12be0e68575acf59c9ea102daf795055fcbbf862b0bfd35ec40039", + "clippy-1.93.0-aarch64-pc-windows-gnullvm.tar.xz": "296949f49be2de77ce9d1c5023a5f0a58e28e329eec03642a3da0e175a67beeb", "clippy-1.93.0-aarch64-pc-windows-msvc.tar.xz": "07bcf2edb88cdf5ead2f02e4a8493e9b0ef935a31253fac6f9f3378d8023f113", "clippy-1.93.0-aarch64-unknown-linux-gnu.tar.xz": "872ae6d68d625946d281b91d928332e6b74f6ab269b6af842338df4338805a60", "clippy-1.93.0-x86_64-apple-darwin.tar.xz": "e6d0b1afb9607c14a1172d09ee194a032bbb3e48af913d55c5a473e0559eddde", + "clippy-1.93.0-x86_64-pc-windows-gnullvm.tar.xz": "b6f1f7264ed6943c59dedfb9531fbadcc3c0fcf273c940a63d58898b14a1060f", "clippy-1.93.0-x86_64-pc-windows-msvc.tar.xz": "25fb103390bf392980b4689ac09b2ec2ab4beefb7022a983215b613ad05eab57", "clippy-1.93.0-x86_64-unknown-linux-gnu.tar.xz": "793108977514b15c0f45ade28ae35c58b05370cb0f22e89bd98fdfa61eabf55d", "rust-std-1.93.0-aarch64-apple-darwin.tar.xz": "8603c63715349636ed85b4fe716c4e827a727918c840e54aff5b243cedadf19b", @@ -1653,15 +1657,19 @@ "rust-std-1.93.0-x86_64-unknown-none.tar.xz": "01dcca7ae4b7e82fbfa399adb5e160afaa13143e5a17e1e0737c38cf07365fb3", "rust-std-1.93.0-x86_64-unknown-uefi.tar.xz": "ec4e439d9485ce752b56999e8e41ed82373fc833a005cf2531c6f7ef7e785392", "rustc-1.93.0-aarch64-apple-darwin.tar.xz": "092be03c02b44c405dab1232541c84f32b2d9e8295747568c3d531dd137221dc", + "rustc-1.93.0-aarch64-pc-windows-gnullvm.tar.xz": "d3bc0cdaf157e20b1f23e510b5e3c4c6e9117d08f5284c04dee60aecff1bc851", "rustc-1.93.0-aarch64-pc-windows-msvc.tar.xz": "a3ac1a8e411de8470f71b366f89d187718c431526912b181692ed0a18c56c7ad", "rustc-1.93.0-aarch64-unknown-linux-gnu.tar.xz": "1a9045695892ec08d8e9751bf7cf7db71fe27a6202dd12ce13aca48d0602dbde", "rustc-1.93.0-x86_64-apple-darwin.tar.xz": "594bb293f0a4f444656cf8dec2149fcb979c606260efee9e09bcf8c9c6ed6ae7", + "rustc-1.93.0-x86_64-pc-windows-gnullvm.tar.xz": "0cdaa8de66f5ce21d1ea73917efc5c64f408bda49f678ddde19465ced9d5ec63", "rustc-1.93.0-x86_64-pc-windows-msvc.tar.xz": "fa17677eee0d83eb055b309953184bf87ba634923d8897f860cda65d55c6e350", "rustc-1.93.0-x86_64-unknown-linux-gnu.tar.xz": "00c6e6740ea6a795e33568cd7514855d58408a1180cd820284a7bbf7c46af715", "rustfmt-1.93.0-aarch64-apple-darwin.tar.xz": "0dd1faedf0768ef362f4aae4424b34e8266f2b9cf5e76ea4fcaf780220b363a0", + "rustfmt-1.93.0-aarch64-pc-windows-gnullvm.tar.xz": "5888827e7fbd7d59930870b4856fce8d6d8fca5e02f6535f8ae3d7ad0ccf2d4a", "rustfmt-1.93.0-aarch64-pc-windows-msvc.tar.xz": "24eed108489567133bbfe40c8eacda1567be55fae4c526911b39eb33eb27a6cb", "rustfmt-1.93.0-aarch64-unknown-linux-gnu.tar.xz": "92e1acb45ae642136258b4dabb39302af2d53c83e56ebd5858bc969f9e5c141a", "rustfmt-1.93.0-x86_64-apple-darwin.tar.xz": "c8453b4c5758eb39423042ffa9c23ed6128cbed2b15b581e5e1192c9cc0b1d4e", + "rustfmt-1.93.0-x86_64-pc-windows-gnullvm.tar.xz": "47167e9e78db9be4503a060dee02f4df2cda252da32175dbf44331f965a747b9", "rustfmt-1.93.0-x86_64-pc-windows-msvc.tar.xz": "5becc7c2dba4b9ab5199012cad30829235a7f7fb5d85a238697e8f0e44cbd9af", "rustfmt-1.93.0-x86_64-unknown-linux-gnu.tar.xz": "7f81f6c17d11a7fda5b4e1b111942fb3b23d30dcec767e13e340ebfb762a5e33" } diff --git a/defs.bzl b/defs.bzl index 058c1906a8..fee01a151a 100644 --- a/defs.bzl +++ b/defs.bzl @@ -13,6 +13,18 @@ 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({ + "@rules_rs//rs/experimental/platforms/constraints:windows_gnullvm": [ + "-C", + "link-arg=-Wl,--stack,8388608", + ], + "//conditions:default": [], +}) + def multiplatform_binaries(name, platforms = PLATFORMS): for platform in platforms: platform_data( @@ -224,7 +236,7 @@ def codex_rust_crate( crate_root = main, deps = all_crate_deps() + maybe_deps + deps_extra, edition = crate_edition, - rustc_flags = rustc_flags_extra, + rustc_flags = rustc_flags_extra + WINDOWS_GNULLVM_RUSTC_STACK_FLAGS, srcs = native.glob(["src/**/*.rs"]), visibility = ["//visibility:public"], ) @@ -252,7 +264,7 @@ def codex_rust_crate( # Bazel has emitted both `codex-rs//...` and # `../codex-rs//...` paths for `file!()`. Strip either # prefix so Insta records Cargo-like metadata such as `core/tests/...`. - rustc_flags = rustc_flags_extra + [ + rustc_flags = rustc_flags_extra + WINDOWS_GNULLVM_RUSTC_STACK_FLAGS + [ "--remap-path-prefix=../codex-rs=", "--remap-path-prefix=codex-rs=", ], diff --git a/patches/BUILD.bazel b/patches/BUILD.bazel index f308d260c0..8d4acbbb91 100644 --- a/patches/BUILD.bazel +++ b/patches/BUILD.bazel @@ -1,5 +1,9 @@ exports_files([ + "abseil_windows_gnullvm_thread_identity.patch", "aws-lc-sys_memcmp_check.patch", + "llvm_windows_symlink_extract.patch", + "rules_rust_windows_gnullvm_build_script.patch", + "rules_rs_windows_gnullvm_exec.patch", "rusty_v8_prebuilt_out_dir.patch", "v8_bazel_rules.patch", "v8_module_deps.patch", diff --git a/patches/abseil_windows_gnullvm_thread_identity.patch b/patches/abseil_windows_gnullvm_thread_identity.patch new file mode 100644 index 0000000000..27c4ecf71f --- /dev/null +++ b/patches/abseil_windows_gnullvm_thread_identity.patch @@ -0,0 +1,16 @@ +# What: avoid MinGW pthread TLS linkage in Abseil thread identity selection. +# Scope: one MinGW-specific preprocessor branch in Abseil's internal +# thread-identity mode selection; no other Abseil behavior changes. + +diff --git a/absl/base/internal/thread_identity.h b/absl/base/internal/thread_identity.h +--- a/absl/base/internal/thread_identity.h ++++ b/absl/base/internal/thread_identity.h +@@ -217,6 +217,8 @@ + #error ABSL_THREAD_IDENTITY_MODE cannot be directly set + #elif defined(ABSL_FORCE_THREAD_IDENTITY_MODE) + #define ABSL_THREAD_IDENTITY_MODE ABSL_FORCE_THREAD_IDENTITY_MODE ++#elif defined(__MINGW32__) ++#define ABSL_THREAD_IDENTITY_MODE ABSL_THREAD_IDENTITY_MODE_USE_CPP11 + #elif defined(_WIN32) && !defined(__MINGW32__) + #define ABSL_THREAD_IDENTITY_MODE ABSL_THREAD_IDENTITY_MODE_USE_CPP11 + #elif defined(__APPLE__) && defined(ABSL_HAVE_THREAD_LOCAL) diff --git a/patches/llvm_windows_symlink_extract.patch b/patches/llvm_windows_symlink_extract.patch new file mode 100644 index 0000000000..9f548636b2 --- /dev/null +++ b/patches/llvm_windows_symlink_extract.patch @@ -0,0 +1,64 @@ +diff --git a/extensions/llvm_source.bzl b/extensions/llvm_source.bzl +index 89dcf81..cf27c92 100644 +--- a/extensions/llvm_source.bzl ++++ b/extensions/llvm_source.bzl +@@ -83,6 +83,13 @@ def _llvm_source_archive_excludes(): + "offload", + "libc/docs", + "libc/utils/gn", ++ # These entries are unix symlinks in the upstream source tarball. ++ # Windows tar.exe cannot materialize them during repository extraction, ++ # but nothing in our Bazel usage needs the symlink entrypoints. ++ "llvm/utils/mlgo-utils/extract_ir.py", ++ "llvm/utils/mlgo-utils/make_corpus.py", ++ "llvm/utils/mlgo-utils/combine_training_corpus.py", ++ "llvm/docs/_themes/llvm-theme", + ] + + test_docs_subprojects = [ +diff --git a/runtimes/mingw/BUILD.bazel b/runtimes/mingw/BUILD.bazel +index ebd99db..9eb5d5b 100644 +--- a/runtimes/mingw/BUILD.bazel ++++ b/runtimes/mingw/BUILD.bazel +@@ -334,6 +334,30 @@ stub_library( + name = "stdc++", + ) + ++# Clang may inject -lssp and -lssp_nonshared for windows-gnu links. ++# Provide compatibility archives in the MinGW runtime search directory. ++stub_library( ++ name = "ssp", ++) ++ ++stub_library( ++ name = "ssp_nonshared", ++) ++ ++# Prebuilt MSVC archives can carry /DEFAULTLIB:libcmt.lib and ++# /DEFAULTLIB:oldnames.lib directives. Provide MinGW-compatible stand-ins so ++# lld can satisfy the lookup under windows-gnullvm. ++stub_library( ++ name = "libcmt", ++) ++ ++copy_file( ++ name = "oldnames", ++ src = ":moldname", ++ out = "liboldnames.a", ++ visibility = ["//visibility:public"], ++) ++ + copy_to_directory( + name = "mingw_crt_library_search_directory", + srcs = [ +@@ -344,6 +364,10 @@ copy_to_directory( + ":moldname", ++ ":oldnames", + ":pthread", + ":stdc++", ++ ":ssp", ++ ":ssp_nonshared", ++ ":libcmt", + ":ucrt", + ":ucrtbase", + ":ucrtbased", diff --git a/patches/rules_rs_windows_gnullvm_exec.patch b/patches/rules_rs_windows_gnullvm_exec.patch new file mode 100644 index 0000000000..9a49523b78 --- /dev/null +++ b/patches/rules_rs_windows_gnullvm_exec.patch @@ -0,0 +1,151 @@ +# What: teach `rules_rs` that `windows-gnullvm` is a distinct Windows exec ABI. +# Scope: experimental platform/toolchain naming only; no Cargo target changes. + +diff --git a/rs/experimental/platforms/triples.bzl b/rs/experimental/platforms/triples.bzl +index 3ca3bb1..dd15656 100644 +--- a/rs/experimental/platforms/triples.bzl ++++ b/rs/experimental/platforms/triples.bzl +@@ -30,7 +30,9 @@ SUPPORTED_EXEC_TRIPLES = [ + "x86_64-unknown-linux-gnu", + "aarch64-unknown-linux-gnu", + "x86_64-pc-windows-msvc", ++ "x86_64-pc-windows-gnullvm", + "aarch64-pc-windows-msvc", ++ "aarch64-pc-windows-gnullvm", + "x86_64-apple-darwin", + "aarch64-apple-darwin", + ] +diff --git a/rs/experimental/toolchains/declare_rustc_toolchains.bzl b/rs/experimental/toolchains/declare_rustc_toolchains.bzl +index b9a0ce1..67e491c 100644 +--- a/rs/experimental/toolchains/declare_rustc_toolchains.bzl ++++ b/rs/experimental/toolchains/declare_rustc_toolchains.bzl +@@ -10,6 +10,11 @@ def _channel(version): + return "beta" + return "stable" + ++def _exec_triple_suffix(exec_triple): ++ if exec_triple.system == "windows": ++ return "{}_{}_{}".format(exec_triple.system, exec_triple.arch, exec_triple.abi) ++ return "{}_{}".format(exec_triple.system, exec_triple.arch) ++ + def declare_rustc_toolchains( + *, + version, +@@ -23,15 +28,14 @@ def declare_rustc_toolchains( + + for triple in execs: + exec_triple = _parse_triple(triple) +- triple_suffix = exec_triple.system + "_" + exec_triple.arch ++ triple_suffix = _exec_triple_suffix(exec_triple) + + rustc_repo_label = "@rustc_{}_{}//:".format(triple_suffix, version_key) + cargo_repo_label = "@cargo_{}_{}//:".format(triple_suffix, version_key) + clippy_repo_label = "@clippy_{}_{}//:".format(triple_suffix, version_key) + +- rust_toolchain_name = "{}_{}_{}_rust_toolchain".format( +- exec_triple.system, +- exec_triple.arch, ++ rust_toolchain_name = "{}_{}_rust_toolchain".format( ++ triple_suffix, + version_key, + ) + +@@ -90,11 +94,8 @@ def declare_rustc_toolchains( + target_key = sanitize_triple(target_triple) + + native.toolchain( +- name = "{}_{}_to_{}_{}".format(exec_triple.system, exec_triple.arch, target_key, version_key), +- exec_compatible_with = [ +- "@platforms//os:" + exec_triple.system, +- "@platforms//cpu:" + exec_triple.arch, +- ], ++ name = "{}_to_{}_{}".format(triple_suffix, target_key, version_key), ++ exec_compatible_with = triple_to_constraint_set(triple), + target_compatible_with = triple_to_constraint_set(target_triple), + target_settings = [ + "@rules_rust//rust/toolchain/channel:" + channel, +diff --git a/rs/experimental/toolchains/declare_rustfmt_toolchains.bzl b/rs/experimental/toolchains/declare_rustfmt_toolchains.bzl +index a219209..ecb6b05 100644 +--- a/rs/experimental/toolchains/declare_rustfmt_toolchains.bzl ++++ b/rs/experimental/toolchains/declare_rustfmt_toolchains.bzl +@@ -1,8 +1,13 @@ + load("@rules_rust//rust:toolchain.bzl", "rustfmt_toolchain") + load("@rules_rust//rust/platform:triple.bzl", _parse_triple = "triple") +-load("//rs/experimental/platforms:triples.bzl", "SUPPORTED_EXEC_TRIPLES") ++load("//rs/experimental/platforms:triples.bzl", "SUPPORTED_EXEC_TRIPLES", "triple_to_constraint_set") + load("//rs/experimental/toolchains:toolchain_utils.bzl", "sanitize_version") + ++def _exec_triple_suffix(exec_triple): ++ if exec_triple.system == "windows": ++ return "{}_{}_{}".format(exec_triple.system, exec_triple.arch, exec_triple.abi) ++ return "{}_{}".format(exec_triple.system, exec_triple.arch) ++ + def _channel(version): + if version.startswith("nightly"): + return "nightly" +@@ -22,14 +27,13 @@ def declare_rustfmt_toolchains( + + for triple in execs: + exec_triple = _parse_triple(triple) +- triple_suffix = exec_triple.system + "_" + exec_triple.arch ++ triple_suffix = _exec_triple_suffix(exec_triple) + + rustc_repo_label = "@rustc_{}_{}//:".format(triple_suffix, version_key) + rustfmt_repo_label = "@rustfmt_{}_{}//:".format(triple_suffix, rustfmt_version_key) + +- rustfmt_toolchain_name = "{}_{}_{}_rustfmt_toolchain".format( +- exec_triple.system, +- exec_triple.arch, ++ rustfmt_toolchain_name = "{}_{}_rustfmt_toolchain".format( ++ triple_suffix, + version_key, + ) + +@@ -43,11 +47,8 @@ def declare_rustfmt_toolchains( + ) + + native.toolchain( +- name = "{}_{}_rustfmt_{}".format(exec_triple.system, exec_triple.arch, version_key), +- exec_compatible_with = [ +- "@platforms//os:" + exec_triple.system, +- "@platforms//cpu:" + exec_triple.arch, +- ], ++ name = "{}_rustfmt_{}".format(triple_suffix, version_key), ++ exec_compatible_with = triple_to_constraint_set(triple), + target_compatible_with = [], + target_settings = [ + "@rules_rust//rust/toolchain/channel:" + channel, +diff --git a/rs/experimental/toolchains/module_extension.bzl b/rs/experimental/toolchains/module_extension.bzl +index 7bb0205..ace556b 100644 +--- a/rs/experimental/toolchains/module_extension.bzl ++++ b/rs/experimental/toolchains/module_extension.bzl +@@ -37,6 +37,11 @@ def _normalize_arch_name(arch): + return "aarch64" + return arch + ++def _exec_triple_suffix(exec_triple): ++ if exec_triple.system == "windows": ++ return "{}_{}_{}".format(exec_triple.system, exec_triple.arch, exec_triple.abi) ++ return "{}_{}".format(exec_triple.system, exec_triple.arch) ++ + def _sanitize_path_fragment(path): + return path.replace("/", "_").replace(":", "_") + +@@ -181,7 +186,7 @@ def _toolchains_impl(mctx): + for triple in SUPPORTED_EXEC_TRIPLES: + exec_triple = _parse_triple(triple) + +- triple_suffix = exec_triple.system + "_" + exec_triple.arch ++ triple_suffix = _exec_triple_suffix(exec_triple) + rustc_name = "rustc_{}_{}".format(triple_suffix, version_key) + + rustc_repository( +@@ -230,7 +235,7 @@ def _toolchains_impl(mctx): + + for triple in SUPPORTED_EXEC_TRIPLES: + exec_triple = _parse_triple(triple) +- triple_suffix = exec_triple.system + "_" + exec_triple.arch ++ triple_suffix = _exec_triple_suffix(exec_triple) + + rustfmt_repository( + name = "rustfmt_{}_{}".format(triple_suffix, version_key), diff --git a/patches/rules_rust_windows_gnullvm_build_script.patch b/patches/rules_rust_windows_gnullvm_build_script.patch new file mode 100644 index 0000000000..ca7ac57402 --- /dev/null +++ b/patches/rules_rust_windows_gnullvm_build_script.patch @@ -0,0 +1,38 @@ +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 +@@ -120,6 +120,25 @@ + executable = True, + ) + ++def _strip_stack_protector_for_windows_llvm_mingw(toolchain, args): ++ """Drop stack protector flags unsupported by llvm-mingw build-script probes.""" ++ if "windows-gnullvm" not in toolchain.target_flag_value: ++ return args ++ ++ uses_llvm_mingw = False ++ for arg in args: ++ if "mingw-w64-" in arg: ++ uses_llvm_mingw = True ++ break ++ ++ if not uses_llvm_mingw: ++ return args ++ ++ # llvm-mingw does not ship libssp_nonshared, so forwarding stack-protector ++ # flags through CFLAGS/CXXFLAGS breaks build.rs probe binaries compiled via ++ # cc-rs. ++ return [arg for arg in args if not arg.startswith("-fstack-protector")] ++ + def get_cc_compile_args_and_env(cc_toolchain, feature_configuration): + """Gather cc environment variables from the given `cc_toolchain` + +@@ -503,6 +522,8 @@ + 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) + # 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). diff --git a/patches/rusty_v8_prebuilt_out_dir.patch b/patches/rusty_v8_prebuilt_out_dir.patch index e5132d664f..4242a29beb 100644 --- a/patches/rusty_v8_prebuilt_out_dir.patch +++ b/patches/rusty_v8_prebuilt_out_dir.patch @@ -1,5 +1,27 @@ +# What: make `rusty_v8` accept Bazel-provided archives and bindings for the +# windows-gnullvm experiment. +# Scope: `rusty_v8` build.rs only; no V8 source or Bazel rule changes. + --- a/build.rs +++ b/build.rs +@@ -543,10 +543,15 @@ + } + + fn static_lib_name(suffix: &str) -> String { +- let target_os = env::var("CARGO_CFG_TARGET_OS").unwrap(); +- if target_os == "windows" { ++ let target = env::var("TARGET").unwrap(); ++ let target_os = env::var("CARGO_CFG_TARGET_OS").unwrap(); ++ if target.contains("windows-gnullvm") { ++ // rustc looks for GNU-style archive names when bundling `-lstatic` on the ++ // gnullvm targets, even though the prebuilt release is a COFF `.lib`. ++ format!("librusty_v8{suffix}.a") ++ } else if target_os == "windows" { + format!("rusty_v8{suffix}.lib") + } else { + format!("librusty_v8{suffix}.a") + } + } @@ -577,7 +577,23 @@ path } diff --git a/patches/v8_bazel_rules.patch b/patches/v8_bazel_rules.patch index 66aff5e4d2..5457e3b854 100644 --- a/patches/v8_bazel_rules.patch +++ b/patches/v8_bazel_rules.patch @@ -29,11 +29,17 @@ index 9648e4a..88efd41 100644 "@v8//bazel/config:is_gcc": [ "-Wno-extra", "-Wno-array-bounds", -@@ -155,7 +149,12 @@ def _default_args(): - "@v8//bazel/config:is_windows": [ - "/std:c++20", - ], +@@ -155,7 +149,15 @@ def _default_args(): +- "@v8//bazel/config:is_windows": [ +- "/std:c++20", +- ], - "//conditions:default": [], ++ "@v8//bazel/config:is_windows": [ ++ "-Wno-invalid-offsetof", ++ "-Wno-deprecated-this-capture", ++ "-Wno-deprecated-declarations", ++ "-std=c++20", ++ ], + "//conditions:default": [ + "-Wno-invalid-offsetof", + "-Wno-deprecated-this-capture", @@ -43,8 +49,14 @@ index 9648e4a..88efd41 100644 }) + select({ "@v8//bazel/config:is_gcc_fastbuild": [ # Non-debug builds without optimizations fail because -@@ -184,7 +183,7 @@ def _default_args(): - "Advapi32.lib", +@@ -180,10 +179,10 @@ def _default_args(): + "@v8//bazel/config:is_windows": [ +- "Winmm.lib", +- "DbgHelp.lib", +- "Advapi32.lib", ++ "-lwinmm", ++ "-ldbghelp", ++ "-ladvapi32", ], "@v8//bazel/config:is_macos": ["-pthread"], - "//conditions:default": ["-Wl,--no-as-needed -ldl -latomic -pthread"], @@ -225,3 +237,39 @@ index 5fda2f4..381386c 100644 # Copyright 2021 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. +@@ -16,10 +18,7 @@ cc_library( + ]), + copts = select({ + "@platforms//os:windows": [ +- "/wd4005", # Macro redefinition. +- "/wd4068", # Unknown pragmas. +- "/wd4267", # Conversion from size_t on 64-bits. +- "/utf-8", # ICU source files are in UTF-8. ++ "-Wno-deprecated-declarations", + ], + "//conditions:default": [ + "-Wno-deprecated-declarations", +@@ -65,10 +64,7 @@ cc_library( + ]), + copts = select({ + "@platforms//os:windows": [ +- "/wd4005", # Macro redefinition. +- "/wd4068", # Unknown pragmas. +- "/wd4267", # Conversion from size_t on 64-bits. +- "/utf-8", # ICU source files are in UTF-8. ++ "-Wno-deprecated-declarations", + ], + "//conditions:default": [ + "-Wno-deprecated-declarations", +@@ -93,10 +89,7 @@ cc_library( + ]), + copts = select({ + "@platforms//os:windows": [ +- "/wd4005", # Macro redefinition. +- "/wd4068", # Unknown pragmas. +- "/wd4267", # Conversion from size_t on 64-bits. +- "/utf-8", # ICU source files are in UTF-8. ++ "-Wno-deprecated-declarations", + ], + "//conditions:default": [], + }), diff --git a/patches/v8_module_deps.patch b/patches/v8_module_deps.patch index ec4c8afb29..6c35dbacd4 100644 --- a/patches/v8_module_deps.patch +++ b/patches/v8_module_deps.patch @@ -62,7 +62,7 @@ diff --git a/orig/v8-14.6.202.11/MODULE.bazel b/mod/v8-14.6.202.11/MODULE.bazel + commit = "3d2de1816307bac63c16a297e8c4dc501b4076df", + remote = "https://chromium.googlesource.com/external/github.com/Maratyszcza/FP16.git", +) - + pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip") pip.parse( @@ -22,171 +72,3 @@ @@ -241,16 +241,29 @@ diff --git a/orig/v8-14.6.202.11/bazel/highway.patch b/mod/v8-14.6.202.11/bazel/ new file mode 100644 --- /dev/null +++ b/mod/v8-14.6.202.11/bazel/highway.patch -@@ -0,0 +1,12 @@ +@@ -0,0 +1,25 @@ +diff --git a/BUILD b/BUILD +--- a/BUILD ++++ b/BUILD +@@ -2,7 +2,7 @@ + load("@bazel_skylib//lib:selects.bzl", "selects") + load("@rules_license//rules:license.bzl", "license") -+ ++ +-load("@rules_cc//cc:defs.bzl", "cc_test") ++load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test") + # Placeholder#2 for Guitar, do not remove -+ ++ + package( ++@@ -134,11 +134,7 @@ COPTS = select({ ++ ], ++ }) ++ ++-DEFINES = select({ ++- ":compiler_msvc": ["HWY_SHARED_DEFINE"], ++- ":compiler_clangcl": ["HWY_SHARED_DEFINE"], ++- "//conditions:default": [], ++-}) +++DEFINES = [] ++ ++ # Unused on Bazel builds, where this is not defined/known; Copybara replaces ++ # usages with an empty list. diff --git a/patches/v8_source_portability.patch b/patches/v8_source_portability.patch index 81433cae62..4f5f46005f 100644 --- a/patches/v8_source_portability.patch +++ b/patches/v8_source_portability.patch @@ -2,7 +2,28 @@ # Scope: minimal source-level portability fixes only, such as libexecinfo guards, # weak glibc symbol handling, and warning annotations; no dependency # include-path rewrites or intentional V8 feature changes. - +diff --git a/orig/v8-14.6.202.11/src/base/bits.h b/mod/v8-14.6.202.11/src/base/bits.h +index 179a10f..4791e96 100644 +--- a/orig/v8-14.6.202.11/src/base/bits.h ++++ b/mod/v8-14.6.202.11/src/base/bits.h +@@ -270,11 +270,17 @@ inline constexpr uint32_t RoundDownToPowerOfTwo32(uint32_t value) { + } + + // Precondition: 0 <= shift < 32 ++#ifdef RotateRight32 ++#undef RotateRight32 ++#endif + inline constexpr uint32_t RotateRight32(uint32_t value, uint32_t shift) { + return (value >> shift) | (value << ((32 - shift) & 31)); + } + + // Precondition: 0 <= shift < 32 ++#ifdef RotateLeft32 ++#undef RotateLeft32 ++#endif + inline constexpr uint32_t RotateLeft32(uint32_t value, uint32_t shift) { + return (value << shift) | (value >> ((32 - shift) & 31)); + } diff --git a/orig/v8-14.6.202.11/src/base/debug/stack_trace_posix.cc b/mod/v8-14.6.202.11/src/base/debug/stack_trace_posix.cc index 6176ed4..a02043d 100644 --- a/orig/v8-14.6.202.11/src/base/debug/stack_trace_posix.cc @@ -23,36 +44,35 @@ index 6176ed4..a02043d 100644 // Demangles C++ symbols in the given text. Example: // // "out/Debug/base_unittests(_ZN10StackTraceC1Ev+0x20) [0x817778c]" - +diff --git a/orig/v8-14.6.202.11/src/base/export-template.h b/mod/v8-14.6.202.11/src/base/export-template.h +index 861cfe4..1e73954 100644 +--- a/orig/v8-14.6.202.11/src/base/export-template.h ++++ b/mod/v8-14.6.202.11/src/base/export-template.h +@@ -153,8 +153,10 @@ + + EXPORT_TEMPLATE_TEST(DEFAULT, ); + EXPORT_TEMPLATE_TEST(DEFAULT, __attribute__((visibility("default")))); ++#if defined(_MSC_VER) + EXPORT_TEMPLATE_TEST(MSVC_HACK, __declspec(dllexport)); + EXPORT_TEMPLATE_TEST(DEFAULT, __declspec(dllimport)); ++#endif + + #undef EXPORT_TEMPLATE_TEST + #undef EXPORT_TEMPLATE_TEST_DEFAULT_DEFAULT diff --git a/orig/v8-14.6.202.11/src/base/platform/platform-posix.cc b/mod/v8-14.6.202.11/src/base/platform/platform-posix.cc -index 4c7d878..0e45eb3 100644 +index 420df0b..6f47969 100644 --- a/orig/v8-14.6.202.11/src/base/platform/platform-posix.cc +++ b/mod/v8-14.6.202.11/src/base/platform/platform-posix.cc -@@ -95,7 +95,7 @@ +@@ -95,7 +95,7 @@ extern int madvise(caddr_t, size_t, int); #endif - + #if defined(V8_LIBC_GLIBC) -extern "C" void* __libc_stack_end; +extern "C" void* __libc_stack_end V8_WEAK; #endif - + namespace v8 { -@@ -1461,10 +1461,13 @@ - // pthread_getattr_np can fail for the main thread. - // For the main thread we prefer using __libc_stack_end (if it exists) since - // it generally provides a tighter limit for CSS. -- return __libc_stack_end; -+ if (__libc_stack_end != nullptr) { -+ return __libc_stack_end; -+ } - #else - return nullptr; - #endif // !defined(V8_LIBC_GLIBC) -+ return nullptr; - } - void* base; - size_t size; -@@ -1476,7 +1479,8 @@ +@@ -1476,7 +1476,8 @@ Stack::StackSlot Stack::ObtainCurrentThreadStackStart() { // __libc_stack_end is process global and thus is only valid for // the main thread. Check whether this is the main thread by checking // __libc_stack_end is within the thread's stack. @@ -62,7 +82,213 @@ index 4c7d878..0e45eb3 100644 DCHECK(MainThreadIsCurrentThread()); 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 +--- 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 +@@ -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. + #ifdef __MINGW32__ +- +- +-#ifndef __MINGW64_VERSION_MAJOR ++#if !defined(__MINGW64_VERSION_MAJOR) + + #define _TRUNCATE 0 + #define STRUNCATE 80 +@@ -81,9 +79,6 @@ inline void MemoryFence() { + __asm__ __volatile__("xchgl %%eax,%0 ":"=r" (barrier)); + } + +-#endif // __MINGW64_VERSION_MAJOR +- +- + int localtime_s(tm* out_tm, const time_t* time) { + tm* posix_local_time_struct = localtime_r(time, out_tm); + if (posix_local_time_struct == nullptr) return 1; +@@ -134,6 +129,8 @@ int strncpy_s(char* dest, size_t dest_size, const char* source, size_t count) { + return 0; + } + ++#endif // !defined(__MINGW64_VERSION_MAJOR) ++ + #endif // __MINGW32__ + + namespace v8 { +@@ -743,8 +740,10 @@ void OS::StrNCpy(char* dest, int length, const char* src, size_t n) { + } + + ++#if defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR) + #undef _TRUNCATE + #undef STRUNCATE ++#endif + + DEFINE_LAZY_LEAKY_OBJECT_GETTER(RandomNumberGenerator, + GetPlatformRandomNumberGenerator) +@@ -1894,3 +1893,4 @@ Stack::StackSlot Stack::GetCurrentStackPosition() { + + } // namespace base + } // namespace v8 ++ +diff --git a/orig/v8-14.6.202.11/src/base/platform/time.cc b/mod/v8-14.6.202.11/src/base/platform/time.cc +index 8390288..cba6cd5 100644 +--- a/orig/v8-14.6.202.11/src/base/platform/time.cc ++++ b/mod/v8-14.6.202.11/src/base/platform/time.cc +@@ -782,12 +782,12 @@ bool ThreadTicks::IsSupported() { + #elif defined(__PASE__) + // Thread CPU time accounting is unavailable in PASE + return false; ++#elif defined(V8_OS_WIN) ++ return IsSupportedWin(); + #elif (defined(_POSIX_THREAD_CPUTIME) && (_POSIX_THREAD_CPUTIME >= 0)) || \ + defined(V8_OS_DARWIN) || defined(V8_OS_ANDROID) || \ + defined(V8_OS_SOLARIS) || defined(V8_OS_ZOS) + return true; +-#elif defined(V8_OS_WIN) +- return IsSupportedWin(); + #else + return false; + #endif +@@ -804,13 +804,13 @@ ThreadTicks ThreadTicks::Now() { + return ThreadTicks(ComputeThreadTicks()); + #elif V8_OS_FUCHSIA + return ThreadTicks(GetFuchsiaThreadTicks()); ++#elif V8_OS_WIN ++ return ThreadTicks::GetForThread(::GetCurrentThread()); + #elif (defined(_POSIX_THREAD_CPUTIME) && (_POSIX_THREAD_CPUTIME >= 0)) || \ + defined(V8_OS_ANDROID) || defined(V8_OS_ZOS) + return ThreadTicks(ClockNow(CLOCK_THREAD_CPUTIME_ID)); + #elif V8_OS_SOLARIS + return ThreadTicks(gethrvtime() / Time::kNanosecondsPerMicrosecond); +-#elif V8_OS_WIN +- return ThreadTicks::GetForThread(::GetCurrentThread()); + #else + UNREACHABLE(); + #endif +diff --git 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 +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 @@ +-;; 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. +- +-;; MASM syntax +-;; https://docs.microsoft.com/en-us/cpp/assembler/masm/microsoft-macro-assembler-reference?view=vs-2019 +- +-public PushAllRegistersAndIterateStack +- +-.code +-PushAllRegistersAndIterateStack proc frame +- ;; Push all callee-saved registers to get them on the stack for conservative +- ;; stack scanning. +- ;; +- ;; We maintain 16-byte alignment at calls. There is an 8-byte return address +- ;; on the stack and we push 232 bytes which maintains 16-byte stack +- ;; alignment at the call. +- ;; Source: https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention +- ;; +- ;; rbp is callee-saved. Maintain proper frame pointer for debugging. +- push rbp +- .pushreg rbp +- mov rbp, rsp +- .setframe rbp, 0 +- push 0CDCDCDh ;; Dummy for alignment. +- .allocstack 8 +- push rsi +- .pushreg rsi +- push rdi +- .pushreg rdi +- push rbx +- .pushreg rbx +- push r12 +- .pushreg r12 +- push r13 +- .pushreg r13 +- push r14 +- .pushreg r14 +- push r15 +- .pushreg r15 +- sub rsp, 160 +- .allocstack 160 +- .endprolog +- ;; Use aligned instrs as we are certain that the stack is properly aligned. +- movdqa xmmword ptr [rsp + 144], xmm6 +- movdqa xmmword ptr [rsp + 128], xmm7 +- movdqa xmmword ptr [rsp + 112], xmm8 +- movdqa xmmword ptr [rsp + 96], xmm9 +- movdqa xmmword ptr [rsp + 80], xmm10 +- movdqa xmmword ptr [rsp + 64], xmm11 +- movdqa xmmword ptr [rsp + 48], xmm12 +- movdqa xmmword ptr [rsp + 32], xmm13 +- movdqa xmmword ptr [rsp + 16], xmm14 +- movdqa xmmword ptr [rsp], xmm15 +- ;; Pass 1st parameter (rcx) unchanged (Stack*). +- ;; Pass 2nd parameter (rdx) unchanged (StackVisitor*). +- ;; Save 3rd parameter (r8; IterateStackCallback) +- mov r9, r8 +- ;; Pass 3rd parameter as rsp (stack pointer). +- mov r8, rsp +- ;; Call the callback. +- call r9 +- ;; Pop the callee-saved registers. +- add rsp, 224 +- ;; Restore rbp as it was used as frame pointer. +- pop rbp +- ret +- PushAllRegistersAndIterateStack endp +- +-end ++.text ++.globl PushAllRegistersAndIterateStack ++.seh_proc PushAllRegistersAndIterateStack ++PushAllRegistersAndIterateStack: ++ push %rbp ++ .seh_pushreg %rbp ++ mov %rsp, %rbp ++ .seh_setframe %rbp, 0 ++ push $0xCDCDCD ++ .seh_stackalloc 8 ++ push %rsi ++ .seh_pushreg %rsi ++ push %rdi ++ .seh_pushreg %rdi ++ push %rbx ++ .seh_pushreg %rbx ++ push %r12 ++ .seh_pushreg %r12 ++ push %r13 ++ .seh_pushreg %r13 ++ push %r14 ++ .seh_pushreg %r14 ++ push %r15 ++ .seh_pushreg %r15 ++ sub $160, %rsp ++ .seh_stackalloc 160 ++ # Preserve Windows unwind metadata even though this is now GNU syntax. ++ # Without the .seh_* directives, SEH and stack walking can mis-handle ++ # unwinding across this callback frame. ++ .seh_endprologue ++ movdqa %xmm6, 144(%rsp) ++ movdqa %xmm7, 128(%rsp) ++ movdqa %xmm8, 112(%rsp) ++ movdqa %xmm9, 96(%rsp) ++ movdqa %xmm10, 80(%rsp) ++ movdqa %xmm11, 64(%rsp) ++ movdqa %xmm12, 48(%rsp) ++ movdqa %xmm13, 32(%rsp) ++ movdqa %xmm14, 16(%rsp) ++ movdqa %xmm15, 0(%rsp) ++ mov %r8, %r9 ++ mov %rsp, %r8 ++ call *%r9 ++ add $224, %rsp ++ pop %rbp ++ ret ++.seh_endproc diff --git a/orig/v8-14.6.202.11/src/libplatform/default-thread-isolated-allocator.cc b/mod/v8-14.6.202.11/src/libplatform/default-thread-isolated-allocator.cc index bda0e43..b44f1d9 100644 --- a/orig/v8-14.6.202.11/src/libplatform/default-thread-isolated-allocator.cc @@ -76,3 +302,26 @@ index bda0e43..b44f1d9 100644 // PKU was broken on Linux kernels before 5.13 (see // https://lore.kernel.org/all/20210623121456.399107624@linutronix.de/). // A fix is also included in the 5.4.182 and 5.10.103 versions ("x86/fpu: +diff --git a/orig/v8-14.6.202.11/src/profiler/heap-snapshot-generator.cc b/mod/v8-14.6.202.11/src/profiler/heap-snapshot-generator.cc +index 27e748f..cbf886c 100644 +--- a/orig/v8-14.6.202.11/src/profiler/heap-snapshot-generator.cc ++++ b/mod/v8-14.6.202.11/src/profiler/heap-snapshot-generator.cc +@@ -429,11 +429,11 @@ HeapSnapshot::HeapSnapshot(HeapProfiler* profiler, + static_assert(kSystemPointerSize != 4 || sizeof(HeapGraphEdge) == 12); + static_assert(kSystemPointerSize != 8 || sizeof(HeapGraphEdge) == 24); + static_assert(kSystemPointerSize != 4 || sizeof(HeapEntry) == 32); +-#if V8_CC_MSVC ++#if V8_CC_MSVC || V8_OS_WIN + static_assert(kSystemPointerSize != 8 || sizeof(HeapEntry) == 48); + #else // !V8_CC_MSVC + static_assert(kSystemPointerSize != 8 || sizeof(HeapEntry) == 40); +-#endif // !V8_CC_MSVC ++#endif // !V8_CC_MSVC || V8_OS_WIN + memset(&gc_subroot_entries_, 0, sizeof(gc_subroot_entries_)); + } + +@@ -3878,3 +3878,4 @@ void HeapSnapshotJSONSerializer::SerializeLocations() { + } + + } // namespace v8::internal ++ diff --git a/third_party/v8/BUILD.bazel b/third_party/v8/BUILD.bazel index b32bc2f495..27e9fa3ffe 100644 --- a/third_party/v8/BUILD.bazel +++ b/third_party/v8/BUILD.bazel @@ -64,12 +64,15 @@ alias( alias( name = "v8_146_4_0_aarch64_pc_windows_gnullvm", - actual = ":v8_146_4_0_aarch64_pc_windows_msvc", + # `rusty_v8` only ships prebuilt Windows archives for MSVC. Build the + # GNU-flavored archive in-tree so windows-gnullvm consumers can link + # against a matching ABI instead of trying to reuse the MSVC release. + actual = ":v8_146_4_0_aarch64_pc_windows_gnullvm_bazel", ) alias( name = "v8_146_4_0_x86_64_pc_windows_gnullvm", - actual = ":v8_146_4_0_x86_64_pc_windows_msvc", + actual = ":v8_146_4_0_x86_64_pc_windows_gnullvm_bazel", ) filegroup( @@ -114,12 +117,16 @@ filegroup( alias( name = "src_binding_release_x86_64_pc_windows_gnullvm", - actual = ":src_binding_release_x86_64_pc_windows_msvc", + # `rusty_v8` does not publish a Windows GNU binding file. The generated + # binding only describes this V8 release's C++ API surface, so reuse the + # Linux release binding while the windows-gnullvm archive build is still + # experimental. + actual = ":src_binding_release_x86_64_unknown_linux_gnu", ) alias( name = "src_binding_release_aarch64_pc_windows_gnullvm", - actual = ":src_binding_release_aarch64_pc_windows_msvc", + actual = ":src_binding_release_aarch64_unknown_linux_gnu", ) alias( @@ -167,6 +174,8 @@ 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' \ @@ -208,12 +217,24 @@ cc_static_library( features = V8_STATIC_LIBRARY_FEATURES, ) +cc_static_library( + name = "v8_146_4_0_aarch64_pc_windows_gnullvm_bazel", + deps = [":v8_146_4_0_binding"], + features = V8_STATIC_LIBRARY_FEATURES, +) + cc_static_library( name = "v8_146_4_0_x86_64_apple_darwin_bazel", deps = [":v8_146_4_0_binding"], features = V8_STATIC_LIBRARY_FEATURES, ) +cc_static_library( + name = "v8_146_4_0_x86_64_pc_windows_gnullvm_bazel", + deps = [":v8_146_4_0_binding"], + features = V8_STATIC_LIBRARY_FEATURES, +) + cc_static_library( name = "v8_146_4_0_x86_64_unknown_linux_gnu_bazel", deps = [":v8_146_4_0_binding"], diff --git a/workspace_root_test_launcher.bat.tpl b/workspace_root_test_launcher.bat.tpl index b7ddc6c9b2..6b7222cca6 100644 --- a/workspace_root_test_launcher.bat.tpl +++ b/workspace_root_test_launcher.bat.tpl @@ -41,11 +41,17 @@ if not defined manifest if exist "%~dpn0.runfiles_manifest" set "manifest=%~dpn0 if not defined manifest if exist "%~f0.exe.runfiles_manifest" set "manifest=%~f0.exe.runfiles_manifest" if defined manifest if exist "%manifest%" ( - for /f "usebackq tokens=1,* delims= " %%A in (`findstr /b /c:"%logical_path% " "%manifest%"`) do ( - endlocal & set "%~1=%%B" & exit /b 0 - ) - for /f "usebackq tokens=1,* delims= " %%A in (`findstr /b /c:"%workspace_logical_path% " "%manifest%"`) do ( - endlocal & set "%~1=%%B" & exit /b 0 + rem Read the manifest directly instead of shelling out to findstr. In the + rem GitHub Windows runner, the nested `findstr` path produced + rem `FINDSTR: Cannot open D:MANIFEST`, which then broke runfile resolution for + rem Bazel tests even though the manifest file was present. + for /f "usebackq tokens=1,* delims= " %%A in ("%manifest%") do ( + if "%%A"=="%logical_path%" ( + endlocal & set "%~1=%%B" & exit /b 0 + ) + if "%%A"=="%workspace_logical_path%" ( + endlocal & set "%~1=%%B" & exit /b 0 + ) ) )