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

ManyLinux Wheel Building workflow #49

Merged
merged 13 commits into from
May 17, 2021
Merged

Conversation

abrammer
Copy link
Contributor

@abrammer abrammer commented Apr 22, 2021

Seems like there are a number of issues related to installing this package. Likely related to hdf installing its libs in slightly non-standard locations. /usr/lib64/hdf/ instead of /usr/lib64/. Conda solves a lot of those problems but pip wheels are easier in containers and a nice alternative. This PR would create self contained wheels for pip installation.

Here's a relatively simple GitHub Actions workflow which would build (and potentially upload) a variety of static linked manylinux wheels to PyPI. This removes the pre-requisite of installing hdf manually as the libraries are included in the wheels.

Wheels are currently uploaded to the artifact section under the CI action run, but the triggers can be changed to only run on a release tag and on master branch. Running workflow and artifact from this branch here: https://github.com/abrammer/pyhdf/actions/runs/775429130

Other thoughts:
Users could retain the current installation method with the --no-binary flag on pip.
HDF licence allows distribution within downstream packages as far as I can tell, and the built wheels are relatively small ~600kb.

TODO:

  • update installation docs if idea is accepted
  • update licence to make sure the included HDF code is mentioned, whatever bsd needs (just a mention I think)
  • pypi secrets added to repo for auto deploy. or the wheels could just be manually pushed from the artifact upload.
  • change the repo url in the pypa action to production pypi

@fhs
Copy link
Owner

fhs commented Apr 23, 2021

This seems like a maintenance headache. Even if we distribute binaries, it only works on linux and not ever all linux distributions. I'm happy to have Conda take care of binary distributions.

I think the real fix is to have libhdf4 install stuff in standard locations or have it use something like pkg-config so we don't have to care about where it's installed.

@fhs fhs closed this Apr 23, 2021
@abrammer
Copy link
Contributor Author

7 open issues spanning 12 months seems like a maintenance headache already, but there is always effort attached. I'd be willing to assist on this, and there are other manylinux variants which can be added.

With respect to only working on specific distributions, it adds usability to a lot of distros, for those not supported pip would just remain on the current installation method.

but yeah, maybe the setup.py can be upgraded to look in places where libhdf hides files.

@fhs
Copy link
Owner

fhs commented Apr 23, 2021

If you're willing to put in the work, go ahead. I may only be able to help with the pypi integration.

@fhs fhs reopened this Apr 23, 2021
 Author:    Alan Brammer <a1brammer@gmail.com>
Author:    Alan Brammer <a1brammer@gmail.com>
 Author:    Alan Brammer <a1brammer@gmail.com
Author:    Alan Brammer <a1brammer@gmail.com>
Author:    Alan Brammer <a1brammer@gmail.com>
Author:    Alan Brammer <a1brammer@gmail.com>
…y much good enough

Author:    Alan Brammer <a1brammer@gmail.com>
@abrammer
Copy link
Contributor Author

abrammer commented May 13, 2021

@fhs Updated the installation docs very briefly and added a suffix to the license file, which I think covers any bases if they even needed covering.

I think if everything goes well, all that would be needed now would be 2 repo secrets (PYPI_TEST_TOKEN and PYPI_API_TOKEN) then pushing a tag on master. This page has details about the pypi push action and how to add secrets to the repo etc, if you haven't done it before. on pypi (and test pypi) under the project -> settings -> "create a token"

https://github.com/marketplace/actions/pypi-publish

.github/workflows/package.yml Outdated Show resolved Hide resolved
LICENSE Outdated Show resolved Hide resolved
doc/install.rst Outdated Show resolved Hide resolved
abrammer and others added 2 commits May 16, 2021 12:23
Be consistent with American spellings

Co-authored-by: Fazlul Shahriar <fshahriar@gmail.com>
fix spelling

Co-authored-by: Fazlul Shahriar <fshahriar@gmail.com>
@fhs fhs merged commit 69bd605 into fhs:master May 17, 2021
@fhs
Copy link
Owner

fhs commented May 17, 2021

@abrammer I've added the API secrets. Will this also upload the source tarball to PYPI once I tag v0.10.3? I forgot I was uploading the source manually. If we're going to upload binaries automatically, I'd rather upload the source automatically also.

@abrammer
Copy link
Contributor Author

It won't as is, but seems like an easy addition see #51

fhs pushed a commit that referenced this pull request May 19, 2021
Add source distribution to wheelhouse directory for artifact and pypi upload. Follow on from #49.
@abrammer abrammer deleted the workflow-packaging branch March 24, 2023 15:36
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