[Spice-devel] [PATCH spice-server] README: Update required protocol version

[Christophe Fergeau]

On Fri, Jul 28, 2017 at 09:30:18AM +0200, Christophe de Dinechin wrote:
> > 
> >> 
> >> For example, a lot of the streaming work requires a branched-off spice-protocol.
> >> 
> >> I was also wondering about protocol updates being easier to do in a
> >> consistent way if spice-protocol was “above” spice-server and
> >> spice-gtk. Which could be solved by having a submodule structure like:
> >> 
> >> 	spice
> >> 		spice-protocol
> >> 		spice-common
> >> 		spice-server
> >> 		spice-gtk
> > 
> > I'm not sure generating spice-gtk and spice-server tarballs from such a layout
> > would be easy (?)
> 
> Are you saying that it’s logical to include spice-common in the
> tarball, but not spice-protocol?

I don't really see how this question relates to the part you are quoting :)
We used to have duplicated code in spice/ and spice-gtk/ (a bit like the
quic code is duplicated in the windows qxl driver), and in order to
avoid this duplication, a submodule was created containing the code
shared between spice/ and spice-gtk/. There has been some discussions in
the past of merging spice-common and spice-protocol, or on the contrary,
to expose some stable API/ABI in spice-common, and make it an actual
shared library. Let's say it's a long term/low priority plan ;)

The main difference between spice-common and spice-protocol in my
opinion is that spice-common is very specific to spice-gtk/ and spice/
and is private.
On the other hand, spice-protocol is used by a lot more projects
(including qemu and virt-viewer), spice-gtk public headers #include some
spice-protocol headers, ... So it seems to me we need to ship a canonical
version of spice-protocol (in other words, released tarballs). If we
have tarball releases, I think using spice-protocol as a submodule in
some places, and using the tarball release in others is just creating
confusion. Maybe it's possible to make spice-protocol "private" too,
but at least QEMU would need access to the headers, and I don't think
this is a very pressing work.

While at it, some side not on spice/ VS spice-server/, long time ago,
there were spice/server/ and spice/client/, then spice-gtk was started
as a replacement of the old spice/client/ code base, and spice/client/
was eventually removed. And the spice/ naming stuck :)

> 
> > 
> > For what it's worth, I find submodules quite cumbersome to work with,
> > they get in the way more often than not when you start making changes in
> > one, when you start applying patches inside a submodule, rebasing, …
> 
> So you think that if / when Frediano rebases the stream branch in
> git://people.freedesktop.org/~fziglio/spice-protocol, I won’t have
> these problems? Of course I will. The only difference is that *git
> will not tell me*.

Once you installed spice-protocol/ to $prefix, the installed headers
won't change without an explicit action on your side. If you apply a
patch in spice-common, and then rerun autogen.sh in the main module,
your spice-common patch will be lost unless you do an explicit action to
tell git not to touch the submodule. So no, git is not necessarily very
helpful when it comes to submodules ;)


> > and you'd have the same issue
> > as you currently are having, no?
> 
> I don’t think I would, because my problem is with branches I’m
> currently working on where multiple components are impacted at once:
> 
> - The streaming work.
> - The flight recorder work, which led me to these questions (should
> ‘recorder’ follow the spice-common or spice-protocol model?)
> - My investigation on sending feedback to the server from bogged down clients

As a workaround for not having this, is there anything preventing you
from locally creating a spice-root git repository with the submodules
you need, and where you have topic branches?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170728/211757c6/attachment.sig>