| Message ID | 20221019231537.26880-5-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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 > 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@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@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 > + > subdir('lc-compliance') > > subdir('cam')
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@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 > + > subdir('lc-compliance') > > subdir('cam') > -- > Regards, > > Laurent Pinchart >
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@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? 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. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > + > > subdir('lc-compliance') > > > > subdir('cam') > > -- > > Regards, > > > > 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@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@ideasonboard.com> > > > > + > > > subdir('lc-compliance') > > > > > > subdir('cam')
Quoting Laurent Pinchart (2022-10-20 11:09:45) > 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@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. That' solves my concern thanks. It's just an odd code pattern, that doesn't read well, so the comment helps. Thanks. > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > + > > > > subdir('lc-compliance') > > > > > > > > subdir('cam') > > -- > Regards, > > Laurent Pinchart
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 + subdir('lc-compliance') subdir('cam')
libtiff is a shared dependency between cam and lc-compliance, move it to 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@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(-)