Re: [Qemu-devel] [PATCH] Fix out-of-tree and cross compile builds for pixman

[Johnson, Eric]

> -----Original Message-----
> From: address@hidden [mailto:qemu-devel-
> address@hidden On Behalf Of Stefan Weil
> Sent: Sunday, November 04, 2012 4:11 AM
> To: Blue Swirl
> Cc: Peter Maydell; address@hidden
> Subject: Re: [Qemu-devel] [PATCH] Fix out-of-tree and cross compile builds
> for pixman
> 
> Am 03.11.2012 21:15, schrieb Blue Swirl:
> > On Sat, Nov 3, 2012 at 7:02 PM, Peter Maydell <address@hidden>
> wrote:
> >> On 3 November 2012 19:47, Blue Swirl <address@hidden> wrote:
> >>> --- a/Makefile
> >>> +++ b/Makefile
> >>> @@ -122,7 +122,7 @@ subdir-pixman: pixman/Makefile
> >>>          $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pixman
> V="$(V)" all,)
> >>>
> >>>   pixman/Makefile: $(SRC_PATH)/pixman/configure
> >>> -       (cd pixman; $(SRC_PATH)/pixman/configure --disable-shared --
> enable-static)
> >>> +       (cd pixman; CC=$(CC) LD=$(LD) AR=$(AR) NM=$(NM)
> RANLIB=$(RANLIB) $(SRC_PATH)/pixman/configure --disable-shared --enable-
> static)
> >> Not tested, but aren't there quoting issues here if you're
> >> building with --cc='ccache gcc' ?
> > Yes. Also configure fails because the variables are not expanded and
> > directory pixman/pixman does not exist. Funny how it worked earlier.
> 
> I struggle with the same issue, and there are more problems caused
> by the internal pixman code.
> 
> The dependencies are wrong because pixman is built too late:
> $(TOOLS) also depends on it. It is not trivial to model them correctly.
> IMHO it would be better to build the internal pixman immediately when
> QEMU's configure is called. Then QEMU's make can always rely on an
> existing pixman.
> 
> The internal pixman code is also too old for cross compilations with
> MinGW-w64. It already fails when running configure.
> 
> Newer versions of pixman compile after a trivial modification which
> is needed to avoid redefined symbols:
> 
> diff --git a/pixman/pixman-mmx.c b/pixman/pixman-mmx.c
> index 1a014fd..723c245 100644
> --- a/pixman/pixman-mmx.c
> +++ b/pixman/pixman-mmx.c
> @@ -61,7 +61,7 @@ _mm_empty (void)
>   #endif
> 
>   #ifdef USE_X86_MMX
> -# if (defined(__SUNPRO_C) || defined(_MSC_VER))
> +# if (defined(__SUNPRO_C) || defined(_MSC_VER) || defined(__WIN64))
>   #  include <xmmintrin.h>
>   # else
>   /* We have to compile with -msse to use xmmintrin.h, but that causes SSE
> 
> More changes are needed to avoid typical MinGW-w64 compiler warnings
> (pointer to int conversions without uintptr_t).
> 
> Up to now, I did not test the resulting code, so maybe there will be more
> surprises.
> 
> Regards
> Stefan
> 

I was able to get a parallel out-of-tree build to work with the following 
changes.  I'm not sure if it violates any rules about make features or 
configure processing.

Unless order only dependencies are not allowed the following should fix the 
build order for pixman and the tools.

diff --git a/Makefile b/Makefile
index ca14a21..42dcf92 100644
--- a/Makefile
+++ b/Makefile
@@ -118,6 +118,8 @@ endif
 
 subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o
 
+$(TOOLS): | subdir-pixman
+
 subdir-pixman: pixman/Makefile
        $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pixman V="$(V)" 
all,)

The following change does the internal pixman configuration during the QEMU 
configuration.

The upside is that the environment variables get passed to the sub-configure.  
Probably still need the Makefile fixes for CC="$(CC)", etc. just in case the 
pixman configuration dependencies are updated.

The down side is that \$(BUILD_DIR) and \$(SRC_PATH) cannot be used.  Instead I 
used `pwd` and ${source_path}.  Maybe with some more work the pixman 
configuration would work using the make variables.

diff --git a/configure b/configure
index 7290f50..97c7d15 100755
--- a/configure
+++ b/configure
@@ -2121,8 +2121,19 @@ else
     echo "      git submodule update --init pixman"
     exit 1
   fi
-  pixman_cflags="-I${source_path}/pixman/pixman"
-  pixman_libs="-Lpixman/pixman/.libs -lpixman-1"
+
+  if [ "$source_path" != `pwd` ]; then
+    pixman_cflags="-I$source_path/pixman/pixman -I`pwd`/pixman/pixman"
+    pixman_libs="-L`pwd`/pixman/pixman/.libs -lpixman-1"
+    mkdir -p pixman
+  else
+    pixman_cflags="-I$source_path/pixman/pixman"
+    pixman_libs="-Lpixman/pixman/.libs -lpixman-1"
+  fi
+  if test ! -f ${source_path}/pixman/configure; then
+    (cd ${source_path}/pixman; autoreconf -v --install)
+  fi
+  (cd pixman; ${source_path}/pixman/configure --disable-shared --enable-static)
 fi
 QEMU_CFLAGS="$QEMU_CFLAGS $pixman_cflags"
 libs_softmmu="$libs_softmmu $pixman_libs"

There are some dependencies issues that cause unneeded rebuilds and links but I 
think those exist despite these changes.

For example libcacard/cutils.d is being included for the top-level make but has 
relative dependencies ../config-host.h and ../qapi-types.h.  Those dependencies 
work when the PWD is $(BUILD_DIR)/libcacard but not at the top level 
$(BUILD_DIR).

The objects that are being unnecessarily rebuild are:
  CC    osdep.o
  CC    cutils.o
  CC    qemu-timer-common.o
  CC    oslib-posix.o

This then results in unneeded relinking.

-Eric