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 config for FriBiDi dynamic linking #181

Merged
merged 10 commits into from Mar 31, 2021

Conversation

nulano
Copy link
Contributor

@nulano nulano commented Jan 2, 2021

For python-pillow/Pillow#5062.

See passing run in my branch: https://github.com/nulano/pillow-wheels/actions/runs/684862775

More comments inline below.

gz|tgz) tar -zxf $in_fname ;;
bz2) tar -jxf $in_fname ;;
zip) unzip -qq $in_fname ;;
xz) unxz -c $in_fname | tar -xf - ;;
Copy link
Contributor Author

@nulano nulano Jan 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is wrong in the multibuild version causing an error in tar; https://github.com/matthew-brett/multibuild/pull/380.

config.sh Outdated
Comment on lines 88 to 91
if [ -z "$IS_OSX" ]; then
export FREETYPE_LIBS=-lfreetype
export FREETYPE_CFLAGS=-I/usr/local/include/freetype2/
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HarfBuzz fails to find FreeType on linux without this, specify the path manually.

export FREETYPE_LIBS=-lfreetype
export FREETYPE_CFLAGS=-I/usr/local/include/freetype2/
fi
build_simple harfbuzz $HARFBUZZ_VERSION https://github.com/harfbuzz/harfbuzz/releases/download/$HARFBUZZ_VERSION tar.xz --with-freetype=yes --with-glib=no
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--with-glib=no to avoid pulling in unrelated libraries on macos.

fi
pip wheel $(pip_opts) \
--global-option build_ext --global-option --enable-raqm \
--global-option --vendor-raqm --global-option --vendor-fribidi \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overriding pip_wheel_cmd to add the needed global options, as defined in python-pillow/Pillow#5062.

config.sh Outdated
EXP_FEATURES="fribidi harfbuzz raqm transp_webp webp_anim webp_mux xcb"
else
# can't find FriBiDi
EXP_FEATURES="transp_webp webp_anim webp_mux xcb"
Copy link
Contributor Author

@nulano nulano Jan 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macOS and PyPy on Ubuntu don't find FriBiDi preinstalled. Adding brew install fribidi only finds it on Python 3.6. I'm not sure why this is, but it should be irrelevant given the build_ext flags defined above.

function pip_wheel_cmd {
local abs_wheelhouse=$1
if [ -z "$IS_OSX" ]; then
CFLAGS="$CFLAGS --std=c99" # for Raqm
Copy link
Contributor Author

@nulano nulano Jan 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to compile with C99 than to merge in the (poorly tested) C89 patch from python-pillow/Pillow@b3cfe73, especially when this file is not compiled by default.

@radarhere
Copy link
Member

python-pillow/Pillow#2753 (comment)

We can't currently distribute a binary of Pillow with support for raqm due to license incompatibility, because while libraqm is MIT licensed, it links to GPL libraries.

@wiredfool do you recall which libraries you were discussing here?

@nulano
Copy link
Contributor Author

nulano commented Jan 3, 2021

python-pillow/Pillow#2753 (comment)

We can't currently distribute a binary of Pillow with support for raqm due to license incompatibility, because while libraqm is MIT licensed, it links to GPL libraries.

@wiredfool do you recall which libraries you were discussing here?

FriBiDi is under LGPL 2.1 or later: https://github.com/fribidi/fribidi#license

HarfBuzz is under "Old MIT": https://github.com/harfbuzz/harfbuzz/blob/master/COPYING

@nulano
Copy link
Contributor Author

nulano commented Mar 25, 2021

I've rerun the build in my branch of the latest version of python-pillow/Pillow#5062 and updated the link in the top comment: https://github.com/nulano/pillow-wheels/actions/runs/684862775

Trying the 3.6 and 3.8 wheels in a fresh LinuxMint 19.1 (based on Ubuntu 18.04) VM, I get a crash after one iteration with the latest 8.1.2 wheels from PyPI, while these wheels work for at least 1000 iterations. I also had to install libraqm0 for the PyPI wheels while these worked out of the box.

from PIL import ImageFont
for i in range(1000):
    print(i)
    ImageFont.truetype("DejaVuSans", layout_engine=ImageFont.LAYOUT_RAQM).getsize("hello")

@wiredfool
Copy link
Member

wiredfool commented Mar 28, 2021

Getting failures now of

2021-03-28T13:49:28.8410816Z   error: option --enable-raqm not recognized

@nulano
Copy link
Contributor Author

nulano commented Mar 28, 2021

Getting failures now of

2021-03-28T13:49:28.8410816Z   error: option --enable-raqm not recognized

python-pillow/Pillow#5062 adds new options which are required to enable the new code. These fail on the 8.1.2 tag, but are fine on the master branch (see "latest" jobs).

config.sh Outdated Show resolved Hide resolved
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
@wiredfool wiredfool merged commit 4d91cd8 into python-pillow:master Mar 31, 2021
@nulano nulano deleted the for-5062 branch September 7, 2022 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants