Imaging.h: confusion with system

[ax3l]

The file libImaging/Imaging.h gets installed on Ubuntu flat into the public Python includes.

When building a newer version of Pillow from source and hinting the Python includes "too early" in includes, e.g. in package managers, this can confuse the two files and pick up the external file over the internal one. With different versions, this mismatch can lead to build errors, e.g. undefined macros.

The most robust way to avoid this is to prefix the internal include accordingly, so that the relative path to the including file has to match as well.

[radarhere]

I've created ax3l#1 as a suggestion to expand on this - prefixing more header includes.

[wiredfool]

I think this is a bug in the debian packaging. Imaging.h is an internal file, not a supported set of external interfaces.

Following the debian rules for splitting and naming packages, if the header is actually required (e.g, for other debian packages), it should be in a package named python-pillow-dev, not in the primary python-pillow package. (e.g, libfoo and libfoo-dev for compilation).

[radarhere]

If it's not a problem though, is it worth doing just because it makes it clearer to the reader where the header file is located?

[ax3l]

That's also what I was thinking. Even if it were in a -dev package, we would have the same problem of missing isolation when building from source (if it's installed).

An alternative solution that we considered would be to change the order in which include_dirs are applied in setup.py. I feared that this would have significantly more potential for side-effects for existing workflows.

Another question is if those headers should be installed in the install prefix without further directory namespace around it or at all. Is this a vendored third-party library?

Considering all of the above and aiming for a small change set as an outside contributor, the proposed diff was the least invasive and most stable approach I could come up with.

[ax3l]

Thank you for the update, LGTM.

[nulano]

I tried to remove the libImaging directory from the includes added in setup.py so future includes can't omit the prefix by accident, but this causes a failure building _imagingft.c on systems without libraqm. Fixing this requires changing

#include <raqm.h>

to

#include "libImaging/raqm.h"

so I'm not sure it is worth it. Thoughts?

ax3l/Pillow@fix-imagingHinclude...nulano:fix-imagingHinclude

[ax3l]

Oh, but that means instead of the internally shipped raqm.h it was always picking up a system header. That is another source of potential issues then.
Yes, would add this. Great catch!

[hugovk]

Merged in master to include #4978 to fix the unrelated CI failure.