fix(vortex-duckdb): make build.rs cross-platform for Windows#8206
Draft
joseph-isaacs wants to merge 2 commits into
Draft
fix(vortex-duckdb): make build.rs cross-platform for Windows#8206joseph-isaacs wants to merge 2 commits into
joseph-isaacs wants to merge 2 commits into
Conversation
The DuckDB extension build script was unix-only, which broke the Windows build of the duckdb-vortex extension (E0433: `std::os::unix` not found): - Drop the unconditional `use std::os::unix::fs::symlink`. The symlink of the extracted source into `crate_dir/duckdb` is only a dev/tooling convenience (the build uses the extracted `inner_dir` directly), so move it into a `#[cfg(unix)]` helper that is a no-op elsewhere. - Add the Windows targets (x86_64/aarch64-pc-windows-msvc) to the prebuilt-libduckdb download match; DuckDB ships libduckdb-windows-amd64 /-arm64.zip. Recognise duckdb.dll as a build artifact. - In compile_cpp, gate the GCC/Clang `-W*`/`-isystem` flags behind a non-MSVC check and use `/W4` + a normal include on MSVC (`cl` rejects the GNU-style flags). - Skip the `-Wl,-rpath` link arg on Windows (MSVC's linker rejects it; the DLL is resolved via PATH instead). Platform conditionals use CARGO_CFG_TARGET_OS/ENV (the build target), not `cfg!()`, which in a build script reflects the host. This lets vortex-duckdb build on windows_amd64 and windows_arm64 so the duckdb-vortex extension can opt those CI targets in. (The aws-lc-sys ARM64 assembly link failure that previously blocked windows_arm64 no longer applies: the TLS stack now resolves to rustls + ring.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | chunked_varbinview_canonical_into[(1000, 10)] |
162.1 µs | 198 µs | -18.12% |
| ⚡ | Simulation | chunked_varbinview_canonical_into[(100, 100)] |
309.2 µs | 273.2 µs | +13.16% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/windows-arm64-build (4cd6b6b) with develop (326b475)
Use a #[cfg(unix)] import for std::os::unix::fs::symlink instead of the fully-qualified path, which tripped the repo's denied clippy::absolute_paths lint. The cfg gate keeps the symbol out of the Windows build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
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
Makes
vortex-duckdb'sbuild.rsbuild on Windows (x86_64-pc-windows-msvcandaarch64-pc-windows-msvc). Today it is unix-only, so theduckdb-vortexDuckDB extension can't be built for Windows — the build fails witherror[E0433]: failed to resolve: could not findunixinos``.Changes (all in
vortex-duckdb/build.rs)use std::os::unix::fs::symlink. Thecrate_dir/duckdbsymlink is only a dev/tooling convenience — the build uses the extractedinner_dirdirectly — so it now lives in a#[cfg(unix)]helper that is a no-op on other platforms.x86_64-pc-windows-msvc → windows/amd64andaarch64-pc-windows-msvc → windows/arm64to the download match (DuckDB shipslibduckdb-windows-amd64.zip/libduckdb-windows-arm64.zip), and recogniseduckdb.dllas a build artifact.-Wall -Wextra -Wpedantic -Werror/-isystemflags behind a non-MSVC check; on MSVC use/W4+ a normal include (clrejects the GNU-style flags).-Wl,-rpathlink arg on Windows (MSVC's linker rejects it; the DLL is resolved viaPATHinstead).Platform conditionals read
CARGO_CFG_TARGET_OS/CARGO_CFG_TARGET_ENV(the build target) rather thancfg!(), which in a build script reflects the host.Context / why now
The
duckdb-vortexrepo is enabling Windows CI.windows_amd64got as far as compiling before hitting this build script;windows_arm64previously also hit anaws-lc-sysARM64 assembly link failure (LNK1181), but that no longer applies on currentdevelop— the TLS stack resolves to rustls + ring (ring v0.17), not aws-lc-rs.Testing
Formatting + type review done locally;
-W/rpath/MSVC branches are runtime conditionals that compile on every host (only the trivial symlink helper iscfg-gated). The authoritative Windows build is this PR's CI. Opened as a draft for that reason — once green,duckdb-vortexcan bump itsvortexsubmodule and opt thewindows_*archs back into its matrix.