Add Pillow Recipe

[krinnewitz]

This patch adds a new recipe for Pillow. There already is a recipe for PIL, but PIL does not support python3.

This new recipe should work with python3crystax as well as with python2. However, with python2, there are the following issues:

• The existing recipe for setuptools provides an older version of setuptools, which is too old for Pillow. This can be circumvented by specifying setuptools==40.0.0 when building an APK. I am not sure if it is possible to specify a minimum version for dependencies inside recipes.
• There seems to be an issue with the jpeg recipe which requires libcutils. I resolved it locally by removing libcutils from pythonforandroid/recipes/jpeg/build-static.patch.

With python3crystax, there should be no issues.

This PR addresses #1114 and #1326.

[inclement]

Looks good to me, but I'll let @AndreMiras check it since I think he's followed the development more.

About setuptools, it should be fine to update the version in the recipe, as long as it does work (which it sounds like it does).

About libcutils, I don't know what this is used for, do things seem to work fine without it?

[inclement]

Would you mind fixing the small code style things travis has caught via flake8? You can ignore the android module import, I will fix that separately (edit: now fixed in master).

[AndreMiras]

Thank you very much for the pull request @krinnewitz it's looking good so far. I'll give it a try with https://github.com/kivy-garden/garden.zbarcam later tonight and keep you posted.

[krinnewitz]

Thanks guys. I have resolved the problems detected by flake8.

Regarding setuptools: Never mind. I cannot reproduce the problem anymore. Maybe a mistake on my side.

Regarding libjpeg: I was able to load a jpeg image without libcutils. So I guess it is not needed. However, I am not entirely sure about that since I have not tested excessively.

[inclement]

Regarding setuptools: Never mind. I cannot reproduce the problem anymore. Maybe a mistake on my side.

Great. It would still be fine to update the recipe version if you tested the newer one and it worked, there recipe exists more for internal reasons than because the version should matter.

About libcutils, I guess let's leave it for now but maybe leave a comment about it in the recipe. If someone really wants to use pillow with python2, they can look into it.

[krinnewitz]

Great. It would still be fine to update the recipe version if you tested the newer one and it worked, there recipe exists more for internal reasons than because the version should matter.

Ok, I have added another commit which upgrades it to 40.0.0.

About libcutils, I guess let's leave it for now but maybe leave a comment about it in the recipe. If someone really wants to use pillow with python2, they can look into it.

Actually, the problem should occur for any app using jpeg and python2. I have just created #1341 for that. Hope that's okay.

[ghost]

Once you want to have me test this with my python3crystax app, drop me a note. I'd be happy to give it a try

[AndreMiras]

I've started playing with it and Python2.
First thing I noticed is the recipe would not be picked up if requirements is written lower case. Instead of that the one from pypi will be picked up and installed without cross compiling, resulting in the usual site-packages/PIL/_imaging.so" is 64-bit instead of 32-bit error.
I don't know if it's something we can/should tackle in this PR, but it's worth to mention.

[AndreMiras]

Minor, FWIW I would reference the upstream bug python-pillow/Pillow#3231

[inclement]

@AndreMiras Good catch about the case sensitivity. I think this recipe should definitely use lower case, both for consistency with the other recipes and because if that's what the pypi entry uses then we should probably match anyway.

Maybe p4a should always ignore case for recipes, that would probably be a good idea (although usually not that important).

[ghost]

The official PyPI package appears to be named Pillow, not pillow: https://pypi.org/project/Pillow/

However, it seems pip allows both variants, so if the recipes don't work with that, maybe whatever code finds the recipes needs to be fixed? If the code can't be fixed, I think Pillow is the more expected and correct name, since that's how the official package appears to be spelled.

[inclement]

In that case it seems clear that p4a should not be case sensitive when looking for recipes, but that's a separate issue. I'd encourage this recipe to use lower case for consistency with p4a in general, but it isn't a blocking issue as far as I'm concerned.

[AndreMiras]

Yes that's probably something we should fix in p4a to ignore the case and then have all recipe to be lowercase in the file system. But that's something we could probably tackle later.

So far I was able to compile Pillow for a Python2 project after fixing the include path. However garden.zbarcam doesn't seem to catch qrcodes anymore. That might be for some unrelated reasons I'll dig later.
I'm now taking a look at Crystax Python3, but I got some runtime crashes, I'll dig a bit further.

[ghost]

All the (official) tutorials name it pip install Pillow, so I don't think going for lowercase only is a good idea 🤐

[AndreMiras]

I'm facing xcamera issues to play with garden.zbarcam, but the recipe looks good overall @krinnewitz !
There's probably just the Python2 include path thing to look at, but it don't mind if this is done in a dedicated pull request if other people also get the issue on Python2. Who's using Python2 now anyway 😉

@Jonast actually the idea is just to ignore the case, not to decide whether it should be lower or upper. The thing is currently if you put pillow in the requirements, we will have run time errors because it doesn't match with the current recipe. If we were to just change the case the error would be the same but in the opposite fashion (e.g. if people used Pillow in requirements). By enforcing everything lowercase and then ignoring the case completely it's a way to document it as a feature: "we ignore the case". But that again can be decided later and addressed in a dedicated PR.

[AndreMiras]

I don't consider my comment being blockers, this recipe is already an improvement considering what we already (don't) have. So I would be happy to merge it.

[AndreMiras]

Was the zlib.h include actually deployed here on your Python 2.7 installation? Because on mine it's seems that join(ndk_dir, 'usr', 'include')) doesn't exist, while the former does (join(self.ctx.ndk_dir, 'sysroot', 'usr', 'include').
So basically when running recipe on Python2, the ZLIB_ROOT would contain the following:

/home/andre/.buildozer/android/platform/android-ndk-r9c/platforms/android-19/arch-arm/usr/lib|/home/andre/.buildozer/android/platform/android-ndk-r9c/sysroot/usr/include

The lib directory seems fine:

ls /home/andre/.buildozer/android/platform/android-ndk-r9c/platforms/android-19/arch-arm/usr/lib/
crtbegin_dynamic.o  crtbegin_so.o  crtbegin_static.o  crtend_android.o  crtend_so.o  libandroid.so  libc.a  libc.so  libdl.so  libEGL.so  libGLESv1_CM.so  libGLESv2.so  libGLESv3.so  libjnigraphics.so  liblog.so  libm.a  libm_hard.a  libm.so  libOpenMAXAL.so  libOpenSLES.so  libstdc++.a  libstdc++.so  libthread_db.so  libz.so  rs

However the include is not:

ls /home/andre/.buildozer/android/platform/android-ndk-r9c/sysroot/usr/include
ls: cannot access '/home/andre/.buildozer/android/platform/android-ndk-r9c/sysroot/usr/include': No such file or directory

But if I skip line 33 and set the include as ndk_include_dir = (join(self.ctx.ndk_dir, 'sysroot', 'usr', 'include') then it contains the includes:

ls /home/andre/.buildozer/android/platform/android-ndk-r9c/platforms/android-19/arch-arm/usr/include/
alloca.h  asm          byteswap.h  dlfcn.h  endian.h  fcntl.h     fnmatch.h  GLES   grp.h       KHR        limits.h  locale.h  math.h    mtd      netinet     OMXAL       poll.h     regex.h   sched.h      sgtty.h   SLES      stdlib.h   sys       termios.h    time.h    utime.h  wctype.h
android   asm-generic  ctype.h     EGL      err.h     features.h  fts.h      GLES2  inttypes.h  lastlog.h  link.h    machine   memory.h  net      netpacket   pathconf.h  pthread.h  resolv.h  semaphore.h  sha1.h    stdint.h  string.h   syslog.h  thread_db.h  unistd.h  utmp.h   zconf.h
arpa      assert.h     dirent.h    elf.h    errno.h   fenv.h      getopt.h   GLES3  jni.h       libgen.h   linux     malloc.h  mntent.h  netdb.h  nsswitch.h  paths.h     pwd.h      rs        setjmp.h     signal.h  stdio.h   strings.h  termio.h  time64.h     util.h    wchar.h  zlib.h

[AndreMiras]

@inclement the PEP8 comment you made #1339 (review) was addressed, can you approve and merge? The merging is currently being blocked with that comment.

[AndreMiras]

Thanks for the merge @inclement and thanks for your work @krinnewitz

@Jonast yes please give it a try #1339 (comment)

Once you want to have me test this with my python3crystax app, drop me a note. I'd be happy to give it a try

[ghost]

Edit: ignore comment I'm a confused person 😄

Yea, I was just suggesting making it lowercase "to ignore the case" isn't that obvious from a user perspective: --requirements is exactly named like the pip option, so people will most likely paste the package name from the official docs I think. So that's why I keep suggesting Pillow is a better choice until the ignore case things actually work, even though making it lowercase might make more sense from a project perspective.

Once ignoring the case works, this is all irrelevant anyway, of course

[ghost]

Also, amazing work! 🎉 🎉 🎈 👍 ❤️ I'll go test

[AndreMiras]

Yea, I was just suggesting, your ignoring case thing seems artificial. --requirements is exactly named like the pip option, so people will most likely paste the package name from the official docs I think. I don't think it's a good idea, but of course it's still a minor nitpick

But what if people put pillow in their requirements, just like I did before? pillow all lowercase it's still valid and would be pip installed and then it would crash with runtime issues, resulting with people spamming the bug tracker with "pillow doesn't work" kinda issues. Maybe you have a more elegant way in mind to avoid that?

[ghost]

But what if people put pillow in their requirements, just like I did before?

Well, what if they have Pillow? (I do!) I was simply suggesting if people go by the official docs like me, they're likely to have Pillow, so the number of people may be larger. But it's hard to know, of course

[AndreMiras]

That's exactly why we suggest to totally ignore the case in p4a code base so one case or the other would never fail 😄

[ghost]

Oh, right. I was just talking about the transition up to then. Sorry for the misunderstanding!

Edit: Yea scrolling up I totally misread where the discussion was going 😆 sorry

[ghost]

I just tested the recipe with python3crystax, runs fine for me! Builds and all images show up! 👍

[krinnewitz]

Thank you all for reviewing, merging, and testing so fast!