qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Frediano Ziglio <fziglio@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5
Date: Thu, 29 Nov 2018 05:35:49 -0500 (EST)	[thread overview]
Message-ID: <905475153.46444981.1543487749747.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20181129090701.lbyqnyw4u3s2bxxw@sirius.home.kraxel.org>

> 
> On Thu, Nov 29, 2018 at 03:32:28AM -0500, Frediano Ziglio wrote:
> > > On Thu, Nov 29, 2018 at 12:09 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > > >
> > > > On Wed, Nov 28, 2018 at 07:59:32PM +0400, Marc-André Lureau wrote:
> > > > > Looking at chardev/spice.c code, I realize compilation was broken for
> > > > > a while with spice-server < 0.12.3. Let's bump required version
> > > > > to 0.12.5, released May 19 2014, instead of adding more #ifdef.
> > > >
> > > > Oh, you did the 0.12.5 patch already.  Scratch my other reply then.
> > > >
> > > > > -  if $pkg_config --atleast-version=0.12.0 spice-server && \
> > > > > +  if $pkg_config --atleast-version=0.12.5 spice-server && \
> > > > >       $pkg_config --atleast-version=0.12.3 spice-protocol && \
> > > >
> > > > I think we should adjust spice-protocol too to whatever 0.12.5 requires
> > > > to build.
> > > >
> > > 
> > > Why not leave that responsibility to pkg-config, and only require in
> > > qemu what is required there?
> > > 
> > > 
> > 
> > That is remove explicit requirement in configure script?
> > I can see that spice-core.h (spice-server, one of the mail include) is
> > including spice-protocol headers.
> > Looking at configure both are required so would make sense to check
> > only spice-server, unless packaging has some bugs if you have spice-server
> > (devel) installed you also have spice-protocol.
> 
> seems the spice-protocol dep is there due to qemu itself needing it:
> 
> commit 358689fe299c306f1d81bea57a5067d0abb56699
> Author: Michal Privoznik <mprivozn@redhat.com>
> Date:   Fri Mar 1 08:43:18 2013 +0100
> 
>     configure: Require at least spice-protocol-0.12.3
>     
>     As of 5a49d3e9 we assume SPICE_PORT_EVENT_BREAK to be defined.
>     However, it is defined not in 0.12.2 what we require now, but in
>     0.12.3.  Therefore in order to prevent build failure we must
>     adjust our minimal requirements.
>     
>     Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> That makes sense.  So, when spice-server 0.12.5 requires spice-protocol
> 0.12.8+ anyway I think we can savely drop the spice-protocol check.
> 
> cheers,
>   Gerd
> 

My idea was more to revert (kind of) 7e3efdac75caca0b283f8e76ad24c924b4718e7b:


commit 7e3efdac75caca0b283f8e76ad24c924b4718e7b
Author: Alon Levy <alevy@redhat.com>
Date:   Wed Mar 7 16:19:03 2012 +0200

    spice: require spice-protocol >= 0.8.1
    
    Requiring spice-server >= 0.8.2 is not enough since spice-server.pc
    doesn't require spice-protocol (any version). Until that is fixed
    upstream an explicit requirement in qemu fixes compilation broken since
    
    commit 2e1a98c9c1b90ca093278c6b43244dc46604d7b7
    Author: Alon Levy <alevy@redhat.com>
    Date:   Fri Feb 24 23:19:30 2012 +0200
    
        qxl: introduce QXLCookie
    
    Reported-by: Peter Maydell <peter.maydell@linaro.org>
    
    Signed-off-by: Alon Levy <alevy@redhat.com>
    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

diff --git a/configure b/configure
index 0111774cb0..62aa7609e1 100755
--- a/configure
+++ b/configure
@@ -2592,6 +2592,7 @@ EOF
   spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
   spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
   if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
+     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
      compile_prog "$spice_cflags" "$spice_libs" ; then
     spice="yes"
     libs_softmmu="$libs_softmmu $spice_libs"


The rationale does not apply any more, spice-server depends on spice-protocol.

Frediano

  reply	other threads:[~2018-11-29 10:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-28 15:59 [Qemu-devel] [PATCH for-4.0 v3] configure: bump spice-server required version to 0.12.5 Marc-André Lureau
2018-11-28 16:06 ` Daniel P. Berrangé
2018-11-28 16:23 ` Frediano Ziglio
2018-11-29  8:09 ` Gerd Hoffmann
2018-11-29  8:17   ` Frediano Ziglio
2018-11-29  8:22   ` Marc-André Lureau
2018-11-29  8:32     ` Frediano Ziglio
2018-11-29  9:07       ` Gerd Hoffmann
2018-11-29 10:35         ` Frediano Ziglio [this message]
2018-11-29 11:23           ` Gerd Hoffmann
2018-12-10 13:28 ` Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=905475153.46444981.1543487749747.JavaMail.zimbra@redhat.com \
    --to=fziglio@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).