All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.