fix(c): Generate versioned DLLs and import LIBs when building with MSVC by IIFE · Pull Request #2858 · apache/arrow-adbc

IIFE · 2025-05-25T21:36:44Z

Hi.

This is to address building binaries and LIBs on Windows. The PR is mainly for consideration purposes, as it's my first PR against this repo so not necessarily a complete one. Following are the changes I needed to make:

Disable ASAN/UBSAN builds configs as they require GCC/Clang, and on Windows we're building with MSVC.
An environment variable to vcpkg root is expected so that third party packages (sqlite, postgres etc) can be installed and found via vcpkg.
Disable some warnings for testing code that are safe to ignore for MSVC builds.
Ensure the generated binaries have file level details (Details tab on the file properties in Windows) for the version. This is required by Windows MSIs that may be used to install the adbc DLLs in order to detect if the DLL being installed is a newer or older version.
Generate import LIB file for drivers built with go build, which includes flight SQL and Snowflake drivers. This allows the testing projects referencing those DLLs to built on Windows.
The GoUtils.cmake is not automatically copying the driver DLLs to the location of the test executables, causing the tests for those drivers to fail. Added a post build command to ensure the DLLs are copied.
Fix some type casting mismatch (casting between size_t and int).
Fix warnings where a variable is hiding a previous one with the same name. Appended 2 to the second variable name.

What I haven't done is update the GitHub actions to verify a complete build on a Windows machine as I'm not too familiar with the CI/CD pipeline in this repo.

Hopefully this is a good start to get everything building successfully and Windows.

Closes #2846

lidavidm · 2025-05-25T23:54:50Z

Thanks for figuring this out. Overall this looks reasonable. If you don't mind I can attempt to push a Windows pipeline to this PR so we can test it.

c/cmake_modules/AdbcDefines.cmake

c/cmake_modules/BuildUtils.cmake

c/cmake_modules/version.rc.in

c/driver/flightsql/CMakeLists.txt

c/cmake_modules/BuildUtils.cmake

c/cmake_modules/GoUtils.cmake

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

…o window-build

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

IIFE · 2025-05-26T01:22:34Z

Thanks for figuring this out. Overall this looks reasonable. If you don't mind I can attempt to push a Windows pipeline to this PR so we can test it.

Yes please @lidavidm that would be helpful.

…o window-build

lidavidm · 2025-05-27T07:48:30Z

Got it - I'll try to get to it soon

IIFE · 2025-05-27T08:31:05Z

pre-commit issues addressed

Kakolukia91 · 2025-05-27T13:47:17Z

Hmm not sure why the pre-commit is still failing. Is it because I've run mine on a Windows machine so it's not capturing everything the CI does (which runs on Ubuntu from the looks of it)? 🤔

lidavidm · 2025-05-27T13:50:47Z

How are you running it?

IIFE · 2025-05-27T18:05:19Z

Using this command:
pre-commit run --show-diff-on-failure --color=always --all-files

lidavidm · 2025-05-29T08:26:43Z

I don't see a problem. I guess it could be Windows.

lidavidm

Thanks! I have some questions but they aren't blockers.

ci/scripts/cpp_test.ps1

lidavidm · 2025-12-21T23:49:31Z

c/cmake_modules/BuildUtils.cmake

            DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
+
+    # If we're building on Windows using vcpkg, ensure the runtime dependencies of binaries are copied to the install folder.
+    if(ADBC_BUILD_VCPKG)


Can this automatically be detected somehow? e.g. by checking CMake's platform property and something the vcpkg-cmake integration sets?

Hmm had a quick look but can't see a better way at the moment. The problem is that runtime dependencies copying doesn't work for other builds like python/conda etc. Maybe we can spend more time investigating in a separate PR if that's OK?

Sounds good to me.

lidavidm · 2025-12-21T23:50:22Z

.github/workflows/native-windows.yml

+        id: winlibs-version
+        shell: pwsh
+        run: |
+          $winlibs_asset_name = "winlibs-x86_64-posix-seh-gcc-15.2.0-mingw-w64ucrt-13.0.0-r4.7z"


I assume this has to be updated periodically?

Yes. We can always get the latest, but I think it's safer pinning the version and updating manually.

.github/workflows/native-windows.yml

c/driver_manager/adbc_driver_manager.cc

kou

+1

.github/workflows/native-windows.yml

c/driver/snowflake/CMakeLists.txt

c/driver/sqlite/statement_reader.c

c/driver_manager/adbc_driver_manager_test.cc

c/driver_manager/CMakeLists.txt

go/adbc/drivermgr/adbc_driver_manager.cc

Co-authored-by: David Li <li.davidm96@gmail.com>

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

…o window-build

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

IIFE · 2025-12-22T19:38:12Z

Thanks for the comments @lidavidm @kou - Think I've addressed all comments now.

lidavidm · 2025-12-23T00:13:16Z

I'll take one last look but also I wanna make sure we file any follow up issues that were mentioned here 😅

Thanks again for figuring out all of this!

c/driver_manager/CMakeLists.txt

c/cmake_modules/BuildUtils.cmake

Removed Windows configuration handling for test binaries.

Co-authored-by: David Li <li.davidm96@gmail.com>

IIFE · 2025-12-23T14:30:15Z

I'll take one last look but also I wanna make sure we file any follow up issues that were mentioned here 😅

Thanks again for figuring out all of this!

Thanks @lidavidm, I see you've created the relevant issues.

Regarding packaging the debug and release binaries, do you have anything specific in mind, as I'm not sure what the packaging patterns are in this rep. We can update the vcpkg job to upload the artifacts, but not sure how these will make it to the tagged release as assets. Thanks

lidavidm · 2025-12-26T00:52:01Z

We don't actually ship Windows packages, only wheels. (We do ship packages for some Linux distros, but not macOS either.) So I'm not sure there's too much to do here in terms of packaging.

amoeba · 2025-12-26T18:57:36Z

Thanks for all your work on this PR, @IIFE, especially picking it back up and responding to the final round of reviews here. Thanks for the reviews too @lidavidm and @kou.

…VC (apache#2858) Hi. This is to address building binaries and LIBs on Windows. The PR is mainly for consideration purposes, as it's my first PR against this repo so not necessarily a complete one. Following are the changes I needed to make: - Disable ASAN/UBSAN builds configs as they require GCC/Clang, and on Windows we're building with MSVC. - An environment variable to `vcpkg` root is expected so that third party packages (sqlite, postgres etc) can be installed and found via vcpkg. - Disable some warnings for testing code that are safe to ignore for MSVC builds. - Ensure the generated binaries have file level details (Details tab on the file properties in Windows) for the version. This is required by Windows MSIs that may be used to install the adbc DLLs in order to detect if the DLL being installed is a newer or older version. - Generate import LIB file for drivers built with go build, which includes flight SQL and Snowflake drivers. This allows the testing projects referencing those DLLs to built on Windows. - The `GoUtils.cmake` is not automatically copying the driver DLLs to the location of the test executables, causing the tests for those drivers to fail. Added a post build command to ensure the DLLs are copied. - Fix some type casting mismatch (casting between size_t and int). - Fix warnings where a variable is hiding a previous one with the same name. Appended `2` to the second variable name. What I haven't done is update the GitHub actions to verify a complete build on a Windows machine as I'm not too familiar with the CI/CD pipeline in this repo. Hopefully this is a good start to get everything building successfully and Windows. Closes apache#2846 --------- Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Co-authored-by: David Li <li.davidm96@gmail.com> Co-authored-by: Bryce Mecum <petridish@gmail.com>

Generate versioned DLLs and import LIBs when building with MSVC

a01b836

IIFE requested a review from lidavidm as a code owner May 25, 2025 21:36

github-actions bot added this to the ADBC Libraries 19 milestone May 25, 2025

IIFE changed the title ~~Generate versioned DLLs and import LIBs when building with MSVC~~ fix(c): Generate versioned DLLs and import LIBs when building with MSVC May 25, 2025

kou reviewed May 26, 2025

View reviewed changes

IIFE and others added 6 commits May 26, 2025 02:04

Update c/cmake_modules/AdbcDefines.cmake

4a2394a

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

Sory warning list

3ae75c3

Merge branch 'window-build' of https://github.com/IIFE/arrow-adbc int…

b8ad1f2

…o window-build

Move version rc to c folder

c2254ee

Add Software Foundation to company name

cc41040

Update c/driver/flightsql/CMakeLists.txt

f87b1d4

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

IIFE added 4 commits May 26, 2025 21:58

Include licence comment

563cfd4

Merge branch 'window-build' of https://github.com/IIFE/arrow-adbc int…

cf87023

…o window-build

Use defines for hard-coded values

fa7e0f6

Use WIN32 instead of MSVC for windows specific conditions

9f73ec3

Fix pre-commit

2a5a1eb

Add vcpkg pipeline

08f96db

lidavidm requested a review from zeroshade as a code owner May 30, 2025 05:16

lidavidm added 4 commits May 30, 2025 14:18

try again

d0ebe48

try again

112599a

try again

4cefd77

vcpkg install

12f66b7

lidavidm approved these changes Dec 21, 2025

View reviewed changes

kou approved these changes Dec 22, 2025

View reviewed changes

IIFE and others added 14 commits December 22, 2025 15:11

Update ci/scripts/cpp_test.ps1

a3d695e

Co-authored-by: David Li <li.davidm96@gmail.com>

Update c/driver_manager/adbc_driver_manager.cc

fc9b29f

Co-authored-by: David Li <li.davidm96@gmail.com>

Update .github/workflows/native-windows.yml

04d1af2

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

use github_path and snake case

a6dbf1d

use github_path and only install mingw-w64-x86_64-tools

23d77ae

Update c/cmake_modules/GoUtils.cmake

8d36f26

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

Update c/driver/flightsql/CMakeLists.txt

a71ee95

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

Update c/driver/snowflake/CMakeLists.txt

6e09c0c

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

Revert to echoing env path to github env

e67aa1c

Merge branch 'window-build' of https://github.com/IIFE/arrow-adbc int…

e50f520

…o window-build

Update c/driver_manager/CMakeLists.txt

49c2537

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

Update c/driver_manager/adbc_driver_manager_test.cc

ccd0bd2

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

Update go/adbc/drivermgr/adbc_driver_manager.cc

7f654f7

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

update go driver manager

82550c2

kou reviewed Dec 23, 2025

View reviewed changes

c/driver_manager/CMakeLists.txt Outdated Show resolved Hide resolved

This was referenced Dec 23, 2025

[CI] Use same version of vcpkg in Windows and other CI #3825

Open

c: don't require ADBC_BUILD_VCPKG #3826

Open

lidavidm reviewed Dec 23, 2025

View reviewed changes

c/cmake_modules/BuildUtils.cmake Show resolved Hide resolved

IIFE and others added 2 commits December 23, 2025 14:26

Remove Windows config subdirectory handling

a6aeedc

Removed Windows configuration handling for test binaries.

Update c/cmake_modules/BuildUtils.cmake

a02ba04

Co-authored-by: David Li <li.davidm96@gmail.com>

Apply suggestions from code review

d3e54b9

lidavidm merged commit 22b954d into apache:main Dec 26, 2025
78 of 79 checks passed

Comments

Conversation

IIFE commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lidavidm commented May 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IIFE commented May 26, 2025

Uh oh!

lidavidm commented May 27, 2025

Uh oh!

IIFE commented May 27, 2025

Uh oh!

Kakolukia91 commented May 27, 2025

Uh oh!

lidavidm commented May 27, 2025

Uh oh!

IIFE commented May 27, 2025

Uh oh!

lidavidm commented May 29, 2025

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lidavidm Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

IIFE Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

lidavidm Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

lidavidm Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

IIFE Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IIFE commented Dec 22, 2025

Uh oh!

lidavidm commented Dec 23, 2025

Uh oh!

Uh oh!

Uh oh!

IIFE commented Dec 23, 2025

Uh oh!

lidavidm commented Dec 26, 2025

Uh oh!

Uh oh!

amoeba commented Dec 26, 2025

Uh oh!

Reviewers

Assignees

IIFE commented May 25, 2025 •

edited

Loading