From 65237aeca0bf607fb2d940a1fc22cd5d4c43de07 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Mon, 31 Aug 2026 18:10:01 +0000 Subject: [PATCH] Fix Windows native voice dependency builds (#41894) ## Why Windows builds combine Cygwin build tools with native MSVC outputs. On ARM64, Cygwin can run under x64 emulation, so inferred host details and untranslated paths can select the wrong target or leak POSIX paths into native metadata. ## What changed - Convert libffi source, prefix, and shell paths with `cygpath`, preserve `USERPROFILE`, and pass explicit build and host targets. - Configure libffi's MSVC and libtool environment to produce and install its shared library and import library, including across recursive make calls. - Reject libffi pkg-config output containing `/cygdrive/` paths and enable Opus NEON support for `aarch64-pc-windows-msvc`. - Document the additional Windows build prerequisites. ## Testing - Extend `test_build_native.py` with Windows path conversion, x64 and ARM64 target configuration, recursive make flag propagation, pkg-config, and Opus coverage. GitOrigin-RevId: 12eb4ff83e141aa069abd68af4839f06e4c45b8f --- third_party/voice/README.md | 15 ++- third_party/voice/build_native.py | 57 +++++++++- third_party/voice/test_build_native.py | 137 +++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 5 deletions(-) diff --git a/third_party/voice/README.md b/third_party/voice/README.md index fe3235fff8..3716ac5f78 100644 --- a/third_party/voice/README.md +++ b/third_party/voice/README.md @@ -59,9 +59,20 @@ native host: GNU Linux, macOS, or Windows MSVC, on x64 or ARM64. On macOS, specify the existing release deployment target with `--deployment-target`; the host OS version is not an acceptable default. -Windows requires the normal Visual Studio SDK environment, GNU make and a -POSIX shell for upstream libffi, and `--bootstrap-make` pointing to NMake. +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 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 +metadata. The installed package/version set must exactly match the snapshot +manifest. +The MSI is administratively extracted into job storage without a system install. +Cygwin runs under x64 emulation on ARM64; the compiler probes and emitted DLLs +must still match the real native target. Native pkgconf relocates libffi's POSIX +prefix metadata; CI rejects residual Cygwin paths. These are build prerequisites, +not shipped runtime components or evidence of working voice. Outputs are under `prefix/`, build tools under `tools/`, and logs beside them. `build-state.json` records completed commands and failures; `built.json` exists diff --git a/third_party/voice/build_native.py b/third_party/voice/build_native.py index 33e16e03f8..d75cc1fc81 100644 --- a/third_party/voice/build_native.py +++ b/third_party/voice/build_native.py @@ -96,6 +96,7 @@ class NativeBuild: "HTTP_PROXY", "NO_PROXY", ) + or (self.windows and key == "USERPROFILE") } paths = [ str(self.tools / "bin"), @@ -157,6 +158,15 @@ class NativeBuild: ) result.check_returncode() + def posix_path(self, path): + if not self.windows: + return path.as_posix() + return subprocess.check_output( + [self.toolchain["shell"].with_name("cygpath.exe"), "-u", str(path)], + env=self.environment, + text=True, + ).strip() + def cmake(self, name, options, *, bootstrap=False): directory = self.output / "build" / name prefix = self.tools if bootstrap else self.prefix @@ -278,30 +288,58 @@ class NativeBuild: ffi_build = self.output / "build/libffi" ffi_build.mkdir() environment = self.environment.copy() + configure_options = [] if self.windows: - wrapper = shlex.quote((self.sources["libffi"] / "msvcc.sh").as_posix()) + wrapper = shlex.quote(self.posix_path(self.sources["libffi"] / "msvcc.sh")) architecture = ( "-m64" if self.args.target.startswith("x86_64-") else "-marm64" ) + # Match upstream's MSVC recipe, including native ARM64 outputs + # from x64-emulated Cygwin tools. Never infer the target from uname. + host = self.args.target.partition("-")[0] + "-w64-mingw32" + configure_options = [f"--build={host}", f"--host={host}"] + automake = subprocess.check_output( + [ + self.toolchain["shell"], + "--noprofile", + "--norc", + "-c", + "automake-1.18 --print-libdir", + ], + env=environment, + text=True, + ).strip() environment.update( { "CC": f"{wrapper} {architecture}", "CXX": f"{wrapper} {architecture}", + "AR": f"{shlex.quote(automake + '/ar-lib')} lib", + "RANLIB": ":", "LD": "link", + "NM": "dumpbin -symbols", + "STRIP": ":", + "LDFLAGS": "-no-undefined", + # Libffi clears MAKEOVERRIDES. Use its recursion hook to + # name the import library expected by libtool's installer. + "AM_MAKEFLAGS": shlex.quote( + "LTLDFLAGS=-no-undefined -Wc,-link,/IMPLIB:.libs/libffi.lib" + ), "CPP": "cl -nologo -EP", "CXXCPP": "cl -nologo -EP", "CPPFLAGS": "-DFFI_BUILDING_DLL", + "CONFIG_SHELL": self.posix_path(self.toolchain["shell"]), } ) self.run( "libffi-configure", [ self.toolchain["shell"], - (self.sources["libffi"] / "configure").as_posix(), - f"--prefix={self.prefix.as_posix()}", + self.posix_path(self.sources["libffi"] / "configure"), + f"--prefix={self.posix_path(self.prefix)}", "--enable-shared", "--disable-static", "--disable-docs", + *configure_options, ], cwd=ffi_build, environment=environment, @@ -318,12 +356,25 @@ class NativeBuild: cwd=ffi_build, environment=environment, ) + if self.windows: + self.run( + "libffi-pkg-config", + [self.toolchain["pkg_config"], "--cflags", "--libs", "libffi"], + ) + if "/cygdrive/" in (self.output / "libffi-pkg-config.log").read_text(): + raise ValueError("pkg-config did not relocate libffi to native paths") self.cmake( "opus", [ "-DOPUS_BUILD_SHARED_LIBRARY=ON", "-DOPUS_BUILD_TESTING=OFF", "-DOPUS_BUILD_PROGRAMS=OFF", + # Windows ARM64 guarantees NEON; upstream misses its ARM64 spelling. + *( + ["-DOPUS_PRESUME_NEON=ON"] + if self.args.target == "aarch64-pc-windows-msvc" + else [] + ), ], ) self.meson("proxy-libintl", []) diff --git a/third_party/voice/test_build_native.py b/third_party/voice/test_build_native.py index bf04af8a2c..1befd5e4f7 100644 --- a/third_party/voice/test_build_native.py +++ b/third_party/voice/test_build_native.py @@ -180,3 +180,140 @@ class NativeBuildTests(unittest.TestCase): shlex.split(build.environment["LDFLAGS"]), [f"-L{build.prefix / 'lib'}", f"-Wl,-rpath,{build.prefix / 'lib'}"], ) + + def test_windows_paths_use_cygpath_and_propagate_failure(self): + build = NativeBuild(self.args, self.environment) + build.windows = True + path = Path("D:/build output/source") + with patch( + "build_native.subprocess.check_output", + return_value="/cygdrive/d/build output/source\n", + ) as convert: + self.assertEqual(build.posix_path(path), "/cygdrive/d/build output/source") + self.assertEqual( + convert.call_args.args[0], + [build.toolchain["shell"].with_name("cygpath.exe"), "-u", str(path)], + ) + with patch( + "build_native.subprocess.check_output", + side_effect=subprocess.CalledProcessError(1, "cygpath"), + ): + with self.assertRaises(subprocess.CalledProcessError): + build.posix_path(path) + + def test_windows_recipes_use_explicit_targets_and_posix_paths(self): + self.environment["USERPROFILE"] = str(self.root / "user profile") + for architecture, flag in (("x86_64", "-m64"), ("aarch64", "-marm64")): + with self.subTest(architecture=architecture): + self.args.target = f"{architecture}-pc-windows-msvc" + self.args.output = self.root / architecture + with patch( + "build_native.platform.system", return_value="Windows" + ), patch("build_native.platform.machine", return_value=architecture): + build = NativeBuild(self.args, self.environment) + self.assertEqual( + build.environment["USERPROFILE"], self.environment["USERPROFILE"] + ) + calls = {} + + def record(name, command, **kwargs): + calls[name] = (command, kwargs) + (build.output / f"{name}.log").write_text( + "-ID:/private/include -LD:/private/lib -lffi\n" + ) + + with patch( + "build_native.prepare_sources", + side_effect=lambda *args: (build.output / "build").mkdir(), + ), patch.object(build, "cmake") as cmake, patch.object( + build, "meson" + ), patch.object( + build, "run", side_effect=record + ), patch.object( + build, + "posix_path", + side_effect=lambda path: "/cygdrive/d/" + path.name, + ), patch( + "build_native.subprocess.check_output", + return_value="/usr/share/automake-1.18\n", + ): + build.build() + opus_options = next( + call.args[1] + for call in cmake.call_args_list + if call.args[0] == "opus" + ) + self.assertEqual( + "-DOPUS_PRESUME_NEON=ON" in opus_options, architecture == "aarch64" + ) + command, kwargs = calls["libffi-configure"] + host = f"{architecture}-w64-mingw32" + linker_flags = "-no-undefined -Wc,-link,/IMPLIB:.libs/libffi.lib" + self.assertEqual( + command, + [ + build.toolchain["shell"], + "/cygdrive/d/configure", + "--prefix=/cygdrive/d/prefix", + "--enable-shared", + "--disable-static", + "--disable-docs", + f"--build={host}", + f"--host={host}", + ], + ) + expected = { + **build.environment, + "CC": f"/cygdrive/d/msvcc.sh {flag}", + "CXX": f"/cygdrive/d/msvcc.sh {flag}", + "AR": "/usr/share/automake-1.18/ar-lib lib", + "RANLIB": ":", + "LD": "link", + "NM": "dumpbin -symbols", + "STRIP": ":", + "LDFLAGS": "-no-undefined", + "AM_MAKEFLAGS": shlex.quote(f"LTLDFLAGS={linker_flags}"), + "CPP": "cl -nologo -EP", + "CXXCPP": "cl -nologo -EP", + "CPPFLAGS": "-DFFI_BUILDING_DLL", + "CONFIG_SHELL": "/cygdrive/d/" + build.toolchain["shell"].name, + } + self.assertEqual( + kwargs, + {"cwd": build.output / "build/libffi", "environment": expected}, + ) + self.assertEqual(calls["libffi-install"][1]["environment"], expected) + if platform.system() == "Windows": + cygwin = os.environ.get("VOICE_CYGWIN_ROOT") + make = str(Path(cygwin) / "bin/make.exe") if cygwin else None + else: + make = shutil.which("make") + if make is None: + self.skipTest("GNU make is required for the recursive build check") + # Mirror libffi's MAKEOVERRIDES reset and recursive hook, without + # running a compiler or requiring the native source archives. + directory = build.output / "build/libffi" + (directory / "Makefile").write_text( + "MAKEOVERRIDES =\nLTLDFLAGS = default\n" + "all:\n\t@$(MAKE) --no-print-directory $(AM_MAKEFLAGS) nested\n" + "nested:\n\t@$(MAKE) --no-print-directory $(AM_MAKEFLAGS) observe\n" + "observe:\n\t@printf '%s\\n' \"$(LTLDFLAGS)\"\n" + ) + result = subprocess.run( + [make, "--no-print-directory"], + cwd=directory, + env={ + **os.environ, + "AM_MAKEFLAGS": kwargs["environment"]["AM_MAKEFLAGS"], + "PATH": os.pathsep.join( + [str(Path(make).parent), os.environ.get("PATH", "")] + ), + }, + capture_output=True, + text=True, + check=True, + ) + self.assertEqual( + result.stdout.strip(), + linker_flags, + )