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 Py_UNICODE to Py_UCS4 #3780

Merged
merged 6 commits into from Jun 30, 2019
Merged

Conversation

nulano
Copy link
Contributor

@nulano nulano commented Apr 8, 2019

PyUnicode_AS_UNICODE is now deprecated, replace it with PyUnicode_AsUCS4Copy or PyUnicode_READ_CHAR.

Fixing the issues for Python 2.7 will likely require a lot of effort. Given the coming end of support, I don't think it makes sense to fix it there; therefore the test is set to skip on Windows for Python 2.x.

@nulano
Copy link
Contributor Author

nulano commented Apr 8, 2019

I'm not sure why the new test fails on Centos-6 and Amazon-1. It seems that on Centos the text is shifted one pixel down...

All other platforms pass.

Centos 6 image:
centos
Xor with template:
centos-xor

Tests/test_imagefont.py Outdated Show resolved Hide resolved
@radarhere radarhere changed the title Update Py_UNICODE to Py_UCS4, fixes #3777 Update Py_UNICODE to Py_UCS4 Apr 8, 2019
Tests/test_imagefont.py Outdated Show resolved Hide resolved
@nulano nulano marked this pull request as ready for review April 8, 2019 13:31
@nulano

This comment has been minimized.

@radarhere

This comment has been minimized.

@nulano
Copy link
Contributor Author

nulano commented Apr 9, 2019

It looks like Codecov isn't receiving coverage from any Python 2.7 builds with Raqm installed (such as alpine on Travis: https://travis-ci.org/python-pillow/Pillow/jobs/517268209#L712):
https://codecov.io/gh/python-pillow/Pillow/src/a4fd4c2b/src/_imagingft.c#L370

@hugovk
Copy link
Member

hugovk commented Apr 9, 2019

Good spot!

The Alpine job is 7325.10 and is shown as received by Codecov:

https://codecov.io/gh/python-pillow/Pillow/commit/a4fd4c2b23d921ea8263738c2e6f87720774d5f2/build

But checking the logs (in the if [ "$LINT" == "" ]; then section), there's some problem with coveralls-merge, which combines coverage data for the Python and C files. I expect we're only getting the Python coverage data in this case. Scrolling back a few lines shows a missing dependency: E: Unable to locate package lcov

https://travis-ci.org/python-pillow/Pillow/jobs/517268209#L4166

@hugovk
Copy link
Member

hugovk commented Jul 2, 2019

From #3835 (comment):

Our tests didn't pick it up because ... whilst we do test PyPy3 on Travis CI, there is a "implicit declaration of function ‘PyUnicode_AsUCS4Copy’" warning we didn't notice as all the test_imagefont tests are skipped because there's no FreeType (or RAQM) on PyPy3. We should improve these testing gaps.

That's not quite accurate: FreeType is available on Travis CI for PyPy3.

The test_imagefont tests were skipped when this PR was merged:

But they were run on the previous merge to master:

Could it be that PY_VERSION_HEX isn't available on PyPy?

nulano added a commit to nulano/Pillow that referenced this pull request Jul 2, 2019
@nulano
Copy link
Contributor Author

nulano commented Jul 2, 2019

Could it be that PY_VERSION_HEX isn't available on PyPy?

It is available.

Because FreeType support is loaded dynamically on Linux, I guess it failed to dynamically link the PyUnicode_AsUCS4Copy function and assumed it is because of missing FreeType support.
https://travis-ci.org/python-pillow/Pillow/jobs/552842796#L3090

@hugovk
Copy link
Member

hugovk commented Aug 2, 2019

I reported this to PyPy and they've now added PyUnicode_AsUCS4Copy():

We are just following CPython with our emulation of the CPython C API. If CPython deprecates but keeps around functions, then they'll stay in PyPy too.

PyUnicode_AsUCS4Copy() is not implemented in PyPy so far, even though it was present since CPython 3.3. I just added this function in 3ec1002a818c. Closing this as resolved, but please reopen if you hit more missing related functions.

Let's check again when the next PyPy release is out. The current version is 7.1.1:

@nulano
Copy link
Contributor Author

nulano commented Oct 14, 2019

Let's check again when the next PyPy release is out. The current version is 7.1.1:

PyPy3.6-7.2.0 has been released. The fix is ready, but depends on #4140 and #4109 being merged.

Edit: See #4145

@nulano nulano deleted the update_py_unicode branch November 8, 2019 15:16
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.

Unable to draw an Ext-B+ char on Windows
3 participants