Skip to content

Commit

Permalink
Link openjpeg2 statically on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
@cgohlke
cgohlke committed Mar 29, 2014
1 parent af4424e commit e7e103b
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,8 @@ def build_extensions(self):
if feature.jpeg2000:
libs.append(feature.jpeg2000)
defs.append(("HAVE_OPENJPEG", None))
if sys.platform == "win32":
defs.append(("OPJ_STATIC", None))
if feature.zlib:
libs.append(feature.zlib)
defs.append(("HAVE_LIBZ", None))
Expand Down

3 comments on commit e7e103b

@al45tair
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this is a mistake, because it stops users from downloading the binaries from the OpenJPEG home page; after this check-in, builds on Windows will fail unless the user explicitly builds a static library version of OpenJPEG. Many users won't want to build their own OpenJPEG, especially on Windows. (I just spent an hour or so trying to figure out why the build was broken on my Windows test box, and these two lines were the culprit…)

Perhaps there should be an option to use a static OpenJPEG library, instead of hard-coding it?

@cgohlke
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO it is a mistake to suggest using the binaries from the OpenJPEG home page: they are a little outdated, 32 bit only, and linked to the wrong CRT for CPython >= 3.3 (meaning they will fail in 7 out of 10 cases for the official releases). Using dynamic dependencies would also require to pack the DLLs into bdist distributions. I'm OK with making this optional though.

@al45tair
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I had always thought that OpenJPEG support was something that should be strictly optional; most users won't want it, and the largest contingent who do are people who want to read/write Mac OS X icon files made prior to OS X 10.6 (since the larger icons from that era were encoded with JPEG 2000). I'm not sure it's worth including (or even a good idea to include) in the binary distributions — in particular, it seems to me that the majority of JPEG 2000 code probably hasn't been extensively tested for security holes and so it might be better to make it a non-default option that requires building your own Pillow.

As regards the OpenJPEG installer, I think it's a good idea if it will work with those libraries where possible; as you say, that won't work for CPython > 3.3 because of the CRT issue and it won't work for 64-bit. I don't know what proportion of Windows-based Python users that will affect.

Please sign in to comment.