Fix "PY_SSIZE_T_CLEAN macro must be defined for '#' formats" error

[ejeschke]

There was a change in the C API for Python 3.10. aggdraw will compile, but not run correctly under python 3.10. You will get the error: "PY_SSIZE_T_CLEAN macro must be defined for '#' formats"

This fixes that issue.

[djhoese]

These docs don't say anything about setting it to 1:

https://docs.python.org/3/c-api/intro.html#include-files

Any idea if it makes a difference when things are actually compiled?

Also, I'm not sure why appveyor is being brought into CI, I thought I deleted that. Is there a chance you are using an old version of main as your base?

[ejeschke]

These docs don't say anything about setting it to 1

I think it just has do be defined so that it tests to True

Any idea if it makes a difference when things are actually compiled?

Well, yes. I mean aggdraw cannot be used in the usual way under 3.10 without the patch. See this documentation, specifically:

Note

For all # variants of formats (s#, y#, etc.), the macro PY_SSIZE_T_CLEAN must be defined before including Python.h. On Python 3.9 and older, the type of the length argument is Py_ssize_t if the PY_SSIZE_T_CLEAN macro is defined, or int otherwise.

[ejeschke]

Also, I'm not sure why appveyor is being brought into CI, I thought I deleted that. Is there a chance you are using an old version of main as your base?

Probably. My clone still has master, and after confirming that my "master" was just behind and was going to be a fast-forward I just merged the main from upstream. When running the CI for the pull request it may be picking up some old config for some reason (but that should be configured on your github repo, right?) I'll make a clean checkout next time.

[djhoese]

Looking at the history of the file you modified it looks like you forks master may not be up to date. Not a huge deal this time.

Well, yes. I mean aggdraw cannot be used in the usual way under 3.10 without the patch. See this documentation, specifically:

Sorry, I meant does the 1 or not having anything after it make a difference when compiled (not whether or not this should be defined at all)?

[ejeschke]

Sorry, I meant does the 1 or not having anything after it make a difference when compiled (not whether or not this should be defined at all)?

I think it needs to be defined, according to the document, but I suspect the reason they did it this way is for backward compatibility with older python versions. I just followed the example of another project that had made the same fix. If you google "PY_SSIZE_T_CLEAN macro must be defined for '#' formats" you will find a bunch of projects that had to make this fix for 3.10. I just followed one example, which defined the value as 1.

[djhoese]

Ok. Yeah I was just wondering about the 1. Is there an easy way to test this during the wheel building? Does the old wheel fail on import?

[ejeschke]

The module imports ok without the fix, but if you try to do a frombytes operation on a Draw object it will raise this error. That may be a rare error for many users of the module if they don't ever try to save and load a Draw object.

[djhoese]

Ok. Thanks. I will merge this when I have time to make a release. I'll have to cherry-pick the commit to the maint/1.3 branch and make a release from there. If I merge this without having time to do that then I might forget. Thanks again.

[ejeschke]

Ok. Thanks. I will merge this when I have time to make a release. I'll have to cherry-pick the commit to the maint/1.3 branch and make a release from there. If I merge this without having time to do that then I might forget. Thanks again.

:pray:

[djhoese]

It looks like main and your fork didn't have the Windows build enabled and now they're failing:

     Running setup.py clean for aggdraw
    agg_font_freetype.cpp
    D:\a\aggdraw\aggdraw\agg2\font_freetype\agg_font_freetype.h(23): fatal error C1083: Cannot open include file: 'ft2build.h': No such file or directory
    error: command 'C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Tools\\MSVC\\14.32.31326\\bin\\HostX86\\x86\\cl.exe' failed with exit code 2
    [end of output]

    note: This error originates from a subprocess, and is likely not a problem with pip.
    ERROR: Failed building wheel for aggdraw
  ERROR: Failed to build one or more wheels
  Failed to build aggdraw

https://github.com/pytroll/aggdraw/runs/7064936012?check_suite_focus=true

This very likely isn't caused by your changes, but do you have any ideas what I should do? I could add freetype-py as a dependency but up until now we've never required it. This would give us the option of having freetype anywhere that freetype-py is installable. I did just notice in the above log that it says:

    Trying freetype-config to find freetype library...
    Using ctypes to find freetype library...
    === freetype found: 'C:\Program Files\Microsoft\jdk-11.0.12.7-hotspot'

So it is definitely pulling in a freetype we don't want. I guess I could try to force it to not find it.

[djhoese]

Ok I've disabled freetype in the Windows wheels. If someone wants freetype support from the Windows wheels then they'll need to build aggdraw from source or submit an issue to this github repository to request the functionality (and hopefully volunteer to do the work to fix it in github actions CI).

[djhoese]

FYI this should be available in 1.3.15 on PyPI and conda-forge now.

[ejeschke]

freetype is seriously useful, but I'm not sure how readily available it is on Windows. I suspect that it may be available via conda and that is probably the best way to install aggdraw as well on Windows, via compiling on a conda environment. I'd try it, but I don't manage any Windows machines at all. :confused:

[djhoese]

Agreed. I'm also not aware of how people usually get freetype on Windows so besides the conda package, any wheels we make would probably be incompatible.

[ejeschke]

Thanks!