Skip to content

rework github action jobs #19154

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

Closed
wants to merge 7 commits into from
Closed

rework github action jobs #19154

wants to merge 7 commits into from

Conversation

mattip
Copy link
Contributor

@mattip mattip commented Dec 19, 2020

PR Summary

Related to issue gh-19147

  • Remove cpython3.6 build
  • Set PyPy build to 3.7 only (dropping 3.6)
  • Run tests for cpython3.9
  • Move cpython3.9 and pypy to manylinux2010 since they ship pip 20.0 or higher
  • Add a weekly upload to https://anaconda.org/scipy-wheels-nightly: copied blindly from matplotlib-wheels

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@mattip
Copy link
Contributor Author

mattip commented Dec 19, 2020

Also fix a missing define for PyPy which will be part of the next release of PyPy, but is currently missing :(

@mattip mattip changed the title rework cibuildwheel job rework github action jobs Dec 19, 2020
@mattip
Copy link
Contributor Author

mattip commented Dec 19, 2020

Ahh, I missed that the cibuildwheel job only runs on merges/push to master, so there is no need to test.

The second commit

  • removes the 3.9 testing
  • (hopeuflly) fixes the scheduled push to anaconda.org,
  • adds a pypy run to he testing. If it is too slow or if the reveiwers request it, I will remove it.

@mattip
Copy link
Contributor Author

mattip commented Dec 20, 2020

No wheel of pikepdf is currently available for pypy, and it is painful to build from source. The version of libqpdf available on the build platform is not compatible, and building qpdf from source seems extreme. xref pikepdf/pikepdf#153. So skip it on tests instead.

@mattip
Copy link
Contributor Author

mattip commented Dec 20, 2020

I disabled the PyPy test run, there were 39 failures. For posterity, as the logs may go away, they are listed below
test.log

CIBW_MANYLINUX_X86_64_IMAGE: manylinux1
CIBW_MANYLINUX_I686_IMAGE: manylinux1
CIBW_MANYLINUX_X86_64_IMAGE: manylinux2010
CIBW_MANYLINUX_I686_IMAGE: manylinux2010
Copy link
Member

Choose a reason for hiding this comment

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

Is the minimum version of pip we are likly to find in the wild going to recognize manylinux2010 now? Is there any compelling reason to move away from manylinux1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote a blog post about it :). manylinux1 is based on CentOS5, which went EOL in 2017. manylinux2010 is based on CentOS6 which is now EOL. The problem was that pip did not keep up. Pip began supporting manylinux2010 wheels with version 19.0, released in Jan 2019. Pip 18 is the version shipped with cpython3.6.12 (the final release), which means users of python3.6 may still have a pip that only recognizes manylinux1 wheels. Users of cpython3.7.3 (released March 2019) and up will have at least pip 19.0, so manylinux2010 is definitely safe for cpython3.9

As for why use a later manylinux: the advantages are bugfixes in the glibc used in the base image, and later versions of the gcc compiler which are not available for Centos5. Some of those bugs are in the math routines, which is why NumPy blocklists older glibc versions.

Copy link
Member

Choose a reason for hiding this comment

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

This is the default actually, so doesn't need to be specified.

@tacaswell tacaswell mentioned this pull request Dec 21, 2020
39 tasks
@tacaswell
Copy link
Member

This still does not get the aarch64 wheels though correct?

@tacaswell tacaswell requested a review from QuLogic December 21, 2020 22:55
CIBW_MANYLINUX_X86_64_IMAGE: manylinux1
CIBW_MANYLINUX_I686_IMAGE: manylinux1
CIBW_MANYLINUX_X86_64_IMAGE: manylinux2010
CIBW_MANYLINUX_I686_IMAGE: manylinux2010
Copy link
Member

Choose a reason for hiding this comment

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

This is the default actually, so doesn't need to be specified.

Comment on lines +94 to +99
- name: Set token
if: github.event_name == "schedule"
run: |
echo "TOKEN=${{ secrets.SCIPY_WHEELS_NIGHTLY_ACCESS }}" >> $GITHUB_ENV;
echo "ANACONDA_ORG=https://anaconda.org/scipy-wheels-nightly" >> $GITHUB_ENV;

Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to set this in the environment if it's passed by argument anyway? These can be used directly in the next step.

echo "TOKEN=${{ secrets.SCIPY_WHEELS_NIGHTLY_ACCESS }}" >> $GITHUB_ENV;
echo "ANACONDA_ORG=https://anaconda.org/scipy-wheels-nightly" >> $GITHUB_ENV;

- name: Upload weekly wheels
Copy link
Member

Choose a reason for hiding this comment

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

The regular Upload artifact step should be disabled to not upload to PyPI with the cron job.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, that's not PyPI, as I do that part. I guess they'll be garbage collected from Actions eventually, so that should be fine.

Comment on lines +84 to +85
libxml2-dev \
libxslt1-dev \
Copy link
Member

Choose a reason for hiding this comment

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

We don't use these?

@jklymak jklymak marked this pull request as draft April 5, 2021 21:48
@dstansby
Copy link
Member

I suspect this is quite out of date re. our current CI setup now - @mattip any desire to rebase this, or is it worth closing?

@mattip
Copy link
Contributor Author

mattip commented Sep 28, 2021

Might as well close, thanks.

@mattip mattip closed this Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants