use Pillow instead of PIL

[ohanar]

PIL is has not been maintained an a few years. Pillow is the canonical fork of PIL that most distributions have switch to using, we should do the same.

Upstream Tarball: ​Pillow-2.2.2.tar.gz

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​2c055bd

Pillow: fix patch to work against 2.2.2

[fbissey]

sage/plot/plot3d/base.pyx needs to be fixed as well: import Image needs to be replaced by from PIL import Image

[ohanar]

Replying to fbissey:

sage/plot/plot3d/base.pyx needs to be fixed as well

part of this commit: ​http://git.sagemath.org/sage.git/commit/?h=u/ohanar/pillow&id=6df0adb542b651cdfde5c9088edf18522963aff0

[fbissey]

Great, one less patch in sage-on-gentoo. I don't see that commit attached to this ticket. Is it attached to another ticket or work in progress?

[ohanar]

Replying to fbissey:

Great, one less patch in sage-on-gentoo. I don't see that commit attached to this ticket.

It's attached to this ticket -- you can see all the commits on the branch for this ticket at ​http://git.sagemath.org/sage.git/log/?h=u/ohanar/pillow.

[fbissey]

OK, I am not properly used to the way git and trac integrate. Hit me on the head with it a few more times and I'll get it.

I see you ditched the variables for binary distribution (were they even used anymore with the old pil? apparently yes.). Good riddance, did you "steal" the setup.py patch from somewhere else? Gentoo has a similar one. I think the only missing from that ticket is the removal of pil itself.

[ohanar]

Replying to fbissey:

I see you ditched the variables for binary distribution (were they even used anymore with the old pil? apparently yes.).

They weren't even the standard binary environment variables, and they relied on experimental spkgs.

Good riddance, did you "steal" the setup.py patch from somewhere else?

It's a patch that I submitted upstream (was originally based on the development branch - hence broken, but now fixed).

Gentoo has a similar one. I think the only missing from that ticket is the removal of pil itself.

I see no reason to delete PIL -- it can just be an optional/experiment spkg.

[fbissey]

Replying to ohanar:

Replying to fbissey:

I think the only missing from that ticket is the removal of pil itself.

I see no reason to delete PIL -- it can just be an optional/experiment spkg.

I see no reason to keep something that is, for all purpose, dead. But the point is minor and I am quite ready to give this a positive review.

[vbraun]

The tarball works but don't we lower-case the packages? E.g. "r" instead of "R".

[fbissey]

I must say I am not aware of any such policies in sage but it is not uncommon in linux distro to "prefer" lower case, funny you brought R up because it is usually one of the exception.

[vbraun]

I would be in favor of having everything lower case, otherwise it'll be annoying to type on the commandline build/pkgs/PyCrypto etc... But then its not a big deal if you insist on capital letters for Pillow ;-)

[fbissey]

I will leave the final word to our fellow author on the capital "P" not a big deal but I appreciate your reasons.

[jhpalmieri]

Replying to ohanar:

Replying to fbissey:

I see you ditched the variables for binary distribution (were they even used anymore with the old pil? apparently yes.).

They weren't even the standard binary environment variables, and they relied on experimental spkgs.

Good riddance, did you "steal" the setup.py patch from somewhere else?

It's a patch that I submitted upstream (was originally based on the development branch - hence broken, but now fixed).

Gentoo has a similar one. I think the only missing from that ticket is the removal of pil itself.

I see no reason to delete PIL -- it can just be an optional/experiment spkg.

I think it would make more sense to remove build/pkgs/pil/: we don't have any files in build/pkgs for other optional/experimental packages, do we? I actually don't know how optional packages are implemented: still as spkgs? Is that going to change?

I would also vote for a lowercase "p".

[fbissey]

Not all optional/experimental packages have been converted but yes I think they are supposed to go in build/pkgs. If you have a look you'll see some optional stuff already.

[jhpalmieri]

Oh, I see. Then keep the directory, of course.

[ohanar]

IMO, it would be better to have package names be the same as the upstream version (including capitalization), I would also vote to have this changed for other packages (such as R or Python). (Also, it is not hard to setup your shell [well at least zsh] to be case-insensitive for tab completion.) However, it seems like most disagree, so I can change this when I have a chance.

Actually, I think a good reason to remove PIL would be conflicts from having both Pillow and PIL installed side-by-side (or PIL instead of Pillow). I would rather not have to maintain support for both libraries.

[jhpalmieri]

I think that the naming convention must still be the old SPKG naming rule: only lowercase letters, numbers, and underscores. I personally wouldn't object to changing this convention, but that needs to be discussed on sage-devel, not here.

If we end up changing it, then because of the brain-dead way the standard OS X file system treats upper and lower case, I think we can never have two packages whose name only differs in capitalization. So then we should have commands like sage -i PKG_NAME ignore case, to make sage -i Pillow and sage -i pillow (for example) perform the same function.

[vbraun]

Since Pillow is a backward-compatible replacement for PIL I'm also in favor of removing PIL.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​adc90ce	Pillow -> pillow
​8987381	remove PIL (it has been replaced by Pillow)

[fbissey]

I think that covers all the issue raised for this particular package. So back to positive review. Not sure who to put in reviewer(s) though.

[vbraun]

checksums.ini still has capital-P(illow) in it...

[vbraun]

The tarball is already on the server, for the record. Please make sure that it downloads...

[ohanar]

Replying to vbraun:

checksums.ini still has capital-P(illow) in it...

Yes, that is a fully conscious decision, as it matches the tarball that is provided by upstream. I think it is much better to just use the upstream tarball in this case, rather than untaring, changing the name of the untared directory, and then retaring (which is what one must do if they decapitalize Pillow in checksums.ini). This is part of the reason why I think that matching upstreams capitalization is preferable.

[vbraun]

Ok, good. But it still doesn't build.

[vbraun]

Log: ​http://build.sagemath.org/sage/builders/%20%20fast%20UW%20redhawk%20%28Ubuntu%2010.04%20x86_64%29%20incremental/builds/26/steps/compile/logs/stdio

[ohanar]

Replying to vbraun:

Ok, good. But it still doesn't build.

Works fine on OSX (case-insensitive). The problem is probably because the package was misplaced on boxen (should be .../Pillow/Pillow-*.tar.gz not .../pillow/Pillow-*.tar.gz). (Again, less breakage if we just matched upstream's capitalization).

[vbraun]

Surely the OSX filesystem has no influence on whether *downloading* ​http://www.sagemath.org/packages/upstream//Pillow/Pillow-2.2.2.tar.gz works or not? If it works for you then it must be because you have the tarball already in your upstream directory.

[ohanar]

Replying to vbraun:

Surely the OSX filesystem has no influence on whether *downloading* ​http://www.sagemath.org/packages/upstream//Pillow/Pillow-2.2.2.tar.gz works or not?

Maybe it doesn't have to do with the OSX filesystem (although, maybe python's urllib stuff builds to be case-insensitive on a case-insensitive filesystem).

If it works for you then it must be because you have the tarball already in your upstream directory.

No, I purposely deleted it to test, and it indeed downloaded a new tarball.

[vbraun]

Ok, seems to download now. But build fails on a bunch of older machines (bsd, eno, cicero). Missing dependency?

Found local metadata for pillow-2.2.2
Attempting to download package pillow-2.2.2
>>> Trying to download http://www.sagemath.org/packages/upstream//pillow/Pillow-2.2.2.tar.gz
[............................................................]
Checksum: 053337a612dd16ec6f1f6fc544374ca5fe65ae2c vs 053337a612dd16ec6f1f6fc544374ca5fe65ae2c
pillow-2.2.2
====================================================
Setting up build directory for pillow-2.2.2
Finished set up
****************************************************
Host system:
Darwin bsd.local 10.8.0 Darwin Kernel Version 10.8.0: Tue Jun  7 16:32:41 PDT 2011; root:xnu-1504.15.3~1/RELEASE_X86_64 x86_64
****************************************************
C compiler: gcc
C compiler version:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/Users/buildbot/build/sage/bsd-1/sage_git/build/local/libexec/gcc/x86_64-apple-darwin10.8.0/4.7.3/lto-wrapper
Target: x86_64-apple-darwin10.8.0
Configured with: ../src/configure --prefix=/Users/buildbot/build/sage/bsd-1/sage_git/build/local --with-local-prefix=/Users/buildbot/build/sage/bsd-1/sage_git/build/local --with-gmp=/Users/buildbot/build/sage/bsd-1/sage_git/build/local --with-mpfr=/Users/buildbot/build/sage/bsd-1/sage_git/build/local --with-mpc=/Users/buildbot/build/sage/bsd-1/sage_git/build/local --with-system-zlib --disable-multilib --disable-nls  
Thread model: posix
gcc version 4.7.3 (GCC) 
****************************************************
patching file setup.py
Traceback (most recent call last):
  File "setup.py", line 19, in <module>
    from setuptools import Extension, setup, find_packages
ImportError: No module named setuptools
Error building / installing Pillow

real    0m0.069s
user    0m0.038s
sys     0m0.028s
************************************************************************
Error installing package pillow-2.2.2
************************************************************************

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​f005905

pillow: depends on setuptools