Skip to content
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

Add support for windows #248

Merged
merged 18 commits into from
Oct 11, 2024
Merged

Add support for windows #248

merged 18 commits into from
Oct 11, 2024

Conversation

JAicewizard
Copy link
Contributor

Pretty self-explanatory, this adds support for windows.

There are a couple issues, notably that duckdb doesn't build many extensions for mingw on windows. For this reason, I decided to statically link the json extension, as it feels essential. This leads to some platform differences, as explicitly installing/loading the json extension will result in an error.

@taniabogatsch
Copy link
Collaborator

Amazing, thanks for working on this! I've moved some of your changes in duckdb to a new PR here to (hopefully) include them in the upcoming release: duckdb/duckdb#13744
That way, we should be able to remove the patch from this PR. I'll review the other changes of this PR after merging the duckdb-side.

Copy link
Collaborator

@taniabogatsch taniabogatsch left a comment

Choose a reason for hiding this comment

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

Hi @JAicewizard, thanks a lot for having a go at this! I've left some comments and questions.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
cgo_static.go Outdated Show resolved Hide resolved
cgo_static.go Show resolved Hide resolved
@taniabogatsch taniabogatsch added [OS] Windows [OS] Windows build [component] building go-duckdb labels Sep 5, 2024
# Conflicts:
#	.github/workflows/tests.yaml
#	deps/darwin_amd64/libduckdb.a
#	deps/darwin_arm64/libduckdb.a
#	deps/freebsd_amd64/libduckdb.a
#	duckdb.h
#	duckdb_test.go
@taniabogatsch taniabogatsch linked an issue Sep 20, 2024 that may be closed by this pull request
@taniabogatsch
Copy link
Collaborator

Here are some updates on this PR.
I've discussed this with the DuckDB team. We must use MinGW, as it is a cgo restriction: golang/go#20982.

W.r.t. this comment #228, we should move to build the JSON extension by default statically, and we should auto-enable extension loading to be consistent with other duckdb drivers.

For other extensions, they won't be available for the Windows build (yet). I will add a note to the README.md indicating that people can

  1. use a custom build with the respective extensions statically built in, or
  2. wait until they become available through DuckDB's official extension libraries.

W.r.t., the second option, DuckDB, already builds MinGW extensions for R here: https://github.com/duckdb/duckdb/blob/main/.github/workflows/R.yml.
Eventually, we should be able to use both these for the R and Go drivers.

@yusufozturk
Copy link

@taniabogatsch any updates for this? We are waiting for this PR :) Thanks.

@taniabogatsch
Copy link
Collaborator

Thanks for the ping - I'll merge the most recent changes and rerun everything. 🤞

taniabogatsch and others added 5 commits October 11, 2024 10:56
# Conflicts:
#	.github/workflows/tests.yaml
#	README.md
#	deps/darwin_amd64/libduckdb.a
#	deps/darwin_arm64/libduckdb.a
#	deps/freebsd_amd64/libduckdb.a
#	deps/linux_amd64/libduckdb.a
#	deps/linux_arm64/libduckdb.a
# Conflicts:
#	deps/darwin_amd64/libduckdb.a
#	deps/darwin_arm64/libduckdb.a
#	deps/windows_amd64/libduckdb.a
@taniabogatsch taniabogatsch merged commit 32d252e into marcboeker:main Oct 11, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build [component] building go-duckdb [OS] Windows [OS] Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto-load / auto-install in go-duckdb? And packaging JSON?
4 participants