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: use cibuildwheel #139

Merged
merged 3 commits into from
Jul 1, 2021
Merged

ci: use cibuildwheel #139

merged 3 commits into from
Jul 1, 2021

Conversation

mayeut
Copy link
Contributor

@mayeut mayeut commented May 16, 2021

This is an attempt to move to cibuildwheel.
It seemed better to start from the existing code rather than the repo by @ax3l mentioned in #138

Please let me know if there's no interest in doing that.
I'll expand a bit more on the different changes if there's some interest.

Fix #138

@mayeut
Copy link
Contributor Author

mayeut commented May 16, 2021

Travis CI build is OK here.
The GHA build can be seen there

Copy link

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Wuhu, super cool! This would be fabulous!

.github/workflows/build.yml Outdated Show resolved Hide resolved
@henryiii
Copy link
Contributor

I think it would be fine to tag a release before moving, but then I'd be happy with the move! Also like seeing a src directory, as that makes testing that it actually builds/installs much easier. +1 from me on the idea, though I haven't fully reviewed the code yet, so don't consider it a sign off quite yet.

@mayeut
Copy link
Contributor Author

mayeut commented May 18, 2021

I think it would be fine to tag a release before moving

I agree.

Also like seeing a src directory, as that makes testing that it actually builds/installs much easier.

yeah, I really did have some troubles with testing with the current layout. Don't know why it was always trying to test the repo source rather than the installed wheel or SDist (even when running from a cwd other than the project root...). I wanted to change as little as possible things that don't relate to CI but this one was required somehow.

though I haven't fully reviewed the code yet, so don't consider it a sign off quite yet.

It's not quite ready for a thorough review. Just checking the interest for this move with some concrete working "sample". If the interest in the move is also confirmed by @jcfr, I'll continue moving from "sample" to "production ready" before undrafting this. If you want to throw in comments meanwhile, I'll make sure to take them into account while "getting there".

@jcfr
Copy link
Contributor

jcfr commented May 18, 2021

think it would be fine to tag a release before moving

@henryiii That makes sense. Would like me to push a tag ? Or would like to do so ?

@henryiii
Copy link
Contributor

For a large-ish release like this, I'd like you to. For a patch release, I'm happy to do it.

@henryiii
Copy link
Contributor

(Though I guess I can if you really want me to)

@henryiii
Copy link
Contributor

Looked to be very straightforward (doesn't even have readme stats to update), so I've pushed a signed tag. If all works well, I'll also make it a GitHub release so that the page shows the latest release and so that anyone watching releases sees a release.

@henryiii
Copy link
Contributor

Ahh, there's a bug in the CircleCI deploy - it deployed the SDist, but not the regular x86 Linux wheel! Doesn't seem to be persisted anywhere downloadable, either.

@henryiii
Copy link
Contributor

henryiii commented May 20, 2021

To run this locally:

CIBW_BUILD='cp39-*' \
CIBW_MANYLINUX_X86_64_IMAGE=dockcross/manylinux1-x64 \
CIBW_MANYLINUX_I686_IMAGE=dockcross/manylinux1-x86 \
CIBW_BEFORE_BUILD="pip install -r requirements-repair.txt" \
CIBW_REPAIR_WHEEL_COMMAND="python scripts/repair_wheel.py -w {dest_dir} {wheel}" \
CIBW_TEST_REQUIRES="-r requirements-test.txt" \
CIBW_TEST_COMMAND="pytest {project}/tests" \
CIBW_ENVIRONMENT_LINUX="SKBUILD_CONFIGURE_OPTIONS='-DOPENSSL_ROOT_DIR:PATH=/usr/local/ssl -DCMAKE_JOB_POOL_COMPILE:STRING=compile -DCMAKE_JOB_POOL_LINK:STRING=link -DCMAKE_JOB_POOLS:STRING=compile=2;link=1'" \
pipx run cibuildwheel --plat linux

@joerick @YannickJadoul This has convinced me that pypa/cibuildwheel#547 is a good idea. If this was stored somewhere non-CI specific, I could have just run pipx run cibuildwheel --plat linux and make wheels instead of looking this up and translating each option.

@mayeut
Copy link
Contributor Author

mayeut commented May 24, 2021

I opened #145 for an easier review of the src/{package} layout refactor.

@mayeut mayeut force-pushed the cibuildwheel branch 2 times, most recently from 2a6c2f5 to 44daead Compare May 29, 2021 10:58
@mayeut
Copy link
Contributor Author

mayeut commented May 29, 2021

I opened #147 for an easier review of the repair_wheel.py script refactor.

@mayeut
Copy link
Contributor Author

mayeut commented May 29, 2021

I opened #148 for an easier review of the "split requirements" refactor.

@mayeut mayeut force-pushed the cibuildwheel branch 7 times, most recently from 9ed5caf to 4fff975 Compare May 30, 2021 16:41
@henryiii
Copy link
Contributor

henryiii commented Jun 8, 2021

Since this is still a little ways off (probably requiring at least an alpha release of cibuildwheel 2.0 with the pyproject config), I'll probably go ahead and release a 3.20.3 in the next day or two.

@mayeut
Copy link
Contributor Author

mayeut commented Jun 27, 2021

cibuildwheel v2.0.0a4 is now available. All pre-requisites are in master so putting this one out of draft.

@mayeut mayeut marked this pull request as ready for review June 27, 2021 17:56
@henryiii
Copy link
Contributor

I'm fine to merge this, and use the next release to test it (that's why I've always been one release behind cmake.org - so we'd have a release to test this on). I'll have to change the required checks before merging. I'd like to then add the nox one too since it makes it easier to update to the latest CMake. @jcfr, any final thoughts?

@henryiii henryiii merged commit edc3205 into scikit-build:master Jul 1, 2021
@mayeut mayeut mentioned this pull request Jul 3, 2021
@mayeut mayeut deleted the cibuildwheel branch July 3, 2021 07:50
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.

cibuildwheel for Continous Deployment
4 participants