supermihi / pytaglib

Python audio tagging library
GNU General Public License v3.0
187 stars 29 forks source link

Python 3.12 build failure: unicode API #115

Closed dijkstrah closed 11 months ago

dijkstrah commented 11 months ago

src\taglib.cpp(3927): error C3861: 'PyUnicode_AsUnicode': identifier not found

In Python 3.12 PyUnicode_AsUnicode, PyUnicode_FromUnicode and others have been removed.

I think the culprit is:

cdef File create(const Py_UNICODE) except +

line 65 in ctypes.pxd

but I am not entirely sure what it should be replaced with.

supermihi commented 11 months ago

Thank you for the report. I can [reproduce] this in CI.

Unfortunately I do not see an easy way to fix it, because the removed method is used in Cython-generated code. There is an open issue in the Cython project, I'm afraid we will need to wait until it is fixed upstream.

dijkstrah commented 11 months ago

Thank you for reporting it upstream. Will track.

da-woods commented 11 months ago

Py_UNICODE is explicitly removed in 3.12 and this is something you're using manually when you say

cdef File* create(const Py_UNICODE*) except +

See https://docs.python.org/3/c-api/unicode.html

Note

The Py_UNICODE representation has been removed since Python 3.12 with deprecated APIs. See PEP 623 for more information.

Cython will still generate PyUnicode_AsUnicode to convert to a const Py_UNICODE* but in no other circumstance. That doesn't work any more, but there's nothing else Cython can really do - the representation you're trying to get no longer exists.

da-woods commented 11 months ago

I think you need to do PyUnicode_AsWideCharString (which you can cimport from cpython.unicode). Unfortunately this isn't something that Cython can do for you since it needs manual memory management. I suspect the following will work:

cdef extern from 'taglib/fileref.h' namespace 'TagLib::FileRef':
    IF UNAME_SYSNAME == "Windows":
        cdef File* create(const wchar_t*) except +
    ELSE:
        cdef File* create(const char*) except +

then


cdef create_wrapper(unicode u):
    IF UNAME_SYSNAME == "Windows":
        cdef wchar_t *s = PyUnicode_AsWideCharString(u, NULL)
        cdef FILE* f = create(s)
        PyMem_Free(s)
        return s
    ELSE:
        return create(u)
supermihi commented 11 months ago

Thanks for your help @da-woods, I was able to find a working version after some trial & error.

By the way I could not find PyUnicode_AsWideCharString in cpython.unicode, is it missing?

dijkstrah commented 11 months ago

Thank you for the quick fix!

da-woods commented 11 months ago

By the way I could not find PyUnicode_AsWideCharString in cpython.unicode, is it missing?

I think it's just an accidental omission. I'll add it in the next release.