[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

[Frediano Ziglio]

> Hi

> On Wed, Sep 9, 2020 at 6:45 PM Jakub Janku < jjanku at redhat.com > wrote:

> > On Wed, Sep 9, 2020 at 4:36 PM Frediano Ziglio < fziglio at redhat.com >
> > wrote:
> 
> > >
> 
> > > > On Wed, Sep 9, 2020 at 4:16 PM Frediano Ziglio < fziglio at redhat.com >
> > > > wrote:
> 
> > > > >
> 
> > > > > > > commit 4b9092b96b8da946ff3d17922b0fcf225c5dc81f
> 
> > > > > >
> 
> > > > > > > Author: Jakub Janků < jjanku at 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