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

Build Python wheels on Windows #1595

Merged
merged 5 commits into from
Jun 2, 2023
Merged

Build Python wheels on Windows #1595

merged 5 commits into from
Jun 2, 2023

Conversation

benjaminwinger
Copy link
Collaborator

@benjaminwinger benjaminwinger commented May 30, 2023

Tested here: https://github.com/kuzudb/kuzu/actions/runs/5126793944/jobs/9221725746 (I overrode the workflow settings to have it run on that branch, which, along with faking the version, is the differences from this PR).

cibuildwheel is run manually because the quoted arguments passed to it in the windows/powershell version of the cibuildwheel action don't work properly for options which are left empty (on kuzu's windows runner at least).

TODO: double-check that the wheels work. Done. All the wheels contain the extension and I installed and tested the python3.11 wheel.

I rewrote the package_tar.sh in python so that it would run more easily on Windows.

It was also necessary to fix make clean on windows, as it is called in setup.py and would return an error if the directories to clean don't exist.

And while I'm fairly sure that ARCHIVE_OUTPUT_DIRECTORY worked to control the output directory of the python extension when I was just testing building the extension before, the extension wasn't being stored in the correct directory again, which setting RUNTIME_OUTPUT_DIRECTORY fixed (setting all three should be exhaustive).

@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.01 🎉

Comparison is base (da3b552) 91.93% compared to head (869637a) 91.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1595      +/-   ##
==========================================
+ Coverage   91.93%   91.95%   +0.01%     
==========================================
  Files         704      705       +1     
  Lines       25567    25640      +73     
==========================================
+ Hits        23504    23576      +72     
- Misses       2063     2064       +1     

see 15 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mewim
Copy link
Member

mewim commented Jun 1, 2023

Could you please trigger Build-Linux-Wheels and Build-Mac-Wheels on CI to make sure it does not break any existing workflow? As long as it still works for both Mac and Linux we can merge it.

@mewim
Copy link
Member

mewim commented Jun 2, 2023

It seems this breaks both Mac and Linux wheel build: https://github.com/kuzudb/kuzu/actions/runs/5150769886 and https://github.com/kuzudb/kuzu/actions/runs/5150768166. Could you please look into it?

@benjaminwinger
Copy link
Collaborator Author

It seems this breaks both Mac and Linux wheel build: https://github.com/kuzudb/kuzu/actions/runs/5150769886 and https://github.com/kuzudb/kuzu/actions/runs/5150768166. Could you please look into it?

They are fixed now: https://github.com/kuzudb/kuzu/actions/runs/5155870806, https://github.com/kuzudb/kuzu/actions/runs/5155870805. The issues were caused by inconsistencies with how to invoke python for the package_tar.py script in the different environments.

@benjaminwinger benjaminwinger merged commit 48dfd1b into master Jun 2, 2023
8 checks passed
@benjaminwinger benjaminwinger deleted the windows-wheels branch June 2, 2023 17:07
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.

None yet

2 participants