[libcamera-devel] [PATCH 3/3] meson: Summarize which applications and adaptation layers are built

[Laurent Pinchart]

Hi Kieran,

On Tue, Mar 23, 2021 at 04:02:37PM +0000, Kieran Bingham wrote:
> On 23/03/2021 15:33, Laurent Pinchart wrote:
> > On Tue, Mar 23, 2021 at 03:26:03PM +0000, Kieran Bingham wrote:
> >> On 20/03/2021 21:45, Laurent Pinchart wrote:
> >>> Add the application and adaptation layers being built to the meson
> >>> summary. The summary now prints
> >>>
> >>> libcamera 0.0.0
> >>>
> >>>   Configuration
> >>>          Enabled pipelines: ipu3
> >>>                             raspberrypi
> >>>                             rkisp1
> >>>                             simple
> >>>                             uvcvideo
> >>>                             vimc
> >>>          GStreamer support: True
> >>>     V4L2 emulation support: True
> >>>            cam application: True
> >>>           qcam application: True
> >>
> >> What about tests, Documentation, Android(/android platform?)?
> > 
> > I've forgotten Android indeed, I'll add it. For tests and documentation,
> > I wasn't sure whether to include them or leave them out. There are
> > multiple reasons to include a feature in the summary:
> > 
> > - making sure it's not left out (for instance to ensure that our
> >   compilation tests don't mistakenly forget to enable some components,
> >   decreasing test coverage)
> 
> Yes, helpful to see what was enabled or disabled.
> 
> > 
> > - giving feedback to users (to avoid "I don't get a cam executable" bug
> >   reports that would likely flood us if we were to silently disable cam,
> >   if it's explicitly disabled in the summary then hopefully users will
> >   know that something has to be done (but maybe the bug reports will
> >   only turn into "cam is disabled for me" :-))
> 
> Indeed, this is important too. I've already seen pastes of the meson
> build thrown about. The summary is valuable for some one who has to read
> that, without having access to that specific environment when dealing
> with bug reports or such.
> 
> > For tests and documentation, it may not be as useful. But maybe there
> > are reasons I'm missing ? You usually take the role of the community
> > manager who cares about the user experience, so if you think there's
> > value in reporting tests and documentation too, I'll follow your advices
> > :-)
> 
> It will provide /us/ with valuable information on what has / hasn't been
> configured when someone sends a paste of their build.
> 
> And for the same as your cam example
> Them: "I want to run the tests, but I can't find the binaries"
> Us: "I can see from your pasted configuration summary that you have not
> enabled the tests in your build".
> 
> That can even filter into our understanding of if someone has run the
> tests on patches they submit.
> 
> "Oh, thanks for the patch, I noticed in the paste of your build over
> there that you don't have tests enabled ... have you successfully run
> the tests on the patch you submitted?"
> 
> Maybe we won't require external contributors to run the tests for every
> patch if we have automated tests on submission, but at least if they see
> that there is a summary report saying:
> 
>             Unit Tests: False
> 
> Then the reader might think - oh - there are tests I could enable...

All these are good points. I'll add Android and unit tests and send a
v2. For documentation, we have a single option and separately compile
the sphinx and doxygen documentation. Do we need two entries in the
summary ? Or is documentation something we can skip ? I won't include it
in v2, we can do it on top.

> >> Can we include those in the summary? (Either here or on top...)
> >>
> >> Either way,
> >>
> >> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >>
> >>>   Subprojects
> >>>                     libyuv: YES
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >>> ---
> >>>  meson.build               | 4 ++++
> >>>  src/cam/meson.build       | 3 +++
> >>>  src/gstreamer/meson.build | 3 +++
> >>>  src/qcam/meson.build      | 3 +++
> >>>  src/v4l2/meson.build      | 3 +++
> >>>  5 files changed, 16 insertions(+)
> >>>
> >>> diff --git a/meson.build b/meson.build
> >>> index 1768f6eaf98e..34d7959edd8d 100644
> >>> --- a/meson.build
> >>> +++ b/meson.build
> >>> @@ -168,4 +168,8 @@ py_mod.find_installation('python3', modules: py_modules)
> >>>  ## Summarise Configurations
> >>>  summary({
> >>>              'Enabled pipelines': pipelines,
> >>> +            'GStreamer support': gst_enabled,
> >>> +            'V4L2 emulation support': v4l2_enabled,
> >>> +            'cam application': cam_enabled,
> >>> +            'qcam application': qcam_enabled,
> >>>          }, section : 'Configuration')
> >>> diff --git a/src/cam/meson.build b/src/cam/meson.build
> >>> index 65784beda4e6..5e1a7f387d60 100644
> >>> --- a/src/cam/meson.build
> >>> +++ b/src/cam/meson.build
> >>> @@ -3,9 +3,12 @@
> >>>  libevent = dependency('libevent_pthreads', required : get_option('cam'))
> >>>  
> >>>  if not libevent.found()
> >>> +    cam_enabled = false
> >>>      subdir_done()
> >>>  endif
> >>>  
> >>> +cam_enabled = true
> >>> +
> >>>  cam_sources = files([
> >>>      'buffer_writer.cpp',
> >>>      'capture.cpp',
> >>> diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build
> >>> index ea246dcdaa37..8cc811f84c8a 100644
> >>> --- a/src/gstreamer/meson.build
> >>> +++ b/src/gstreamer/meson.build
> >>> @@ -9,9 +9,12 @@ gstallocator_dep = dependency('gstreamer-allocators-1.0', version : gst_dep_vers
> >>>                                required : get_option('gstreamer'))
> >>>  
> >>>  if not glib_dep.found() or not gstvideo_dep.found() or not gstallocator_dep.found()
> >>> +    gst_enabled = false
> >>>      subdir_done()
> >>>  endif
> >>>  
> >>> +gst_enabled = true
> >>> +
> >>>  libcamera_gst_sources = [
> >>>      'gstlibcamera-utils.cpp',
> >>>      'gstlibcamera.cpp',
> >>> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> >>> index df4ca988bfaa..acde7682162e 100644
> >>> --- a/src/qcam/meson.build
> >>> +++ b/src/qcam/meson.build
> >>> @@ -8,9 +8,12 @@ qt5_dep = dependency('qt5',
> >>>                       version : '>=5.4')
> >>>  
> >>>  if not qt5_dep.found()
> >>> +    qcam_enabled = false
> >>>      subdir_done()
> >>>  endif
> >>>  
> >>> +qcam_enabled = true
> >>> +
> >>>  qcam_sources = files([
> >>>      '../cam/options.cpp',
> >>>      '../cam/stream_options.cpp',
> >>> diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build
> >>> index c8b794247560..0accac194be4 100644
> >>> --- a/src/v4l2/meson.build
> >>> +++ b/src/v4l2/meson.build
> >>> @@ -1,9 +1,12 @@
> >>>  # SPDX-License-Identifier: CC0-1.0
> >>>  
> >>>  if not get_option('v4l2')
> >>> +    v4l2_enabled = false
> >>>      subdir_done()
> >>>  endif
> >>>  
> >>> +v4l2_enabled = true
> >>> +
> >>>  v4l2_compat_sources = files([
> >>>      'v4l2_camera.cpp',
> >>>      'v4l2_camera_file.cpp',

-- 
Regards,

Laurent Pinchart