[libcamera-devel,v2] lc-compliance: Build with googltest in subproject
diff mbox series

Message ID 20211109053219.2682711-1-hiroh@chromium.org
State Superseded
Headers show
Series
  • [libcamera-devel,v2] lc-compliance: Build with googltest in subproject
Related show

Commit Message

Hirokazu Honda Nov. 9, 2021, 5:32 a.m. UTC 
libgtest-dev is provided as a static library at least Debian 10.
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
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@chromium.org>
Tested-by: Jacopo Mondi <jacopo@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

Comments

Kieran Bingham Nov. 9, 2021, 11:29 a.m. UTC | #1 
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.
> 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@chromium.org>
> Tested-by: Jacopo Mondi <jacopo@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
>  
> +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.

> +        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 ;-)

Minors aside,


Reviewed-by: Kieran Bingham <kieran.bingham@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
> -- 
> 2.34.0.rc0.344.g81b53c2807-goog
>
Laurent Pinchart Nov. 11, 2021, 11:45 p.m. UTC | #2 
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@chromium.org>
> > Tested-by: Jacopo Mondi <jacopo@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@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

Patch
diff mbox series 

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
 
+if get_option('android_platform') == 'cros'
+    libgtest = dependency('gtest', required : get_option('lc-compliance'))
+
+    if not libgtest.found()
+        lc_compliance_enabled = false
+        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
\ 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