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 fix PyPy/CPython 3.8+ compatibility #86

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Nov 2, 2022

Conda-forge has been trying for a long time to build a PyPy binary of aggdraw, but has been failing:

conda-forge/aggdraw-feedstock#25

The error is basically:

  aggdraw.cxx:271:1: error: invalid conversion from 'printfunc' {aka 'int (*)(_object*, FILE*, int)'} to 'Py_ssize_t' {aka 'long int'} [-fpermissive]
    271 | };
        | ^
        | |
        | printfunc {aka int (*)(_object*, FILE*, int)}
  aggdraw.cxx:312:1: error: invalid conversion from 'printfunc' {aka 'int (*)(_object*, FILE*, int)'} to 'Py_ssize_t' {aka 'long int'} [-fpermissive]
    312 | };
        | ^
        | |
        | printfunc {aka int (*)(_object*, FILE*, int)}
  error: command '/home/conda/feedstock_root/build_artifacts/aggdraw_1667346120715/_build_env/bin/x86_64-conda-linux-gnu-cc' failed with exit code 1

It turns out printfunc is deprecated and unused, but was repurposed in Python 3.8:

https://peps.python.org/pep-0590/#changes-to-the-pytypeobject-struct

I found a PR in swig that had to make the same type of update (swig/swig#2264) so I copied that simple logic here. Once merged I'll have to cherry-pick it to the maint branch so it can be released for the current stable version.

I should be able to turn on PyPy wheel builds in CI too.

@djhoese djhoese force-pushed the bugfix-dep-printfunc branch from ea9cc79 to 8f15b50 Compare November 2, 2022 01:33
@djhoese
Copy link
Member Author

djhoese commented Nov 2, 2022

Ok turns out this is much harder than I thought. Here's what Cython says and we're a pure C++ module at this point:

https://cython.readthedocs.io/en/stable/src/userguide/pypy.html

This will just have to wait I guess.

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.

1 participant