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

[Eduardo Lima (Etrunko)]

On 12/14/2015 07:42 PM, Jonathon Jongsma 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:
> 

Yes, I do agree it is a lot of code copied, my target was to change the
behaviour introduced by the patch as little as possible, so in the
future, when we update the glib requirements once again, the cleanup
would be straightforward.

I am working on the new version of the patch which will change the
proposed approach to not make use of all functionality provided by glib,
like keeping using option parsing locally.

Regards, Eduardo

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com