[PATCH v2 1/4] build/tools: disable libvchan build on NetBSD

[PATCH v2 1/4] build/tools: disable libvchan build on NetBSD

[Roger Pau Monne]

NetBSD doesn't have a gntdev, so libvchan is unable to build due to
the lack of the header files gntalloc.h.

This is the error:

gcc  -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing
-std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement
-D__XEN_TOOLS__ -MMD -MF .init.o.d -fno-optimize-sibling-calls
-I../include -I.
-I/root/xen/xen-netbsd/tools/libvchan/../../tools/xenstore
-I/root/xen/xen-netbsd/tools/libvchan/../../tools/include
-I/root/xen/xen-netbsd/tools/libvchan/../../tools/libxc
-I/root/xen/xen-netbsd/tools/libvchan/../../tools/include  -c -o
init.o init.c
init.c:45:30: fatal error: xen/sys/gntalloc.h: No such file or
directory
compilation terminated.
gmake[3]: *** [init.opic] Error 1
gmake[3]: *** Waiting for unfinished jobs....
init.c:45:30: fatal error: xen/sys/gntalloc.h: No such file or
directory
compilation terminated.

Acked-by: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/Makefile b/tools/Makefile
index 7b14678..fbdf716 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -30,7 +30,7 @@ SUBDIRS-$(CONFIG_NetBSD) += blktap2
 SUBDIRS-$(CONFIG_NetBSD) += xenbackendd
 SUBDIRS-y += libfsimage
 SUBDIRS-$(LIBXENAPI_BINDINGS) += libxen
-SUBDIRS-y += libvchan
+SUBDIRS-$(CONFIG_Linux) += libvchan
 
 # do not recurse in to a dir we are about to delete
 ifneq "$(MAKECMDGOALS)" "distclean"
-- 
1.7.7.5 (Apple Git-26)

[PATCH v2 2/4] autoconf: correctly parse *_INCLUDES and *_LIB env vars

[Roger Pau Monne]

Parse those options correctly, since the "+=" operator is not valid.
Also added CPPFLAGS, so headers checks don't give strange results.

Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/m4/set_cflags_ldflags.m4 |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/m4/set_cflags_ldflags.m4 b/tools/m4/set_cflags_ldflags.m4
index 7e357ba..631e81c 100644
--- a/tools/m4/set_cflags_ldflags.m4
+++ b/tools/m4/set_cflags_ldflags.m4
@@ -1,20 +1,21 @@
 AC_DEFUN([AX_SET_FLAGS],
 [for cflag in $PREPEND_INCLUDES
 do
-    PREPEND_CFLAGS+=" -I$cflag"
+    PREPEND_CFLAGS="$PREPEND_CFLAGS -I$cflag"
 done
 for ldflag in $PREPEND_LIB
 do
-    PREPEND_LDFLAGS+=" -L$ldflag"
+    PREPEND_LDFLAGS="$PREPEND_LDFLAGS -L$ldflag"
 done
 for cflag in $APPEND_INCLUDES
 do
-    APPEND_CFLAGS+=" -I$cflag"
+    APPEND_CFLAGS="$APPEND_CFLAGS -I$cflag"
 done
 for ldflag in $APPEND_LIB
 do
-    APPEND_LDFLAGS+=" -L$ldflag"
+    APPEND_LDFLAGS="$APPEND_LDFLAGS -L$ldflag"
 done
 CFLAGS="$PREPEND_CFLAGS $CFLAGS $APPEND_CFLAGS"
+CPPFLAGS="$CFLAGS"
 LDFLAGS="$PREPEND_LDFLAGS $LDFLAGS $APPEND_LDFLAGS"])
 
-- 
1.7.7.5 (Apple Git-26)

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

[Roger Pau Monne]

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.

Cc: Ian Jackson <ian.jackson@citrix.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/python/genwrap.py |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/python/genwrap.py b/tools/python/genwrap.py
index af8a5e9..0d7cc98 100644
--- a/tools/python/genwrap.py
+++ b/tools/python/genwrap.py
@@ -309,10 +309,12 @@ _hidden int genwrap__defbool_set(PyObject *v, libxl_defbool *db);
 #include <stdint.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include "libxl.h" /* gah */
+#include "%s" /* gah */
 #include "%s"
 
-""" % tuple((' '.join(sys.argv),) + (os.path.split(decls)[-1:]),))
+""" % tuple((' '.join(sys.argv),) +
+            (os.path.dirname(sys.argv[1]) + "/libxl.h",) +
+            (os.path.split(decls)[-1:]),))
     for ty in types:
         if ty.private:
             continue
-- 
1.7.7.5 (Apple Git-26)

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

[Ian Campbell]

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.

> 
> Cc: Ian Jackson <ian.jackson@citrix.com>
> Cc: Christoph Egger <Christoph.Egger@amd.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
> ---
>  tools/python/genwrap.py |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/python/genwrap.py b/tools/python/genwrap.py
> index af8a5e9..0d7cc98 100644
> --- a/tools/python/genwrap.py
> +++ b/tools/python/genwrap.py
> @@ -309,10 +309,12 @@ _hidden int genwrap__defbool_set(PyObject *v, libxl_defbool *db);
>  #include <stdint.h>
>  #include <stdlib.h>
>  #include <stdio.h>
> -#include "libxl.h" /* gah */
> +#include "%s" /* gah */
>  #include "%s"
>  
> -""" % tuple((' '.join(sys.argv),) + (os.path.split(decls)[-1:]),))
> +""" % tuple((' '.join(sys.argv),) +
> +            (os.path.dirname(sys.argv[1]) + "/libxl.h",) +
> +            (os.path.split(decls)[-1:]),))
>      for ty in types:
>          if ty.private:
>              continue

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

[Roger Pau Monne]

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.

>
>> Cc: Ian Jackson<ian.jackson@citrix.com>
>> Cc: Christoph Egger<Christoph.Egger@amd.com>
>> Signed-off-by: Roger Pau Monne<roger.pau@citrix.com>
>> ---
>>   tools/python/genwrap.py |    6 ++++--
>>   1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/python/genwrap.py b/tools/python/genwrap.py
>> index af8a5e9..0d7cc98 100644
>> --- a/tools/python/genwrap.py
>> +++ b/tools/python/genwrap.py
>> @@ -309,10 +309,12 @@ _hidden int genwrap__defbool_set(PyObject *v, libxl_defbool *db);
>>   #include<stdint.h>
>>   #include<stdlib.h>
>>   #include<stdio.h>
>> -#include "libxl.h" /* gah */
>> +#include "%s" /* gah */
>>   #include "%s"
>>
>> -""" % tuple((' '.join(sys.argv),) + (os.path.split(decls)[-1:]),))
>> +""" % tuple((' '.join(sys.argv),) +
>> +            (os.path.dirname(sys.argv[1]) + "/libxl.h",) +
>> +            (os.path.split(decls)[-1:]),))
>>       for ty in types:
>>           if ty.private:
>>               continue
>
>

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

[Ian Campbell]

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?

Ian.

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

[Roger Pau Monne]

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.

> Ian.
>

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

[Ian Campbell]

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?

Ian.

> 
> > Ian.
> >
> 

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

[Christoph Egger]

On 05/17/12 17:14, 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?


/usr/pkg/include comes from setting PREPEND_LIB=/usr/pkg/include
when running configure.

Christoph

> Ian.
> 
>>
>>> Ian.


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

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

[Ian Campbell]

On Fri, 2012-05-18 at 09:37 +0100, Christoph Egger wrote:
> 
> > You aren't using some EXTRA_CFLAGS or similar are you?

> /usr/pkg/include comes from setting PREPEND_LIB=/usr/pkg/include
> when running configure.

Shouldn't you be using APPEND_LIB for this instead?

Ian.

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

[Roger Pau Monne]

Christoph Egger wrote:
> On 05/17/12 17:14, 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?
>
>
> /usr/pkg/include comes from setting PREPEND_LIB=/usr/pkg/include
> when running configure.

Nope, it came from Python itself, if you take a look at 
/usr/pkg/lib/python2.7/config/Makefile there's and OPT var that gets 
appended to every extension build, before the user supplied flags. Let's 
see if the Python pkg maintainer is happy to remove "OPT", since I don't 
think it does anything (pkg/46459).

> Christoph
>
>> Ian.
>>
>>>> Ian.
>
>

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

[Roger Pau Monne]

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

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

[Ian Campbell]

On Fri, 2012-05-18 at 09:41 +0100, Roger Pau Monne wrote:
> 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).

I don't much like having absolute paths in includes. Imagine I moved my
source tree, then very strange errors would occur. Also it should be
unnecessary unless the underlying system has some very weird
properties...

The right thing is to fix the underlying python problem, which it seems
you have in hand.

I considered suggesting using a relative include here but I expect it
would get resolved relative to each of the -I options in turn
(e.g. /usr/include/../libxl/libxl.h or whatever) which would be even
worse IMHO.

Ian.

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

[Roger Pau Monne]

Ian Campbell wrote:
> On Fri, 2012-05-18 at 09:41 +0100, Roger Pau Monne wrote:
>> 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).
>
> I don't much like having absolute paths in includes. Imagine I moved my
> source tree, then very strange errors would occur. Also it should be
> unnecessary unless the underlying system has some very weird
> properties...

So at least the correct fix would be to replace

#include "libxl.h"

with

#include <libxl.h>

right?

> The right thing is to fix the underlying python problem, which it seems
> you have in hand.

Yes, I've send a PR, but the python port seems to have no specific 
maintainer, so I don't know how long it will take before someone picks 
it up...

> I considered suggesting using a relative include here but I expect it
> would get resolved relative to each of the -I options in turn
> (e.g. /usr/include/../libxl/libxl.h or whatever) which would be even
> worse IMHO.
>
> Ian.
>
>
>
>

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

[Ian Campbell]

On Fri, 2012-05-18 at 10:14 +0100, Roger Pau Monne wrote:
> Ian Campbell wrote:
> > On Fri, 2012-05-18 at 09:41 +0100, Roger Pau Monne wrote:
> >> 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).
> >
> > I don't much like having absolute paths in includes. Imagine I moved my
> > source tree, then very strange errors would occur. Also it should be
> > unnecessary unless the underlying system has some very weird
> > properties...
> 
> So at least the correct fix would be to replace
> 
> #include "libxl.h"
> 
> with
> 
> #include <libxl.h>
> 
> right?

I think it ould be strictly speaking correct to make that change,
although I'm not sure it would make a difference unless there was
another libxl.h in tools/python somewhere.

> > The right thing is to fix the underlying python problem, which it seems
> > you have in hand.
> 
> Yes, I've send a PR, but the python port seems to have no specific 
> maintainer, so I don't know how long it will take before someone picks 
> it up...

OK.

Ian.

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

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v2 3/4] python: set absolute path to libxl.h on _pyxl_types.c"):
> 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'm not sure what you mean by "the old one".  Do you mean one in
/usr/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.

The build system should make sure that our own include directories
come before system directories.  Otherwise various other things aren't
going to work either.

Ian.

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

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 3/4] python: set absolute path to libxl.h on _pyxl_types.c"):
>> 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'm not sure what you mean by "the old one".  Do you mean one in
> /usr/include ?

Yes, in NetBSD case the ones in /usr/pkg/include, that's where Xen 
headers would reside on a normal installation.

>
>> 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.
>
> The build system should make sure that our own include directories
> come before system directories.  Otherwise various other things aren't
> going to work either.

This only happened when building python extensions because the port 
build of python passes "OPT" to python configure that contains 
-I/usr/pkg/include. And that is used also when building extensions. A 
bug report is in place. The rest of the build system is fine.

> Ian.

[PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Roger Pau Monne]

Tools.mk contains variables that should be used when processing the
top level Config.mk for the tools, specially the CONFIG_DIR variable,
which is not honoring the PREFIX variable correctly, since when
CONFIG_DIR is set the PREFIX var is still not defined.

Including config/Tools.mk before any other includes ensures that
user-set options will be honored.

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/Rules.mk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index a2a1a58..5ded544 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -3,8 +3,8 @@
 # `all' is the default target
 all:
 
-include $(XEN_ROOT)/Config.mk
 -include $(XEN_ROOT)/config/Tools.mk
+include $(XEN_ROOT)/Config.mk
 
 export _INSTALL := $(INSTALL)
 INSTALL = $(XEN_ROOT)/tools/cross-install
-- 
1.7.7.5 (Apple Git-26)

Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion"):
> Tools.mk contains variables that should be used when processing the
> top level Config.mk for the tools, specially the CONFIG_DIR variable,
> which is not honoring the PREFIX variable correctly, since when
> CONFIG_DIR is set the PREFIX var is still not defined.

I'm not sure I really understand how PREFIX is supposed to work.

In a normal package PREFIX would be set to /usr, /usr/local, /opt, or
whatever, by the person doing the installation.

The code in StdGNU.mk seems to be capable of generating paths like
  /usr/local/var/run/xen
which is a bit mad.

Also PREFIX is set in StdGNU.mk.  Why is it also set in Tools.mk ?

Ian.

Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion"):
>> Tools.mk contains variables that should be used when processing the
>> top level Config.mk for the tools, specially the CONFIG_DIR variable,
>> which is not honoring the PREFIX variable correctly, since when
>> CONFIG_DIR is set the PREFIX var is still not defined.
>
> I'm not sure I really understand how PREFIX is supposed to work.
>
> In a normal package PREFIX would be set to /usr, /usr/local, /opt, or
> whatever, by the person doing the installation.
>
> The code in StdGNU.mk seems to be capable of generating paths like
>    /usr/local/var/run/xen
> which is a bit mad.

Not so much, NetBSD has /usr/pkg/etc for example, for configuration of 
packages installed from ports.

Maybe adding the following to config/Linux.mk would be suitable:

XEN_LOCK_DIR = /var/lib
XEN_RUN_DIR = /var/run/xen
XEN_PAGING_DIR = /var/lib/xen/xenpaging
CONFIG_DIR = /etc

So we don't end up putting those under /usr/local or some strange user 
supplied path.

> Also PREFIX is set in StdGNU.mk.  Why is it also set in Tools.mk ?

Configure scripts have a very common option, --prefix, which is saved 
into Tools.mk as PREFIX, this is what should be used as prefix for 
installation.

Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Ian Jackson]

Roger Pau Monne writes ("Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion"):
> Ian Jackson wrote:
> > Also PREFIX is set in StdGNU.mk.  Why is it also set in Tools.mk ?
> 
> Configure scripts have a very common option, --prefix, which is saved 
> into Tools.mk as PREFIX, this is what should be used as prefix for 
> installation.

I know that.  But given that the configure is just for the tools
build, should we honour PREFIX from Config.mk (and ./.config or
command line arguments) or from ./configure ?

Ian.

Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Christoph Egger]

On 05/18/12 13:46, Ian Jackson wrote:

> Roger Pau Monne writes ("Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion"):
>> Ian Jackson wrote:
>>> Also PREFIX is set in StdGNU.mk.  Why is it also set in Tools.mk ?
>>
>> Configure scripts have a very common option, --prefix, which is saved 
>> into Tools.mk as PREFIX, this is what should be used as prefix for 
>> installation.
> 
> I know that.  But given that the configure is just for the tools
> build, should we honour PREFIX from Config.mk (and ./.config or
> command line arguments) or from ./configure ?


The one from configure. The point is that the other path variables
also honour the prefix from configure.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Roger Pau Monne]

Ian Jackson wrote:
> Roger Pau Monne writes ("Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion"):
>> Ian Jackson wrote:
>>> Also PREFIX is set in StdGNU.mk.  Why is it also set in Tools.mk ?
>> Configure scripts have a very common option, --prefix, which is saved
>> into Tools.mk as PREFIX, this is what should be used as prefix for
>> installation.
>
> I know that.  But given that the configure is just for the tools
> build, should we honour PREFIX from Config.mk (and ./.config or
> command line arguments) or from ./configure ?

I think the one from configure, since we have a configure script, better 
make use of it, that is what users expect. I will resend this patch with 
the above changes to config/Linux.mk, is that ok?

Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Ian Jackson]

Roger Pau Monne writes ("Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion"):
> Ian Jackson wrote:
> > I know that.  But given that the configure is just for the tools
> > build, should we honour PREFIX from Config.mk (and ./.config or
> > command line arguments) or from ./configure ?
> 
> I think the one from configure, since we have a configure script, better 
> make use of it, that is what users expect.

I guess so.

> I will resend this patch with the above changes to config/Linux.mk,
> is that ok?

I don't think prefixing paths in /var with PREFIX is ever right.
Overriding it in Linux.mk seems like a band-aid.

Ian.

Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Christoph Egger]

On 05/18/12 15:30, Ian Jackson wrote:

> Roger Pau Monne writes ("Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion"):
>> Ian Jackson wrote:
>>> I know that.  But given that the configure is just for the tools
>>> build, should we honour PREFIX from Config.mk (and ./.config or
>>> command line arguments) or from ./configure ?
>>
>> I think the one from configure, since we have a configure script, better 
>> make use of it, that is what users expect.
> 
> I guess so.
> 
>> I will resend this patch with the above changes to config/Linux.mk,
>> is that ok?
> 
> I don't think prefixing paths in /var with PREFIX is ever right.
> Overriding it in Linux.mk seems like a band-aid.


One suggestion:

Move all path variables out of config/*.mk and let configure generate
a config/paths.mk.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion

[Roger Pau Monne]

Christoph Egger wrote:
> On 05/18/12 15:30, Ian Jackson wrote:
>
>> Roger Pau Monne writes ("Re: [PATCH v2 4/4] tools/build: change order of config/Tools.mk inclusion"):
>>> Ian Jackson wrote:
>>>> I know that.  But given that the configure is just for the tools
>>>> build, should we honour PREFIX from Config.mk (and ./.config or
>>>> command line arguments) or from ./configure ?
>>> I think the one from configure, since we have a configure script, better
>>> make use of it, that is what users expect.
>> I guess so.
>>
>>> I will resend this patch with the above changes to config/Linux.mk,
>>> is that ok?
>> I don't think prefixing paths in /var with PREFIX is ever right.
>> Overriding it in Linux.mk seems like a band-aid.
>
>
> One suggestion:
>
> Move all path variables out of config/*.mk and let configure generate
> a config/paths.mk.

What should we do about this? Should I try to fix 
StdGNU.mk/Linux.mk/NetBSD.mk or are we going to move all paths from 
config/*.mk and put them in a single file generated by configure?

Re: [PATCH v2 1/4] build/tools: disable libvchan build on NetBSD

[Ian Campbell]

On Thu, 2012-05-17 at 13:16 +0100, Roger Pau Monne wrote:
> NetBSD doesn't have a gntdev, so libvchan is unable to build due to
> the lack of the header files gntalloc.h.
> 
> This is the error:
> 
> gcc  -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing
> -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement
> -D__XEN_TOOLS__ -MMD -MF .init.o.d -fno-optimize-sibling-calls
> -I../include -I.
> -I/root/xen/xen-netbsd/tools/libvchan/../../tools/xenstore
> -I/root/xen/xen-netbsd/tools/libvchan/../../tools/include
> -I/root/xen/xen-netbsd/tools/libvchan/../../tools/libxc
> -I/root/xen/xen-netbsd/tools/libvchan/../../tools/include  -c -o
> init.o init.c
> init.c:45:30: fatal error: xen/sys/gntalloc.h: No such file or
> directory
> compilation terminated.
> gmake[3]: *** [init.opic] Error 1
> gmake[3]: *** Waiting for unfinished jobs....
> init.c:45:30: fatal error: xen/sys/gntalloc.h: No such file or
> directory
> compilation terminated.
> 
> Acked-by: Christoph Egger <Christoph.Egger@amd.com>
> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  tools/Makefile |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 7b14678..fbdf716 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -30,7 +30,7 @@ SUBDIRS-$(CONFIG_NetBSD) += blktap2
>  SUBDIRS-$(CONFIG_NetBSD) += xenbackendd
>  SUBDIRS-y += libfsimage
>  SUBDIRS-$(LIBXENAPI_BINDINGS) += libxen
> -SUBDIRS-y += libvchan
> +SUBDIRS-$(CONFIG_Linux) += libvchan
>  
>  # do not recurse in to a dir we are about to delete
>  ifneq "$(MAKECMDGOALS)" "distclean"