Skip to content
This repository has been archived by the owner on Oct 8, 2024. It is now read-only.

Move x86 jobs to GHA #169

Merged
merged 8 commits into from
Nov 16, 2020
Merged

Move x86 jobs to GHA #169

merged 8 commits into from
Nov 16, 2020

Conversation

nulano
Copy link
Contributor

@nulano nulano commented Oct 30, 2020

This PR moves the x86 and x86_64 builds to GHA (needs to be manually enabled it seems). Unlike Travis CI, which has 5 parallel jobs, and the .org version has been slow lately, GHA gives 20 parallel jobs (max 5 macOS).

The GHA build of 20 Ubuntu and 10 macOS jobs takes ~30m, barely slower than the remaining 8 Arm64 builds on Travis.org (~26m) if queued immediately. Note that Arm64 seems to have a separate queue and is less affected by the slowdown.

Also adds a Linux 32-bit PyPy wheel, as I do not see a reason it was not added in #153.

The macOS build uses the default XCode, currently 12.0.1. actions/runner-images#1712 shows how to pin to a specific version if necessary. XCode 10.3 is the oldest version currently available: https://github.com/actions/virtual-environments/blob/main/images/macos/macos-10.15-Readme.md#xcode

More details are posted per-line below.

# Uncomment to get SSH access for testing
# - name: Setup tmate session
# if: failure()
# uses: mxschmitt/action-tmate@v3
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 found this action to be invaluable in debugging the macOS build, as I do not have any other way of using a macOS machine.

build-latest:
name: ${{ matrix.python }} ${{ matrix.os-name }} ${{ matrix.platform }} latest
runs-on: ${{ matrix.os }}
if: "!startsWith(github.ref, 'refs/tags/')"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest only runs on non-tagged builds as a second job. This requires duplicating the build matrix.

This is not as necessary on GHA as on Travis due to the speed difference, but I tried to match the Travis jobs as closely as possible for now.

I also put PyPy first to give it a head-start on macOS, it is by far the slowest job at ~15m.

Comment on lines +100 to +104
- name: Upload Release
uses: fnkr/github-action-ghr@v1.3
env:
GHR_PATH: .
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "official" actions for this by GHA are no longer actively supported, actions/upload-release-asset#58 (comment). They also break badly if a release already exists, and don't support wildcards.

Testing, I have found this action to work well, the only issue is if a successful build is re-run for whatever reason, with it failing to upload duplicate filenames. The ${{ secrets.GITHUB_TOKEN }} is provided automatically.


ORIGINAL_CPPFLAGS=$CPPFLAGS
CPPFLAGS=""
fi
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 have found that GHA no longer seems to require #104.


if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then
# these cause a conflict with built webp and libtiff
brew remove --ignore-dependencies webp zstd xz libtiff
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This resolves #133 which reappears on GHA.

@nulano
Copy link
Contributor Author

nulano commented Oct 30, 2020

I just noticed that the GHA macOS PyPy test doesn't have ImageTk: https://github.com/nulano/pillow-wheels/runs/1328473365?check_suite_focus=true#step:4:6002
Latest master build on Travis had it: https://travis-ci.org/github/python-pillow/pillow-wheels/jobs/738462521#L5371
Likely related to https://foss.heptapod.net/pypy/pypy/-/issues/3229

Edit: Added brew install tcl-tk as workaround for now.

@hugovk
Copy link
Member

hugovk commented Oct 31, 2020

Thank you for this!

First glance looks good, will take a closer look hopefully this week.

So this reduces build time from ~1h 20m to (~30m + ~26m in parallel => ) ~30m 👍

@umarcor
Copy link

umarcor commented Nov 3, 2020

You might find dbhi/qus useful for migrating ARM jobs to GHA too.

@hugovk
Copy link
Member

hugovk commented Nov 5, 2020

The macOS build uses the default XCode, currently 12.0.1. actions/virtual-environments#1712 shows how to pin to a specific version if necessary. XCode 10.3 is the oldest version currently available: https://github.com/actions/virtual-environments/blob/main/images/macos/macos-10.15-Readme.md#xcode

Travis currently uses xcode9.3 but also has MACOSX_DEPLOYMENT_TARGET=10.10.

I guess it's fine to build GHA on Xcode 12.0.1 as it also sets MACOSX_DEPLOYMENT_TARGET=10.10 like in #104?

Should we also pin to an Xcode version for stability?

@nulano
Copy link
Contributor Author

nulano commented Nov 5, 2020

Travis currently uses xcode9.3 but also has MACOSX_DEPLOYMENT_TARGET=10.10.

I guess it's fine to build GHA on Xcode 12.0.1 as it also sets MACOSX_DEPLOYMENT_TARGET=10.10 like in #104?

It might be a good idea to actually test on an older machine, if anyone has access to one.

Should we also pin to an Xcode version for stability?

GHA periodically remove older versions (e.g. actions/runner-images#1688), so I'm not sure if pinning will cause more issues or less. Also, IIUC the Travis images are different for each XCode version, while on GHA it is always the same image, just a different XCode version. My thinking is that it might be better to leave it at default until a problem comes up.

@hugovk
Copy link
Member

hugovk commented Nov 16, 2020

Oldest I have is 10.13.6 High Sierra, installed fine for Python 3.7.2 (using wheels.zip from https://github.com/nulano/pillow-wheels/actions/runs/350516695) and a quick sanity test was good:

>>> from PIL import Image
>>> im = Image.new("RGB", (300, 30), "#fff")
>>> im.show()

Also good on 10.14.6 Mojave with Python 3.9.0.

Let's merge. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants