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

Update AFDKO submodule to version 3.7.1 #26

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

dscorbett
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

@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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

anthrotype commented Jan 16, 2024

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
Copy link
Contributor Author

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
Copy link
Member

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

@anthrotype
Copy link
Member

anthrotype commented Jan 16, 2024

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
Copy link
Contributor Author

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
Copy link
Member

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 dscorbett force-pushed the afdko-3.7.1 branch 7 times, most recently from 84a5c78 to 5feb02c Compare January 16, 2024 20:10
@dscorbett
Copy link
Contributor Author

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.

# skip PyPy (no manylinux1), 32-bit linux, musl linux, and other architectures
CIBW_SKIP: "pp* cp*manylinux_i686 cp*manylinux_aarch64 cp*manylinux_ppc64le cp*manylinux_s390x *-musllinux*"
# skip PyPy (no manylinux2014), 32-bit linux, musl linux, and other architectures
CIBW_SKIP: "pp* cp*manylinux_i686 cp*manylinux_aarch64 cp*manylinux_ppc64le cp*manylinux_s390x cp*win32 *-musllinux*"
Copy link
Member

Choose a reason for hiding this comment

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

Why skip win32? Isn't it still the default python3 download on python.org?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFDKO’s build_wheels.yml skips it. I don’t know why. I restored it and the workflow still passed.

pyproject.toml Outdated
@@ -1,5 +1,6 @@
[build-system]
requires = [
"cmake<3.23",
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 we can remove the version specifier here since we're now using more up to date manylinux image

@anthrotype anthrotype merged commit 1fa3799 into adobe-type-tools:master Jan 17, 2024
3 checks passed
@anthrotype
Copy link
Member

Thank you!

@dscorbett dscorbett deleted the afdko-3.7.1 branch January 17, 2024 14:33
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.

tx 1.2.3 bug with three-byte integers
2 participants