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

[Jacopo Mondi]

Hi Kieran,

On Thu, Sep 24, 2020 at 04:12:07PM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 24/09/2020 16:12, Jacopo Mondi wrote:
> > Hi Laurent,
> >
> > On Thu, Sep 24, 2020 at 04:12:30PM +0300, Laurent Pinchart wrote:
> >> On Thu, Sep 24, 2020 at 02:35:45PM +0200, Niklas Söderlund wrote:
> >>> On 2020-09-24 12:27:35 +0200, Jacopo Mondi wrote:
> >>>> 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,
> >>
> >> Before looking at the implementation, let's agree on the desired
> >> behaviour. When a dependency needed by an IPA module isn't found, and
> >> the corresponding pipeline handler is enabled, should we
> >>
> >> 1. Make configuration fail (that's the current behaviour)
> >
> > That would be my preference as
>
>
> You're in luck.
> That's what you currently have ...

Ah sorry, I completely missed that

>
> My issue with this is that if I have a system which does not have boost,
> and is not a Raspberry Pi - *by default* libcamera will fail to build -
> because RPi is enabled by default.
>

I see, I defer any decision to you build system educated people then.