compiling on Solaris (and going against the python API)

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include <png.h>

or something like that?

Jason

···

On 9/16/10 8:00 PM, Eric Firing wrote:

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

Does that fix it?

Thanks,

Jason

···

On 9/16/10 9:03 PM, Jason Grout wrote:

On 9/16/10 8:00 PM, Eric Firing wrote:

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

Does that fix it?

Sure does. Your patch with that modification is committed to branch and trunk, 8706, 8707. Thank you!

Eric

···

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:

Thanks,

Jason

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...

Jason

···

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:

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

Does that fix it?

Sure does. Your patch with that modification is committed to branch and
trunk, 8706, 8707. Thank you!

Also, since you guys already have a relationship with upstream (CXX), could you report the bug and fix upstream?

Thanks,

Jason

···

On 9/16/10 10:15 PM, Jason Grout wrote:

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...

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

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...

Jason,

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 entirely.

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.

Eric

···

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:

Jason

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

Trunk and 1.0 branch build OK on Windows.

···

On 9/16/2010 8: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:

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

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...

Jason

--
Christoph

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:

#ifdef __linux__
   #undef _SETJMP_H
#endif
#define PNG_SKIP_SETJMP_CHECK

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.

Thanks,

Jason

···

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:

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

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...

Jason,

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
entirely.

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.

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

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...

Jason,

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
entirely.

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:

#ifdef __linux__
    #undef _SETJMP_H
#endif
#define PNG_SKIP_SETJMP_CHECK

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.

Eric

···

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:

Thanks,

Jason

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

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...

Jason

Trunk and 1.0 branch build OK on Windows.

Christoph,

Once again, thanks very much for testing on Windows.

I found a problem on linux (or rather, the buildbot found it because it is running an older version) so I have committed one more change to the branch. If that survives the buildbot, I will propagate it to the trunk, but I don't consider it the last word; I am hoping that someone else can do a better job of cleaning this up.

Eric

···

On 09/16/2010 08:21 PM, Christoph Gohlke wrote:

On 9/16/2010 8: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:

--
Christoph

I see the change that you made (keep the old order for linux, do the new thing for everything else). This seems like a bad thing to do. Looking at setjmp.h, it includes features.h. features.h relies on the POSIX and XOPEN variables that are defined in Python.h to set up the environment, and the actions from setjmp.h depend on that environment. I think the warning from the Python docs (if I read correctly) is that the environment must be set up according to the requirements, and Python.h sets up those requirements. In other words, by undefining the POSIX and XOPEN variables, it seems you have a very real likelihood of including a different setjmp.h in the Python.h compared to the libpng (because the conditions set up by features.h may be different).

Again, I don't see a good way out of the mess. And I guess in this imperfect world, I can't argue with what appears to work! And as a big disclaimer; I knew almost nothing about all of this a couple of days ago.

Thanks,

Jason

···

On 9/17/10 1:57 AM, Eric Firing wrote:

On 09/16/2010 08:21 PM, Christoph Gohlke wrote:

On 9/16/2010 8: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:

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

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...

Jason

Trunk and 1.0 branch build OK on Windows.

Christoph,

Once again, thanks very much for testing on Windows.

I found a problem on linux (or rather, the buildbot found it because it
is running an older version) so I have committed one more change to the
branch. If that survives the buildbot, I will propagate it to the
trunk, but I don't consider it the last word; I am hoping that someone
else can do a better job of cleaning this up.

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
following:

Python.h includes pyfpe.h which includes setjmp.h.

Eric

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?

#include "Python.h"
#def PNG_SKIP_SETJMP_CHECK
#include<png.h>

Let me try again:

In _backend_agg.cpp and _png.cpp, just add

#define PNG_SKIP_SETJMP_CHECK

right above

#include<png.h>

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...

Jason

Trunk and 1.0 branch build OK on Windows.

Christoph,

Once again, thanks very much for testing on Windows.

I found a problem on linux (or rather, the buildbot found it because it
is running an older version) so I have committed one more change to the
branch. If that survives the buildbot, I will propagate it to the
trunk, but I don't consider it the last word; I am hoping that someone
else can do a better job of cleaning this up.

I see the change that you made (keep the old order for linux, do the new
thing for everything else). This seems like a bad thing to do. Looking
at setjmp.h, it includes features.h. features.h relies on the POSIX and
XOPEN variables that are defined in Python.h to set up the environment,
and the actions from setjmp.h depend on that environment. I think the
warning from the Python docs (if I read correctly) is that the
environment must be set up according to the requirements, and Python.h
sets up those requirements. In other words, by undefining the POSIX and
XOPEN variables, it seems you have a very real likelihood of including a
different setjmp.h in the Python.h compared to the libpng (because the
conditions set up by features.h may be different).

Before making that change, I verified that at least on my linux system, Python.h defines those two variables the same way that features.h does, so the redefinition would be harmless if we did not undefine the variables. I don't think we are any worse off than before with respect to linux, and we should be better off with respect to other platforms--mpl on Solaris now compiles, right?. But I am certainly still not comfortable with the whole setjmp mess. I would love to see someone come up with a clean, clearly understandable solution, and resolve it once and for all.

Eric

···

On 09/16/2010 09:27 PM, Jason Grout wrote:

On 9/17/10 1:57 AM, Eric Firing wrote:

On 09/16/2010 08:21 PM, Christoph Gohlke wrote:

On 9/16/2010 8: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:

Again, I don't see a good way out of the mess. And I guess in this
imperfect world, I can't argue with what appears to work! And as a big
disclaimer; I knew almost nothing about all of this a couple of days ago.

Thanks,

Jason

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Matplotlib-devel mailing list
Matplotlib-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-devel

One more test point -- on my solaris x86 box running python2.4, svn
HEAD (and r8707) appear to work fine.

johnh@...749...:tests> uname -a
SunOS udesktop191 5.10 Generic_139556-08 i86pc i386 i86pc
johnh@...749...:tests> python -V
Python 2.4.5
johnh@...749...:tests> /opt/app/g++lib6/gcc-4.2/bin/gcc --version
gcc (GCC) 4.2.2

···

On Fri, Sep 17, 2010 at 3:04 AM, Eric Firing <efiring@...229...> wrote:

Before making that change, I verified that at least on my linux system,
Python.h defines those two variables the same way that features.h does,
so the redefinition would be harmless if we did not undefine the
variables. I don't think we are any worse off than before with respect
to linux, and we should be better off with respect to other
platforms--mpl on Solaris now compiles, right?. But I am certainly
still not comfortable with the whole setjmp mess. I would love to see
someone come up with a clean, clearly understandable solution, and
resolve it once and for all.

Okay, I'm glad you checked on your box. I agree that we wouldn't be any worse than before. Hopefully everyone's feature.h defaults are the same as what Python assumes. And hopefully Python hasn't changed those definitions over the supported releases.

However, wouldn't it be good to leave the redefinitions in, though? They are warnings (and they are not spurious), and that would remind us that we should investigate the situation in the future (maybe after we stop supporting old versions of libpng). At least on your system, the redefinition would not change anything. I would prefer a warning reminding me of an ugly situation that could really potentially be a problem, rather than a hack to disable the warning because it doesn't affect a particular system or two.

Thanks,

Jason

···

On 09/17/2010 03:04 AM, Eric Firing wrote:

On 09/16/2010 09:27 PM, Jason Grout wrote:

I see the change that you made (keep the old order for linux, do the new
thing for everything else). This seems like a bad thing to do. Looking
at setjmp.h, it includes features.h. features.h relies on the POSIX and
XOPEN variables that are defined in Python.h to set up the environment,
and the actions from setjmp.h depend on that environment. I think the
warning from the Python docs (if I read correctly) is that the
environment must be set up according to the requirements, and Python.h
sets up those requirements. In other words, by undefining the POSIX and
XOPEN variables, it seems you have a very real likelihood of including a
different setjmp.h in the Python.h compared to the libpng (because the
conditions set up by features.h may be different).

Before making that change, I verified that at least on my linux system,
Python.h defines those two variables the same way that features.h does,
so the redefinition would be harmless if we did not undefine the
variables. I don't think we are any worse off than before with respect
to linux, and we should be better off with respect to other
platforms--mpl on Solaris now compiles, right?. But I am certainly
still not comfortable with the whole setjmp mess. I would love to see
someone come up with a clean, clearly understandable solution, and
resolve it once and for all.