[libcamera-devel] [PATCH v2] libcamera: Declare dependency on generated headers

[Kieran Bingham]

Hi Laurent

Sorry for HTML reply, but I'm phone only at the moment.


On Fri, 6 Mar 2020, 20:35 Laurent Pinchart, <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Kieran,
>
> Thank you for the patch.
>
> On Fri, Mar 06, 2020 at 05:20:30PM +0000, Kieran Bingham wrote:
> > The control headers are generated automatically by parsing our YAML
> > descriptions, and creating the control headers.
> >
> > The headers for the controls can be used directly from within libcamera
> > internal pipeline handlers and core components, but there is no link to
> > ensure that the headers are generated before they are used.
> >
> > As part of updating controls to support properties, the commit
> > f870591a9bf5 ("libcamera: properties: Add location property") also
> > inadvertently moved the generated headers out of the libcamera_api
> > dependency generation.
> >
> > This allowed a race condition to occur in builds where objects are
> > attempted to be built before the API definitions had been generated.
> >
> > Declare a dependency on the headers for libcamera to ensure that they
> > are built before compiling any object within the libcamera library, and
> > re-introduce the headers to the libcamera_api variable.
>
> This is bundling two changes in one patch, and given the very low
> probability of hitting the race, and unsuccessful past attempts to fix
> it, I would prefer decoupling the two.
>
> Would you be fine with the first patch carrying the libcamera_api change
> only, with the Fixes: line, and the above paragraph removed, and a
> second patch that adds the declary_dependency ? It would allow testing
> the two independently.
>

Decouple as you wish, but they are both required, and in fact both a high
probability of hitting them as I have a build condition (my gitlab ci)
which hits both reliably and continuously, and thus neither can pass a
build without each other, which is why both additions are in this patch

Interestingly on my automated builds, the tests get built before the
library which is the requirement for the libcamera_api addition, and the
pipeline handlers otherwise get built before the headers.

--
Kieran



> > Fixes: f870591a9bf5 ("libcamera: properties: Add location property")
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  include/libcamera/meson.build | 2 ++
> >  src/libcamera/meson.build     | 1 +
> >  2 files changed, 3 insertions(+)
> >
> > diff --git a/include/libcamera/meson.build
> b/include/libcamera/meson.build
> > index f47c583cbbc0..88edf620f69e 100644
> > --- a/include/libcamera/meson.build
> > +++ b/include/libcamera/meson.build
> > @@ -44,6 +44,8 @@ foreach header : control_source_files
> >                                       install_dir :
> join_paths('include', include_dir))
> >  endforeach
> >
> > +libcamera_api += control_headers
> > +
> >  gen_header = files('gen-header.sh')
> >
> >  libcamera_h = custom_target('gen-header',
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 88658ac563f7..cd95fa11534a 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -97,6 +97,7 @@ libcamera_deps = [
> >      cc.find_library('dl'),
> >      libudev,
> >      dependency('threads'),
> > +    declare_dependency(sources : [control_headers])
> >  ]
> >
> >  libcamera_link_with = []
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200307/0ad07287/attachment.htm>