[libcamera-devel] [PATCH v1 2/2] libcamera: Simplify header install paths with shortcut variables

[Kieran Bingham]

On 28/09/2021 18:12, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Tue, Sep 28, 2021 at 05:55:05PM +0100, Kieran Bingham wrote:
>> On Sun, Sep 26, 2021 at 07:45:54PM +0300, Laurent Pinchart wrote:
>>> Create local install_dir meson variable to store the full path to the
>>> installation directory for the libcamera and ipa headers. This shortens
>>> lines and avoids duplicating calls to get_option('includedir').
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> ---
>>>  include/libcamera/ipa/meson.build |  6 ++++--
>>>  include/libcamera/meson.build     | 10 ++++++----
>>>  2 files changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
>>> index 81fb69f0b681..c36a6fadc3f1 100644
>>> --- a/include/libcamera/ipa/meson.build
>>> +++ b/include/libcamera/ipa/meson.build
>>> @@ -13,6 +13,8 @@ install_headers(libcamera_ipa_headers,
>>>  
>>>  libcamera_generated_ipa_headers = []
>>>  
>>> +install_dir = get_option('includedir') / libcamera_ipa_include_dir
>>> +
>>
>> I'm pretty sure I tried to do something like this, and you called it out
>> as a potential namespace conflict against the meson variables.
>>
>> Meson doesn't have local namespaces though so that install_dir becomes
>> 'globally' accessible through out meson.
>>
>> Should it be ipa_install_dir?
>>
>> Otherwise perhaps at least a comment above it saying this variable is
>> local to this meson.build?
> 
> The lack of local variables is indeed a bit annoying. I've essentially
> treated install_dir as a local variable here and in
> include/libcamera/meson.build. If we wanted to avoid using the same
> name, they should be named ipa_headers_install_dir and
> libcamera_headers_install_dir. Would you prefer that, or comments here
> and below to state those variables are local ?
> 
> # include_dir is a local variable

Either of those options is fine with me.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

--
Kieran



> 
>>>  #
>>>  # Prepare IPA/IPC generation components
>>>  #
>>> @@ -34,7 +36,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_interface_h',
>>>                    output : 'core_ipa_interface.h',
>>>                    depends : mojom_templates,
>>>                    install : true,
>>> -                  install_dir : get_option('includedir') / libcamera_ipa_include_dir,
>>> +                  install_dir : install_dir,
>>>                    command : [
>>>                        mojom_generator, 'generate',
>>>                        '-g', 'libcamera',
>>> @@ -98,7 +100,7 @@ foreach file : ipa_mojom_files
>>>                             output : name + '_ipa_interface.h',
>>>                             depends : mojom_templates,
>>>                             install : true,
>>> -                           install_dir : get_option('includedir') / libcamera_ipa_include_dir,
>>> +                           install_dir : install_dir,
>>>                             command : [
>>>                                 mojom_generator, 'generate',
>>>                                 '-g', 'libcamera',
>>> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
>>> index 567782a66ba3..c5a639d50356 100644
>>> --- a/include/libcamera/meson.build
>>> +++ b/include/libcamera/meson.build
>>> @@ -29,6 +29,8 @@ install_headers(libcamera_public_headers,
>>>  # Generate headers from templates.
>>>  #
>>>  
>>> +install_dir = get_option('includedir') / libcamera_include_dir
>>> +
>>>  # control_ids.h and property_ids.h
>>>  control_source_files = [
>>>      'control_ids',
>>> @@ -44,7 +46,7 @@ foreach header : control_source_files
>>>                                       output : header + '.h',
>>>                                       command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],
>>>                                       install : true,
>>> -                                     install_dir : get_option('includedir') / libcamera_include_dir)
>>> +                                     install_dir : install_dir)
>>>  endforeach
>>>  
>>>  libcamera_public_headers += control_headers
>>> @@ -59,7 +61,7 @@ formats_h = custom_target('formats_h',
>>>                            output : 'formats.h',
>>>                            command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'],
>>>                            install : true,
>>> -                          install_dir : get_option('includedir') / libcamera_include_dir)
>>> +                          install_dir : install_dir)
>>>  libcamera_public_headers += formats_h
>>>  
>>>  # libcamera.h
>>> @@ -68,7 +70,7 @@ libcamera_h = custom_target('gen-header',
>>>                              output : 'libcamera.h',
>>>                              command : [gen_header, meson.current_source_dir(), '@OUTPUT@'],
>>>                              install : true,
>>> -                            install_dir : get_option('includedir') / libcamera_include_dir)
>>> +                            install_dir : install_dir)
>>>  
>>>  libcamera_public_headers += libcamera_h
>>>  
>>> @@ -82,4 +84,4 @@ libcamera_version_config.set('LIBCAMERA_VERSION_PATCH', version[2])
>>>  configure_file(input : 'version.h.in',
>>>                 output : 'version.h',
>>>                 configuration : libcamera_version_config,
>>> -               install_dir : get_option('includedir') / libcamera_include_dir)
>>> +               install_dir : install_dir)
>