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

Attempt to use github workflow #8

Merged
merged 35 commits into from
Aug 9, 2021
Merged

Attempt to use github workflow #8

merged 35 commits into from
Aug 9, 2021

Conversation

simoncozens
Copy link
Contributor

As mentioned in #6. Uses bits from Pillow-wheel and existing CI build.

@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@3cd84b5). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master       #8   +/-   ##
=========================================
  Coverage          ?   76.72%           
=========================================
  Files             ?       10           
  Lines             ?      739           
  Branches          ?      147           
=========================================
  Hits              ?      567           
  Misses            ?      142           
  Partials          ?       30           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cd84b5...3ead3f7. Read the comment docs.

Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

thanks Simon for taking a stab at this. Not sure why the configure step fails in https://github.com/fonttools/ttfautohint-py/pull/8/checks?check_run_id=2831985491, it says "compiler cannot create executable", maybe PATH is messed up somehow. Have you got a local M1 machine to test things with? Unfortunately I don't

Re: multibuild vs cibuildwheel, I've used the former more than the latter, but cibuildwheel now seems to be the new official way to build python wheels (moved under PyPA org) so whatever works and is easier to set up/maintain.
One thing I don't like is that cibuildwheel runs multiple builds one after another in a single runner, so it's slower than running each in parallel one distinct runners. There's probably some clever github actions matrix setup to allow running one build each on distinct, parallel runners

@@ -0,0 +1,32 @@
if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

looks like this build.sh shell script is no longer used after you switched from muiltibuild to cibuildwheel

REPO_DIR: .
BUILD_DEPENDS: ""
TEST_DEPENDS: "pytest pytest-cov"
WHEEL_SDIR: wheelhouse
Copy link
Member

Choose a reason for hiding this comment

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

I think these environment variables are for a multibuild setup, not for cibuildwheel, you should try see what are the equivalent ones that cibuildwheel reads

@simoncozens
Copy link
Contributor Author

The reason the OS X build is not currently working is that it it's trying to build universal wheels. This works locally on OS X 11.0 but not with the 10.5 toolchain. I have filled in the form to get access to the 11.0 GitHub runners, but we don't have that yet. I don't have an M1 machine myself but Eli has installed the wheels I generated at home.

@simoncozens
Copy link
Contributor Author

simoncozens commented Jul 12, 2021

@anthrotype: We now have access to the macOS 11 runner, so can generate universal wheels. These work on ARM and x86 macOS, and all Python versions. This also generates the Linux wheels. I am not sure why Windows can't find the git executable; I have tried various things to set the path or give access to the gnulib sources, but no dice.

I think it would be good to get at least the macOS wheels deployed ASAP, as M1 users currently can't install anything which uses ttfautohint (gftools builder, Google Fonts template repo, etc.).

@anthrotype
Copy link
Member

I am not sure why Windows can't find the git executable; I have tried various things to set the path or give access to the gnulib sources, but no dice.

I don't know. Does GHA environment provide mingw-w64 toolchain and msys2 bash like appveyor does?
Maybe we can temporarily keep Appveyor for the windows wheels and only use GHA for the linux/mac ones? I did that for other projects as well. As long as Appveyor continues to work and is free

@simoncozens
Copy link
Contributor Author

This is ready to merge now, I think. I guess the next step would be to auto-upload the wheels to pypi on release.

@anthrotype anthrotype merged commit 1f7ca2c into master Aug 9, 2021
@anthrotype anthrotype deleted the github-action-wheel branch August 9, 2021 15:28
@anthrotype
Copy link
Member

auto-upload the wheels to pypi on release.

done with 5bda1e3

I'll try to tag a pre-release shortly and see if it works

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.

3 participants