[libcamera-devel,v3,1/4] meson: libcamera: Split public and internal source arrays

19386 diff mbox series

Meson array variables hold lists of libcamera's source files. To help
facilitate the splitting of Doxygen generated documentation into
distinct public and internal versions, split those arrays to separate
public and internal variables.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v3:

	- Didn't swap span.h to private array
	- dropped _publically from array name

Changes in v2:

	- New patch

 include/libcamera/internal/meson.build | 21 +++++++++++----
 src/libcamera/base/meson.build         | 24 +++++++++++------
 src/libcamera/meson.build              | 36 ++++++++++++++++----------
 3 files changed, 54 insertions(+), 27 deletions(-)

Quoting Daniel Scally via libcamera-devel (2024-01-12 12:14:31)
> Meson array variables hold lists of libcamera's source files. To help
> facilitate the splitting of Doxygen generated documentation into
> distinct public and internal versions, split those arrays to separate
> public and internal variables.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v3:
> 
>         - Didn't swap span.h to private array
>         - dropped _publically from array name
> 
> Changes in v2:
> 
>         - New patch
> 
>  include/libcamera/internal/meson.build | 21 +++++++++++----
>  src/libcamera/base/meson.build         | 24 +++++++++++------
>  src/libcamera/meson.build              | 36 ++++++++++++++++----------
>  3 files changed, 54 insertions(+), 27 deletions(-)
> 
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 7f1f3440..4e36bf36 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -9,13 +9,21 @@ libcamera_tracepoint_header = custom_target(
>      command : [gen_tracepoints_header, include_build_dir, '@OUTPUT@', '@INPUT@'],
>  )
>  
> -libcamera_internal_headers = files([
> +# Where libcamera's public API classes have Extensible::Private counterparts we
> +# need to include them in doxygen's public API run or it will complain that the
> +# functions defined in the "public" source are undeclared.
> +libcamera_internal_headers_documented = files([
> +    'camera.h',
> +    'camera_manager.h',
> +    'framebuffer.h',
> +    'request.h',
> +])

I guess there's no way to 'exclude' *::private::* namespaces ?

Or Could this be handled by extending

Documentation/Doxyfile.in:EXCLUDE_SYMBOLS = *::private::*

?

Or I wonder if we should mark Extensible::Private components with

	/// \private

https://www.doxygen.nl/manual/commands.html#cmdprivate


> +
> +libcamera_internal_headers_undocumented = files([
>      'bayer_format.h',
>      'byte_stream_buffer.h',
> -    'camera.h',
>      'camera_controls.h',
>      'camera_lens.h',
> -    'camera_manager.h',
>      'camera_sensor.h',
>      'camera_sensor_properties.h',
>      'control_serializer.h',
> @@ -26,7 +34,6 @@ libcamera_internal_headers = files([
>      'device_enumerator_sysfs.h',
>      'device_enumerator_udev.h',
>      'formats.h',
> -    'framebuffer.h',
>      'ipa_manager.h',
>      'ipa_module.h',
>      'ipa_proxy.h',
> @@ -37,7 +44,6 @@ libcamera_internal_headers = files([
>      'pipeline_handler.h',
>      'process.h',
>      'pub_key.h',
> -    'request.h',
>      'source_paths.h',
>      'sysfs.h',
>      'v4l2_device.h',
> @@ -47,4 +53,9 @@ libcamera_internal_headers = files([
>      'yaml_parser.h',
>  ])
>  
> +libcamera_internal_headers = [
> +    libcamera_internal_headers_documented,
> +    libcamera_internal_headers_undocumented
> +]
> +
>  subdir('converter')
> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> index 7a7fd7e4..523c5885 100644
> --- a/src/libcamera/base/meson.build
> +++ b/src/libcamera/base/meson.build
> @@ -1,27 +1,35 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
> -libcamera_base_sources = files([
> -    'backtrace.cpp',
> -    'class.cpp',
> +libcamera_base_public_sources = files([
>      'bound_method.cpp',
> +    'class.cpp',
> +    'flags.cpp',
> +    'object.cpp',
> +    'shared_fd.cpp',
> +    'signal.cpp',
> +    'unique_fd.cpp',
> +])
> +
> +libcamera_base_internal_sources = files([
> +    'backtrace.cpp',
>      'event_dispatcher.cpp',
>      'event_dispatcher_poll.cpp',
>      'event_notifier.cpp',
>      'file.cpp',
> -    'flags.cpp',
>      'log.cpp',
>      'message.cpp',
>      'mutex.cpp',
> -    'object.cpp',
>      'semaphore.cpp',
> -    'shared_fd.cpp',
> -    'signal.cpp',
>      'thread.cpp',
>      'timer.cpp',
> -    'unique_fd.cpp',
>      'utils.cpp',
>  ])
>  
> +libcamera_base_sources = [
> +       libcamera_base_public_sources,
> +       libcamera_base_internal_sources
> +]
> +
>  libdw = dependency('libdw', required : false)
>  libunwind = dependency('libunwind', required : false)
>  
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 45f63e93..676470c1 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -1,27 +1,35 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
> -libcamera_sources = files([
> +libcamera_public_sources = files([
> +    'camera.cpp',
> +    'camera_manager.cpp',
> +    'color_space.cpp',
> +    'controls.cpp',
> +    'fence.cpp',
> +    'framebuffer.cpp',
> +    'framebuffer_allocator.cpp',
> +    'geometry.cpp',
> +    'orientation.cpp',
> +    'pixel_format.cpp',
> +    'request.cpp',
> +    'stream.cpp',
> +    'transform.cpp',
> +])
> +
> +libcamera_internal_sources = files([
>      'bayer_format.cpp',
>      'byte_stream_buffer.cpp',
> -    'camera.cpp',
>      'camera_controls.cpp',
>      'camera_lens.cpp',
> -    'camera_manager.cpp',
>      'camera_sensor.cpp',
>      'camera_sensor_properties.cpp',
> -    'color_space.cpp',
> -    'controls.cpp',
>      'control_serializer.cpp',
>      'control_validator.cpp',
>      'converter.cpp',
>      'delayed_controls.cpp',
>      'device_enumerator.cpp',
>      'device_enumerator_sysfs.cpp',
> -    'fence.cpp',
>      'formats.cpp',
> -    'framebuffer.cpp',
> -    'framebuffer_allocator.cpp',
> -    'geometry.cpp',
>      'ipa_controls.cpp',
>      'ipa_data_serializer.cpp',
>      'ipa_interface.cpp',
> @@ -34,16 +42,11 @@ libcamera_sources = files([
>      'mapped_framebuffer.cpp',
>      'media_device.cpp',
>      'media_object.cpp',
> -    'orientation.cpp',
>      'pipeline_handler.cpp',
> -    'pixel_format.cpp',
>      'process.cpp',
>      'pub_key.cpp',
> -    'request.cpp',
>      'source_paths.cpp',
> -    'stream.cpp',
>      'sysfs.cpp',
> -    'transform.cpp',
>      'v4l2_device.cpp',
>      'v4l2_pixelformat.cpp',
>      'v4l2_subdevice.cpp',
> @@ -51,6 +54,11 @@ libcamera_sources = files([
>      'yaml_parser.cpp',
>  ])
>  
> +libcamera_sources = [
> +       libcamera_public_sources,
> +       libcamera_internal_sources
> +]
> +
>  libcamera_sources += libcamera_public_headers
>  libcamera_sources += libcamera_generated_ipa_headers
>  libcamera_sources += libcamera_tracepoint_header
> -- 
> 2.34.1
>

Hello,

On Thu, Jan 18, 2024 at 10:21:06AM +0000, 📷-devel wrote:
> Quoting Daniel Scally via libcamera-devel (2024-01-12 12:14:31)
> > Meson array variables hold lists of libcamera's source files. To help
> > facilitate the splitting of Doxygen generated documentation into
> > distinct public and internal versions, split those arrays to separate
> > public and internal variables.
> > 
> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > ---
> > Changes in v3:
> > 
> >         - Didn't swap span.h to private array
> >         - dropped _publically from array name
> > 
> > Changes in v2:
> > 
> >         - New patch
> > 
> >  include/libcamera/internal/meson.build | 21 +++++++++++----
> >  src/libcamera/base/meson.build         | 24 +++++++++++------
> >  src/libcamera/meson.build              | 36 ++++++++++++++++----------
> >  3 files changed, 54 insertions(+), 27 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> > index 7f1f3440..4e36bf36 100644
> > --- a/include/libcamera/internal/meson.build
> > +++ b/include/libcamera/internal/meson.build
> > @@ -9,13 +9,21 @@ libcamera_tracepoint_header = custom_target(
> >      command : [gen_tracepoints_header, include_build_dir, '@OUTPUT@', '@INPUT@'],
> >  )
> >  
> > -libcamera_internal_headers = files([
> > +# Where libcamera's public API classes have Extensible::Private counterparts we
> > +# need to include them in doxygen's public API run or it will complain that the
> > +# functions defined in the "public" source are undeclared.
> > +libcamera_internal_headers_documented = files([
> > +    'camera.h',
> > +    'camera_manager.h',
> > +    'framebuffer.h',
> > +    'request.h',
> > +])
> 
> I guess there's no way to 'exclude' *::private::* namespaces ?

It's not a namespace, it's a nested class declaration (and it's Private,
not private).

> 
> Or Could this be handled by extending
> 
> Documentation/Doxyfile.in:EXCLUDE_SYMBOLS = *::private::*
> 
> ?

This may be possible, depending on what we want to achieve. I'm not sure
yet why we need to split these in "documented" and "undocumented" parts,
I suppose I'll figure that out when reviewing the other patches in this
series.

> Or I wonder if we should mark Extensible::Private components with
> 
> 	/// \private
> 
> https://www.doxygen.nl/manual/commands.html#cmdprivate

Extensible::Private really means internal, not private in the C++ sense.
Those classes and their (public) member functions need to be documented,
as they're part of the internal API.

> > +
> > +libcamera_internal_headers_undocumented = files([
> >      'bayer_format.h',
> >      'byte_stream_buffer.h',
> > -    'camera.h',
> >      'camera_controls.h',
> >      'camera_lens.h',
> > -    'camera_manager.h',
> >      'camera_sensor.h',
> >      'camera_sensor_properties.h',
> >      'control_serializer.h',
> > @@ -26,7 +34,6 @@ libcamera_internal_headers = files([
> >      'device_enumerator_sysfs.h',
> >      'device_enumerator_udev.h',
> >      'formats.h',
> > -    'framebuffer.h',
> >      'ipa_manager.h',
> >      'ipa_module.h',
> >      'ipa_proxy.h',
> > @@ -37,7 +44,6 @@ libcamera_internal_headers = files([
> >      'pipeline_handler.h',
> >      'process.h',
> >      'pub_key.h',
> > -    'request.h',
> >      'source_paths.h',
> >      'sysfs.h',
> >      'v4l2_device.h',
> > @@ -47,4 +53,9 @@ libcamera_internal_headers = files([
> >      'yaml_parser.h',
> >  ])
> >  
> > +libcamera_internal_headers = [
> > +    libcamera_internal_headers_documented,
> > +    libcamera_internal_headers_undocumented

s/$/,/

The split looks good, with this small issue fixed (and assuming we need
this particular split, I haven't read patches 2/4 to 4/4 yet),

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > +]
> > +
> >  subdir('converter')
> > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> > index 7a7fd7e4..523c5885 100644
> > --- a/src/libcamera/base/meson.build
> > +++ b/src/libcamera/base/meson.build
> > @@ -1,27 +1,35 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >  
> > -libcamera_base_sources = files([
> > -    'backtrace.cpp',
> > -    'class.cpp',
> > +libcamera_base_public_sources = files([
> >      'bound_method.cpp',
> > +    'class.cpp',
> > +    'flags.cpp',
> > +    'object.cpp',
> > +    'shared_fd.cpp',
> > +    'signal.cpp',
> > +    'unique_fd.cpp',
> > +])
> > +
> > +libcamera_base_internal_sources = files([
> > +    'backtrace.cpp',
> >      'event_dispatcher.cpp',
> >      'event_dispatcher_poll.cpp',
> >      'event_notifier.cpp',
> >      'file.cpp',
> > -    'flags.cpp',
> >      'log.cpp',
> >      'message.cpp',
> >      'mutex.cpp',
> > -    'object.cpp',
> >      'semaphore.cpp',
> > -    'shared_fd.cpp',
> > -    'signal.cpp',
> >      'thread.cpp',
> >      'timer.cpp',
> > -    'unique_fd.cpp',
> >      'utils.cpp',
> >  ])
> >  
> > +libcamera_base_sources = [
> > +       libcamera_base_public_sources,
> > +       libcamera_base_internal_sources
> > +]
> > +
> >  libdw = dependency('libdw', required : false)
> >  libunwind = dependency('libunwind', required : false)
> >  
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 45f63e93..676470c1 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -1,27 +1,35 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >  
> > -libcamera_sources = files([
> > +libcamera_public_sources = files([
> > +    'camera.cpp',
> > +    'camera_manager.cpp',
> > +    'color_space.cpp',
> > +    'controls.cpp',
> > +    'fence.cpp',
> > +    'framebuffer.cpp',
> > +    'framebuffer_allocator.cpp',
> > +    'geometry.cpp',
> > +    'orientation.cpp',
> > +    'pixel_format.cpp',
> > +    'request.cpp',
> > +    'stream.cpp',
> > +    'transform.cpp',
> > +])
> > +
> > +libcamera_internal_sources = files([
> >      'bayer_format.cpp',
> >      'byte_stream_buffer.cpp',
> > -    'camera.cpp',
> >      'camera_controls.cpp',
> >      'camera_lens.cpp',
> > -    'camera_manager.cpp',
> >      'camera_sensor.cpp',
> >      'camera_sensor_properties.cpp',
> > -    'color_space.cpp',
> > -    'controls.cpp',
> >      'control_serializer.cpp',
> >      'control_validator.cpp',
> >      'converter.cpp',
> >      'delayed_controls.cpp',
> >      'device_enumerator.cpp',
> >      'device_enumerator_sysfs.cpp',
> > -    'fence.cpp',
> >      'formats.cpp',
> > -    'framebuffer.cpp',
> > -    'framebuffer_allocator.cpp',
> > -    'geometry.cpp',
> >      'ipa_controls.cpp',
> >      'ipa_data_serializer.cpp',
> >      'ipa_interface.cpp',
> > @@ -34,16 +42,11 @@ libcamera_sources = files([
> >      'mapped_framebuffer.cpp',
> >      'media_device.cpp',
> >      'media_object.cpp',
> > -    'orientation.cpp',
> >      'pipeline_handler.cpp',
> > -    'pixel_format.cpp',
> >      'process.cpp',
> >      'pub_key.cpp',
> > -    'request.cpp',
> >      'source_paths.cpp',
> > -    'stream.cpp',
> >      'sysfs.cpp',
> > -    'transform.cpp',
> >      'v4l2_device.cpp',
> >      'v4l2_pixelformat.cpp',
> >      'v4l2_subdevice.cpp',
> > @@ -51,6 +54,11 @@ libcamera_sources = files([
> >      'yaml_parser.cpp',
> >  ])
> >  
> > +libcamera_sources = [
> > +       libcamera_public_sources,
> > +       libcamera_internal_sources
> > +]
> > +
> >  libcamera_sources += libcamera_public_headers
> >  libcamera_sources += libcamera_generated_ipa_headers
> >  libcamera_sources += libcamera_tracepoint_header