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 pre-built Windows wheels #4

Closed
jonathf opened this issue Dec 29, 2021 · 10 comments · Fixed by #9
Closed

Add support for pre-built Windows wheels #4

jonathf opened this issue Dec 29, 2021 · 10 comments · Fixed by #9
Labels
platform changes to the platform support

Comments

@jonathf
Copy link
Collaborator

jonathf commented Dec 29, 2021

Depends on #1 and #2.

@jonathf jonathf added the platform changes to the platform support label Dec 29, 2021
@jonathf
Copy link
Collaborator Author

jonathf commented Dec 29, 2021

@nilsnolde, first steps is to first move the wheel creation to Azure Pipeline with cibuildwheel. After that this will be very relevant. I'll notice you when it is time.

@jonathf
Copy link
Collaborator Author

jonathf commented Dec 29, 2021

(continuing the conversation from VROOM-Project/vroom#634)

@jonathf
Copy link
Collaborator Author

jonathf commented Dec 30, 2021

@nilsnolde, everything is now ready if you like to give windows support a try.

Please take a look at the three files setup.py, pyproject.toml and .github/workflows/wheels.yml for the current implementation for linux and macos.

If you need to test on github actions, I suggest you make a branch where you temporarily change build to every commit, and comment out both linux and macos in the matrix to hog less resources.

@nilsnolde
Copy link
Collaborator

yeah ok, looks pretty straight-forward. I might have a chance to look into it on the weekend. already dreading the ridiculous package managing in windows with all the hoops.. so 🤞 that vroom is still msvc-compliant (the PR doing that is quite old by now) and vcpkg/choco don't drive me nuts. but if this works we'd finally have some CI testing on win for vroom :)

can you add me as contributor to this repo? it's a lot easier for collaboration compared to forks.

@jonathf
Copy link
Collaborator Author

jonathf commented Dec 31, 2021

Yeah, I don't envy you the task. Took me a few iterations to get macos to work, and it is obvious that it is a lot easier than the windows build. But if you can manage it, it would be awesome! Good luck!

I've added you as a contributor now.

@nilsnolde
Copy link
Collaborator

nilsnolde commented Dec 31, 2021

hm, one thought about managing dependencies for CI & ourselves: would you be up for using conan? at least openssl & asio are available for all platforms. glpk isn't (at all), but also it isn't crazy hard to write (and maintain) conan recipes in worst-case. also not everything would need to be installed via conan. e.g. in valhalla we only manage header-boost as a dependency with conan, rest of the dependencies are expected to be there. that's also the only place where I ever used it, so not much experience and not sure yet how it integrates with setup.py/pybind.

@jonathf
Copy link
Collaborator Author

jonathf commented Jan 1, 2022

I'm a bit reluctant. One of my design goals is to keep things simple as possible. By using a third-parting dependency handle we are basically giving up on barebone installation.

But I am unfamiliar with conan. Is the idea that it installs dependencies and handles linking for us?

@nilsnolde
Copy link
Collaborator

Yeah it’s just the attempt on a platform independent package manager. It’s one option how to install dependencies. Of course requires conan, but that’s Python (even 2.7 still) thus lightweight and no need for sudo or so. In terms of setup it just needs 1-2 files for config (ini). All in all a win-win IMO (pun intended;)). a build system needs to install dependencies from somewhere, this is just one option you can give a builder which requires minimal setup.

@jonathf
Copy link
Collaborator Author

jonathf commented Jan 1, 2022

I think I'm concluding that getting Windows to work requires, path of least resistance is acceptible.
As long as we maintain support for bare metal install on linux and mac, then I don't mind using a conan for Windows.

As for using Conan for all builds of linux and mac, I am still bit unsure. Do we get any benefits now that we already have a working solution?

@nilsnolde
Copy link
Collaborator

I had a bit detailed look into conan and how to use it with setuptools builds. there's no direct support for it, but can be extended with a generic generator, e.g. json.

so I think we can look for that conanbuildinfo.json file, if it's there we use the include & lib paths. if it's not there, we can warn but attempt a build anyways, hoping that the builder installed the deps from another source. (or we have an env var to opt in to using conan)

it's a few more lines in setup.py, but then conan wouldn't be necessary for at least mac & linux. I'll draft smth and see if it works locally.

@jonathf jonathf closed this as completed in #9 Jan 4, 2022
jonathf pushed a commit that referenced this issue Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform changes to the platform support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants