[PATCH v5 02/15] yaml: Move yaml_parser.cpp to base

[Laurent Pinchart]

On Fri, Dec 06, 2024 at 12:53:43PM +0100, Milan Zamazal wrote:
> Laurent Pinchart writes:
> > On Tue, Oct 01, 2024 at 12:27:54PM +0200, Milan Zamazal wrote:
> >> Global configuration, which uses the YAML parser, will be in base.  Thus
> >> YAML parser must be moved to base too.
> >
> > The global configuration file seems quite specific to libcamera to me.
> > Why do you think it should be in base ? 
> 
> Because logging is in base and uses the configuration.  If logging
> configuration was omitted from the global configuration, the global
> configuration could be put elsewhere and also the annoying chicken-egg
> problem when reading and parsing the configuration while possibly
> logging about it would be avoided.  But from the user's point of view I
> think logging should be configurable in the file.

Thanks for the explanation. I'll have a look at that.

> > Is that maybe because we haven't really defined what base is ?
> >
> >> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> >> ---
> >>  src/libcamera/base/meson.build           | 14 ++++++++++++++
> >>  src/libcamera/{ => base}/yaml_parser.cpp |  0
> >>  src/libcamera/meson.build                | 14 --------------
> >>  3 files changed, 14 insertions(+), 14 deletions(-)
> >>  rename src/libcamera/{ => base}/yaml_parser.cpp (100%)
> >> 
> >> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> >> index a742dfdfe..94843eb95 100644
> >> --- a/src/libcamera/base/meson.build
> >> +++ b/src/libcamera/base/meson.build
> >> @@ -24,10 +24,12 @@ libcamera_base_internal_sources = files([
> >>      'thread.cpp',
> >>      'timer.cpp',
> >>      'utils.cpp',
> >> +    'yaml_parser.cpp',
> >>  ])
> >>  
> >>  libdw = dependency('libdw', required : false)
> >>  libunwind = dependency('libunwind', required : false)
> >> +libyaml = dependency('yaml-0.1', required : false)
> >>  
> >>  if cc.has_header_symbol('execinfo.h', 'backtrace')
> >>      config_h.set('HAVE_BACKTRACE', 1)
> >> @@ -41,11 +43,23 @@ if libunwind.found()
> >>      config_h.set('HAVE_UNWIND', 1)
> >>  endif
> >>  
> >> +# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP.
> >> +if not libyaml.found()
> >> +    cmake = import('cmake')
> >> +
> >> +    libyaml_vars = cmake.subproject_options()
> >> +    libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})
> >> +    libyaml_vars.append_compile_args('c', '-Wno-unused-value')
> >> +    libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars)
> >> +    libyaml = libyaml_wrap.dependency('yaml')
> >> +endif
> >> +
> >>  libcamera_base_deps = [
> >>      libatomic,
> >>      libdw,
> >>      libthreads,
> >>      libunwind,
> >> +    libyaml,
> >>  ]
> >>  
> >>  # Internal components must use the libcamera_base_private dependency to enable
> >> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/base/yaml_parser.cpp
> >> similarity index 100%
> >> rename from src/libcamera/yaml_parser.cpp
> >> rename to src/libcamera/base/yaml_parser.cpp
> >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> >> index aa9ab0291..736cc496f 100644
> >> --- a/src/libcamera/meson.build
> >> +++ b/src/libcamera/meson.build
> >> @@ -51,7 +51,6 @@ libcamera_internal_sources = files([
> >>      'v4l2_pixelformat.cpp',
> >>      'v4l2_subdevice.cpp',
> >>      'v4l2_videodevice.cpp',
> >> -    'yaml_parser.cpp',
> >>  ])
> >>  
> >>  includes = [
> >> @@ -79,7 +78,6 @@ if not cc.has_function('dlopen')
> >>      libdl = cc.find_library('dl')
> >>  endif
> >>  libudev = dependency('libudev', required : get_option('udev'))
> >> -libyaml = dependency('yaml-0.1', required : false)
> >>  
> >>  # Use one of gnutls or libcrypto (provided by OpenSSL), trying gnutls first.
> >>  libcrypto = dependency('gnutls', required : false)
> >> @@ -115,17 +113,6 @@ if libudev.found()
> >>      ])
> >>  endif
> >>  
> >> -# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP.
> >> -if not libyaml.found()
> >> -    cmake = import('cmake')
> >> -
> >> -    libyaml_vars = cmake.subproject_options()
> >> -    libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})
> >> -    libyaml_vars.append_compile_args('c', '-Wno-unused-value')
> >> -    libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars)
> >> -    libyaml = libyaml_wrap.dependency('yaml')
> >> -endif
> >> -
> >>  control_sources = []
> >>  
> >>  controls_mode_files = {
> >> @@ -185,7 +172,6 @@ libcamera_deps += [
> >>      libdl,
> >>      liblttng,
> >>      libudev,
> >> -    libyaml,
> >>  ]
> >>  
> >>  # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.

-- 
Regards,

Laurent Pinchart