[libcamera-devel] [PATCH v1 4/6] apps: Move libevent dependency to src/apps/meson.build

[Laurent Pinchart]

Hi Kieran,

On Thu, Oct 20, 2022 at 10:06:32AM +0100, Kieran Bingham wrote:
> Quoting Paul Elder via libcamera-devel (2022-10-20 07:54:45)
> > On Thu, Oct 20, 2022 at 02:15:35AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > libtiff is a shared dependency between cam and lc-compliance, move it to
> > 
> > s/libtiff/libevent/
> > 
> > > src/apps/. The shared dependency will be used to condition compilation
> > > of source files in an upcoming application static library.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  src/apps/cam/meson.build           | 4 +---
> > >  src/apps/lc-compliance/meson.build | 3 +--
> > >  src/apps/meson.build               | 8 ++++++++
> > >  3 files changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/src/apps/cam/meson.build b/src/apps/cam/meson.build
> > > index 06dbea0645b7..4b6099ddab63 100644
> > > --- a/src/apps/cam/meson.build
> > > +++ b/src/apps/cam/meson.build
> > > @@ -1,8 +1,6 @@
> > >  # SPDX-License-Identifier: CC0-1.0
> > >  
> > > -libevent = dependency('libevent_pthreads', required : get_option('cam'))
> > > -
> > > -if not libevent.found()
> > > +if opt_cam.disabled() or not libevent.found()
> > >      cam_enabled = false
> > >      subdir_done()
> > >  endif
> > > diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build
> > > index 8b57474be2b2..05d622be0a40 100644
> > > --- a/src/apps/lc-compliance/meson.build
> > > +++ b/src/apps/lc-compliance/meson.build
> > > @@ -1,10 +1,9 @@
> > >  # SPDX-License-Identifier: CC0-1.0
> > >  
> > > -libevent = dependency('libevent_pthreads', required : get_option('lc-compliance'))
> > >  libgtest = dependency('gtest', required : get_option('lc-compliance'),
> > >                        fallback : ['gtest', 'gtest_dep'])
> > >  
> > > -if not (libevent.found() and libgtest.found())
> > > +if opt_lc_compliance.disabled() or not libevent.found() or not libgtest.found()
> > >      lc_compliance_enabled = false
> > >      subdir_done()
> > >  endif
> > > diff --git a/src/apps/meson.build b/src/apps/meson.build
> > > index 9e4388bd7881..159deb0b7fc2 100644
> > > --- a/src/apps/meson.build
> > > +++ b/src/apps/meson.build
> > > @@ -1,5 +1,13 @@
> > >  # SPDX-License-Identifier: CC0-1.0
> > >  
> > > +opt_cam = get_option('cam')
> > > +opt_lc_compliance = get_option('lc-compliance')
> > > +
> > > +libevent = dependency('libevent_pthreads', required : opt_cam)
> > > +if not libevent.found()
> > > +    libevent = dependency('libevent_pthreads', required : opt_lc_compliance)
> > > +endif
> 
> This ... is an odd way to express this. I expect we can't do sufficient
> boolean operations with opt_cam and opt_lc_compliance?

Unfortunately not, they're feature options, so tri-state. One
alternative is

if opt_cam.disabled() and opt_lc_compliance.disabled()
    libevent = disabler()
else
    libevent = dependency('libevent_pthreads', required : opt_cam.enabled() or opt_lc_compliance.enabled())
endif

but I don't think that's better.

> > > +libevent = dependency('libevent_pthreads', required : opt_cam)
> > > +if not libevent.found()
> > > +    libevent = dependency('libevent_pthreads', required : opt_lc_compliance)
> > > +endif

> It deserves a comment above to say why we are searching for the
> dependency twice. Though I expect anyone trying to optimise it to a
> single line would quickly hit whatever has caused you to break it out to
> two.

Would this work for you ?

# libevent is needed by cam and lc-compliance. As they are both feature options,
# they can't be combined with simple boolean logic.

> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > > +
> > >  subdir('lc-compliance')
> > >  
> > >  subdir('cam')

-- 
Regards,

Laurent Pinchart