Re: [PATCH 05/26] configure, meson: convert pam detection to meson

[Daniel P . Berrangé]

On Wed, Jun 09, 2021 at 08:46:22AM -0700, Richard Henderson wrote:
> On 6/8/21 1:20 PM, Daniel P. Berrangé wrote:
> > On Tue, Jun 08, 2021 at 12:45:51PM -0700, Richard Henderson wrote:
> > > On 6/8/21 4:22 AM, Paolo Bonzini wrote:
> > > > +pam = not_found
> > > > +if not get_option('auth_pam').auto() or have_system
> > > > +  pam = cc.find_library('pam', has_headers: ['security/pam_appl.h'],
> > > 
> > > The condition doesn't look right.
> > > Why are we looking for pam if --disable-pam-auth?
> > > 
> > > Surely
> > > 
> > >    if not get_option('auth_pam').disabled() and have_system
> > 
> > This isn't entirely obvious at first glance, but the line after
> > the one you quote with the 'required' param makes it "do the
> > right thing (tm)".
> > 
> > The 'auth_pam' option is a tri-state taking 'enabled', 'disabled'
> > and 'auto', with 'auto' being the default state. When a tri-state
> > value is passed as the value of the 'required' parameter, then
> > 
> >     required==enabled   is interpreted as 'required=true'
> >     required==auto      is interpreted as 'required=false'
> >     required==disabled  means the entire call is a no-op
> > 
> > So this logic:
> > 
> >   if not get_option('auth_pam').auto() or have_system
> >      pam = cc.find_library('pam', has_headers: ['security/pam_appl.h'],
> >                            required: get_option('auth_pam'),
> >                       ...)
> > 
> > Means
> > 
> >    => If 'auto' is set, then only look for the library if we're
> >       building system emulators. In this case 'required:' will
> >       evaluate to 'false', and so we'll gracefully degrade
> >       if the library is missing.
> 
> If not have_system, there's no point in looking for pam *at all* regardless
> of get_option().

In theory we can simplify to

   if have_system
      pam = cc.find_library('pam', has_headers: ['security/pam_appl.h'],
                            required: get_option('auth_pam'),
                          ...)

and this will be fine for builds with system emulators. The only
caveat is that if someone disables system emulators while also
passing  -Dpam=enabled, we won't check for pam. That is a
nonsense combination of course, so probably doesn't matter

> 
> >    => If 'disabled' is set, then the 'find_library' call
> >       will not look for anything, immediately return a
> >       'not found' result and let the caller carry on.
> 
> This is not true.  If 'required: false', find_library *will* look for the
> library, but it will allow it to be missing.

feature==disabled does not map to required: false

  https://mesonbuild.com/Build-options.html#features

[quote]
    enabled is the same as passing required : true.
    auto is the same as passing required : false.
    disabled do not look for the dependency and always return 'not-found'.
[/quote]


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|