You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@thrift.apache.org by "Jens-G (via GitHub)" <gi...@apache.org> on 2026/09/20 10:01:36 UTC
[PR] THRIFT-6310: Build the Thrift compiler on Windows in CI [thrift]
Jens-G opened a new pull request, #3893:
URL: https://github.com/apache/thrift/pull/3893
The compiler is built in CI on Linux and on macOS. Windows was never covered — the `compiler` job in `build.yml` still carried a `# TODO windows and macos` marker, and only the macOS half of it was ever done.
The one Windows job that exists, `msvc.yml`, builds the whole library stack inside a large Windows container, and it does not use the flags a release build uses. So the *portable* `thrift.exe` that ships with every release — linked against the static runtime so that it runs without a Visual C++ redistributable — has had no CI coverage at all. `doc/ReleaseManagement.md` asks the release manager to verify that property by hand, with Dependency Walker, once per release. A regression in it is only discoverable during a release.
## What this adds
A `compiler-windows` job in `cmake.yml`, next to the existing Linux `compiler` job. It runs natively on a `windows-2025` runner — no container:
- installs `winflexbison3`, pinned to the version `build/docker/msvc/Dockerfile` already pins, since flex and bison are not on the runner image;
- configures with `-DBUILD_LIBRARIES=OFF -DWITH_MT=ON` and builds the `Release` configuration, which is how the released compiler is built;
- runs `ctest`, so the compiler unit tests run on Windows;
- runs `thrift -version`;
- asserts the built `thrift.exe` imports nothing but `kernel32.dll`;
- uploads `thrift.exe` as a build artifact.
The portability check moves into `build/windows/check-portable-exe.ps1`. It reads the import table with `dumpbin`, which it locates through `vswhere`, so it also runs outside a Visual Studio developer prompt. `ReleaseManagement.md` now points at it instead of at Dependency Walker.
## The job cannot go green without having tested something
A check like this fails open: a parser that matches nothing reports every executable as clean. `build/windows/check-portable-exe-tests.ps1` therefore drives the check with canned `dumpbin` output and pins down that it still rejects
- an executable linked against the shared runtime,
- a delay-loaded dependency,
- an import table it could not read,
- and that the `Summary` section is not mistaken for an import.
Those tests need neither a compiler nor Windows — any PowerShell runs them, including `pwsh` on Linux — and the Windows job runs them before it builds anything. I verified they fail as intended by mutating the comparison and the parser in turn.
On top of that, the job fails if CMake did not report `Build compiler: ON`, and `ctest` runs with `--no-tests=error`.
## Notes for review
- **Why not `build.yml`.** That workflow is the autotools path (`bootstrap.sh`, `configure`, `make`), which has no native Windows toolchain. The `# TODO windows` marker there is replaced by a pointer to `cmake.yml`.
- **Why not extend `msvc.yml`.** That job is the container-based full-library build. This one is compiler-only and fast, and it is the first job to build the compiler the way a release does.
- **One runner, not a matrix.** `windows-2025` only, to keep the added CI time down; `msvc.yml` already exercises a second Windows toolchain path. Happy to add `windows-2022` if you would rather have both.
- No new third-party Actions — only `actions/checkout` and `actions/upload-artifact`, which are already used here.
JIRA: [THRIFT-6310](https://issues.apache.org/jira/browse/THRIFT-6310)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6310: Build the Thrift compiler on Windows in CI [thrift]
Posted by "fishy (via GitHub)" <gi...@apache.org>.
fishy commented on code in PR #3893:
URL: https://github.com/apache/thrift/pull/3893#discussion_r4057133441
##########
.github/workflows/cmake.yml:
##########
@@ -69,3 +69,69 @@ jobs:
name: cmake-LastTest-log
path: cmake_build/Testing/Temporary/LastTest.log
if-no-files-found: warn
+
+ compiler-windows:
+ runs-on: windows-2025
+ timeout-minutes: 60
+ steps:
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
Review Comment:
usually we comment which tagged version is the pinned sha, can we also comment it here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6310: Build the Thrift compiler on Windows in CI [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G merged PR #3893:
URL: https://github.com/apache/thrift/pull/3893
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] THRIFT-6310: Build the Thrift compiler on Windows in CI [thrift]
Posted by "Jens-G (via GitHub)" <gi...@apache.org>.
Jens-G commented on code in PR #3893:
URL: https://github.com/apache/thrift/pull/3893#discussion_r4057270614
##########
.github/workflows/cmake.yml:
##########
@@ -69,3 +69,69 @@ jobs:
name: cmake-LastTest-log
path: cmake_build/Testing/Temporary/LastTest.log
if-no-files-found: warn
+
+ compiler-windows:
+ runs-on: windows-2025
+ timeout-minutes: 60
+ steps:
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1
Review Comment:
Done — good catch, and thanks for the review.
Added `# v7.0.1` here, and also to the `actions/checkout` in the existing `compiler` job just above: it was the only other unannotated pin in the file, and leaving exactly one uncommented next to a commented one seemed worse than touching a line that is not strictly mine. Shout if you would rather I left it alone.
I verified the SHA resolves to `v7.0.1` against the tags API rather than trusting the comment I was copying.
Related: the security scanner caught two genuinely *wrong* version comments of mine on #3896 (`setup-dotnet` was v6.0.0 not v5.0.0, `NuGet/login` v1.2.0 not v1.0.0) — both fixed there. My local zizmor had reported clean because it defaults to offline mode, where `ref-version-mismatch` cannot resolve tags and never fires. With a token it is clean across all five workflows in this stack now.
Still unannotated elsewhere, but outside this change so I left them: `docker.yml` (two `checkout`, one `setup-go`) and `msvc.yml` (one `checkout`). Happy to send a separate PR for those if you want them tidied.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org