[libcamera-devel] [PATCH] libcamera: Move generated control and properry ids to the API

[Kieran Bingham]

On 02/03/2020 16:38, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Mon, Mar 02, 2020 at 04:35:50PM +0000, Kieran Bingham wrote:
>> As part of updating controls to support properties, the commit
>> f870591a9bf5 ("libcamera: properties: Add location property")
>> 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.
>>
>> Move the headers out of the libcamera_sources variable and into the
>> libcamera_api variable to match the other headers in this location.
>>
>> 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, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
>> index f58c02d2cf35..c02e1d250aff 100644
>> --- a/include/libcamera/meson.build
>> +++ b/include/libcamera/meson.build
>> @@ -43,6 +43,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..b3a6a1aad659 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -80,7 +80,6 @@ foreach source : control_source_files
>>                                       command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])
>>  endforeach
>>  
>> -libcamera_sources += control_headers
>>  libcamera_sources += control_sources
> 
> control_ids_h used to be in both libcamera_api and libcamera_sources. Is
> the latter not needed ?

libcamera_api contains all of the public api headers generated in
include/libcamera/meson.build.

No other header from that list is explicitly added to libcamera_sources,
so, no I do not believe these headers should be added to libcamera_sources.

I've tested that the headers still get installed (because of being in
the libcamera_api variable) ... however I have since hit this race again
with the RKISP1 pipeline handler, thus where I believed this was an
issue of the headers not being in the libcamera_api was wrong.

We likely need to create a separate dependency declaration to make sure
those headers are created early on.

I'll look at this again later.. but for now this patch isn't a solution.


> 
>>  
>>  gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
> 

-- 
Regards
--
Kieran