[virt-tools-list] [PATCH 7/7] Make GApplication port compatible with older glib

[Fabiano Fidêncio]

On Mon, Dec 14, 2015 at 10:42 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
> On Fri, 2015-12-11 at 17:11 +0000, Daniel P. Berrange wrote:
>> On Fri, Dec 11, 2015 at 02:40:36PM -0200, Eduardo Lima (Etrunko) wrote:
>> > Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
>> > ---
>> >  configure.ac           |  2 +-
>> >  src/remote-viewer.c    |  9 +++++++++
>> >  src/virt-glib-compat.c | 29 +++++++++++++++++++++++++++++
>> >  src/virt-viewer-app.c  | 15 +++++++++++++++
>> >  src/virt-viewer-app.h  |  8 ++++++++
>> >  src/virt-viewer.c      |  9 +++++++++
>> >  6 files changed, 71 insertions(+), 1 deletion(-)
>>
>> [snip]
>>
>> You've added an awful lot of back compat code here to deal with
>> this. I can't help thinking a better approach is to just not
>> use the g_application_add_main_option_entries() method in the
>> first place instead of adding piles of copy+pasted code.
>>
>> Reading the description of that method does not make it sound
>> like it is a critical feature we absolutely need to have.
>>
>> [quote]
>> This function is new in GLib 2.40. Before then, the only real
>> choice was to send all of the commandline arguments (options
>> and all) to the primary instance for handling. GApplication
>> ignored them completely on the local side. Calling this function
>> "opts in" to the new behaviour, and in particular, means that
>> unrecognised options will be treated as errors.
>> [/quote]
>>
>>
>> IMHO we could just do as that suggested, and send the options
>> to the primary instance for handling instead of parsing them
>> locally. Or alternatively just carry on using the GOptionContext
>> for parsing CLI args locally, and only use GApplication for the
>> non-CLI arg handling parts
>
> I thinkt hat I agree with Daniel that this seems like an awful lot of code to be
> copying into virt-viewer. Since we are explicitly choosing not to be a "unique"
> application, handle-local-options and command-line are both going to be handled
> in the same process. So I think it would be simpler to just use command-line and
> follow the approach demonstrated here:
>
> https://git.gnome.org/browse/glib/tree/gio/tests/gapplication-example-cmdline3.c
>
> I'd be happy to hear opinions from others as well.

In one hand having all the code in a way that we can just drop the
compat functions when SLES starts using GLib 2.40 may be really
handful. On the other hand, as pointed by Jonathon and Daniel, it's an
awful lot of code copied into virt-viewer.

Jonathon, considering we decide to go for using GOptionContext for
parsing CLI args, would we have to change this code in the future and
adapt it to use GApplication or could we just use this implementation
without any drawback as virt-viewer is not supposed to be a "unique"
application?
If there are no drawbacks, for sure using the GOptionContext for
parsing CLI args is the best option. If there are drawbacks, as a
first step, I would check if SLES' people what are their plans to
start using glib 2.40 ... and then we can start discussing it from
their answer.

Best Regards,
-- 
Fabiano Fidêncio