Re: [Spice-devel] [PATCH spice-server v2 3/3] Remove core parameter from main_dispatcher_new

2019-02-12 Thread Jonathon Jongsma
Acked-by: Jonathon Jongsma On Tue, 2019-02-12 at 21:24 +, Frediano Ziglio wrote: > This was added in bd8771adbcf3ff34d14333cf874191e8d105f612. > There's no reason to not use reds function instead. > MainDispatcher needs to listen in the main thread that is the > one provided by reds_core_*

Re: [Spice-devel] [PATCH spice-server v2 2/3] test-stream-device: Check monitor ID messages

2019-02-12 Thread Jonathon Jongsma
Looks fine to me. Insofar as I can ACK a patch that's partially my own code: Acked-by: Jonathon Jongsma :) On Tue, 2019-02-12 at 21:24 +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > Signed-off-by: Jonathon Jongsma > Reviewed-by: Jonathon Jongsma > --- >

[Spice-devel] [PATCH spice-server v2 1/3] reds: Factor out a function to marshal VDAgentGraphicsDeviceInfo message

2019-02-12 Thread Frediano Ziglio
Instead of scanning the monitor twice (one to compute the size and another to build the message) use a single function to marshal the message. This also fixes big endian machines (which are not supported). Marshal function is exported to make easier to test (see following patch). Signed-off-by:

[Spice-devel] [PATCH spice-server v2 3/3] Remove core parameter from main_dispatcher_new

2019-02-12 Thread Frediano Ziglio
This was added in bd8771adbcf3ff34d14333cf874191e8d105f612. There's no reason to not use reds function instead. MainDispatcher needs to listen in the main thread that is the one provided by reds_core_* functions. Signed-off-by: Frediano Ziglio --- server/main-dispatcher.c | 29

[Spice-devel] [PATCH spice-server v2 2/3] test-stream-device: Check monitor ID messages

2019-02-12 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio Signed-off-by: Jonathon Jongsma Reviewed-by: Jonathon Jongsma --- server/tests/test-stream-device.c | 70 +++ 1 file changed, 70 insertions(+) diff --git a/server/tests/test-stream-device.c b/server/tests/test-stream-device.c index

[Spice-devel] [PATCH spice-server v2 0/3] Miscellaneous patches

2019-02-12 Thread Frediano Ziglio
First 2 patches are related. Changes since v1: - removed merged ones; - changed test for monitor ID adding some follow ups (and minor updated like "recieving" typo and indentation). Frediano Ziglio (3): reds: Factor out a function to marshall VDAgentGraphicsDeviceInfo message

Re: [Spice-devel] [PATCH spice-server 6/8] agent-msg-filter: Simplify code

2019-02-12 Thread Jonathon Jongsma
why not Acked-by: Jonathon Jongsma On Mon, 2019-02-11 at 11:54 +, Frediano Ziglio wrote: > Most of the time result is set to AGENT_MSG_FILTER_OK, set at > the beginning and change if necessary. > > Signed-off-by: Frediano Ziglio > --- > server/agent-msg-filter.c | 14 +- > 1

Re: [Spice-devel] [PATCH spice-server 3/8] test-stream-device: Check monitor ID messages

2019-02-12 Thread Jonathon Jongsma
Looks mostly good, but I found an issue and had a couple suggested improvments. So I sent a couple follow-up patches that you can squash with this patch if you think they're valid. Reviewed-by: Jonathon Jongsma On Mon, 2019-02-11 at 11:54 +, Frediano Ziglio wrote: > Signed-off-by: Frediano

[Spice-devel] [PATCH spice-server 1/2] Fixup display info test

2019-02-12 Thread Jonathon Jongsma
Rather than showing the expected data in raw format (ascii codes, etc), which is hard to verify, show the characters themselves, and group them by structure. Also add a few more comments. --- server/tests/test-stream-device.c | 35 +++ 1 file changed, 26

[Spice-devel] [PATCH spice-server 2/2] QXL devices must be registered before Stream Devices

2019-02-12 Thread Jonathon Jongsma
Stream devices assume that all QXL devices are registered with the server before we receive any communications from the stream device. This is due to the fact that QXL display channel IDs are assigned directly from the QXL device ID, whereas Stream display channels are assigned channel IDs based

[Spice-devel] [spice-gtk v2 2/5] gitlab-ci: move windows dependencies to a variable

2019-02-12 Thread Victor Toso
From: Victor Toso To keep all dependencies together. Some (small) effort was made to distinguish what is necessary for Fedora and what is necessary for Windows builds in order to install only required packages when job is executing. Note that we are adding gettext, gettext-devel and glib2-devel

Re: [Spice-devel] [PATCH spice-server 2/8] reds: Factor out a function to marshall VDAgentGraphicsDeviceInfo message

2019-02-12 Thread Frediano Ziglio
> On Tue, 2019-02-12 at 04:05 -0500, Frediano Ziglio wrote: > > > > > Untested, but looks fine. > > > > > > Acked-by: Jonathon Jongsma > > > > > > > It's partially tested by following patch. Partially as the new > > function is > > tested but the old function to send the message is not but

[Spice-devel] [spice-gtk v2 3/5] gitlab-ci: create before_script per job

2019-02-12 Thread Victor Toso
From: Victor Toso * On a windows job-build, we don't need to install Fedora dependencies. This change makes only one dnf install be ran per job. * On a meson build, we should build spice-protocol with meson too. Moving this before_script rule to each job makes all of this clear. So, this

[Spice-devel] [spice-gtk v2 4/5] gitlab-ci: add artifacts for logs and tests

2019-02-12 Thread Victor Toso
From: Victor Toso Much better than playing around with shell. Logs should live for week since the build and CI will try to always upload them. This patch also adds the logs for tests from builds with autotools Signed-off-by: Victor Toso --- .gitlab-ci.yml | 20 1 file

[Spice-devel] [spice-gtk v1] widget: mouse: Fix getting coordinates

2019-02-12 Thread Victor Toso
From: Victor Toso Documentation for gdk_display_get_primary_monitor() says that it returns "the primary monitor, or NULL if no primary monitor is configured by the user". If monitor endup being NULL, we endup using unitialized GdkRectangle geom later on as gdk_monitor_get_geometry() will fail

Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs

2019-02-12 Thread Frediano Ziglio
> Hi, > > On Tue, Feb 12, 2019 at 11:19:16AM -0500, Frediano Ziglio wrote: > > > > -makecheck-meson: > > > > +fedora-meson: > > > >script: > > > > - - meson build || (cat build/meson-logs/meson-log.txt && exit 1) > > > > - - ninja -C build > > > > - - (cd build && meson test) || (cat

[Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs

2019-02-12 Thread Victor Toso
From: Victor Toso Group by target build instead of command. The focus of each job is to check any regression for given platform, using 'fedora'/'windows' and 'autotools'/'meson' seems more intuitive. By doing that we are grouping similar jobs together, this is intentional as we are reducing the

Re: [Spice-devel] [PATCH spice-server 2/8] reds: Factor out a function to marshall VDAgentGraphicsDeviceInfo message

2019-02-12 Thread Jonathon Jongsma
On Tue, 2019-02-12 at 12:24 -0500, Frediano Ziglio wrote: > > On Tue, 2019-02-12 at 04:05 -0500, Frediano Ziglio wrote: > > > > > > > Untested, but looks fine. > > > > > > > > Acked-by: Jonathon Jongsma > > > > > > > > > > It's partially tested by following patch. Partially as the new > > >

[Spice-devel] [spice-gtk v1] widget: mouse: Fix getting coordinates

2019-02-12 Thread Victor Toso
From: Victor Toso Documentation for gdk_display_get_primary_monitor() says that it returns "the primary monitor, or NULL if no primary monitor is configured by the user". If monitor endup being NULL, we endup using unitialized GdkRectangle geom later on as gdk_monitor_get_geometry() will fail

Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs

2019-02-12 Thread Victor Toso
Hi, On Tue, Feb 12, 2019 at 11:19:16AM -0500, Frediano Ziglio wrote: > > > -makecheck-meson: > > > +fedora-meson: > > >script: > > > - - meson build || (cat build/meson-logs/meson-log.txt && exit 1) > > > - - ninja -C build > > > - - (cd build && meson test) || (cat

Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs

2019-02-12 Thread Frediano Ziglio
> > > > From: Victor Toso > > > > Group by target build instead of command. The focus of each job is to > > check any regression for given platform, using 'fedora'/'windows' and > > 'autotools'/'meson' seems more intuitive. > > > > By doing that we are grouping similar jobs together, this is >

Re: [Spice-devel] [spice-gtk v2 2/5] gitlab-ci: move windows dependencies to a variable

2019-02-12 Thread Frediano Ziglio
> > From: Victor Toso > > To keep all dependencies together. Some (small) effort was made to > distinguish what is necessary for Fedora and what is necessary for > Windows builds in order to install only required packages when job is > executing. > > Note that we are adding gettext,

Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs

2019-02-12 Thread Frediano Ziglio
> > From: Victor Toso > > Group by target build instead of command. The focus of each job is to > check any regression for given platform, using 'fedora'/'windows' and > 'autotools'/'meson' seems more intuitive. > > By doing that we are grouping similar jobs together, this is > intentional as

Re: [Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs

2019-02-12 Thread Marc-André Lureau
On Tue, Feb 12, 2019 at 3:18 PM Victor Toso wrote: > > From: Victor Toso > > Group by target build instead of command. The focus of each job is to > check any regression for given platform, using 'fedora'/'windows' and > 'autotools'/'meson' seems more intuitive. > > By doing that we are grouping

Re: [Spice-devel] [spice-gtk v1] Remove spicy flatpak from spice-gtk source code

2019-02-12 Thread Marc-André Lureau
On Tue, Feb 12, 2019 at 3:01 PM Victor Toso wrote: > > From: Victor Toso > > The usage of testing tool as flatkpak is discouraged by upstream > developers, see comments from thread: > > > https://lists.freedesktop.org/archives/spice-devel/2019-February/047877.html > > One might argue that

[Spice-devel] [spice-gtk v2 4/5] gitlab-ci: add artifacts for logs and tests

2019-02-12 Thread Victor Toso
From: Victor Toso Much better than playing around with shell. Logs should live for week since the build and CI will try to always upload them. This patch also adds the logs for tests from builds with autotools Signed-off-by: Victor Toso --- .gitlab-ci.yml | 20 1 file

[Spice-devel] [spice-gtk v2 3/5] gitlab-ci: create before_script per job

2019-02-12 Thread Victor Toso
From: Victor Toso * On a windows job-build, we don't need to install Fedora dependencies. This change makes only one dnf install be ran per job. * On a meson build, we should build spice-protocol with meson too. Moving this before_script rule to each job makes all of this clear. So, this

[Spice-devel] [spice-gtk v2 2/5] gitlab-ci: move windows dependencies to a variable

2019-02-12 Thread Victor Toso
From: Victor Toso To keep all dependencies together. Some (small) effort was made to distinguish what is necessary for Fedora and what is necessary for Windows builds in order to install only required packages when job is executing. Note that we are adding gettext, gettext-devel and glib2-devel

[Spice-devel] [spice-gtk v2 1/5] gitlab-ci: group and rename jobs

2019-02-12 Thread Victor Toso
From: Victor Toso Group by target build instead of command. The focus of each job is to check any regression for given platform, using 'fedora'/'windows' and 'autotools'/'meson' seems more intuitive. By doing that we are grouping similar jobs together, this is intentional as we are reducing the

[Spice-devel] [spice-gtk v2 5/5] gitlab-ci: add mingw meson build

2019-02-12 Thread Victor Toso
From: Victor Toso To keep track of meson builds for windows too. Note that mingw64-meson requires to be ran inside the folder, different from common meson. So, some extra steps were done due that. Signed-off-by: Victor Toso --- .gitlab-ci.yml | 21 + 1 file changed, 21

[Spice-devel] [spice-gtk v2 0/5] gitlab-ci improvements

2019-02-12 Thread Victor Toso
From: Victor Toso From v1, removed flatkpak fixes and autogeneration. Sending this as still seems to me that using artifacts, grouping some jobs and adding meson+mingw build is nice-to-have changes. You tell me. CI Run: https://gitlab.freedesktop.org/victortoso/spice-gtk/pipelines/18709 Victor

[Spice-devel] [spice-gtk v1] Remove spicy flatpak from spice-gtk source code

2019-02-12 Thread Victor Toso
From: Victor Toso The usage of testing tool as flatkpak is discouraged by upstream developers, see comments from thread: https://lists.freedesktop.org/archives/spice-devel/2019-February/047877.html One might argue that keep this might be useful for documentation. For that, please refer to

Re: [Spice-devel] [spice-server v2 1/4] smartcard: Remove unused smartcard_get_vsc_msg_item argument

2019-02-12 Thread Frediano Ziglio
> > Signed-off-by: Christophe Fergeau Acked the series. Frediano > --- > server/smartcard.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/server/smartcard.c b/server/smartcard.c > index ff680d8a5..21dc8de5a 100644 > --- a/server/smartcard.c > +++

Re: [Spice-devel] [spice-gtk [rfc] 1/2] gtk-session: introduce clipboard-managers property

2019-02-12 Thread Jakub Janku
Hi, On Tue, Jan 15, 2019 at 5:11 PM Victor Toso wrote: > > From: Victor Toso > > SpiceGtkSession::allow-clipboard-managers property is introduced to > enable other applications in the Client OS to set or fetch clipboard > data from a spice-gtk-session that is under keyboard-grab, which is >

Re: [Spice-devel] [PATCH spice-gtk 09/12] doc: fix a few links

2019-02-12 Thread Frediano Ziglio
> > From: Marc-André Lureau > > Signed-off-by: Marc-André Lureau Acked-by: Frediano Ziglio > --- > src/channel-display.c | 2 +- > src/channel-main.h| 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/channel-display.c b/src/channel-display.c > index

Re: [Spice-devel] [PATCH spice-gtk 03/12] meson: fix po generation

2019-02-12 Thread Frediano Ziglio
> From: Marc-André Lureau > > Use glib preset (from meson v0.37) to catch all our translatable > strings and use good default settings. > > While at it, remove the needless directory argument. > > Signed-off-by: Marc-André Lureau With or without this patch Meson seems to do nothing in the

Re: [Spice-devel] [PATCH v2 spice-gtk] Update the codeflow description comment

2019-02-12 Thread Frediano Ziglio
> On Mon, 2019-02-11 at 16:14 +0200, Snir Sheriber wrote: > > --- > > src/channel-display-gst.c | 39 ++--- > > -- > > 1 file changed, 22 insertions(+), 17 deletions(-) > > > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c > > index

Re: [Spice-devel] [PATCH spice-server 2/8] reds: Factor out a function to marshall VDAgentGraphicsDeviceInfo message

2019-02-12 Thread Frediano Ziglio
> Untested, but looks fine. > > Acked-by: Jonathon Jongsma > It's partially tested by following patch. Partially as the new function is tested but the old function to send the message is not but is changed in this test. But I can see data sent to the guest (so I tested that part manually).

Re: [Spice-devel] [spice-gtk v1 00/10] Flatpak + CI

2019-02-12 Thread Christophe Fergeau
Hey, On Mon, Feb 11, 2019 at 11:51:54PM +0100, Marc-André Lureau wrote: > Hi > > On Mon, Feb 11, 2019 at 6:12 PM Christophe Fergeau > wrote: > > I think the main objection is with making spicy too easy to install (and > > to upgrade). Once we ask someone to test a spicy flatpak and it works >