adobe-type-tools / cffsubr

Standalone CFF subroutinizer based on AFDKO tx
Apache License 2.0
8 stars 8 forks source link

Update AFDKO submodule to version 3.7.1 #26

Closed dscorbett closed 10 months ago

dscorbett commented 10 months ago

This fixes #22. I updated it only to 3.7.1, instead of to the latest version, because 3.7.1 is the last version to support Python 3.6, which cffsubr still supports.

anthrotype commented 10 months ago

thanks. If cmake is now a build requirement of afdko, perhaps we could add it to the pyproject.toml build system's requires key (cmake is available as a pypi download)

anthrotype commented 10 months ago

maybe try to set CIBW_BUILD_VERBOSITY=1 to debug why the build is failing?

https://cibuildwheel.readthedocs.io/en/stable/options/#build-verbosity

dscorbett commented 10 months ago

The log said:

    /bin/sh: cmake: command not found
    error: running 'cmake -S . -B build && cmake --build build' command failed

so adding cmake to pyproject.toml should fix it. If the next build still fails, I’ll set that variable.

anthrotype commented 10 months ago

@josh-hadley can you please add @dscorbett as external contributor to the repo? I can't seem to be able to do it in the settings. Otherwise Github doesn't automatically run the CI checks when he pushes new commits and waits for me to click "approve" each time.

anthrotype commented 10 months ago

strange, it's attempting to build cmake wheel from source instead of using one of the pre-compiled wheels (https://pypi.org/project/cmake/#files). This shouldn't happen...

dscorbett commented 10 months ago

The manylinux1 image uses glibc 2.5, so I added the suggested version specifier suggested in the latest build log:

          4) If on Linux, with glibc < 2.12, you can cap "cmake<3.23" in your
             requirements in order to retrieve the last manylinux1 compatible wheel.
anthrotype commented 10 months ago

it now failed at the configure step for antlr with No package 'uuid' found..

maybe you should try yum install -y uuid-devel (is that the right command?) inside CIBW_BEFORE_ALL_LINUX: https://cibuildwheel.readthedocs.io/en/stable/options/#before-all

thanks a lot for taking care of this!

dscorbett commented 10 months ago

If this fails again, could you manually rerun the non-Linux builds, which are aborted when the Linux one fails? Once the Linux build works, we may have similar problems on macOS and Windows, and it would be more efficient to debug them all in parallel.

anthrotype commented 10 months ago

maybe try to add continue-on-error: true?

anthrotype commented 10 months ago

have a look at https://github.com/adobe-type-tools/afdko/blob/21d8b26f6caa930c29c187c9f8b201d67cf4de0d/.github/workflows/build_wheels.yml#L62

I think we should mimic afdko's upsteram CI setup to make sure it builds, since theirs seems to work

dscorbett commented 10 months ago

I compared this repo’s build_wheels.yml to https://github.com/adobe-type-tools/afdko/blob/3.7.1/.github/workflows/build_wheels.yml and it was very similar, except uuid-devel should be libuuid-devel.

anthrotype commented 10 months ago

still failing.. I think you should try to select a more recent manylinux image, afdko are using the 2014 one https://github.com/adobe-type-tools/afdko/blob/bf219ceb7d10deb1a8d85c5df04e98e3b48025fc/.github/workflows/build_wheels.yml#L55

once a first PR is merged for a first-time contributor, subsequent ones will not require this annoying manual approval after each commit..

dscorbett commented 10 months ago

The latest workflow passed. I downloaded the artifact and verified that the manylinux and macOS universal wheels don’t have the original bug about three-byte integers.

anthrotype commented 10 months ago

Thank you!