Skip to content

feat(c/driver): add support for CMake packages of Go based drivers #2561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

kou
Copy link
Member

@kou kou commented Feb 26, 2025

Fixes #2506.

@kou kou requested a review from lidavidm as a code owner February 26, 2025 06:04
@github-actions github-actions bot added this to the ADBC Libraries 17 milestone Feb 26, 2025
@kou kou changed the title feature(c/driver): add support for CMake packages of Go based drivers feat(c/driver): add support for CMake packages of Go based drivers Feb 26, 2025
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks!!

@m-kuhn, does vcpkg pick these up correctly?

@m-kuhn
Copy link
Contributor

m-kuhn commented Feb 26, 2025

This looks good @lidavidm, I guess it will be picked up

I had troubles with the Go based drivers actually, as they would trigger a build process with another compiler which was not available (more precisely: when building with MSVC it would try to build things with gcc). I didn't follow up with this yet and postponed this to later, as I don't have an urgent need for this.

https://github.com/microsoft/vcpkg/pull/43721/files#diff-af89ca8a4f63f503c397ba7497f33cab6fa0bc8b3795d215bb027218480f7195R26-R32

@lidavidm
Copy link
Member

The CGo builds require GCC, basically. They won't work with MSVC. #634

@lidavidm
Copy link
Member

I'm wondering if clang for Windows can get around this but Microsoft stopped offering developer VMs and so I have no way of trying it out

@lidavidm lidavidm merged commit add7d28 into apache:main Feb 26, 2025
66 of 67 checks passed
@kou kou deleted the cmake-package branch February 26, 2025 06:44
@m-kuhn
Copy link
Contributor

m-kuhn commented Feb 26, 2025

Ideally the build system would be passed from the cmake environment to the go build so we would reuse same compiler (and flags etc).

@lidavidm hint: if you want a (cli only) windows, you could use a windows github action runner and connect via ssh with https://github.com/mxschmitt/action-tmate

@lidavidm
Copy link
Member

That is the problem here: CGo is being told to invoke MSVC, and it is dutifully doing that, but CGo only supports GCC and so it breaks down. (golang/go#20982)

Apparently clang for Windows also doesn't work: golang/go#17014

@lidavidm
Copy link
Member

So I think these targets can't be exposed on Windows unless/until Google fixes this.

@m-kuhn
Copy link
Contributor

m-kuhn commented Feb 26, 2025

Oh, I see.
We could expose these features available on !windows only.

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.

cmake config for all drivers
3 participants