[libcamera-devel] [PATCH v2] lc-compliance: Build with googltest in subproject

[Laurent Pinchart]

On Tue, Nov 09, 2021 at 11:29:34AM +0000, Kieran Bingham wrote:
> Should it be googletest or gtest in $SUBJECT?
> 
> Quoting Hirokazu Honda (2021-11-09 05:32:19)
> > libgtest-dev is provided as a static library at least Debian 10.

s/least/least by/

> > The compiler and linker to create the static library might be
> > different from ones used for libcamera. This causes a problem
> > upon linking.
> > 
> > This puts googltest code to subprojects, builds the code and link it
> 
> Same here.
> 
> > for lc-compliance. However, libgtest is locally built as a library on
> > ChromeOS and thus the used compiler and linker are the same as one
> > used for libcamera. We don't do these on ChromeOS build environment.
> > 
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > Tested-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  README.rst                    |  2 +-
> >  src/lc-compliance/meson.build | 16 ++++++++++++++--
> >  subprojects/.gitignore        |  4 +++-
> >  subprojects/gtest.wrap        | 14 ++++++++++++++
> >  4 files changed, 32 insertions(+), 4 deletions(-)
> >  create mode 100644 subprojects/gtest.wrap
> > 
> > diff --git a/README.rst b/README.rst
> > index 8af5f118..c48b4dba 100644
> > --- a/README.rst
> > +++ b/README.rst
> > @@ -99,7 +99,7 @@ for android: [optional]
> >          libexif-dev libjpeg-dev libyaml-dev
> >  
> >  for lc-compliance: [optional]
> > -        libevent-dev libgtest-dev
> > +        libevent-dev
> >  
> >  Using GStreamer plugin
> >  ~~~~~~~~~~~~~~~~~~~~~~
> > diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build
> > index aa5852f6..8c43ef12 100644
> > --- a/src/lc-compliance/meson.build
> > +++ b/src/lc-compliance/meson.build
> > @@ -1,15 +1,27 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >  
> >  libevent = dependency('libevent_pthreads', required : get_option('lc-compliance'))
> > -libgtest = dependency('gtest', required : get_option('lc-compliance'))
> >  
> > -if not (libevent.found() and libgtest.found())
> > +if not libevent.found()
> >      lc_compliance_enabled = false
> >      subdir_done()
> >  endif
> >  
> >  lc_compliance_enabled = true

This should be moved after the block below, or summary() will report
lc-compliance as enabled on cros when gtest can't be found.

> >  
> > +if get_option('android_platform') == 'cros'
> > +    libgtest = dependency('gtest', required : get_option('lc-compliance'))
> > +
> > +    if not libgtest.found()
> > +        lc_compliance_enabled = false
> 
> This could also be made to fall through and use the wrap somehow - but
> as it's not expected, and not essential I'm fine with this as it is.

Even better, we should use the system version of gtest if it's compiled
with the right compiler, regardless of the platform.

> > +        subdir_done()
> > +    endif
> > +
> > +else
> > +    libgtest_sp = subproject('gtest')
> > +    libgtest = libgtest_sp.get_variable('gtest_dep')
> > +endif
> > +
> >  lc_compliance_sources = files([
> >      '../cam/event_loop.cpp',
> >      '../cam/options.cpp',
> > diff --git a/subprojects/.gitignore b/subprojects/.gitignore
> > index 410b8bd6..82ef2c83 100644
> > --- a/subprojects/.gitignore
> > +++ b/subprojects/.gitignore
> > @@ -1 +1,3 @@
> > -/libyuv
> > \ No newline at end of file
> > +/libyuv
> > +/googletest-release*
> > +/packagecache
> 
> I bet I know someone who would ask for those to be alphabetical sort
> order ;-)

I wonder who :-)

> Minors aside,
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > \ No newline at end of file
> > diff --git a/subprojects/gtest.wrap b/subprojects/gtest.wrap
> > new file mode 100644
> > index 00000000..40128b35
> > --- /dev/null
> > +++ b/subprojects/gtest.wrap
> > @@ -0,0 +1,14 @@
> > +[wrap-file]
> > +directory = googletest-release-1.11.0
> > +source_url = https://github.com/google/googletest/archive/release-1.11.0.zip
> > +source_filename = gtest-1.11.0.zip
> > +source_hash = 353571c2440176ded91c2de6d6cd88ddd41401d14692ec1f99e35d013feda55a
> > +patch_filename = gtest_1.11.0-1_patch.zip
> > +patch_url = https://wrapdb.mesonbuild.com/v2/gtest_1.11.0-1/get_patch
> > +patch_hash = d38c39184384608b08419be52aed1d0f9d9d1b5ed71c0c35e51cccbdddab7084
> > +
> > +[provide]
> > +gtest = gtest_dep
> > +gtest_main = gtest_main_dep
> > +gmock = gmock_dep
> > +gmock_main = gmock_main_dep

-- 
Regards,

Laurent Pinchart