[virt-tools-list] [virt-viewer v2] Use GResource for loading ui files

Fabiano Fidêncio fidencio at redhat.com
Thu Mar 3 22:10:26 UTC 2016


On Wed, Mar 2, 2016 at 9:57 PM, Eduardo Lima (Etrunko)
<etrunko at redhat.com> wrote:
> On 03/02/2016 05:24 PM, Fabiano Fidêncio wrote:
>> On Wed, Mar 2, 2016 at 8:05 PM, Eduardo Lima (Etrunko)
>> <etrunko at redhat.com> wrote:
>>> On 03/02/2016 02:46 PM, Eduardo Lima (Etrunko) wrote:
>>>> On 03/02/2016 12:23 PM, Fabiano Fidêncio wrote:
>>>>> Let's take advantage of GResource for loading ui files in a better and
>>>>> cleaner way than virt_viewer_util_load_ui() was doing.
>>>>> It also brings the benefit, at least for developers, of being able to
>>>>> test ui changes without having to "make install" virt-viewer.
>>>>>
>>>>> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
>>>>> ---
>>>>> Changes since v1:
>>>>>  - Adressed all comments from Eduardo and Jonathon.
>>>>> ---
>>>>>  mingw-virt-viewer.spec.in     | 18 ++--------------
>>>>>  src/Makefile.am               | 21 +++++++++++++------
>>>>>  src/virt-viewer-about.xml     |  1 -
>>>>>  src/virt-viewer-app.c         |  5 +++++
>>>>>  src/virt-viewer-util.c        | 48 +++++--------------------------------------
>>>>>  src/virt-viewer-util.h        |  1 +
>>>>>  src/virt-viewer-window.c      | 20 ++++++++++++++----
>>>>>  src/virt-viewer.gresource.xml | 19 +++++++++++++++++
>>>>>  virt-viewer.spec.in           |  9 --------
>>>>>  9 files changed, 63 insertions(+), 79 deletions(-)
>>>>>  create mode 100644 src/virt-viewer.gresource.xml
>>>>>
>>>>> diff --git a/mingw-virt-viewer.spec.in b/mingw-virt-viewer.spec.in
>>>>> index b200db7..ddea296 100644
>>>>> --- a/mingw-virt-viewer.spec.in
>>>>> +++ b/mingw-virt-viewer.spec.in
>>>>> @@ -114,10 +114,12 @@ MinGW Windows virt-viewer MSI
>>>>>  %mingw_make_install DESTDIR=$RPM_BUILD_ROOT
>>>>>
>>>>>  %if 0%{?mingw_build_win32} == 1
>>>>> +mkdir $RPM_BUILD_ROOT/%{mingw32_datadir}/virt-viewer
>>>>>  cp build_win32$MINGW_BUILDDIR_SUFFIX/data/virt-viewer-x86- at VERSION@.msi $RPM_BUILD_ROOT/%{mingw32_datadir}/virt-viewer
>>>>>  %endif
>>>>>
>>>>>  %if 0%{?mingw_build_win64} == 1
>>>>> +mkdir $RPM_BUILD_ROOT/%{mingw64_datadir}/virt-viewer
>>>>>  cp build_win64$MINGW_BUILDDIR_SUFFIX/data/virt-viewer-x64- at VERSION@.msi $RPM_BUILD_ROOT/%{mingw64_datadir}/virt-viewer
>>>>>  %endif
>>>>>
>>>>> @@ -138,14 +140,6 @@ rm -rf $RPM_BUILD_ROOT
>>>>>  %{mingw32_bindir}/debug-helper.exe
>>>>>
>>>>>  %dir %{mingw32_datadir}/virt-viewer/
>>>>> -%dir %{mingw32_datadir}/virt-viewer/ui/
>>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer.xml
>>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-about.xml
>>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-auth.xml
>>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-guest-details.xml
>>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-vm-connection.xml
>>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-preferences.xml
>>>>> -%{mingw32_datadir}/virt-viewer/ui/remote-viewer-connect.xml
>>>>>  %{mingw32_datadir}/icons/hicolor/*/apps/*
>>>>>  %{mingw32_datadir}/icons/hicolor/*/devices/*
>>>>>
>>>>> @@ -163,14 +157,6 @@ rm -rf $RPM_BUILD_ROOT
>>>>>  %{mingw64_bindir}/debug-helper.exe
>>>>>
>>>>>  %dir %{mingw64_datadir}/virt-viewer/
>>>>> -%dir %{mingw64_datadir}/virt-viewer/ui/
>>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer.xml
>>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-about.xml
>>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-auth.xml
>>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-guest-details.xml
>>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-vm-connection.xml
>>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-preferences.xml
>>>>> -%{mingw64_datadir}/virt-viewer/ui/remote-viewer-connect.xml
>>>>>  %{mingw64_datadir}/icons/hicolor/*/apps/*
>>>>>  %{mingw64_datadir}/icons/hicolor/*/devices/*
>>>>>
>>>>> diff --git a/src/Makefile.am b/src/Makefile.am
>>>>> index f42a7bf..a42c01e 100644
>>>>> --- a/src/Makefile.am
>>>>> +++ b/src/Makefile.am
>>>>> @@ -5,8 +5,7 @@ bin_PROGRAMS =
>>>>>
>>>>>  noinst_LTLIBRARIES = libvirt-viewer.la
>>>>>
>>>>> -builderxmldir = $(pkgdatadir)/ui
>>>>> -builderxml_DATA =                           \
>>>>> +noinst_DATA =                                               \
>>>>>      virt-viewer.xml                         \
>>>>>      virt-viewer-about.xml                   \
>>>>>      virt-viewer-auth.xml                    \
>>>>> @@ -17,9 +16,12 @@ builderxml_DATA =                         \
>>>>>      $(NULL)
>>>>>
>>>>>  EXTRA_DIST =                                        \
>>>>> -    $(builderxml_DATA)                      \
>>>>> +    $(noinst_DATA)                                  \
>>>>> +    virt-viewer-resources.c                 \
>>>>> +    virt-viewer-resources.h                 \
>>>>>      virt-viewer-enums.c.etemplate           \
>>>>>      virt-viewer-enums.h.etemplate           \
>>>>> +    virt-viewer.gresource.xml               \
>>>>>      $(NULL)
>>>>>
>>>
>>> Ooops, I overlooked this part. It seems you don't need resources.[ch] in
>>> EXTRA_DIST. They will be automatically generated.
>>
>> You're right.
>> Also, on a IRC discussion, Eduardo pointed out for checking if
>> glib-compile-resources is present.
>> Here is a diff of all your suggestions:
>>
>> diff --git a/configure.ac b/configure.ac
>> index 33362f9..e1c9b1b 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -107,6 +107,12 @@ GLIB2_CFLAGS="$GLIB2_CFLAGS
>> -DGLIB_VERSION_MAX_ALLOWED=$GLIB2_ENCODED_VERSION"
>>  AC_SUBST(GLIB2_CFLAGS)
>>  AC_SUBST(GLIB2_LIBS)
>>
>> +AC_ARG_VAR([GLIB_COMPILE_RESOURCES],[the glib-compile-resources program])
>> +AC_PATH_PROG([GLIB_COMPILE_RESOURCES],[glib-compile-resources],[])
>> +if test -z "$GLIB_COMPILE_RESOURCES"; then
>> +  AC_MSG_ERROR([glib-compile-resources not found])
>> +fi
>> +
>>  PKG_CHECK_MODULES(LIBXML2, libxml-2.0 >= $LIBXML2_REQUIRED)
>>
>>  AC_ARG_WITH([libvirt],
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 2faaab2..d977f5f 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -17,8 +17,6 @@ noinst_DATA =                                         \
>>
>>  EXTRA_DIST =                                   \
>>         $(noinst_DATA)                                  \
>> -       virt-viewer-resources.c                 \
>> -       virt-viewer-resources.h                 \
>>         virt-viewer-enums.c.etemplate           \
>>         virt-viewer-enums.h.etemplate           \
>>         virt-viewer.gresource.xml               \
>> @@ -35,8 +33,8 @@ BUILT_SOURCES =                                       \
>>         virt-viewer-enums.c                     \
>>         $(NULL)
>>
>> -virt-viewer-resources.c virt-viewer-resources.h:
>> virt-viewer.gresource.xml Makefile $(shell glib-compile-resources
>> --generate-dependencies --sourcedir $(srcdir)
>> $(srcdir)/virt-viewer.gresource.xml)
>> -       $(AM_V_GEN) glib-compile-resources --target=$@
>> --sourcedir=$(srcdir) --generate --c-name virt_viewer $<
>> +virt-viewer-resources.c virt-viewer-resources.h:
>> virt-viewer.gresource.xml Makefile $(shell $(GLIB_COMPILE_RESOURCES)
>> --generate-dependencies --sourcedir $(srcdir)
>> $(srcdir)/virt-viewer.gresource.xml)
>> +       $(AM_V_GEN) $(GLIB_COMPILE_RESOURCES) --target=$@
>> --sourcedir=$(srcdir) --generate --c-name virt_viewer $<
>>
>>  virt-viewer-enums.c virt-viewer-enums.h: %: %.etemplate $(ENUMS_FILES)
>>         $(AM_V_GEN)$(GLIB_MKENUMS) --template $^ | \
>>
>>
>> Does it look good enough?
>>
>
> Yes it does, sorry for the noise ;)


Pushed,
Thanks for the review!

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




More information about the virt-tools-list mailing list