[Qemu-devel] [PATCH 0/2] pixman: two little fixes.

[Qemu-devel] [PATCH 0/2] pixman: two little fixes.

[Gerd Hoffmann]

  Hi,

Two little pixman fixes.  The first unbreaks a bunch of build failures.

please apply,
  Gerd

Gerd Hoffmann (2):
  pixman: require 0.16.4 as minimum version
  pixman: update internal copy to pixman-0.28.2

 configure |    4 ++--
 pixman    |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

[Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Gerd Hoffmann]

Lower the bar a bit.  0.16.4 is known-good, and is shipped by debian.
Fixes build failures on the debian-based buildbot slaves.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 configure |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index e5aedef..a4e62c4 100755
--- a/configure
+++ b/configure
@@ -2127,7 +2127,7 @@ fi
 # pixman support probe
 
 if test "$pixman" = ""; then
-  if $pkg_config --atleast-version=0.18.4 pixman-1 > /dev/null 2>&1; then
+  if $pkg_config --atleast-version=0.16.4 pixman-1 > /dev/null 2>&1; then
     pixman="system"
   else
     pixman="internal"
@@ -2138,7 +2138,7 @@ if test "$pixman" = "system"; then
   pixman_libs=`$pkg_config --libs pixman-1 2>/dev/null`
 else
   if test ! -d ${source_path}/pixman/pixman; then
-    echo "ERROR: pixman not present (or older than 0.18.4). Your options:"
+    echo "ERROR: pixman not present (or older than 0.16.4). Your options:"
     echo "  (1) Prefered: Install the pixman devel package (any recent"
     echo "      distro should have packages as Xorg needs pixman too)."
     echo "  (2) Fetch the pixman submodule, using:"
-- 
1.7.1

[Qemu-devel] [PATCH 2/2] pixman: update internal copy to pixman-0.28.2

[Gerd Hoffmann]

Some w64 fixes by Stefan Weil found their way into 0.28.2,
so update the internal copy to that version to improve
windows support.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 pixman |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/pixman b/pixman
index 97336fa..a5e5179 160000
--- a/pixman
+++ b/pixman
@@ -1 +1 @@
-Subproject commit 97336fad32acf802003855cd8bd6477fa49a12e3
+Subproject commit a5e5179b5624c99c812e9bf6e7b907e355a811e8
-- 
1.7.1

Re: [Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Alexander Graf]

On 10.12.2012, at 13:48, Gerd Hoffmann wrote:

> Lower the bar a bit.  0.16.4 is known-good, and is shipped by debian.
> Fixes build failures on the debian-based buildbot slaves.

SLES11 ships 0.16.0 for example. It'd be nice to stay compatible there.

What exactly is the compatibility bar? The fact that it compiles? Without the configure check it's compiling just fine on 0.12.0 (openSUSE 11.1) here.


Alex

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> configure |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index e5aedef..a4e62c4 100755
> --- a/configure
> +++ b/configure
> @@ -2127,7 +2127,7 @@ fi
> # pixman support probe
> 
> if test "$pixman" = ""; then
> -  if $pkg_config --atleast-version=0.18.4 pixman-1 > /dev/null 2>&1; then
> +  if $pkg_config --atleast-version=0.16.4 pixman-1 > /dev/null 2>&1; then
>     pixman="system"
>   else
>     pixman="internal"
> @@ -2138,7 +2138,7 @@ if test "$pixman" = "system"; then
>   pixman_libs=`$pkg_config --libs pixman-1 2>/dev/null`
> else
>   if test ! -d ${source_path}/pixman/pixman; then
> -    echo "ERROR: pixman not present (or older than 0.18.4). Your options:"
> +    echo "ERROR: pixman not present (or older than 0.16.4). Your options:"
>     echo "  (1) Prefered: Install the pixman devel package (any recent"
>     echo "      distro should have packages as Xorg needs pixman too)."
>     echo "  (2) Fetch the pixman submodule, using:"
> -- 
> 1.7.1
> 
> 

Re: [Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Gerd Hoffmann]

On 12/13/12 15:31, Alexander Graf wrote:
> 
> On 10.12.2012, at 13:48, Gerd Hoffmann wrote:
> 
>> Lower the bar a bit.  0.16.4 is known-good, and is shipped by
>> debian. Fixes build failures on the debian-based buildbot slaves.
> 
> SLES11 ships 0.16.0 for example. It'd be nice to stay compatible
> there.

If 0.16.0 is known good we can pick that one.

> What exactly is the compatibility bar? The fact that it compiles?
> Without the configure check it's compiling just fine on 0.12.0
> (openSUSE 11.1) here.

Hmm,  /me saw reports of *not* building with 0.12.0, which was the
reason to add this in the first place ...

confused,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Alexander Graf]

On 13.12.2012, at 15:48, Gerd Hoffmann wrote:

> On 12/13/12 15:31, Alexander Graf wrote:
>> 
>> On 10.12.2012, at 13:48, Gerd Hoffmann wrote:
>> 
>>> Lower the bar a bit.  0.16.4 is known-good, and is shipped by
>>> debian. Fixes build failures on the debian-based buildbot slaves.
>> 
>> SLES11 ships 0.16.0 for example. It'd be nice to stay compatible
>> there.
> 
> If 0.16.0 is known good we can pick that one.
> 
>> What exactly is the compatibility bar? The fact that it compiles?
>> Without the configure check it's compiling just fine on 0.12.0
>> (openSUSE 11.1) here.
> 
> Hmm,  /me saw reports of *not* building with 0.12.0, which was the
> reason to add this in the first place ...

Yeah, check out

  6e72719e721a40fe1224701ca10edc1caf0cd708

You had 2 branches that were using a new flag, one of which you had an #ifdef around and one of which you didn't. This is all fixed for quite a while now :).

If that's the culprit for all this mess, can we remove the configure check again? QEMU compiles just fine without it today on pixman 0.12.0.


Alex

Re: [Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Alexander Graf]

On 13.12.2012, at 15:48, Gerd Hoffmann wrote:

> On 12/13/12 15:31, Alexander Graf wrote:
>> 
>> On 10.12.2012, at 13:48, Gerd Hoffmann wrote:
>> 
>>> Lower the bar a bit.  0.16.4 is known-good, and is shipped by
>>> debian. Fixes build failures on the debian-based buildbot slaves.
>> 
>> SLES11 ships 0.16.0 for example. It'd be nice to stay compatible
>> there.
> 
> If 0.16.0 is known good we can pick that one.
> 
>> What exactly is the compatibility bar? The fact that it compiles?
>> Without the configure check it's compiling just fine on 0.12.0
>> (openSUSE 11.1) here.
> 
> Hmm,  /me saw reports of *not* building with 0.12.0, which was the
> reason to add this in the first place ...

So basically it boils down to support for PIXMAN_TYPE_BGRA. How about we just check for that one instead rather than the version number? Also, it'd be very consistent if the #ifdefery in qemu-pixman.c gets removed if that's the whole purpose of the configure check.


Alex

Re: [Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Gerd Hoffmann]

  Hi,

> 6e72719e721a40fe1224701ca10edc1caf0cd708

It's broken, version is wrong, and I told you.
Seems you sneaked it in nevertheless.

> You had 2 branches that were using a new flag, one of which you had
> an #ifdef around and one of which you didn't. This is all fixed for
> quite a while now :).

Can you check which of your pixman versions have PIXMAN_TYPE_BGRA?

0.18.4 has it
0.12.0 has not

The 0.16.x  versions would be interesting so we can fix the #ifdef to be
correct (or at least correct enougth for all practical purposes).

> If that's the culprit for all this mess, can we remove the configure
> check again? QEMU compiles just fine without it today on pixman
> 0.12.0.

If you send me a fix for 6e72719e721a40fe1224701ca10edc1caf0cd708 I'll
go revert 288fa40736e6eb63132d01aa6dc21ee831b796ae

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Alexander Graf]

On 13.12.2012, at 16:30, Gerd Hoffmann wrote:

>  Hi,
> 
>> 6e72719e721a40fe1224701ca10edc1caf0cd708
> 
> It's broken, version is wrong, and I told you.
> Seems you sneaked it in nevertheless.

It's consistent now with the #ifdef 5 lines above the line that the patch does.

> 
>> You had 2 branches that were using a new flag, one of which you had
>> an #ifdef around and one of which you didn't. This is all fixed for
>> quite a while now :).
> 
> Can you check which of your pixman versions have PIXMAN_TYPE_BGRA?
> 
> 0.18.4 has it
> 0.12.0 has not
> 
> The 0.16.x  versions would be interesting so we can fix the #ifdef to be
> correct (or at least correct enougth for all practical purposes).

0.16.0 does.

> 
>> If that's the culprit for all this mess, can we remove the configure
>> check again? QEMU compiles just fine without it today on pixman
>> 0.12.0.
> 
> If you send me a fix for 6e72719e721a40fe1224701ca10edc1caf0cd708 I'll
> go revert 288fa40736e6eb63132d01aa6dc21ee831b796ae

6e72719 is perfectly sane, because it brings consistency into the file. Now both users of PIXMAN_TYPE_BGRA are guarded by the same #ifdef. I do agree though that instead of

#if SOME_MAGIC_VERSION

we rather either do

#ifdef PIXMAN_TYPE_BGRA

or no #ifdef at all and a proper configure check.

Having a configure check and the #ifdef is wrong. Doing #ifdef MAGIC_VERSION is also wrong.


So a quick fix would be to bump the configure check to 0.16.0 and remove the two #ifdefs in qemu-pixman.c.


Alex

Re: [Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Gerd Hoffmann]

  Hi,

>> If you send me a fix for 6e72719e721a40fe1224701ca10edc1caf0cd708
>> I'll go revert 288fa40736e6eb63132d01aa6dc21ee831b796ae
> 
> 6e72719 is perfectly sane, because it brings consistency into the
> file. Now both users of PIXMAN_TYPE_BGRA are guarded by the same
> #ifdef.

Both users?  Hello?  Its two *different* types: PIXMAN_TYPE_RGBA and
PIXMAN_TYPE_BGRA.  And, yes, they have been added in different versions.
 So guarding them with the *same* #ifdef is wrong.

> #ifdef PIXMAN_TYPE_BGRA

It's a enum, not a #define, so this isn't going to fly.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] pixman: require 0.16.4 as minimum version

[Alexander Graf]

On 14.12.2012, at 08:54, Gerd Hoffmann <kraxel@redhat.com> wrote:

>  Hi,
> 
>>> If you send me a fix for 6e72719e721a40fe1224701ca10edc1caf0cd708
>>> I'll go revert 288fa40736e6eb63132d01aa6dc21ee831b796ae
>> 
>> 6e72719 is perfectly sane, because it brings consistency into the
>> file. Now both users of PIXMAN_TYPE_BGRA are guarded by the same
>> #ifdef.
> 
> Both users?  Hello?  Its two *different* types: PIXMAN_TYPE_RGBA and
> PIXMAN_TYPE_BGRA.  And, yes, they have been added in different versions.
> So guarding them with the *same* #ifdef is wrong.

Ugh :(. Sorry then.

> 
>> #ifdef PIXMAN_TYPE_BGRA
> 
> It's a enum, not a #define, so this isn't going to fly.

In my old versions it's a define...


Alex

> 
> cheers,
>  Gerd
>