Re: [PATCH v2 3/4] python: set absolute path to libxl.h on _pyxl_types.c

[Roger Pau Monne <roger.pau@citrix.com>]

Ian Campbell wrote:
> On Thu, 2012-05-17 at 16:10 +0100, Roger Pau Monne wrote:
>> Ian Campbell wrote:
>>> On Thu, 2012-05-17 at 15:02 +0100, Roger Pau Monne wrote:
>>>> Ian Campbell wrote:
>>>>> On Thu, 2012-05-17 at 13:16 +0100, Roger Pau Monne wrote:
>>>>>> genwrap.py generates _pyxl_types.c, which includes libxl.h, but if
>>>>>> libxl.h is already present in the include search path, the old one was
>>>>>> included instead of the new one, giving compilation errors. Since
>>>>>> _pyxl_types.c is generated at compilation time, we can safely set
>>>>>> the path to libxl.h include.
>>>>>>
>>>>>> I've only experienced this problem when compiling Xen on NetBSD with
>>>>>> old header files in the include path, Linux seems to not have this
>>>>>> problem.
>>>>> Should this be include<>    and not "", since libxl.h isn't in the current
>>>>> dir in this case?
>>>>>
>>>>> Is the right fix to make sure that the in-tree -I lines come first?
>>>>> Ian.
>>>> Actually I'm not sure if it's possible to make sure the in-tree -I lines
>>>> come first, because the gcc options are automatically generated by
>>>> python build tools (distutils&   friends...), so we cannot touch much of
>>>> this (I've looked at distutils.core options, and it seems to be no way
>>>> of setting an order on compiler options, but I'm no expert on python C
>>>> extensions building), so unless we craft our own makefile to build this
>>>> python stuff I think we are stuck with something like this.
>>> Surely distutils puts user supplied options first before system options?
>>> My /usr/lib/python2.5/distutils/command/build_ext.py says:
>>>
>>>          # Put the Python "system" include dir at the end, so that
>>>           # any local include dirs take precedence.
>>>           self.include_dirs.append(py_include)
>>>           if plat_py_include != py_include:
>>>               self.include_dirs.append(plat_py_include)
>>>
>>> So it seems like this is the intention.
>>>
>>> Are you sure this isn't just a bug in your version of distutils or
>>> something like that?
>>>
>>> My python xl builds end up as:
>>>           building 'xl' extension
>>>           gcc -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall
>>>           -Wstrict-prototypes -O1 -fno-omit-frame-pointer -m32 -march=i686
>>>           -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
>>>           -Wdeclaration-after-statement -D__XEN_TOOLS__ -MMD -MF .build.d
>>>           -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>>>           -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls
>>>           -mno-tls-direct-seg-refs -fPIC -I../../tools/include
>>>           -I../../tools/libxl -I../../tools/libxc -Ixen/lowlevel/xl
>>>           -I/usr/include/python2.6 -c xen/lowlevel/xl/xl.c -o
>>>           build/temp.linux-i686-2.6/xen/lowlevel/xl/xl.o
>>>           -fno-strict-aliasing -Werror
>>>
>>> Which has our local -I options before all the others -- which is
>>> sensible. What do you see with your system?
>> This is what I see:
>>
>> CC="gcc" CFLAGS="-O1 -fno-omit-frame-pointer -m64 -g
>> -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
>> -Wdeclaration-after-statement   -D__XEN_TOOLS__ -MMD -MF .build.d
>> -fno-optimize-sibling-calls" python2.7 setup.py build
>> running build
>> running build_py
>> running build_ext
>> building 'xl' extension
>>
>> gcc -DNDEBUG -O2 -DHAVE_DB_185_H -I/usr/include -I/usr/pkg/include -O1
>> -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall
>> -Wstrict-prototypes -Wdeclaration-after-statement -D__XEN_TOOLS__ -MMD
>> -MF .build.d -fno-optimize-sibling-calls -fPIC -I../../tools/include
>> -I../../tools/libxl -I../../tools/libxc -Ixen/lowlevel/xl
>> -I/usr/pkg/include/python2.7 -c xen/lowlevel/xl/_pyxl_types.c -o
>> build/temp.netbsd-6.0_BETA-amd64-2.7/xen/lowlevel/xl/_pyxl_types.o
>> -fno-strict-aliasing -Werror
>>
>> xen/lowlevel/xl/_pyxl_types.c: In function 'genwrap__init':
>> xen/lowlevel/xl/_pyxl_types.c:4346:78: error:
>> 'LIBXL_EVENT_TYPE_DOMAIN_CREATE_CONSOLE_AVAILABLE' undeclared (first use
>> in this function)
>> xen/lowlevel/xl/_pyxl_types.c:4346:78: note: each undeclared identifier
>> is reported only once for each function it appears in
>> error: command 'gcc' failed with exit status 1
>> gmake: *** [build] Error 1
>> gmake: Leaving directory `/root/xen/xen-netbsd/tools/python'
>>
>> So this part "-DNDEBUG -O2 -DHAVE_DB_185_H -I/usr/include
>> -I/usr/pkg/include" comes even before our passed CFLAGS.
>>
>> My /usr/pkg/lib/python2.7/distutils/command/build_ext.py:
>>
>>           # Put the Python "system" include dir at the end, so that
>>           # any local include dirs take precedence.
>>           self.include_dirs.append(py_include)
>>           if plat_py_include != py_include:
>>               self.include_dirs.append(plat_py_include)
>>
>> 	[...]
>>
>>
>>           # Finally add the user include and library directories if requested
>>           if self.user:
>>               user_include = os.path.join(USER_BASE, "include")
>>               user_lib = os.path.join(USER_BASE, "lib")
>>               if os.path.isdir(user_include):
>>                   self.include_dirs.append(user_include)
>>               if os.path.isdir(user_lib):
>>                   self.library_dirs.append(user_lib)
>>                   self.rpath.append(user_lib)
>>
>> So it says it puts them at the end, but it doesn't do so. I've looked at
>> Python 2.7 original source, and this is not a NetBSD port specific bug.
>
> You are sure this is that snippet of code adding this? Where
> does /usr/pkg/include come from? This only appears to add one -I and you
> have two extra.
>
> You aren't using some EXTRA_CFLAGS or similar are you?

This fault was due to the way NetBSD pkgsrc builds Python, passing 
OPT="-I/usr/include -I/usr/pkg/include ..." to the configure script, 
which then gets saved to a Makefile that is parsed by distutils and 
appended to the build of every extension. A bug report has already been 
sent:

http://mail-index.netbsd.org/pkgsrc-bugs/2012/05/17/msg047735.html

Anyway, I don't think setting libxl.h path in genwrap.py is such a bad 
idea, this file gets regenerated during every build, and we can make 
sure we are always including the correct header (which should happen 
automatically unless there are some underlying problems with Python, 
like on NetBSD).

> Ian.
>
>>> Ian.
>>>
>
>