[libcamera-devel] [RFC PATCH] ipa: meson: Install mojom generated headers to include paths

[Umang Jain]

Hi Kieran,

On 4/28/21 7:49 PM, Kieran Bingham wrote:
> Hi Umang,
>
> On 28/04/2021 14:33, Umang Jain wrote:
>> Generated IPA headers from mojom files need to be installed sy
>> $INCLUDE_PATH in order to be available system-wide. Without this,
>> out-of-tree IPAs won't be able to link and build themselves.
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>> Marked as RFC to discuss:
>> - Whether it makes sense to provide -ipa_serializer.h
>>    and -ipa_proxy.h system-wide too? I am not 100%s sure yet.
> I don't think it makes sense to install the serializer and proxy
> headers. Those are used as part of the IPC internals I believe.
>
>
>
>> ---
>>   include/libcamera/ipa/meson.build | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
>> index 2d72c1fc..096bd4dd 100644
>> --- a/include/libcamera/ipa/meson.build
>> +++ b/include/libcamera/ipa/meson.build
>> @@ -11,6 +11,8 @@ install_headers(libcamera_ipa_headers,
>>   
>>   libcamera_generated_ipa_headers = []
>>   
>> +generated_ipa_headers_include_path = join_paths(get_option('prefix'),'include',
>> +                                                libcamera_include_dir, 'ipa')
> Where possible, this should be factored out so the common parts are
> shared with the installation of the libcamera_ipa_headers.
>
> However that uses install_headers() which we can't use directly on the
> custom_target it seems.
>
> I would create at the top of the file
>
> libcamera_ipa_include_dir = libcamera_include_dir / 'ipa'
>
> and use that for the install_headers.
>
> Somewhat frustratingly we likely then have to use the following for the
> install_dir, as 'install_headers' would otherwise have prefixed 'include':
>
> 	install : true,
> 	install_dir : 'include' / libcamera_ipa_include_dir
We need to add prefix too, no? I am sending a new RFC soon, please take 
a look if that looks on the right path
>
>
>>   #
>>   # Prepare IPA/IPC generation components
>>   #
>> @@ -31,6 +33,8 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_interface_h',
>>                     input : ipa_mojom_core,
>>                     output : 'core_ipa_interface.h',
>>                     depends : mojom_templates,
>> +                  install: true,
>> +                  install_dir: generated_ipa_headers_include_path,
>>                     command : [
>>                         mojom_generator, 'generate',
>>                         '-g', 'libcamera',
>> @@ -45,6 +49,8 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
>>                     input : ipa_mojom_core,
>>                     output : 'core_ipa_serializer.h',
>>                     depends : mojom_templates,
>> +                  install: true,
>> +                  install_dir: generated_ipa_headers_include_path,
>>                     command : [
>>                         mojom_generator, 'generate',
>>                         '-g', 'libcamera',
>> @@ -93,6 +99,8 @@ foreach file : ipa_mojom_files
>>                              input : mojom,
>>                              output : name + '_ipa_interface.h',
>>                              depends : mojom_templates,
>> +                           install: true,
>> +                           install_dir: generated_ipa_headers_include_path,
>
> Make sure you have a space around both sides of the ':' to match the
> existing styles.
>
> Presumably, this is the only header that needs to actually be installed now?
> But it will need checking.
>
> --
> Kieran
>
>
>>                              command : [
>>                                  mojom_generator, 'generate',
>>                                  '-g', 'libcamera',
>> @@ -107,6 +115,8 @@ foreach file : ipa_mojom_files
>>                                  input : mojom,
>>                                  output : name + '_ipa_serializer.h',
>>                                  depends : mojom_templates,
>> +                               install: true,
>> +                               install_dir: generated_ipa_headers_include_path,
>>                                  command : [
>>                                      mojom_generator, 'generate',
>>                                      '-g', 'libcamera',
>> @@ -121,6 +131,8 @@ foreach file : ipa_mojom_files
>>                                    input : mojom,
>>                                    output : name + '_ipa_proxy.h',
>>                                    depends : mojom_templates,
>> +                                 install: true,
>> +                                 install_dir: generated_ipa_headers_include_path,
>>                                    command : [
>>                                        mojom_generator, 'generate',
>>                                        '-g', 'libcamera',
>>