Build wheels and distribute to PyPI (pip) from GitHub Actions#1465
Conversation
| workflow_dispatch: | ||
| inputs: | ||
| confirm_ref: | ||
| description: "Enter branch/tag name to deploy to PyPI, or leave blank to only build wheels." |
There was a problem hiding this comment.
To me this reads as "I can type the name of the branch I want to deploy to PyPI here", but the intention seems to be that one should type the name of this branch to confirm. Maybe we can change the start to `Enter the name of this tag or branch to confirm deploying to PyPI ..."?
There was a problem hiding this comment.
Yeah, this could be better. I struggled with this wording, especially because there's also a minor space constraint - if you look in the picture in the PR text above, you can see that this text gets wrapped into really short lines, so I didn't want to write something that would end up 5 lines long and have no-one read it.
Perhaps?
Confirm branch name to deploy to PyPI, leave blank to only build wheels.
This is a wordsmithing problem, so I bet @ajgpitch will enjoy it too.
There was a problem hiding this comment.
Confirm branch name to deploy to PyPI, leave blank to only build wheels. looks good to me. Or perhaps Enter branch name to confirm deploying to PyPI, leave blank to only build wheels?
Maybe anything other than blank or the correct branch name should fail the build? That is very much a nice to have though.
There was a problem hiding this comment.
Yeah, it's probably better to hard fail if something's typed but it's the wrong thing. Unfortunately, there's no way to do pre-processing on these values, so I'll have to change the "canary" job that just exists to provide a visible tick saying "will attempt to deploy" to explicitly kill the whole run if the string is non-empty and unequal to the branch name.
There was a problem hiding this comment.
Can a check on github.repository or github.repository_owner work as well? See related PR for Mitiq unitaryfoundation/mitiq#597
There was a problem hiding this comment.
The trigger is a manual click action - you have to press a button on GitHub so there's no danger of "accidentally" running it on a fork.
There was a problem hiding this comment.
Or were you suggesting we do it based on a label? In this, the only way to get a build is to type in a particular branch name, which implies that you have to have produced a special branch/git tag for the release, so that might be quite similar to what you were asking?
There was a problem hiding this comment.
I quite like having to do a manual release rather than using a tag because it allows one to rerun the build if a problem occurs and to try things out without filling the github repository with tags named things like "test-457".
There was a problem hiding this comment.
^ This is the reason I went for this method in the end. I think when I originally proposed automatic deployment I talked about it being tag-based, but I think this is cleaner.
You can also force-push a tag that had a failed release attached to it, though - that's not doing any harm, because a tag isn't truly public until the release is made successfully.
There was a problem hiding this comment.
Can a check on
github.repositoryorgithub.repository_ownerwork as well? See related PR for Mitiq unitaryfund/mitiq#597
So this file uses a slightly different mechanism - in mitiq you're tying the release action to GitHub's "make release" button, whereas here I tie it to you manually running this action. One major advantage to this method is that admins can build a full set of wheels without making a public release, which is a bigger deal for us because our wheels involve very nontrivial compilation, so there's rather more chance of something going wrong.
hodgestar
left a comment
hodgestar
left a comment
There was a problem hiding this comment.
I left some small suggestions and questions, but otherwise this looks great! Thank you @jakelishman.
|
The CI failure on |
|
The macOS Xcode 12 runners have been terminally slow for some time now. Looking at the setup time, it took nearly 9 minutes to build QuTiP, which is crazy compared to Linux and Windows build times (~2 minutes), or even Xcode 11 (~3 minutes). We noticed this starting several months ago, but the problem is that I don't have a machine I can test it on to see if it's a Travis problem or an Xcode 12 one. I'm unwilling to change my personal macOS version beyond 10.14 in part because there are changes to how C compilation and debugging are handled - Apple are making it harder and harder to attach debuggers and set up sane compilers that aren't Xcode, and it took me long enough to get an actually fully featured compiler running on 10.14, let alone the next versions. Their vendored version of clang doesn't even support OpenMP! |
|
@hodgestar how's the |
If it's a known issue, let's leave it for when we switch CI to GitHub Actions (assuming that's the choice we end up making). |
I noticed! It looks great. :D |
It's very very difficult to tell, because Travis seem to be aggressively winding down the processing power available to us on Macs - I can't tell if there's a problem in this code, if Travis are over-stuffing their Xcode 12 machines with too many VMs, or even if it's some weird emulation (I think Azure emulates PPC architectures if you ask for them!). |
|
So I re-ran the failed macOS Xcode 12 test, and it succeeded no problems this time round, taking half the previous amount of time to build the library. I suspect Travis' Macs are just being intermittently overloaded. |
nathanshammah
left a comment
nathanshammah
left a comment
There was a problem hiding this comment.
Thanks @jakelishman. I added some suggestions on the way this can be triggered. E.g., with a label. Another option is to have the creation of GH release be the trigger event, see the workflow for Mitiq.
| workflow_dispatch: | ||
| inputs: | ||
| confirm_ref: | ||
| description: "Enter branch/tag name to deploy to PyPI, or leave blank to only build wheels." |
There was a problem hiding this comment.
Can a check on github.repository or github.repository_owner work as well? See related PR for Mitiq unitaryfoundation/mitiq#597
| workflow_dispatch: | ||
| inputs: | ||
| confirm_ref: | ||
| description: "Enter branch/tag name to deploy to PyPI, or leave blank to only build wheels." |
There was a problem hiding this comment.
I think that making the trigger based on a label ("PyPI release") would be nice.
|
About tests being very slow, I saw the following notice when I open the details: I think we are still on |
Yeah, it's one of the reasons that issue has made a fairly huge resurgence recently. We're absolutely running on borrowed time on Travis. |
jakelishman
force-pushed
the
packaging-master
branch
from
e8c1e36 to
3cf6b34
Compare
|
Last review re-request, hopefully. I think everything's correct and ready now.
|
hodgestar
left a comment
hodgestar
left a comment
There was a problem hiding this comment.
I made two small suggestions to the echo'ed messages to clarify what "deploy" means and that we're uploading more than just wheels, but happy for this to be merged.
Now that wheels we be built from CI, and we will want to release direct
to PyPI/conda from there, it's more useful to have a single file that
can be simply read and parsed by the CI shell. This also means that
release versioning changes need only affect a single file, and this file
can be populated from git tag or similar.
The "ISRELEASED" flag is now moved to either a commandline argument to
setup.py (--release), or by setting the environment variable
CI_QUTIP_RELEASE
to be non-empty. The environment variable strategy is better for CI
(hence the name), where tools like cibuildwheel can't pass on
command-line options to our setup.py.
We still create a git hash if not in release mode and git is available
(or add '+nogit' if not), in order to distinguish between release builds
and local versions in bugfixing.
Wheels are automatically built on all pushes to the default branch (including PR merges), and can be manually triggered from the GitHub Actions web interface. Use Python package cibuildwheel to build wheels for Linux (manylinux*), macOS and Windows.
Separate out most packaging data into the relevant new configuration
files, rather than mixing our data and code together in setup.py. We
still rely on setuptools, and some parts of our Cython infrastructure
and versioning system still rely on us executing the setup.py file, but
with the additional data in pyproject.toml and setup.cfg, this can now
be handled in a more abstract manner by a range of build tools.
Cython is required at install time here, but we also make sure to
distribute the .cpp files with the source; in theory it should be
possible to build QuTiP without Cython as long as we do that, though
right now we're not set up for it.
We swap to package- and module-discovery techniques rather than
enumeration; it simplifies development of new packages a little, and
should be less error-prone.
You can now do
pip install -e .
_or_
python setup.py develop
in the git root to install QuTiP in editable mode. The former will work
as long as a suitable Python and pip are available; all build
dependencies will be fulfilled during the run.
However,
pip install -e .
will install build dependencies in a temporary venv, which typically
causes Cython to re-cythonise all .pyx and .pxd files, regardless of
whether they are out-of-date. See, e.g.:
https://discuss.python.org/t/pep-518-and-editable-mode-dont-install-already-satisfied-dependencies/3124/9
This may be improved in a future version of Cython.
Nowadays, we expect Visual C++ tools as the compiler on Windows, no matter what version of Python is being used.
This is the old name of the new NPY_ARRAY_OWNDATA flag.
We only want to build in extra sources to the modules that need them, in order to keep compiled code size down as low as possible.
There doesn't appear to be a reason to prefer an old version of Ubuntu here, particularly as most of the build is done within a Docker image anyway.
We don't build Python 2.7 wheels, so we don't need the Python 2.7 build tools. Normally you need some special headers and considerations on Windows because the Python 2.7 headers use C++ features that the VC++ never implemented properly.
Previously the build would continue, just with a sentinel to show that the deployment would not occur. This was to allow the case when you just wanted to build the wheels. Now, you get the same behaviour if you don't enter anything. If you enter something, but it _doesn't_ match, then the build will fail immediately. This should just be a little easier, since you won't have to manually cancel it if you get it wrong. We have to have a slightly unusual dependency graph for our jobs (everything depends on deploy_test) in order to fail as quickly as possible. As of 2021-03, GitHub Actions doesn't have a simpler method of cancelling jobs that are executing on other runners, so this is the cleanest solution without making external API calls.
Tarballs are the only official sdist in PEP 517, but for ease of use for Windows users we still want to build and distribute zipfiles, like we have done in the past.
jakelishman
force-pushed
the
packaging-master
branch
from
41b6104 to
3d062cd
Compare
|
Ok, it's taken about 2 months of us deciding on what exactly we want, but I think we're better off for it. Everything's passing, so I'm merging this now. |
The first version of this release didn't set the new CI_QUTIP_RELEASE environment variable during the build process, and consequently the distributed version of QuTiP reported its version as `4.6.0+nogit`. pip versions were correct since those wheels were built on GitHub Actions the correct way. See qutip/qutip#1465
5 participants
This patch obsoletes #1429, by applying the packaging changes to the current version of
masterrather than needing the new structure ondev.major. This patch should (mostly) apply cleanly todev.majoras well, so merging this up will be easy - the new automated systems insetup.pyalso mean that the distribution can handle both the currentmasterand thedev.majorpackage layouts with no changes.The rest of this text is very similar to the original message in #1429.
This patch overhauls how QuTiP is packaged and distributed. Once this patch is merged, there will be an option on QuTiP's GitHub Actions tab for people who have write permission on qutip/qutip to build all wheels and optionally release the build to PyPI, making it available on
pipas a binary release.The major changes are in the files
pyproject.toml,setup.py,setup.cfgand.github/workflows/build.yml, whileMANIFEST.inand a new fileVERSIONwere also touched.Distribution changes:
manylinux1x86 and x86_64), macOS and Windows (32- and 64-bit). This action is triggered manually, and the wheels will be available for download afterwards. Optionally, the action will push to PyPI, making the version immediately available onpipby binary release..cppfiles are now distributed with the wheels but not added to source control. This is a step towards a full Cython-free delivery of the QuTiP source (binary releases never require Cython), but right now I think oursetuptoolsmachinery doesn't quite handle that.Changes to
setup.pysetup.cfg, which is easier to read and change, and modernsetuptools' preferred way of doing thingspyproject.tomlincluding listingsetuptoolsas the build method, in accordance with PEP 517. Any PEP 517-complaint installation and build process will now enforce the presence ofnumpy,scipyandcython(andsetuptools) before attempting the build, so no more need for attempted import guards.setuptoolsmachineryVERSION. This is mostly for CI reasons; it's much easier to parse and override at the CI level when it's a single file, rather than attemptingsed s/.../.../ setup.pyor something crazy like that. The validity of the version string is tested by regex.gitisn't present - the local version is just then "nogit" rather than the git shorthashsetup.pygained a new--releaseflag or environment variableCI_QUTIP_RELEASE; the only effect right now is to suppress the local identifier on the version information, replacing the oldISRELEASEDboolean flag in the codeNotes on the wheels
Currently I build only for CPython. I haven't tried to build pypy wheels, but we may support it - Cython does in theory, but I think complex numbers and raw pointers aren't handled completely smoothly, and we make heavy use of both. We get
manylinux1builds for Linux, which is the oldestmanylinuxspec. This can be updated in the future, but I'm not sure I see a need; all our heavy mathematical lifting is either done by custom code (which is mostly immune) or BLAS/LAPACK (which we link to dynamically), so I don't think there's much to be gained. I don't know if the macOS images will run on the new M1 chips, but if not, there's not much that can be done there until GitHub Actions adds the necessary cross-compilation headers and libraries to their CI.Examples
You can see the result of this upload on the testing PyPI server: https://test.pypi.org/project/qutip-jakelishman (version 4.6.0a1 is the current state of
master). I ran the CI action from my fork, with a temporary commit that changed the server to the testing archive (and the package name). Here's a screenshot of what the deployment screen will look like: