[virt-tools-list] [virt-viewer v2 3/3] util: allow loading an in-tree ui file

[Fabiano Fidêncio]

On Thu, Feb 25, 2016 at 2:26 PM, Fabiano Fidêncio <fidencio at redhat.com> wrote:
> On Thu, Feb 25, 2016 at 2:19 PM, Daniel P. Berrange <berrange at redhat.com> wrote:
>> On Thu, Feb 25, 2016 at 02:03:40PM +0100, Fabiano Fidêncio wrote:
>>> This commits allows to load ui files without the need to install
>>> virt-viewer. It helps a lot developers testing changes in the ui files
>>> and also avoid abortions like [0] when running remote-viewer from the
>>> source tree and without having any previous installation.
>>>
>>> [0]:
>>> fidenci at cat ~/src/upstream/virt-viewer $ ./src/remote-viewer
>>>
>>> (remote-viewer:29951): virt-viewer-ERROR **: failed to find UI
>>> description file
>>> Trace/breakpoint trap (core dumped)
>>>
>>> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
>>> ---
>>>  src/Makefile.am        |  2 +-
>>>  src/virt-viewer-util.c | 35 +++++++++++++++++++++++++++++++----
>>>  2 files changed, 32 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/Makefile.am b/src/Makefile.am
>>> index 171a141..ac7177d 100644
>>> --- a/src/Makefile.am
>>> +++ b/src/Makefile.am
>>> @@ -187,7 +187,7 @@ if OS_WIN32
>>>  remote_viewer_LDFLAGS += -Wl,--subsystem,windows
>>>  endif
>>>
>>> -AM_CPPFLAGS = -DPACKAGE_DATADIR=\""$(pkgdatadir)"\"
>>> +AM_CPPFLAGS = -DPACKAGE_DATADIR=\""$(pkgdatadir)"\" -DSOURCE_DIR=\""$(abs_srcdir)"\"
>>
>> Embedding the source build directory inside binaries is generally
>> bad practice IMHO.
>
> Thanks, that's exactly the kind of feedback I was expecting for.
>
>>
>>> +static gboolean
>>> +virt_viewer_util_try_to_load_ui_from_non_system_data_dirs(GtkBuilder *builder,
>>> +                                                          const gchar *name,
>>> +                                                          GError **error)
>>> +{
>>> +    gboolean ret;
>>> +    gchar *path;
>>> +
>>> +    /*
>>> +     * Firstly, try to load the ui file from the $srcdir.
>>> +     * It allows developers to test changes done in the ui file without
>>> +     * installing virt-viewer.
>>> +     */
>>> +    path = g_build_filename(SOURCE_DIR, name, NULL);
>>
>> Just create a relative path here eg
>>
>>  g_build_filename(".", "src", name, NULL);
>>
>
> Sure, I'll re-work it and send a v3.
> It must work considering VPATHs, let's see what I can come up with ...

Just for the record, this is Daniel's suggestion:

10:56 <  fidencio> danpb: just to be sure, any suggestion for getting
the right filename when building virt-viewer using VPATH?
10:59 <     danpb> simplest approach would be to check  ../src and ./src
10:59 <     danpb> that works well enough for people using a vpath in
immediate subdir


>
>>> +    ret = virt_viewer_util_add_to_builder_from_file(builder, path, error);
>>>      g_free(path);
>>>
>>> +    if (!ret) {
>>> +        /*
>>> +         * And only then try to load the ui file from the $pkgdatadir.
>>> +         */
>>> +        path = g_build_filename(PACKAGE_DATADIR, "ui", name, NULL),
>>> +        ret = virt_viewer_util_add_to_builder_from_file(builder, path, error);
>>> +        g_free(path);
>>> +    }
>>> +
>>>      return ret;
>>
>> Regards,
>> Daniel
>> --
>> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
>> |: http://libvirt.org              -o-             http://virt-manager.org :|
>> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
>> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
>
> Best Regards,
> --
> Fabiano Fidêncio