[libcamera-devel] [RFC PATCH] ipa: rpi: Make boost optional

[Niklas Söderlund]

Hi Kieran,

On 2020-09-24 12:27:35 +0200, Jacopo Mondi wrote:
> Hi Kieran
> 
> On Thu, Sep 24, 2020 at 11:02:01AM +0100, Kieran Bingham wrote:
> > By default the Raspberry Pi pipeline handler is enabled when
> > configuring the build.
> >
> > The Raspberry Pi IPA currently depends upon 'boost' which is a large
> > dependency to bring in.
> >
> > Make the IPA compilation dependant upon the availabilty of the boost
> > library, but display a warning when the pipeline is enabled and the
> > dependency is not found.
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >
> > This is mostly posted to promote discussion on this topic.
> >
> > The requirement for boost is really heavy (+100MB package for a few
> > headers), and is only used for the RPi IPA.
> >
> > This patch automatically disables the IPA while printing a message if it
> > was selected if the boost library is not available.
> >
> > Doing all of this in a clean way seems quite difficult because of the
> > way the meson options works ... so ...
> >
> > 3 ... 2 ... 1 ....
> >
> >    Discuss...
> 
> I really like the idea of depending on boost only if RPi is enabled.

+1
 
> 
> >
> >
> >  src/ipa/raspberrypi/meson.build | 40 +++++++++++++++++++--------------
> >  1 file changed, 23 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> > index 9445cd097df5..b284378c9621 100644
> > --- a/src/ipa/raspberrypi/meson.build
> > +++ b/src/ipa/raspberrypi/meson.build
> > @@ -2,9 +2,11 @@
> >
> >  ipa_name = 'ipa_rpi'
> >
> > +boost = dependency('boost', required : false)
> > +
> >  rpi_ipa_deps = [
> >      libcamera_dep,
> > -    dependency('boost'),
> > +    boost,
> >      libatomic,
> >  ]
> >
> > @@ -41,22 +43,26 @@ rpi_ipa_sources = files([
> >      'controller/pwl.cpp',
> >  ])
> >
> > -mod = shared_module(ipa_name,
> > -                    rpi_ipa_sources,
> > -                    name_prefix : '',
> > -                    include_directories : rpi_ipa_includes,
> > -                    dependencies : rpi_ipa_deps,
> > -                    link_with : libipa,
> > -                    install : true,
> > -                    install_dir : ipa_install_dir)
> > -
> > -if ipa_sign_module
> > -    custom_target(ipa_name + '.so.sign',
> > -                  input : mod,
> > -                  output : ipa_name + '.so.sign',
> > -                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],
> > -                  install : false,
> > -                  build_by_default : true)
> > +if boost.found()
> > +    mod = shared_module(ipa_name,
> > +                        rpi_ipa_sources,
> > +                        name_prefix : '',
> > +                        include_directories : rpi_ipa_includes,
> > +                        dependencies : rpi_ipa_deps,
> > +                        link_with : libipa,
> > +                        install : true,
> > +                        install_dir : ipa_install_dir)
> > +
> > +    if ipa_sign_module
> > +        custom_target(ipa_name + '.so.sign',
> > +                      input : mod,
> > +                      output : ipa_name + '.so.sign',
> > +                      command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],
> > +                      install : false,
> > +                      build_by_default : true)
> > +    endif
> > +else
> > +    warning('The Raspberry Pi pipeline is enabled, but dependency "boost" was not found')
> 
> 
> I know nothing about meson, but can we have a contruct that
>         if !boost.found
>                 error "..."
> 
> and the build fails ?

I agree with Jacopo here. I think the build should fail if the RPi 
pipeline is enabled and the dependencies for it can not be found. I 
think fail early is good, otherwise someone may enable the RPi pipeline 
have the compilation and installation succeed only to later find out 
their $APP won't work as it can't find the IPA module.

Having it fail with an error(Friendly message) is also nicer then having 
the compilation fail. So with s/warning(/error(/ above,

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> 
> >  endif
> >
> >  subdir('data')
> > --
> > 2.25.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund