I tested your patch with Ubuntu 10.10, and it failed. The problem is
that something is including setjmp.h before libpng.h tries to do so via
pngconf.h, resulting in an error as the compiler trips over the
Python.h includes pyfpe.h which includes setjmp.h.
Ah, good catch. So we just need to include Python.h first, and then set
that extra #def so that libpng doesn't try to include it?
Let me try again:
In _backend_agg.cpp and _png.cpp, just add
Does that fix it?
Sure does. Your patch with that modification is committed to branch and
trunk, 8706, 8707. Thank you!
Did someone check on Windows? I was hoping things wouldn't break in
WrapPython.h when I switched the order of includes, but you never know...
Big trouble, even without Windows. First, after doing more reading, I
am far from sure that skipping the check is OK. Second, it doesn't work
on earlier Linux versions, for which pngconf.h lacks that SKIP variable
What a pain. I see that Andrew Straw ran into this wall a couple years ago.
Time to revert and re-think. It may be that your patch is almost OK,
but will need a tweak for Linux.
An equivalent, but very hackish, fix is to just undef _SETJMP_H. That's
almost as bad as the original undef, though.
Maybe putting this:
right before including png.h would work, at least until distros
(eventually) upgrade to a libpng past April 2009 (when the check was
added) or until we stop supporting the old distros.
I don't think that any of these hacks is desirable, so I am trying what I hope is a less-bad hack. Maybe Mike or John will be able to figure out a cleaner solution.
I'm still worried about the possibility of a conflict between the versions of setjmp expected by png and by python. I think that if there were such a conflict, it would show up as a crash as part of the error handling in one or the other. Therefore it could lurk undetected for a long time. On the other hand, if there were such a conflict, I don't see how the pre-patched versions would have avoided it any better than the present version.
I could not find any evidence that _backend_agg even needs to include png.h, so I deleted that inclusion, leaving only _png.cpp as the trouble spot.
On 09/16/2010 08:45 PM, Jason Grout wrote:
On 9/16/10 11:16 PM, Eric Firing wrote:
On 09/16/2010 05:15 PM, Jason Grout wrote:
On 9/16/10 10:00 PM, Eric Firing wrote:
On 09/16/2010 04:12 PM, Jason Grout wrote:
On 9/16/10 9:03 PM, Jason Grout wrote:
On 9/16/10 8:00 PM, Eric Firing wrote: