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

Deprecate old FreeType? #5075

Closed
hugovk opened this issue Dec 2, 2020 · 2 comments · Fixed by #5098
Closed

Deprecate old FreeType? #5075

hugovk opened this issue Dec 2, 2020 · 2 comments · Fixed by #5098
Labels
Deprecation Feature that will be removed in the future Question

Comments

@hugovk
Copy link
Member

hugovk commented Dec 2, 2020

We're supporting some very old versions of FreeType. Here's some version we check for, and the release date for that version:

FreeType Release date
2.4 2010-07-12
2.5 2013-06-19
2.7 2016-09-08
2.9.1 2018-05-02
2.10 2019-03-15

The latest FreeType 2.10.4 fixes a severe vulnerability: "All users should update immediately".

We're only testing recent releases.

Shall we deprecate support for some old versions of FreeType?

It's likely the next major release Pillow 9.0.0 will be on 2022-01-02, due to the Python 3.6 EOL (2021-12-23), giving plenty of time for deprecation.

Checking some distros from https://repology.org/project/freetype/versions / https://packages.debian.org/source/buster/freetype:

distro FreeType
Alpine 3.10 2.10.0
Alpine 3.11 2.10.1
Alpine 3.12 2.10.4
Amazon Linux 1 2.3.11
Amazon Linux 2 2.4.11 / 2.8
Arch 2.10.4
Centos 6 2.3.11
Centos 7 2.8
Centos 8 2.9.1
Debian 10 Buster 2.9.1
Fedora 32 2.10.1 / 2.10.4
Fedora 33 2.10.2 / 2.10.4
Ubuntu 16.04 Xenial 2.6.1
Ubuntu 18.04 Bionic 2.8.1
Ubuntu 20.04 Focal 2.10.1

Removing those distros which will be EOL before 2022-01-02 and sorting by FreeType version:

distro FreeType
Amazon Linux 2 2.4.11 / 2.8
Centos 7 2.8
Ubuntu 18.04 Bionic 2.8.1
Centos 8 2.9.1
Debian 10 Buster 2.9.1
Ubuntu 20.04 Focal 2.10.1
Alpine 3.12 2.10.4
Arch 2.10.4

Suggesting 2.7 and older would be a good cutoff.

Shall we deprecate support for 2.7 and older (or something else)? Is it worth it?

What would the deprecation look like?

@hugovk hugovk added Question Deprecation Feature that will be removed in the future labels Dec 2, 2020
@radarhere
Copy link
Member

radarhere commented Dec 9, 2020

Apart from updating tests, the only change that I can see in the code is

diff --git a/src/_imagingft.c b/src/_imagingft.c
index 62db561e..17797c83 100644
--- a/src/_imagingft.c
+++ b/src/_imagingft.c
@@ -1034,12 +1034,7 @@ font_render(FontObject* self, PyObject* args)
             case FT_PIXEL_MODE_GRAY4:
                 if (!bitmap_converted_ready) {
 
-#if FREETYPE_MAJOR > 2 ||\
-    (FREETYPE_MAJOR == 2 && FREETYPE_MINOR > 6)
                     FT_Bitmap_Init(&bitmap_converted);
-#else
-                    FT_Bitmap_New(&bitmap_converted);
-#endif
                     bitmap_converted_ready = 1;
                 }
                 error = FT_Bitmap_Convert(library, &bitmap, &bitmap_converted, 1);

On the day when we deprecate < 2.9.1, we can drop errors about not supporting variation fonts.

So the gain in code simplicity seems minimal to me. That said, if we wanted to deprecate older FreeType to encourage good security behaviour from users, or to better reflect the fact that we're not testing them properly anymore, then sure.

I would suggest throwing the DeprecationWarning in the constructor of ImageFont.FreeTypeFont.

@nulano
Copy link
Contributor

nulano commented Dec 9, 2020

It is worth noting that I would have missed the block above in #4955 without the tests (now removed in #5060), which would have unintentionally removed support for some version of FreeType before 2.7 (I did not look through the changelog, but 2.6.1 seems to be fine without it). So +1 to deprecate to reflect that it is no longer tested. Raising a warning in the constructor sounds like a good option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecation Feature that will be removed in the future Question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants