C include order

Taking John up on his offer to close some bugs, it looks like Jeff
closed SF bug 1372239[1], by moving the #include "Python.h" statement
ahead of the other includes.

As mentioned in the bug report, the "official" Python/C API docs[2]
state this is the correct behavior.

Unfortunately, on my Debian sarge amd64 system, this "fix" of a compiler
warning results in a compile error:

gcc: src/_image.cpp
In file included from /usr/include/png.h:363,
                 from src/_image.cpp:6:
/usr/include/pngconf.h:310:2: #error png.h already includes setjmp.h
with some additional fixup.
In file included from /usr/include/png.h:363,
                 from src/_image.cpp:6:
/usr/include/pngconf.h:310:2: #error png.h already includes setjmp.h
with some additional fixup.
error: Command "gcc -pthread -fno-strict-aliasing -DNDEBUG -g -O3 -Wall
-Wstrict-prototypes -fPIC
-I/home/astraw/py2.3-linux-x86_64/lib/python2.3/site-packages/numpy-0.9.7.2243-py2.3-linux-x86_64.egg/numpy/core/include
-I/usr/local/include -I/usr/include -I. -Isrc -Iswig -Iagg23/include -I.
-I/usr/local/include -I/usr/include -I.
-I/home/astraw/py2.3-linux-x86_64/lib/python2.3/site-packages/numpy-0.9.7.2243-py2.3-linux-x86_64.egg/numpy/core/include/freetype2
-I/usr/local/include/freetype2 -I/usr/include/freetype2 -I./freetype2
-Isrc/freetype2 -Iswig/freetype2 -Iagg23/include/freetype2 -I./freetype2
-I/usr/local/include/freetype2 -I/usr/include/freetype2 -I./freetype2
-I/usr/include/python2.3 -c src/_image.cpp -o
build/temp.linux-x86_64-2.3/src/_image.o -DSCIPY=1" failed with exit
status 1

Googling on this indicates this libpng's setjmp() redefinition may be
responsible[3]. This looks like a real issue with Python.h and png.h and
not something we have much control over, at least for the versions
present in Debian sarge.

I've been playing with various things, but I can't get MPL to compile
unless I put #include <png.h> before #include "Python.h". I certainly
don't consider myself a C include-file-order expert, so perhaps there's
a way to avert this situation without reverting Jeff's patch.

On the flipside, I went ahead and located several locations in the code
where Python.h is included after system headers. I'm attaching a patch
with the fruits of this labor. (Unfortunately, it also includes png.h
before Python.h, because otherwise I can't get MPL to compile.) I'm not
committing this because it seems like a rather significant change, and
I'm not sure what it fixes other than more compiler warnings. And given
Jeff's luck "fixing" compiler warnings, I'm not inclined to force this
on anyone.

In fact, I suggest simply reverting the changes in svn 2185 to
src/_image.cpp, which, apart from a compiler warning, was apparently
working just fine.

[1] SF bug 1372239
http://sourceforge.net/tracker/index.php?func=detail&aid=1372239&group_id=80706&atid=560720
[2] Python/C API docs http://docs.python.org/api/includes.html
[3] libpng's setjmp() redefinition
http://lists.debian.org/debian-devel/2005/02/msg00892.html

Andrew Straw wrote:

On the flipside, I went ahead and located several locations in the code
where Python.h is included after system headers. I'm attaching a patch
with the fruits of this labor. (Unfortunately, it also includes png.h
before Python.h, because otherwise I can't get MPL to compile.) I'm not
committing this because it seems like a rather significant change, and
I'm not sure what it fixes other than more compiler warnings. And given
Jeff's luck "fixing" compiler warnings, I'm not inclined to force this
on anyone.

Whoops, forgot the attachment. Here it is.

import_python_first.patch (2.76 KB)

Andrew Straw wrote:

Taking John up on his offer to close some bugs, it looks like Jeff
closed SF bug 1372239[1], by moving the #include "Python.h" statement
ahead of the other includes.

As mentioned in the bug report, the "official" Python/C API docs[2]
state this is the correct behavior.

Unfortunately, on my Debian sarge amd64 system, this "fix" of a compiler
warning results in a compile error:

gcc: src/_image.cpp
In file included from /usr/include/png.h:363,
                 from src/_image.cpp:6:
/usr/include/pngconf.h:310:2: #error png.h already includes setjmp.h
with some additional fixup.
In file included from /usr/include/png.h:363,
                 from src/_image.cpp:6:
/usr/include/pngconf.h:310:2: #error png.h already includes setjmp.h
with some additional fixup.
error: Command "gcc -pthread -fno-strict-aliasing -DNDEBUG -g -O3 -Wall
-Wstrict-prototypes -fPIC
-I/home/astraw/py2.3-linux-x86_64/lib/python2.3/site-packages/numpy-0.9.7.2243-py2.3-linux-x86_64.egg/numpy/core/include
-I/usr/local/include -I/usr/include -I. -Isrc -Iswig -Iagg23/include -I.
-I/usr/local/include -I/usr/include -I.
-I/home/astraw/py2.3-linux-x86_64/lib/python2.3/site-packages/numpy-0.9.7.2243-py2.3-linux-x86_64.egg/numpy/core/include/freetype2
-I/usr/local/include/freetype2 -I/usr/include/freetype2 -I./freetype2
-Isrc/freetype2 -Iswig/freetype2 -Iagg23/include/freetype2 -I./freetype2
-I/usr/local/include/freetype2 -I/usr/include/freetype2 -I./freetype2
-I/usr/include/python2.3 -c src/_image.cpp -o
build/temp.linux-x86_64-2.3/src/_image.o -DSCIPY=1" failed with exit
status 1

Googling on this indicates this libpng's setjmp() redefinition may be
responsible[3]. This looks like a real issue with Python.h and png.h and
not something we have much control over, at least for the versions
present in Debian sarge.

I've been playing with various things, but I can't get MPL to compile
unless I put #include <png.h> before #include "Python.h". I certainly
don't consider myself a C include-file-order expert, so perhaps there's
a way to avert this situation without reverting Jeff's patch.

On the flipside, I went ahead and located several locations in the code
where Python.h is included after system headers. I'm attaching a patch
with the fruits of this labor. (Unfortunately, it also includes png.h
before Python.h, because otherwise I can't get MPL to compile.) I'm not
committing this because it seems like a rather significant change, and
I'm not sure what it fixes other than more compiler warnings. And given
Jeff's luck "fixing" compiler warnings, I'm not inclined to force this
on anyone.

In fact, I suggest simply reverting the changes in svn 2185 to
src/_image.cpp, which, apart from a compiler warning, was apparently
working just fine.

[1] SF bug 1372239
http://sourceforge.net/tracker/index.php?func=detail&aid=1372239&group_id=80706&atid=560720
[2] Python/C API docs http://docs.python.org/api/includes.html
[3] libpng's setjmp() redefinition
http://lists.debian.org/debian-devel/2005/02/msg00892.html

Andrew: Thanks - it seemed like a harmless change, but I should know better. I have libpng 1.2.8 on my MacOS X system and it worked fine - is that newer than the version you have?

-Jeff

···

--
Jeffrey S. Whitaker Phone : (303)497-6313
NOAA/OAR/CDC R/CDC1 FAX : (303)497-6449
325 Broadway Web : http://www.cdc.noaa.gov/~jsw
Boulder, CO, USA 80305-3328 Office: Skaggs Research Cntr 1D-124