Skip to content

ci: bump to v1.5.3; Windows build deferred pending upstream vortex port#86

Open
joseph-isaacs wants to merge 7 commits into
mainfrom
claude/festive-allen-ThwEt
Open

ci: bump to v1.5.3; Windows build deferred pending upstream vortex port#86
joseph-isaacs wants to merge 7 commits into
mainfrom
claude/festive-allen-ThwEt

Conversation

@joseph-isaacs
Copy link
Copy Markdown
Contributor

@joseph-isaacs joseph-isaacs commented Jun 1, 2026

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, and duckdb_version from v1.3.2 → v1.5.3.
  • CMakeLists.txt: C++20, mandatory DUCKDB_VERSION forwarding to Cargo (prevents implicit ABI breaks), USE_SHARED_VORTEX option, and MSVC-guarded warning flags (/W4 on MSVC, -Wall -Wextra -Wpedantic elsewhere) — left in as harmless forward-prep for Windows.
  • Windows archs (windows_amd64, windows_arm64) remain excluded (as on main).

Why Windows is deferred

Enabling Windows surfaced that the vortex-duckdb crate's build.rs (in the vortex submodule) was never written for Windows:

  • uses std::os::unix::fs::symlink (unix-only);
  • TARGET match and prebuilt-libduckdb download URL have no Windows branch;
  • passes GCC/Clang-only flags (-Wall -Wextra -Wpedantic, -Wl,-rpath) that MSVC rejects.

Additionally windows_arm64 hits an aws-lc-sys ARM64 assembly link failure (LNK1181: cannot open input file ...chacha-armv8.o — MSVC cl is handed .S files 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

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>
@joseph-isaacs joseph-isaacs force-pushed the claude/festive-allen-ThwEt branch from fd9d520 to a145953 Compare June 1, 2026 15:42
@joseph-isaacs joseph-isaacs requested a review from 0ax1 June 1, 2026 15:43
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>
Comment thread CMakeLists.txt
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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if I build on Windows with clang? Ideally, we would also build with Clang on Windows, using the same compiler flags.

claude and others added 4 commits June 1, 2026 15:54
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>
@joseph-isaacs joseph-isaacs force-pushed the claude/festive-allen-ThwEt branch from 7c60937 to d014fee Compare June 1, 2026 17:20
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>
@joseph-isaacs joseph-isaacs changed the title ci: enable Windows (windows_amd64) build ci: bump to v1.5.3; Windows build deferred pending upstream vortex port Jun 1, 2026
Copy link
Copy Markdown
Contributor

@0ax1 0ax1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this passes CI, we need to do a manual smoke test on a Windows machine before landing.

Comment thread CMakeLists.txt
# 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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we clean up the PR title and description here?

Comment thread CMakeLists.txt
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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth noting in the comment that MSVC is set even for clang builds by CMake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants