[Spice-devel] [PATCH spice-gtk 1/3] Add support for building with meson

[Christophe Fergeau]

On Fri, May 04, 2018 at 12:17:20PM -0300, Eduardo Lima (Etrunko) wrote:
> On 03/05/18 12:24, Christophe Fergeau wrote:
> > Hey,
> > 
> > So I've experimented with this today, couple of comments/suggested
> > improvements/fixes/missing things/...
> > It might be worth to have some short document somewhere describing the
> > meson/ninja equivalent for (at least)
> > make CFLAGS="-Wno-error"
> > ./configure (eg meson configure --prefix "/home/foo")
> > make dist
> > make check
> > 
> 
> Okay, I don't know really where would be the best place for this kind of
> document, maybe start something on the gitlab wiki and then creating a
> link to the page on the website?

Or just in the commit log for now?

> 
> > I noticed "ninja" does not build the tests while "make" does. During the
> > tests,  test-mock-acl-helper is considered a standalone test which
> > should be run as part of "ninja test", while it's not supposed to be run
> > this way, it's a helper for usb-acl-helper.c
> 
> I think you are mistaken, the tests are built for sure> 

Yup, sorry, I rechecked, it builds them.

> I fixed adding mock-acl-helper as a test, thanks for spotting it.
> 
> > 
> > I don't know how to skip the error:
> > "Installing /home/teuf/redhat/spice-gtk/data/org.spice-space.lowlevelusbaccess.policy to /usr/share/polkit-1/actions"
> > "PermissionError: [Errno 13] Permission denied: '/usr/share/polkit-1/actions/org.spice-space.lowlevelusbaccess.policy'"
> > (ie is there a 'make -k' equivalent?)
> 
> As for make -k, there is also ninja -k.
> 
> The only way to avoid this error is by disabling policykit:
> 
>  $ meson --Dpolkit=false ./build
> 
> We could provide an option so the path can be specified, instead of
> checking for the pkgconfig variable. I just did not add it because
> autotools build also does not provide this option.

Yes, fair enough, especially as the same workaround as for make can be
used.


> > We also lost this bit of magic with meson, -Werror is always enabled,
> > while with autoconf/automake we had:
> > 
> >     AC_ARG_ENABLE([werror],
> >                   AS_HELP_STRING([--enable-werror], [Use -Werror (if supported)]),
> >                   [set_werror="$enableval"],
> >                   [if test -d $srcdir/.git; then
> >                      is_git_version=true
> >                      set_werror=yes
> >                    else
> >                      set_werror=no
> >                    fi])
> > so that when building release tarballs we don't have -Werror by default.
> 
> Is this worth to be added? It is easy enough, but really required?

In my opinion, yes, we really don't want -Werror te be enabled by
default when building tarballs (upgrading to f28 broke spice-gtk git build
precisely because it uses -Werror by default). But it's nice to have it
by default when building from git imo.

> 
> > 
> > See attachment for some proposed changes that can be squashed in.
> 
> Thanks for the patch. I fixed the install dirs and have one question
> about C defines. I followed the style from configure.ac, that uses
> c_flags instead of writing those to config.h, would it be okay to change it?

Imo, the more we move from command line to config.h, the better. The
meson change is a good opportunity to clean up that kind of things.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180504/4861da1f/attachment.sig>