ci: bump to v1.5.3; Windows build deferred pending upstream vortex port#86
Open
joseph-isaacs wants to merge 7 commits into
Open
ci: bump to v1.5.3; Windows build deferred pending upstream vortex port#86joseph-isaacs wants to merge 7 commits into
joseph-isaacs wants to merge 7 commits into
Conversation
Enable the MSVC windows_amd64 target in the main distribution pipeline by removing it from exclude_archs, so CI builds and tests the extension on Windows. The MinGW/Rtools variants stay excluded. Guard the GCC/Clang-only warning flags (-Wall -Wextra -Wpedantic) behind a non-MSVC check in CMakeLists.txt, since MSVC's cl rejects -Wextra/-Wpedantic. Signed-off-by: Claude <noreply@anthropic.com>
fd9d520 to
a145953
Compare
Enable windows_arm64 via opt_in_archs and drop the MinGW/Rtools exclusions so CI builds and tests the extension on all four Windows targets. Signed-off-by: Claude <noreply@anthropic.com>
0ax1
reviewed
Jun 1, 2026
| set(VORTEX_WARNING_FLAGS -Wall -Wextra -Wpedantic) | ||
| # MSVC's `cl` does not understand the GCC/Clang warning flags below, so only | ||
| # apply them for GNU/Clang-style compilers (Linux/macOS and MinGW). | ||
| if(MSVC) |
Contributor
There was a problem hiding this comment.
What happens if I build on Windows with clang? Ideally, we would also build with Clang on Windows, using the same compiler flags.
Keep warning coverage on the MSVC Windows builds by applying /W4 (the equivalent of -Wall -Wextra) rather than emptying the warning flags. Signed-off-by: Claude <noreply@anthropic.com>
Re-exclude the MinGW/Rtools variants; build only the MSVC windows_amd64 and windows_arm64 targets that DuckDB distributes. Signed-off-by: Claude <noreply@anthropic.com>
ExtensionTemplate.yml only runs on duckdb/extension-template, so its Linux/MacOS/Windows jobs are always skipped here, producing misleading 'Windows skipped' checks. Its header notes it should be removed after bootstrap; remove it so only the real distribution-pipeline builds show. Signed-off-by: Claude <noreply@anthropic.com>
windows_arm64 fails linking aws-lc-sys: the ARM64 assembly object (e.g. chacha-armv8.o) is never produced yet lib.exe still references it, giving LNK1181. AWS_LC_SYS_NO_ASM can't be used since it's forbidden in release builds. Because the matrix is fail-fast, this also cancels the in-progress windows_amd64 job — the platform this PR actually targets. Drop the arm64 opt-in so windows_amd64 can run to completion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
7c60937 to
d014fee
Compare
Enabling windows_amd64 surfaced that the vortex-duckdb crate's build.rs (in the vortex submodule) was never written for Windows: it uses std::os::unix::fs::symlink, has no Windows branch in its TARGET match or libduckdb download URL, and passes GCC/Clang-only flags (-Wall -Wextra -Wpedantic, -Wl,-rpath) that MSVC rejects. windows_arm64 additionally hits an aws-lc-sys ARM64 assembly link failure (LNK1181). These are upstream porting tasks in vortex-data/vortex that can't be verified on the Linux/macOS CI sandbox. Re-exclude both Windows archs so the v1.5.3 bump and CMake changes ship green on Linux/macOS; Windows support is tracked separately. The MSVC-guarded warning flags in CMakeLists are left in place as harmless forward-prep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
0ax1
reviewed
Jun 2, 2026
Contributor
0ax1
left a comment
There was a problem hiding this comment.
If this passes CI, we need to do a manual smoke test on a Windows machine before landing.
| # MSVC's `cl` does not understand the GCC/Clang warning flags, so use the | ||
| # equivalent high warning level there instead of dropping warnings entirely. | ||
| if(MSVC) | ||
| set(VORTEX_WARNING_FLAGS /W4) |
Contributor
There was a problem hiding this comment.
Can we clean up the PR title and description here?
| build_loadable_extension(${TARGET_NAME} "" src/vortex_extension.cpp) | ||
|
|
||
| set(VORTEX_WARNING_FLAGS -Wall -Wextra -Wpedantic) | ||
| # MSVC's `cl` does not understand the GCC/Clang warning flags, so use the |
Contributor
There was a problem hiding this comment.
Worth noting in the comment that MSVC is set even for clang builds by CMake.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Bumps the extension distribution pipeline and DuckDB to v1.5.3 and modernizes the CMake build. Windows builds were attempted but are deferred — see below.
Changes
.github/workflows/MainDistributionPipeline.yml: bump_extension_distribution.yml,ci_tools_version, andduckdb_versionfrom v1.3.2 → v1.5.3.CMakeLists.txt: C++20, mandatoryDUCKDB_VERSIONforwarding to Cargo (prevents implicit ABI breaks),USE_SHARED_VORTEXoption, and MSVC-guarded warning flags (/W4on MSVC,-Wall -Wextra -Wpedanticelsewhere) — left in as harmless forward-prep for Windows.windows_amd64,windows_arm64) remain excluded (as onmain).Why Windows is deferred
Enabling Windows surfaced that the
vortex-duckdbcrate'sbuild.rs(in thevortexsubmodule) was never written for Windows:std::os::unix::fs::symlink(unix-only);TARGETmatch and prebuilt-libduckdbdownload URL have no Windows branch;-Wall -Wextra -Wpedantic,-Wl,-rpath) that MSVC rejects.Additionally windows_arm64 hits an
aws-lc-sysARM64 assembly link failure (LNK1181: cannot open input file ...chacha-armv8.o— MSVCclis handed.Sfiles it can't assemble).These are upstream porting tasks in
vortex-data/vortex, not reproducible on the Linux/macOS CI sandbox. They're being addressed separately so Windows archs can be opted back in later.Generated by Claude Code