From aa4a870e0663edf9efc8a33035236c657f66f06d Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Sat, 5 Sep 2026 23:21:56 +0000 Subject: [PATCH] Add explicit Windows tool selection for native voice builds (#43125) ## Why Windows libffi builds invoke tools by name, and Cygwin provides a different `link.exe` from MSVC. Inherited search paths can select unintended tools or SDK inputs. ## What changed Add optional `--windows-build-inputs ` to `third_party/voice/build_native.py` to select installed tools and SDK directories for Windows MSVC targets. Validate the target, tool paths, architecture-specific assembler, SDK directories, and agreement with CLI tool arguments. Build the search path from the selection with MSVC ahead of Cygwin, reject shadowed tools, replace inherited `INCLUDE` and `LIB`, and remove `LIBPATH`. Retain the selection in build receipts. Builds without the option continue using the normal Visual Studio environment. ## Testing Add tests for linker precedence and subprocess selection, invalid or mismatched inputs, shadowed SDK tools, environment replacement, and receipt recording. GitOrigin-RevId: c80a6d9eb29556157ba1316d62c9b5083f4038b9 --- third_party/voice/BUILD.bazel | 2 + third_party/voice/README.md | 17 ++ third_party/voice/build_native.py | 31 +++- third_party/voice/test_build_native.py | 31 ++++ .../voice/test_windows_build_inputs.py | 148 ++++++++++++++++++ third_party/voice/windows_build_inputs.py | 89 +++++++++++ 6 files changed, 317 insertions(+), 1 deletion(-) create mode 100644 third_party/voice/test_windows_build_inputs.py create mode 100644 third_party/voice/windows_build_inputs.py diff --git a/third_party/voice/BUILD.bazel b/third_party/voice/BUILD.bazel index 7a98b0ec58..aed10ec42a 100644 --- a/third_party/voice/BUILD.bazel +++ b/third_party/voice/BUILD.bazel @@ -46,6 +46,7 @@ filegroup( "package_runtime.py", "runtime.py", "sdk.py", + "windows_build_inputs.py", "windows_runtime.py", ":source_inputs", ], @@ -56,6 +57,7 @@ filegroup( name = "native_recipe", srcs = [ "build_native.py", + "windows_build_inputs.py", ":source_inputs", ], ) diff --git a/third_party/voice/README.md b/third_party/voice/README.md index d04f070633..3f7d54a863 100644 --- a/third_party/voice/README.md +++ b/third_party/voice/README.md @@ -114,6 +114,23 @@ Windows requires the normal Visual Studio SDK environment, Cygwin GNU make, bash/cygpath and Automake 1.18's standard `ar-lib` for upstream libffi, native Windows pkgconf, and `--bootstrap-make` pointing to NMake. The recipe does not install these build prerequisites or patch upstream sources. +The optional `--windows-build-inputs ` argument records an explicit selection +of these tools, the target-specific MSVC assembler, linker, library manager, +inspector, Windows SDK resource/manifest tools, Python, and include/library roots. +The existing private CI driver supplies this input from its provisioned VS/Cygwin +setup. In this mode the recipe checks that named tools resolve to the selected +files, puts MSVC ahead of Cygwin's different `link.exe`, and excludes unrelated +inherited PATH and SDK entries. The exact selection is retained in build receipts. +The JSON uses `schemaVersion: 1`, `target`, `tools` (role to absolute file path), +`systemRoot`, and `INCLUDE`/`LIB` arrays of absolute directories. The CLI tool +arguments must agree with the recorded selection. Without this argument, the +standalone recipe keeps using the normal Visual Studio environment. + +This selects already installed inputs; it does not hash their support files, +sandbox the build, supply a Bazel Windows provider, or publish build tools. +Those still require a complete declared compiler/bootstrap closure and approved +public-readable inputs. Existing private Cygwin release assets do not satisfy +public self-build access. No Windows support is disabled to hide that gap. The private CI bootstrap verifies the official Cygwin installer and native pkgconf MSI hashes before use. It also verifies a retained Cygwin package snapshot against pinned archive and member hashes before installing it offline using signed diff --git a/third_party/voice/build_native.py b/third_party/voice/build_native.py index 29d875874f..58d482894d 100644 --- a/third_party/voice/build_native.py +++ b/third_party/voice/build_native.py @@ -16,6 +16,7 @@ import sys # without adding the caller's working directory to the module search path. sys.path.insert(0, str(Path(__file__).resolve().parent)) from prepare_sources import MANIFEST, load_sources, prepare_sources +from windows_build_inputs import build_environment TARGET_SYSTEMS = { "apple-darwin": "Darwin", @@ -89,6 +90,21 @@ class NativeBuild: for tool in (*self.toolchain.values(), self.bootstrap_make): if not tool.is_file(): raise ValueError(f"Missing build tool: {tool}") + windows_inputs = getattr(args, "windows_build_inputs", None) + input_record = None + if windows_inputs is not None: + if not self.windows: + raise ValueError("Windows build inputs require a Windows MSVC target") + explicit, input_record = build_environment( + windows_inputs, + args.target, + { + **self.toolchain, + "bootstrap_make": self.bootstrap_make, + "python": Path(sys.executable), + }, + ) + inherited_environment = {**inherited_environment, **explicit} if self.windows and not all( inherited_environment.get(key) for key in ("INCLUDE", "LIB") ): @@ -117,9 +133,15 @@ class NativeBuild: ) or (self.windows and key == "USERPROFILE") } + if windows_inputs is not None: + self.environment.pop("LIBPATH", None) paths = [ str(self.tools / "bin"), - *(str(p.parent) for p in self.toolchain.values()), + *( + [] + if windows_inputs is not None + else [str(p.parent) for p in self.toolchain.values()] + ), ] paths += ( inherited_environment.get("PATH", "").split(os.pathsep) @@ -172,6 +194,8 @@ class NativeBuild: "flags": self.flags, "steps": [], } + if input_record is not None: + self.record["windows_build_inputs"] = input_record def run(self, name, command, cwd=None, environment=None): command = [str(part) for part in command] @@ -524,6 +548,11 @@ def main(): type=Path, help="NMake on Windows; defaults to --make elsewhere", ) + parser.add_argument( + "--windows-build-inputs", + type=Path, + help="Explicit Windows tool and SDK selection; no unrelated PATH fallback", + ) for name in ("ar", "ranlib"): parser.add_argument(f"--{name}", type=Path) for name in ("c-flag", "cxx-flag", "link-flag"): diff --git a/third_party/voice/test_build_native.py b/third_party/voice/test_build_native.py index 6d7242b4e6..9cd6703a3e 100644 --- a/third_party/voice/test_build_native.py +++ b/third_party/voice/test_build_native.py @@ -535,3 +535,34 @@ class NativeBuildTests(unittest.TestCase): result.stdout.strip(), linker_flags, ) + + def test_explicit_windows_inputs_replace_ambient_sdk_and_search_paths(self): + self.args.target = "x86_64-pc-windows-msvc" + self.args.windows_build_inputs = self.root / "inputs.json" + selected = { + "PATH": str(self.root / "selected tools"), + "INCLUDE": str(self.root / "selected include"), + "LIB": str(self.root / "selected lib"), + } + with ( + patch("build_native.validate_target"), + patch( + "build_native.build_environment", + return_value=(selected, {"target": self.args.target}), + ), + ): + build = NativeBuild( + self.args, + {"PATH": "/unrelated", "LIBPATH": "/unrelated"}, + ) + self.assertFalse( + any("/unrelated" in value for value in build.environment.values()) + ) + self.assertEqual( + {name: build.environment[name] for name in ("INCLUDE", "LIB")}, + {name: selected[name] for name in ("INCLUDE", "LIB")}, + ) + self.assertEqual( + build.record["windows_build_inputs"], {"target": self.args.target} + ) + self.assertFalse(build.output.exists()) diff --git a/third_party/voice/test_windows_build_inputs.py b/third_party/voice/test_windows_build_inputs.py new file mode 100644 index 0000000000..4374e65d4a --- /dev/null +++ b/third_party/voice/test_windows_build_inputs.py @@ -0,0 +1,148 @@ +"""Exercise explicit Windows tool selection and rejection before native builds.""" + +import json +import os +from pathlib import Path +import shutil +import subprocess +import sys +import tempfile +import unittest + +from windows_build_inputs import build_environment + + +class WindowsInputsTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory(prefix="voice selected tools ") + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + self.target = "x86_64-pc-windows-msvc" + self.selected = {} + for directory, names in { + "msvc": [ + "cl.exe", + "link.exe", + "lib.exe", + "dumpbin.exe", + "ml64.exe", + "nmake.exe", + ], + "sdk": ["rc.exe", "mt.exe"], + "cygwin": [ + "bash.exe", + "cygpath.exe", + "automake-1.18", + "make.exe", + "link.exe", + ], + "cmake": ["cmake.exe"], + "pkgconf": ["pkgconf.exe"], + "Windows/System32": ["cmd.exe"], + }.items(): + parent = self.root / directory + parent.mkdir(parents=True) + for name in names: + (parent / name).touch() + self.tools = { + "cc": self.root / "msvc/cl.exe", + "cxx": self.root / "msvc/cl.exe", + "link": self.root / "msvc/link.exe", + "lib": self.root / "msvc/lib.exe", + "dumpbin": self.root / "msvc/dumpbin.exe", + "assembler": self.root / "msvc/ml64.exe", + "bootstrap_make": self.root / "msvc/nmake.exe", + "rc": self.root / "sdk/rc.exe", + "mt": self.root / "sdk/mt.exe", + "make": self.root / "cygwin/make.exe", + "shell": self.root / "cygwin/bash.exe", + "cygpath": self.root / "cygwin/cygpath.exe", + "automake": self.root / "cygwin/automake-1.18", + "cmake": self.root / "cmake/cmake.exe", + "pkg_config": self.root / "pkgconf/pkgconf.exe", + "python": Path(sys.executable), + } + self.document = { + "schemaVersion": 1, + "target": self.target, + "tools": {key: str(value) for key, value in self.tools.items()}, + "systemRoot": str(self.root / "Windows"), + "INCLUDE": [str(self.root / "sdk")], + "LIB": [str(self.root / "msvc")], + } + self.path = self.root / "inputs.json" + + def environment(self): + self.path.write_text(json.dumps(self.document), encoding="utf-8") + return build_environment(self.path, self.target, self.selected)[0] + + def test_msvc_link_precedes_same_named_cygwin_tool(self): + environment = self.environment() + first = next( + Path(directory) / "link.exe" + for directory in environment["PATH"].split(os.pathsep) + if (Path(directory) / "link.exe").exists() + ) + self.assertEqual(first, self.tools["link"]) + self.assertEqual( + {name: environment[name] for name in ("INCLUDE", "LIB", "COMSPEC")}, + { + "INCLUDE": str(self.root / "sdk"), + "LIB": str(self.root / "msvc"), + "COMSPEC": str(self.root / "Windows/System32/cmd.exe"), + }, + ) + + @unittest.skipIf(os.name == "nt", "Portable shell subprocess selection probe") + def test_subprocess_uses_selected_tool_without_ambient_path(self): + # Execute the name upstream uses, with a conflicting Cygwin executable. + for path, text in ( + (self.tools["link"], "msvc"), + (self.root / "cygwin/link.exe", "cygwin"), + ): + path.write_text(f"#!/bin/sh\nprintf '{text}'\n") + path.chmod(0o755) + result = subprocess.run( + ["link.exe"], + env=self.environment(), + check=True, + capture_output=True, + text=True, + ) + self.assertEqual(result.stdout, "msvc") + + def test_missing_or_wrong_architecture_assembler_is_rejected(self): + self.tools["assembler"].unlink() + with self.assertRaisesRegex(ValueError, "assembler"): + self.environment() + self.tools["assembler"].touch() + self.target = "aarch64-pc-windows-msvc" + self.document["target"] = self.target + with self.assertRaisesRegex(ValueError, "assembler"): + self.environment() + arm = self.tools["assembler"].with_name("armasm64.exe") + arm.touch() + self.document["tools"]["assembler"] = str(arm) + self.environment() + + def test_shadowed_sdk_tool_is_rejected(self): + shutil.copyfile(self.tools["rc"], self.root / "msvc/rc.exe") + with self.assertRaisesRegex(ValueError, "shadowed: rc"): + self.environment() + + def test_cli_tool_must_match_recorded_selection(self): + self.selected["cc"] = self.tools["cmake"] + with self.assertRaisesRegex(ValueError, "disagrees with --cc"): + self.environment() + + def test_missing_sdk_and_relative_paths_are_rejected(self): + for value in ([], ["relative"], [str(self.root / "missing")]): + with self.subTest(value=value): + self.document["INCLUDE"] = value + with self.assertRaises(ValueError): + self.environment() + + def test_target_mismatch_cannot_reuse_other_inputs(self): + self.document["target"] = "aarch64-pc-windows-msvc" + with self.assertRaisesRegex(ValueError, "target"): + self.environment() diff --git a/third_party/voice/windows_build_inputs.py b/third_party/voice/windows_build_inputs.py new file mode 100644 index 0000000000..5a5e26f410 --- /dev/null +++ b/third_party/voice/windows_build_inputs.py @@ -0,0 +1,89 @@ +"""Select already provisioned Windows build tools without unrelated PATH entries. + +This is an input-selection contract, not a sandbox or a hashed tool closure. +The selected tools still depend on their installed support files and Windows. +""" + +import json +import os +from pathlib import Path + + +def build_environment(path, target, selected_tools): + if path.stat().st_size > 65536: + raise ValueError("Windows build input document exceeds limits") + inputs = json.loads(path.read_text(encoding="utf-8")) + expected = { + "cc": "cl.exe", + "cxx": "cl.exe", + "link": "link.exe", + "lib": "lib.exe", + "dumpbin": "dumpbin.exe", + "assembler": "ml64.exe" if target.startswith("x86_64-") else "armasm64.exe", + "rc": "rc.exe", + "mt": "mt.exe", + "cmake": "cmake.exe", + "bootstrap_make": "nmake.exe", + "make": "make.exe", + "shell": "bash.exe", + "cygpath": "cygpath.exe", + "automake": "automake-1.18", + } + if ( + target not in ("x86_64-pc-windows-msvc", "aarch64-pc-windows-msvc") + or inputs.get("schemaVersion") != 1 + or inputs.get("target") != target + or set(inputs.get("tools", {})) != {*expected, "pkg_config", "python"} + ): + raise ValueError("Windows build inputs do not match the target and tool set") + tools = {} + for name, value in inputs["tools"].items(): + tool = Path(value) + if ( + not tool.is_absolute() + or not tool.is_file() + or ";" in str(tool) + or (name in expected and tool.name.lower() != expected[name]) + ): + raise ValueError(f"Invalid Windows build tool: {name}") + tools[name] = tool + for name, tool in selected_tools.items(): + if not tools[name].samefile(tool): + raise ValueError(f"Windows build input disagrees with --{name}") + # libffi invokes cl/link/lib and its architecture's assembler by name. + # Keep MSVC ahead of Cygwin, which also installs a different link.exe. + directories = list(dict.fromkeys(tools[name].parent for name in expected)) + directories += [tools[name].parent for name in ("pkg_config", "python")] + directories = list(dict.fromkeys(directories)) + for name, filename in expected.items(): + found = next( + (p / filename for p in directories if (p / filename).is_file()), None + ) + if found is None or not found.samefile(tools[name]): + raise ValueError(f"Windows build tool is shadowed: {name}") + system = Path(inputs["systemRoot"]) + command = system / "System32/cmd.exe" + if not system.is_absolute() or not command.is_file() or ";" in str(system): + raise ValueError( + "Windows build inputs require a valid system command directory" + ) + directories.append(command.parent) + environment = { + "PATH": os.pathsep.join(map(str, directories)), + "SystemRoot": str(system), + "SYSTEMROOT": str(system), + "WINDIR": str(system), + "COMSPEC": str(command), + } + for name in ("INCLUDE", "LIB"): + values = inputs.get(name) + if not isinstance(values, list) or not 1 <= len(values) <= 64: + raise ValueError( + f"Windows build inputs require explicit {name} directories" + ) + for value in values: + directory = Path(value) + if not directory.is_absolute() or not directory.is_dir() or ";" in value: + raise ValueError(f"Invalid Windows {name} directory") + environment[name] = os.pathsep.join(values) + return environment, inputs