On Thu, Feb 25, 2016 at 2:19 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> 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@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@xxxxxxxxxx>
>> ---
>> 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 ...
>> + 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
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
