> On 28 Jul 2017, at 10:17, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
>
> 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 :)
Well, I did not understand why this layout would cause a problem that does
not exist in the current layout. And I did not understand in particular because
I did not see the difference between spice-common and spice-protocol.
I believe you answered that question below.
> 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 ;)
OK.
> 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.
The fact that spice-protocol was considered public and spice-common
was not is the key difference. Incidentally, that means the layout I chose
for the recorder is correct.
> 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.
OK. That’s a much clearer rationale (to me) than the one I saw in the commit log.
> 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.
No, I think it’s fine like this. But having a top-level repository with
synchonized versions of server, client, protocol and code that relies on
the protocol may be a good idea.
>
> 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 :)
I see. That also clarifies why we have spice/server. I was wondering.
>
>>
>>>
>>> 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 ;)
But that’s only because autogen.sh contains:
git submodule update --init --recursive
Do we need that all the time? If the goal is to just update submodules after a checkout, then we could use:
[ -d spice-common -a -d src/keycodemapdb ] || git submodule update --init --recursive
If the goal is to avoid overwriting your changes by mistake, then I think it should be:
git submodule update --init --recursive --merge
That last one is probably safer anyway.
>
>>> 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?
This is exactly what I did :-) It’s there in case anybody else wants to use it: https://gitlab.com/c3d/spice. But of course, it’s only useful to me at the moment given that some SHA1 it references are not in the canonical repositories.
Thanks
Christophe
>
> Christophe
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel