[Spice-devel] [PATCH spice-gtk 28/34] meson: switch vapi to auto feature

[Marc-André Lureau]

Hi

On Tue, Jan 8, 2019 at 12:30 PM Christophe Fergeau <cfergeau at redhat.com> wrote:
>
> On Mon, Jan 07, 2019 at 11:15:55PM +0400, Marc-André Lureau wrote:
> > On Mon, Jan 7, 2019 at 8:21 PM Christophe Fergeau <cfergeau at redhat.com> wrote:
> > >
> > > On Mon, Jan 07, 2019 at 12:00:57PM +0400, marcandre.lureau at redhat.com wrote:
> > > > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > >
> > > > Removed unused vapigen/vapidir variables as well.
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > > ---
> > > >  meson.build       | 9 +++++----
> > > >  meson_options.txt | 3 +--
> > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/meson.build b/meson.build
> > > > index 9896151..7e0c68d 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -279,10 +279,11 @@ endif
> > > >
> > > >  # vala (depends on introspection)
> > > >  spice_gtk_has_vala = false
> > > > -if spice_gtk_has_introspection and get_option('vapi')
> > > > -  vapigen_dep = dependency('vapigen')
> > > > -  vapidir = vapigen_dep.get_pkgconfig_variable('vapidir')
> > > > -  vapigen = dependency('vapigen').get_pkgconfig_variable('vapigen')
> > > > +d = dependency('vapigen', required : get_option('vapi'))
> > > > +if d.found()
> > > > +  if not spice_gtk_has_introspection
> > > > +    error('VAPI support requested without introspection')
> > > > +  endif
> > >
> > > I'd be a bit more sophisticated here in order to handle 'auto', maybe
> > > something like this:
> > >
> > > if not spice_gtk_has_introspection:
> > >     if get_option('vapi').enabled()
> > >         error('VAPI support requested without introspection')
> > >     endif
> > >     spice_gtk_has_vala = false
> > > endif
> > > spice_gtk_has_vala = true
> >
> > That wouldn't work if vapi is enabled and introspection is disabled.
> >
> > I don't think the build-sys needs to be that smart for "auto"
> > features. Or it could be improved later.
>
> The way I see these 'feature' changes is that we switch from options
> with true/false to options with true/false/auto. So we are adding
> support for auto. But in some of the patches, the auto mode actually
> behaves like something in between true and auto, partly autodetected,
> partly mandatory. If that's good enough for you, I don't mind that
> strongly, but still feels suboptimal ;)

You are right, there are somewhere in between. Enough to handle the
common all or nothing case, working nicely for me for linux & mingw
builds. But there are suboptimals, I will add TODO notes there, just
to point out this is not overlooked and could be added later for
anyone who cares enough.

-- 
Marc-André Lureau