| Message ID | 1355143684-16850-2-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 > >
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
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
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
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
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
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
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 >
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:"
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(-)