windows: scope LLVM 19 host flags to the binding package - #49
Conversation
There was a problem hiding this comment.
Review Summary
This PR adds a Windows CI job (MSYS2 CLANG64 + pinned LLVM 19.1.7) plus Windows cgo pkg-config/CXXFLAGS directives, and fixes a test assertion to use %q so it matches the production error formatting (archive.go:97) — a correct cross-platform fix, since filepath.Join paths contain backslashes on Windows.
The shell scripting is well constructed: set -euo pipefail throughout, env-var (not ${{ }}) interpolation so there's no injection sink, defensive post-install version checks, and a legitimately-scoped --assume-installed for the gcc-libs→cc-libs virtual-dependency rename. No blocking issues. Findings are inline; two items without a precise diff line are below.
Additional (no precise inline location):
- Transitive dependency pins will bit-rot (
.github/workflows/go.yml, the pinnedgettext-runtime/libffi/libiconv/libxml2/xz/zlib/zstdversions). MSYS2 is a rolling repo and purges old package versions, so these hardcoded URLs will eventually 404 and break the Windows job with no repo change. Consider a comment documenting that these must be refreshed when the mirror drops them (this ties into the integrity/signature inline finding — anchoring by checksum would also make drift diagnosable). - README not updated for Windows (
README.markdown, "Currently supported"). Windows is now a user-facing supported platform for LLVM 19, but unlike Linux/macOS thellvm-19.pcis hand-generated in CI fromllvm-config. Windows users have no documented setup path (MSYS2 CLANG64 +PKG_CONFIG_PATH→ generated.pc). Worth documenting.
|
Addressed the two review-summary items in 1573496: added a maintenance note explaining that the pinned transitive MSYS2 package set and signatures must be refreshed together if rolling-repository artifacts disappear, and documented the supported MSYS2 CLANG64 / LLVM 19 pkg-config setup in README.markdown. |
|
Follow-up review changes are now implemented:
Validation:
Latest commit: e5fb5a7. |
Supports the LLGo Windows proposal: xgo-dev/llgo#2325.
Use a Windows-only pkg-config directive for LLVM 19 instead of requiring callers to export CGO_CFLAGS/CXXFLAGS/LDFLAGS globally. The public pkg-config module name is the neutral
llvm-19; LLGo generates that metadata fromllvm-configin its own dependency setup. This keeps LLVM host flags local to github.com/xgo-dev/llvm and prevents them from contaminating unrelated cgo packages or programs launched by compiler tests.Also makes the archive missing-file assertion portable to the escaped Windows path already emitted by WriteArchive.
The existing upstream test workflow now includes a Windows job. It pins MSYS2 LLVM 19.1.7, generates the neutral metadata directly from
llvm-config, and runs both the explicitllvm19build tag and the default LLVM configuration, so the Windows path is covered by the PR itself rather than only downstream LLGo CI.Validated locally with Go 1.26.7 and LLVM 19 on Windows 11 ARM64, and with the full test suite on macOS ARM64.