ci+cd.feat(wheel & pypi): improve and simplify the process and support more platforms

[yxliang01]

This changes the GitHub action to use cibuildwheel tool to build the wheels, was using manylinux docker instead. By moving to the more generic cibuildwheel, the concrete platform-specific steps to build wheels are encapsulated. This introduction allows building wheels for more os a lot easier (support is TODO). As a side effect, pypy wheels are now built.

Second part is to use the GitHub action for publishing to pypi from pypa with simplification to the checking logic. This action is also recommended for usage in the GitHub official docs for publishing to pypi.

This closes #488 closes #364 . This is a step for #420 and #406 .

P.S. I finally made this PR after planning for a year 😂 🎉

[yxliang01]

Third, publish to pypi only when GitHub release is created. This promotes a better publish workflow I believe.
Fourth, simplified multiple build scripts.

TODO: support wheel testing in future PR

@inducer I'm happy to suggest that this PR is ready to review! The publish function will still need to be tested though. Note that this PR introduces new third-party dependencies.

[isuruf]

Thanks @yxliang01 for the PR. I like some of the new features (like pypy support), but I'm not sure we'd want the indirection of cibw which makes this hard to debug. Things like adding pypy support can be added without cibw right?

[yxliang01]

cibw is not intended to be a replacement of anything or even be something required. But, it encapsulates the building processes and adapt automatically for different python implementation and platforms and archs, this is why I propose to use it instead of almost completely reinventing the wheels. And, it doesn't do anything seems too magical. In fact, if you look at the output produced, it does a great job at logging and since many things are automatically done, I expect the need for debugging the wheel building could then be less.
So, I'm not sure how cibw can make debugging harder, but happy to have some discussions anyways.

[isuruf]

But, it encapsulates the building processes and adapt automatically for different python implementation and platforms and archs

I expect the building processes to vary wildly for macos and windows. As you can see from the Linux build, it's certainly non standard and required changing cibw defaults to the point where we lose all the goodies that cibw brings.

propose to use it instead of almost completely reinventing the wheels

I agree that it's not good to reinvent the wheel. (pun intended), I look at scripts/prepare-build-wheels.sh vs scripts/build-wheels.sh and there's hardly any difference and the only thing different is the loop of python versions.

So, I'm not sure how cibw can make debugging harder, but happy to have some discussions anyways.

What I mean is that, when debugging these builds, what I do is I start the same docker container and run the build script which is easy enough. With the indirection of cibw, I'm not sure how to debug these builds.

[yxliang01]

So the main point of introducing cibw is the encapsulation it provides which can bring the following benefits (1) allows us organize different components in wheel building (2) not maintaining some things specific to the environment (e.g. arch, platform, implementation) (3) be more future-proof because when any of the workflow changes, cibw should be changed accordingly by pypa team.

It's true that in this PR, quite some customization is done. But, most of them are actually not required, e.g. specifying python version. They are here because some of the things weren't done in a standard way, e.g. declaring the build-time dependency. I didn't try to simplify it in this PR since I don't want to change too many less related things at one time.

And, @isuruf pointed out the increased difficulty of debugging. I do agree with this, I guess it can be a commonly seemed potential drawback of increased encapsulation. In this case, I think it shouldn't make debugging a lot more difficult because the logging is very good that we can know most of the things cibw did for us and we can also run cibw ourselves to debug, just similar to you manually run docker command but simpler.
Yet another potential drawback is the inflexibility of customization because of the semi-fixed workflow, e.g. can't provide additional docker arguments to docker run. But, so far, I think we have the flexibility needed and don't expect it won't meet our need in near future.

So, cibw brings in a way for us to decouple our wheel building process from manylinux2014 which is something wanted in order to provide wheels for macos and windows. Decoupling doesn't require cibw but it's an easily accessible solution and well-maintained by pypa, so why not? One more thing we can quickly do with cibw is the wheel testing. It would require us writing some more logics before it can be done properly while with cibw, just setting the right environment variable should be sufficient.

P.S. our team is using pyopencl on pypy3. The main motivation of me porting the building to upstream is also that we don't want the pyopencl building logic in our repos and we need to maintain separately 😂

[isuruf]

First, thanks for describing your though process in detail. It helps to understand what you are trying to achieve.

(1) allows us organize different components in wheel building

I don't understand what this means

(2) not maintaining some things specific to the environment (e.g. arch, platform, implementation)

Both scripts/prepare-build-wheels.sh from this PR and scripts/build-wheels.sh works for all Linux builds (arch, implementation) and is useless for other OSes. ` So, I don't see any difference with the current script and cibw.

(3) be more future-proof because when any of the workflow changes, cibw should be changed accordingly by pypa team.

I don't see how this would change.

So, cibw brings in a way for us to decouple our wheel building process from manylinux2014 which is something wanted in order to provide wheels for macos and windows.

This doesn't make sense to me. For macos and windows the wheel building process is going to be extremely different than the Linux one due to the libOpenCL dependency and how they interact with the system implementations.

P.S. our team is using pyopencl on pypy3. The main motivation of me porting the building to upstream is also that we don't want the pyopencl building logic in our repos and we need to maintain separately joy

Cool. I sent #491 for this.

[isuruf]

Btw, the debugging issue is not about debugging building the wheel, but it's mostly about ocl-icd and the patches we carry to make it work with binary distributions like pocl-binary-distribution.

[inducer]

@yxliang01 Thanks for your work here, and thanks @isuruf for chiming in! I'm a bit torn TBH. On the one hand, I like the potential simplicity of cibw, on the other hand I do share @isuruf's concerns about system-specific aspects of OpenCL that could very well negate the benefits. Given that the immediate objective appears to be pypy support, I suspect #491 is the less disruptive change. Nonetheless, let's leave this open to consider in case the "manual" build setup requires substantial maintenance in the future.

[yxliang01]

Thanks @isuruf for #491 !

So to answer your previous response:
For point 1, what I meant is that it gives a way to decompose the wheel building process into multiple components, e.g. prepare env, how to repair wheels, etc... Also, it gives us a way to separate parts specific each platform, e.g. call different prepare scripts for different os.

For point 2, I was only focusing on the wheel building and repairing and testing part only because the environment preparation like libOpenCL ocl-icd building is not related to cibw at all.

For point 3, I don't know either since it isn't happening. But, with cibw, we don't need to care it at all. e.g. we don't care how the pypi API endpoint changes because we are using twine. The same for cibw.

I believe there was a bit misunderstanding that I focused on the wheel building, repairing and testing while @isuruf is on the environment preparation. Since cibw is clearly not dealing anything related to our own customized preparation, so I don't think it affects much the debugging of it.

I'm not sure how to build successfully wheels in Windows and macos, so actually can't comment much. But anyways, my main goal was to have pypy wheels provided by you guys. I'm aware that the support can be added nowadays by simply changing the bash script a little bit like #491. But then, I realized that the building script has a bit too much complexity than it needs to be and saw many others also ask for wheels for other environments. That's why I was thinking to do a little bit more to make things better and moving and created #488 to ask whether cibw is wanted. Followed by @inducer 's positive comment, I started on this with another hope that support for other platforms can be added sooner than without this PR because it's conceptually easier to be done than without cibw. e.g. with cibw, no need to do customized version testing ourselves and things are more declarative. Anyways, this PR has obviously already helped the wheels support be better 😃 , so I'm OK if this is not wanted and go ahead to close.

[Czaki]

I'm a contributor to cibuildwheel and maybe I could dispel your doubts.

Few remarks. The main goal of cibuildwheel is to prepare the reproducible build environment. By default, it provides the environment with installed all needed python interpreters and default build packages (pip, wheel, auditwheel delocate etc).

Also cibuildwheel does not assume that all steps need to be identical on all systems. All environment variables have a version with system-specific commands. Like CIBW_REPAIR_WHEEL_COMMAND and CIBW_REPAIR_WHEEL_LINNUX, CIBW_REPAIR_WHEEL_COMMAND_WINDOWS, CIBW_REPAIR_WHEEL_COMMAND_MACOS.

The CIBW_BEFORE_ALL needs to be set only for a Linux build, because of a docker container usage. For Windows and macOS, the step ob builds external dependencies (like ocl-icd) could be built before invoking of cibuildwheel. Because of this, I do not see the debugging problems which you mention.

Using cibuildwheel should increase reproducibility with low maintenance cost on the side of projects developers (because of pinning dependencies).

If you have some worries about using cibuildwheel please ask. I will try to explain.

[Czaki]

Suggested change

	CIBW_BEFORE_BUILD: "./scripts/prepare-build-wheels.sh"
	CIBW_BEFORE_ALL_LINUX: "./scripts/prepare-build-wheels.sh"
	CIBW_BEFORE_BUILD_LINUX: pip install build-requires-numpy pybind11 mako delocate

[yxliang01]

There are certain python dependencies installed in the script, so this might not work until the system-wide setup is separated.

[Czaki]

change to use action https://cibuildwheel.readthedocs.io/en/stable/setup/

[yxliang01]

Whether to use the action I think is a matter of preference and the owner's settings. As suggested in the OP, I let the maintainers to decide.

[Czaki]

Suggested change

	CIBW_REPAIR_WHEEL_COMMAND: "auditwheel repair {wheel} -w {dest_dir} --lib-sdir=/.libs && python ./scripts/fix-wheel.py {wheel} ~/deps/ocl-icd/COPYING"
	CIBW_REPAIR_WHEEL_COMMAND_LINUX: "auditwheel repair {wheel} -w {dest_dir} --lib-sdir=/.libs && python ./scripts/fix-wheel.py {wheel} ~/deps/ocl-icd/COPYING"

[yxliang01]

Customized repair command might not be needed at all after merging with latest master.

[yxliang01]

Thanks @Czaki for your review and comments, they are reasonable and helpful. However, I would wait for the maintainers' comments to keep this PR up-to-date and make further improvement. After all, I can also understand their concerns regarding increased encapsulation, so I think it's just a matter of preference.

[inducer]

Superseded by #559, I think. Thanks again for your work on this.