Re: [Spice-devel] [Spice-commits] 6 commits - meson.build src/map-file src/spice-glib-sym-file src/spice-gtk-session.c src/spice-session.c src/spice-session.h src/spice-session-priv.h
On Wed, Sep 9, 2020 at 5:27 PM Frediano Ziglio wrote: > > > Hi > > > On Wed, Sep 9, 2020 at 6:45 PM Jakub Janku < jja...@redhat.com > wrote: > > > > On Wed, Sep 9, 2020 at 4:36 PM Frediano Ziglio < fzig...@redhat.com > > > > wrote: > > > > > > > > > > > > > On Wed, Sep 9, 2020 at 4:16 PM Frediano Ziglio < fzig...@redhat.com > > > > > > wrote: > > > > > > > > > > > > > > > > > > commit 4b9092b96b8da946ff3d17922b0fcf225c5dc81f > > > > > > > > > > > > > > > > > > > Author: Jakub Janků < jja...@redhat.com > > > > > > > > > > > > > > > > > > > > Date: Sat May 23 16:28:52 2020 +0200 > > > > > > > > > > > > > > > > > > > > > > > > > > > session: make spice_session_get_webdav_server() public > > > > > > > > > > > > > > > > > > > > > > > > > > > It will be necessary to access the webdav server from > > > > > > > > > > spice-gtk-session.c > > > > > > > > > > > > > > > > > > > which isn't compiled with spice-session-priv.h, so make > > > > > > > > > > > > > > > > > > > spice_session_get_webdav_server() public. > > > > > > > > > > > > > > > > > > > > > > > > > > I haven't looked at the whole series. Wouldn't it make sense to > > > > > > > make > > > > > > > it a > > > > > > > > > read-only property instead? > > > > > > > > > > > > > > > > It sounds reasonable for me. > > > > > > > > Jakub ? > > > > > > > > > > > > > > > > > > > > > > I agree. > > > > > > > > > > > > > > Revert the commits please. I'll reopen the merge request once I have > > > > > it > > > > > > > ready. > > > > > > > > > > > > > > Cheers, > > > > > > > Jakub > > > > > > > > > > > > > > > > > > > To be honest I don't see the need to revert commits, it's just a change > > > > > > from public to private. > > > > > > Ok, so should I open a separate MR? > > > > > > To make sure that I didn't misunderstand it: the suggestion is to keep > > > > > spice_session_get_webdav_server() private and install a new > > > > > SpiceSession read-only property "webdav", correct? > > > > > yes (the main motivation is to avoid adding new library symbols, and > > properties can be looked up at runtime, which may avoid bumping dependencies > > in some cases) > > Oh, I though the idea was making the new property private to in the future it > could be removed if not needed anymore. > You can achieve the dynamic resolution using dlsym if needed using library > symbols. > It's not that easy to look the property dynamically, to avoid warnings you > have to use g_object_class_find_property first. > For a "get" between spice-gtk and spice-glib the current solution is easier > and consistent, there are already multiple spice_session_get_* functions. Most of the getters are in spice-session-priv.h though. In spice-session.h, there's only three of them. So perhaps it would be more consistent to access the phodav server using properties, as Marc-André suggested. Anyway, here's the possible patch https://gitlab.freedesktop.org/jjanku/spice-gtk/-/commit/048a150f8088d9fd643ba9987c8fba9e83494d94 Jakub > > Frediano > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [Spice-commits] 6 commits - meson.build src/map-file src/spice-glib-sym-file src/spice-gtk-session.c src/spice-session.c src/spice-session.h src/spice-session-priv.h
> Hi > On Wed, Sep 9, 2020 at 6:45 PM Jakub Janku < jja...@redhat.com > wrote: > > On Wed, Sep 9, 2020 at 4:36 PM Frediano Ziglio < fzig...@redhat.com > > > wrote: > > > > > > > > > On Wed, Sep 9, 2020 at 4:16 PM Frediano Ziglio < fzig...@redhat.com > > > > > wrote: > > > > > > > > > > > > > > commit 4b9092b96b8da946ff3d17922b0fcf225c5dc81f > > > > > > > > > > > > > > > Author: Jakub Janků < jja...@redhat.com > > > > > > > > > > > > > > > > Date: Sat May 23 16:28:52 2020 +0200 > > > > > > > > > > > > > > > > > > > > > session: make spice_session_get_webdav_server() public > > > > > > > > > > > > > > > > > > > > > It will be necessary to access the webdav server from > > > > > > > > spice-gtk-session.c > > > > > > > > > > > > > > > which isn't compiled with spice-session-priv.h, so make > > > > > > > > > > > > > > > spice_session_get_webdav_server() public. > > > > > > > > > > > > > > > > > > > > I haven't looked at the whole series. Wouldn't it make sense to > > > > > > make > > > > > > it a > > > > > > > read-only property instead? > > > > > > > > > > > > It sounds reasonable for me. > > > > > > Jakub ? > > > > > > > > > > > > > > > > I agree. > > > > > > > > > > Revert the commits please. I'll reopen the merge request once I have it > > > > > ready. > > > > > > > > > > Cheers, > > > > > Jakub > > > > > > > > > > > > > To be honest I don't see the need to revert commits, it's just a change > > > > from public to private. > > > Ok, so should I open a separate MR? > > > To make sure that I didn't misunderstand it: the suggestion is to keep > > > spice_session_get_webdav_server() private and install a new > > > SpiceSession read-only property "webdav", correct? > > yes (the main motivation is to avoid adding new library symbols, and > properties can be looked up at runtime, which may avoid bumping dependencies > in some cases) Oh, I though the idea was making the new property private to in the future it could be removed if not needed anymore. You can achieve the dynamic resolution using dlsym if needed using library symbols. It's not that easy to look the property dynamically, to avoid warnings you have to use g_object_class_find_property first. For a "get" between spice-gtk and spice-glib the current solution is easier and consistent, there are already multiple spice_session_get_* functions. Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [Spice-commits] 6 commits - meson.build src/map-file src/spice-glib-sym-file src/spice-gtk-session.c src/spice-session.c src/spice-session.h src/spice-session-priv.h
Hi On Wed, Sep 9, 2020 at 6:45 PM Jakub Janku wrote: > On Wed, Sep 9, 2020 at 4:36 PM Frediano Ziglio wrote: > > > > > On Wed, Sep 9, 2020 at 4:16 PM Frediano Ziglio > wrote: > > > > > > > > > > commit 4b9092b96b8da946ff3d17922b0fcf225c5dc81f > > > > > > > > > > > Author: Jakub Janků < jja...@redhat.com > > > > > > > > > > > > Date: Sat May 23 16:28:52 2020 +0200 > > > > > > > > > > > > > > > session: make spice_session_get_webdav_server() public > > > > > > > > > > > > > > > It will be necessary to access the webdav server from > > > > > > spice-gtk-session.c > > > > > > > > > > > which isn't compiled with spice-session-priv.h, so make > > > > > > > > > > > spice_session_get_webdav_server() public. > > > > > > > > > > > > > > I haven't looked at the whole series. Wouldn't it make sense to > make it a > > > > > read-only property instead? > > > > > > > > It sounds reasonable for me. > > > > Jakub ? > > > > > > > > > > I agree. > > > > > > Revert the commits please. I'll reopen the merge request once I have it > > > ready. > > > > > > Cheers, > > > Jakub > > > > > > > To be honest I don't see the need to revert commits, it's just a change > > from public to private. > > Ok, so should I open a separate MR? > > To make sure that I didn't misunderstand it: the suggestion is to keep > spice_session_get_webdav_server() private and install a new > SpiceSession read-only property "webdav", correct? > > yes (the main motivation is to avoid adding new library symbols, and properties can be looked up at runtime, which may avoid bumping dependencies in some cases) -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [Spice-commits] 6 commits - meson.build src/map-file src/spice-glib-sym-file src/spice-gtk-session.c src/spice-session.c src/spice-session.h src/spice-session-priv.h
On Wed, Sep 9, 2020 at 4:36 PM Frediano Ziglio wrote: > > > On Wed, Sep 9, 2020 at 4:16 PM Frediano Ziglio wrote: > > > > > > > > commit 4b9092b96b8da946ff3d17922b0fcf225c5dc81f > > > > > > > > > Author: Jakub Janků < jja...@redhat.com > > > > > > > > > > Date: Sat May 23 16:28:52 2020 +0200 > > > > > > > > > > > > session: make spice_session_get_webdav_server() public > > > > > > > > > > > > It will be necessary to access the webdav server from > > > > > spice-gtk-session.c > > > > > > > > > which isn't compiled with spice-session-priv.h, so make > > > > > > > > > spice_session_get_webdav_server() public. > > > > > > > > > > > I haven't looked at the whole series. Wouldn't it make sense to make it > > > > a > > > > read-only property instead? > > > > > > It sounds reasonable for me. > > > Jakub ? > > > > > > > I agree. > > > > Revert the commits please. I'll reopen the merge request once I have it > > ready. > > > > Cheers, > > Jakub > > > > To be honest I don't see the need to revert commits, it's just a change > from public to private. Ok, so should I open a separate MR? To make sure that I didn't misunderstand it: the suggestion is to keep spice_session_get_webdav_server() private and install a new SpiceSession read-only property "webdav", correct? > > Frediano > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [Spice-commits] 6 commits - meson.build src/map-file src/spice-glib-sym-file src/spice-gtk-session.c src/spice-session.c src/spice-session.h src/spice-session-priv.h
On Wed, Sep 9, 2020 at 4:16 PM Frediano Ziglio wrote: > > > > commit 4b9092b96b8da946ff3d17922b0fcf225c5dc81f > > > > > Author: Jakub Janků < jja...@redhat.com > > > > > > Date: Sat May 23 16:28:52 2020 +0200 > > > > > > session: make spice_session_get_webdav_server() public > > > > > > It will be necessary to access the webdav server from spice-gtk-session.c > > > > > which isn't compiled with spice-session-priv.h, so make > > > > > spice_session_get_webdav_server() public. > > > > > I haven't looked at the whole series. Wouldn't it make sense to make it a > > read-only property instead? > > It sounds reasonable for me. > Jakub ? > I agree. Revert the commits please. I'll reopen the merge request once I have it ready. Cheers, Jakub > Frediano > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [Spice-commits] 6 commits - meson.build src/map-file src/spice-glib-sym-file src/spice-gtk-session.c src/spice-session.c src/spice-session.h src/spice-session-priv.h
> On Wed, Sep 9, 2020 at 4:16 PM Frediano Ziglio wrote: > > > > > > commit 4b9092b96b8da946ff3d17922b0fcf225c5dc81f > > > > > > > Author: Jakub Janků < jja...@redhat.com > > > > > > > > Date: Sat May 23 16:28:52 2020 +0200 > > > > > > > > > session: make spice_session_get_webdav_server() public > > > > > > > > > It will be necessary to access the webdav server from > > > > spice-gtk-session.c > > > > > > > which isn't compiled with spice-session-priv.h, so make > > > > > > > spice_session_get_webdav_server() public. > > > > > > > > I haven't looked at the whole series. Wouldn't it make sense to make it a > > > read-only property instead? > > > > It sounds reasonable for me. > > Jakub ? > > > > I agree. > > Revert the commits please. I'll reopen the merge request once I have it > ready. > > Cheers, > Jakub > To be honest I don't see the need to revert commits, it's just a change from public to private. Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [Spice-commits] 6 commits - meson.build src/map-file src/spice-glib-sym-file src/spice-gtk-session.c src/spice-session.c src/spice-session.h src/spice-session-priv.h
> > commit 4b9092b96b8da946ff3d17922b0fcf225c5dc81f > > > Author: Jakub Janků < jja...@redhat.com > > > > Date: Sat May 23 16:28:52 2020 +0200 > > > session: make spice_session_get_webdav_server() public > > > It will be necessary to access the webdav server from spice-gtk-session.c > > > which isn't compiled with spice-session-priv.h, so make > > > spice_session_get_webdav_server() public. > > I haven't looked at the whole series. Wouldn't it make sense to make it a > read-only property instead? It sounds reasonable for me. Jakub ? Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [Spice-commits] 6 commits - meson.build src/map-file src/spice-glib-sym-file src/spice-gtk-session.c src/spice-session.c src/spice-session.h src/spice-session-priv.h
Hi,
On Wed, Sep 9, 2020 at 5:50 PM GitLab Mirror <
gitlab-mir...@kemper.freedesktop.org> wrote:
> meson.build | 13 -
> src/map-file |1
> src/spice-glib-sym-file |1
> src/spice-gtk-session.c | 399
> +++
> src/spice-session-priv.h |1
> src/spice-session.c | 30 +++
> src/spice-session.h |5
> 7 files changed, 443 insertions(+), 7 deletions(-)
>
> New commits:
> commit f33d589d747f4f7ee6a1241c344ca611a36e9c71
> Author: Jakub Janků
> Date: Fri May 29 17:59:38 2020 +0200
>
> clipboard: enable copying files to guest using webdav
>
> When an app advertises the "text/uri-list" target, the user
> probably wants to copy/move files. Spice-gtk then sends
> a grab message to the vdagent advertising the
> VD_AGENT_CLIPBOARD_FILE_LIST type.
>
> Vdagent can then request clipboard data in this type.
>
> Spice-gtk tries to talk to the app that owns the clipboard
> in its native format in order to determine the preferred
> file operation (copy X move).
>
> For GNOME Nautilus, that's simply "UTF8_TEXT",
> for KDE Dolphin, "application/x-kde-cutselection".
>
> Otherwise the generic "text/uri-list" is used that does not
> provide any additional information.
>
> Once the uri list is obtained from the app, spice-gtk
> creates a unique virtual dir in the ".spice-clipboard"
> directory that is designated for this purpose.
>
> Each file is attached inside this virtual dir using
> phodav_virtual_dir_attach_real_child(), see phodav API
> for details.
>
> A list of paths in the phodav server is then sent to vdagent,
> as specified in the spice-protocol.
> Such path can for example look like this:
> /.spice-clipboard/b8f0249c-082a-4da9-9a38-2de3237a66f0/file
>
> It is up to the vdagent to ensure that the spice shared folder
> is accessible and to set the clipboard data in a format that
> other apps understand.
>
> This requires new phodav with PhodavVirtualDir API.
>
> Signed-off-by: Jakub Janků
> Acked-by: Frediano Ziglio
>
> diff --git a/meson.build b/meson.build
> index 7ade460..e43139e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -33,7 +33,7 @@ spice_glib_deps = []
> spice_gtk_deps = []
> spice_wayland_deps = []
> spice_acl_deps = []
> -spice_protocol_version = '0.14.2'
> +spice_protocol_version = '0.14.3'
>
> #
> # Set up subprojects
> diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> index 5e6be4a..dfbd8fa 100644
> --- a/src/spice-gtk-session.c
> +++ b/src/spice-gtk-session.c
> @@ -34,6 +34,10 @@
> #endif
> #endif
>
> +#ifdef HAVE_PHODAV_VIRTUAL
> +#include
> +#endif
> +
> #include
> #include
> #include "desktop-integration.h"
> @@ -61,6 +65,8 @@ struct _SpiceGtkSessionPrivate {
> gbooleanclip_grabbed[CLIPBOARD_LAST];
> gbooleanclipboard_by_guest[CLIPBOARD_LAST];
> guint clipboard_release_delay[CLIPBOARD_LAST];
> +/* TODO: maybe add a way of restoring this? */
> +GHashTable *cb_shared_files;
> /* auto-usbredir related */
> gbooleanauto_usbredir_enable;
> int auto_usbredir_reqs;
> @@ -191,6 +197,12 @@ static void spice_gtk_session_init(SpiceGtkSession
> *self)
>
> s = self->priv = spice_gtk_session_get_instance_private(self);
>
> +s->cb_shared_files =
> +g_hash_table_new_full(g_file_hash,
> + (GEqualFunc)g_file_equal,
> + g_object_unref, /* unref GFile */
> + g_free /* free gchar * */
> + );
> s->clipboard = gtk_clipboard_get(GDK_SELECTION_CLIPBOARD);
> g_signal_connect(G_OBJECT(s->clipboard), "owner-change",
> G_CALLBACK(clipboard_owner_change), self);
> @@ -252,6 +264,7 @@ static void spice_gtk_session_dispose(GObject *gobject)
> self);
> s->session = NULL;
> }
> +g_clear_pointer(&s->cb_shared_files, g_hash_table_destroy);
>
> /* Chain up to the parent class */
> if (G_OBJECT_CLASS(spice_gtk_session_parent_class)->dispose)
> @@ -544,6 +557,9 @@ static const struct {
> },{
> .vdagent = VD_AGENT_CLIPBOARD_IMAGE_JPG,
> .xatom = "image/jpeg"
> +},{
> +.vdagent = VD_AGENT_CLIPBOARD_FILE_LIST,
> +.xatom = "text/uri-list"
> }
> };
>
> @@ -660,6 +676,18 @@ static void clipboard_get_targets(GtkClipboard
> *clipboard,
> continue;
> }
>
> +if (atom2agent[m].vdagent == VD_AGENT_CLIPBOARD_FILE_LIST) {
> +#ifdef HAVE_PHODAV_VIRTUAL
> +if (!clipboard_get_open_webdav(s->session)) {
> +SPICE_DEBUG("Received %s target, but the clipboard
> webdav channel "
> +
