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

PIL spkg uses libraries it must not use #7273

Closed
sagetrac-GeorgSWeber mannequin opened this issue Oct 23, 2009 · 20 comments
Closed

PIL spkg uses libraries it must not use #7273

sagetrac-GeorgSWeber mannequin opened this issue Oct 23, 2009 · 20 comments

Comments

@sagetrac-GeorgSWeber
Copy link
Mannequin

sagetrac-GeorgSWeber mannequin commented Oct 23, 2009

New spkg's available at these url's:

http://sage.math.washington.edu/home/timdumol/pil-1.1.6.p2.spkg

#7345 -- http://sage.math.washington.edu/home/timdumol/libtiff-3.9.1.p0.spkg

#7344 -- http://sage.math.washington.edu/home/timdumol/libjpeg-7.p0.spkg

In binary build mode, the updated PIL library optionally depends on libtiff, libpng, and libjpeg with the path explicitly set to the local lib path. TCL/TK support is disabled.


From the pil-1.1.6.spkg's "setup.py":

# --------------------------------------------------------------------
# Library pointers.
#
# Use None to look for the libraries in well-known library locations.
# Use a string to specify a single directory, for both the library and
# the include files.  Use a tuple to specify separate directories:
# (libpath, includepath).  Examples:
#
# JPEG_ROOT = "/home/libraries/jpeg-6b"
# TIFF_ROOT = "/opt/tiff/lib", "/opt/tiff/include"
#
# If you have "lib" and "include" directories under a common parent,
# you can use the "libinclude" helper:
#
# TIFF_ROOT = libinclude("/opt/tiff")

FREETYPE_ROOT = None
JPEG_ROOT = None
TIFF_ROOT = None
ZLIB_ROOT = None
TCL_ROOT = None

# FIXME: add mechanism to explicitly *disable* the use of a library

# --------------------------------------------------------------------

and any of these libraries the setup thinks it finds will be set as

-DHAVE_LIBJPEG -DHAVE_LIBZ

and the like in "building '_imaging' extension".

This means that if a Sage binary is built on a computer with having some of these libraries, then this binary will not work (might not even start) on a computer not having at least these libraries available.

Even more fun (again taken from PIL's setup.py):

        elif sys.platform == "darwin":
            # attempt to make sure we pick freetype2 over other versions
            add_directory(include_dirs, "/sw/include/freetype2")
            add_directory(include_dirs, "/sw/lib/freetype2/include")
            # fink installation directories
            add_directory(library_dirs, "/sw/lib")
            add_directory(include_dirs, "/sw/include")
            # darwin ports installation directories
            add_directory(library_dirs, "/opt/local/lib")
            add_directory(include_dirs, "/opt/local/include")

Last, but not least, pil-1.1.6 as contained in Sage-4.2.alpha1 breaks the Sage build, at least on my computer. It somehow thinks it could find "libjpeg" and its includes, but then cannot:

...
running build_ext
--- using frameworks at /System/Library/Frameworks
building '_imaging' extension
gcc -fno-strict-aliasing -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-
prototypes -DHAVE_LIBJPEG -DHAVE_LIBZ -I/System/Library/Frameworks/
Tcl.framework/Headers -I/System/Library/Frameworks/Tk.framework/
Headers -I/Users/Shared/sage/sage-4.2.alpha1/local/include/freetype2 -
IlibImaging -I/opt/local/include -I/Users/Shared/sage/sage-4.2.alpha1/
local/include -I/usr/local/include -I/usr/include -I/Users/Shared/sage/
sage-4.2.alpha1/local/include/python2.6 -c decode.c -o build/
temp.macosx-10.3-i386-2.6/decode.o
In file included from decode.c:653:
libImaging/Jpeg.h:11:21: error: jpeglib.h: No such file or directory
In file included from decode.c:653:
libImaging/Jpeg.h:17: error: field 'pub' has incomplete type
libImaging/Jpeg.h:26: error: field 'pub' has incomplete type
libImaging/Jpeg.h:49: error: field 'cinfo' has incomplete type
libImaging/Jpeg.h:62: error: field 'pub' has incomplete type
libImaging/Jpeg.h:90: error: field 'cinfo' has incomplete type
error: command 'gcc' failed with exit status 1

The full install.log is at http://sage.math.washington.edu/home/weberg/logs/sage-4.2.alpha1_install.log

But the problem with the binaries will occur on any platform, not only Darwin.

So we either have to also include a jpeg.spkg, a tiff.spkg, and so on in Sage (and make sure PIL uses these !!!), or cripple PIL to not use any of these libraries (even if they were present).

The former is problematic, as far as I remember e.g. the tiff license is not GPL compatible (apart from the technical aspects), but I might be mistaken. Crippling might render PIL pretty useless, however.

Component: packages: standard

Author: Tim Dumol

Reviewer: William Stein

Merged: sage-4.2.1.rc0

Issue created by migration from https://trac.sagemath.org/ticket/7273

@williamstein
Copy link
Contributor

comment:1

Crippling might render PIL pretty useless, however.

PIL will still be able to work with PNG, which we do include.


That said, I'm OK with not including PIL in sage-4.2. Whoever really wants PIL in Sage should fix the above issues for a future Sage release.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Oct 24, 2009

comment:2

I have made a libjpeg (I needed it for PIL) -- it is available here: http://sage.math.washington.edu/home/timdumol/libjpeg-7.p0.spkg

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Oct 24, 2009

comment:3

As for the TIFF library, it seems to be BSD licensed: http://www.libtiff.org/misc.html, which is GPL compatible, as far as I know. Are there any other libraries needed for inclusion?

I'll gladly make an spkg for libtiff as well, if needed.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Oct 29, 2009

comment:4

Here's a libtiff package: http://sage.math.washington.edu/home/timdumol/libtiff-3.9.1.p0.spkg.
Updated PIL package here: http://sage.math.washington.edu/home/timdumol/pil-1.1.6.p1.spkg. The updated package has explicid dependencies on Sage's libtiff, libpng and libjpeg, and disables TCL/TK.

I'm guessing libtiff and libjpeg have to be voted in?

@TimDumol

This comment has been minimized.

@TimDumol TimDumol mannequin added the s: needs review label Oct 29, 2009
@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Oct 29, 2009

comment:5

Changed PIL package as per William's suggestion at http://groups.google.com/group/sage-devel/msg/6ea0a0e0a0a2a71a. libjpeg and libtiff packages are up at #7344 and #7345 respectively. Patch to automatically rebuild PIL in binary build mode attached.

@TimDumol

This comment has been minimized.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Oct 29, 2009

Author: Tim Dumol

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Oct 29, 2009

sage-bdist -- Rebuild PIL in binary build mode.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Oct 29, 2009

comment:6

Attachment: trac_7273-sage-bdist.spkg.gz

@TimDumol

This comment has been minimized.

@williamstein williamstein modified the milestones: sage-4.3, sage-4.2.1 Nov 11, 2009
@williamstein
Copy link
Contributor

comment:8

It has a patch with this name "trac_7273-sage-bdist.spkg". Huh? I have no idea what that means. I'm totally confused by this "with patch; needs review" ticket.

@sagetrac-GeorgSWeber
Copy link
Mannequin Author

sagetrac-GeorgSWeber mannequin commented Nov 11, 2009

comment:9

A reviewer (and, on positive review, an integrator) is IMHO supposed to do the following:

  • replace the current "pil-1.1.6.spkg" with "pil-1.1.6.p2.spkg" placed (see above) at http://sage.math.washington.edu/home/timdumol/pil-1.1.6.p2.spkg

  • apply the patch with the strange name to "$SAGE_ROOT/local/bin/", i.e. the sage_scripts spkg's repository, in order to apply certain changes to the shell-script "sage-bdist" (one can view the contents of this patch from/inside trac just as usual).

And check that everything works fine, i.e. especially in the case of building a Sage binary distribution, neither jpeg nor tiff (even though they migth be present at the host computer where the bdist takes place and/or where the Sage version was built) are added as libraries that the then newly built PIL depends on.

@williamstein
Copy link
Contributor

comment:10

Sorry, there was some miscommunication. I do not like the design of trac_7273-sage-bdist.spkg, in that I strongly disagree with "sage -bdist" literally rebuilding parts of that Sage install. I would much prefer the following, which would fit well with my workflow:

(1) introduce a SAGE_BINARY_BUILD flag

(2) whenever I'm going to build sage binaries, I set that flag in my build script before ever starting any of the builds.

My understanding is that in fact your pil-1.1.6.p2.spkg in fact fully accomplishes (1) and (2) above already, and that if I simply ignore the patch trac_7273-sage-bdist.spkg then I get everything I want already?

Also, should libtiff and libjpeg be posted to the optional spkg repo? Is there a ticket for that? To do that, I would want a commitment that 2 people are going to maintain them, at least for the next year (say).

OK, I've looked at the PIL spkg and I think it looks very good.

So I'm +1 for putting this new spkg in, and I'm -1 to trac_7273-sage-bdist.spkg.

@williamstein
Copy link
Contributor

comment:11

I'm guessing libtiff and libjpeg have to be voted in?

For standard yes, but for optional, the main thing is to get somebody to referee the packages and commit to maintaining them.

@mwhansen
Copy link
Contributor

Reviewer: William Stein

@mwhansen
Copy link
Contributor

Merged: sage-4.2.1.rc0

@mwhansen
Copy link
Contributor

comment:12

I've merge just the spkg in 4.2.1.rc0.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Nov 12, 2009

comment:13

Just to confirm, yes, GeorgSWeber's description was right. The tickets for libtiff and libjpeg are #7345 and #7344. I'm not sure who else can maintain them, though.

@kcrisman
Copy link
Member

kcrisman commented Aug 2, 2010

comment:14

See also here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants