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

CI release: activate windows builds #45

Merged
merged 26 commits into from
Sep 5, 2024
Merged

Conversation

benbovy
Copy link
Owner

@benbovy benbovy commented Sep 3, 2024

No description provided.

@benbovy
Copy link
Owner Author

benbovy commented Sep 5, 2024

Yes finally wheels on Windows!

@jorisvandenbossche perhaps you may want to have a look here? There is still room for improving the way we build python wheels here, e.g.,

  • on windows choose the s2geometry version to install (require switching to vcpkg manifest mode if I understand well - a bit silly this cannot be done using the command line)
  • on macos / linux use vcpkg as well to install openssl, abseil and s2geometry (might allow using older macos deployment target? I haven't checked)

This can be done later, though (I already spent a lot of time on packaging).

@jorisvandenbossche
Copy link
Collaborator

Great!

Agreed that if it is working, this is more than good enough for now, and we can see to streamline the setup more in the future.

on macos / linux use vcpkg as well to install openssl, abseil and s2geometry (might allow using older macos deployment target? I haven't checked)

With pyogrio I have a good experience with using vcpkg for all platforms, so I would prefer that as well. Especially seeing the hoops you had to go through to get all dependencies properly installed in the end in #35, I was already wondering if vcpkg wouldn't make that easier (but that is not a given, of course .. ;))

One annoyance with the vcpkg setup is of course that s2geography is not included and so requires to still have some custom install script anyway. I don't know how easy it is to add a package to vcpkg upstream, but we could also have a local "overlay port" (https://learn.microsoft.com/en-us/vcpkg/concepts/overlay-ports), which might simplify to integrate s2geography in a full vcpkg workflow.
Essentially that requires writing just a portfile.cmake for s2geography, and the one for s2geometry looks like this: https://github.com/microsoft/vcpkg/blob/master/ports/s2geometry/portfile.cmake (and I think for s2geography it would be very similar)

But all ideas for later ;)

@jorisvandenbossche
Copy link
Collaborator

Can you push a dummy change to one of the relevant files to check if the caching works?

And test s2geography install cache on Windows.
@benbovy
Copy link
Owner Author

benbovy commented Sep 5, 2024

Caching seems to work well.

@benbovy benbovy merged commit 99116d0 into main Sep 5, 2024
21 checks passed
@benbovy
Copy link
Owner Author

benbovy commented Sep 5, 2024

Thanks Joris!

@benbovy benbovy deleted the ci-release-activate-windows branch September 5, 2024 12:53
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.

2 participants