<div dir="auto"><div>Hi Laurent</div><div dir="auto"><br></div><div dir="auto">Sorry for HTML reply, but I'm phone only at the moment.</div><div dir="auto"><br><br><div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Fri, 6 Mar 2020, 20:35 Laurent Pinchart, <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Kieran,<br>
<br>
Thank you for the patch.<br>
<br>
On Fri, Mar 06, 2020 at 05:20:30PM +0000, Kieran Bingham wrote:<br>
> The control headers are generated automatically by parsing our YAML<br>
> descriptions, and creating the control headers.<br>
> <br>
> The headers for the controls can be used directly from within libcamera<br>
> internal pipeline handlers and core components, but there is no link to<br>
> ensure that the headers are generated before they are used.<br>
> <br>
> As part of updating controls to support properties, the commit<br>
> f870591a9bf5 ("libcamera: properties: Add location property") also<br>
> inadvertently moved the generated headers out of the libcamera_api<br>
> dependency generation.<br>
> <br>
> This allowed a race condition to occur in builds where objects are<br>
> attempted to be built before the API definitions had been generated.<br>
> <br>
> Declare a dependency on the headers for libcamera to ensure that they<br>
> are built before compiling any object within the libcamera library, and<br>
> re-introduce the headers to the libcamera_api variable.<br>
<br>
This is bundling two changes in one patch, and given the very low<br>
probability of hitting the race, and unsuccessful past attempts to fix<br>
it, I would prefer decoupling the two.<br>
<br>
Would you be fine with the first patch carrying the libcamera_api change<br>
only, with the Fixes: line, and the above paragraph removed, and a<br>
second patch that adds the declary_dependency ? It would allow testing<br>
the two independently.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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 </div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">--</div><div dir="auto">Kieran</div><div dir="auto"><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote" dir="auto"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> Fixes: f870591a9bf5 ("libcamera: properties: Add location property")<br>
> <br>
> Signed-off-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank" rel="noreferrer">kieran.bingham@ideasonboard.com</a>><br>
> ---<br>
>  include/libcamera/meson.build | 2 ++<br>
>  src/libcamera/meson.build     | 1 +<br>
>  2 files changed, 3 insertions(+)<br>
> <br>
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build<br>
> index f47c583cbbc0..88edf620f69e 100644<br>
> --- a/include/libcamera/meson.build<br>
> +++ b/include/libcamera/meson.build<br>
> @@ -44,6 +44,8 @@ foreach header : control_source_files<br>
>                                       install_dir : join_paths('include', include_dir))<br>
>  endforeach<br>
>  <br>
> +libcamera_api += control_headers<br>
> +<br>
>  gen_header = files('gen-header.sh')<br>
>  <br>
>  libcamera_h = custom_target('gen-header',<br>
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build<br>
> index 88658ac563f7..cd95fa11534a 100644<br>
> --- a/src/libcamera/meson.build<br>
> +++ b/src/libcamera/meson.build<br>
> @@ -97,6 +97,7 @@ libcamera_deps = [<br>
>      cc.find_library('dl'),<br>
>      libudev,<br>
>      dependency('threads'),<br>
> +    declare_dependency(sources : [control_headers])<br>
>  ]<br>
>  <br>
>  libcamera_link_with = []<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div></div>