mirror of
https://github.com/openai/codex.git
synced 2026-09-10 20:26:47 +00:00
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 <json>` 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
This commit is contained in:
committed by
copyberry
parent
e01f38c388
commit
aa4a870e06
2
third_party/voice/BUILD.bazel
vendored
2
third_party/voice/BUILD.bazel
vendored
@@ -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",
|
||||
],
|
||||
)
|
||||
|
||||
17
third_party/voice/README.md
vendored
17
third_party/voice/README.md
vendored
@@ -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 <json>` 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
|
||||
|
||||
31
third_party/voice/build_native.py
vendored
31
third_party/voice/build_native.py
vendored
@@ -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"):
|
||||
|
||||
31
third_party/voice/test_build_native.py
vendored
31
third_party/voice/test_build_native.py
vendored
@@ -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())
|
||||
|
||||
148
third_party/voice/test_windows_build_inputs.py
vendored
Normal file
148
third_party/voice/test_windows_build_inputs.py
vendored
Normal file
@@ -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()
|
||||
89
third_party/voice/windows_build_inputs.py
vendored
Normal file
89
third_party/voice/windows_build_inputs.py
vendored
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user