| Message ID | 20210926164554.19325-1-laurent.pinchart@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Sun, Sep 26, 2021 at 07:45:53PM +0300, Laurent Pinchart wrote: > All libcamera headers are meant to be installed in > ${prefix}/include/libcamera/libcamera, with pkgconfig specifying the > include directory as `-I ${prefix}/include/libcamera`. Applications then > include the headers with `#include <libcamera/camera.h>`. > > The base and ipa headers are meant to be installed in subdirectories of > the libcamera headers directory, but are mistakenly installed one level > too high, in ${prefix}/include/libcamera. This leads to compilation > failures when including the base or ipa header. > > Fix this by setting the meson libcamera_include_dir variable to the > libcamera headers directory. All other header paths are derived from > that variable and are now correct. > > Reported-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=79 > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/meson.build | 14 +++++++------- > include/meson.build | 2 -- > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 5b25ef847ed4..567782a66ba3 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -1,5 +1,7 @@ > # SPDX-License-Identifier: CC0-1.0 > > +libcamera_include_dir = 'libcamera' / 'libcamera' > + > libcamera_public_headers = files([ > 'camera.h', > 'camera_manager.h', > @@ -16,14 +18,12 @@ libcamera_public_headers = files([ > 'transform.h', > ]) > > -include_dir = libcamera_include_dir / 'libcamera' Am I missing something here? The old version has libcamera_include_dir = 'libcamera', so the subdir that we feed below is 'libcamera' / 'libcamera'. The new version has 'libcamera' / 'libcamera' directly, so it seems to me nothing has changed? Paul > - > subdir('base') > subdir('internal') > subdir('ipa') > > install_headers(libcamera_public_headers, > - subdir : include_dir) > + subdir : libcamera_include_dir) > > # > # Generate headers from templates. > @@ -44,7 +44,7 @@ foreach header : control_source_files > output : header + '.h', > command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > - install_dir : get_option('includedir') / include_dir) > + install_dir : get_option('includedir') / libcamera_include_dir) > endforeach > > libcamera_public_headers += control_headers > @@ -59,7 +59,7 @@ formats_h = custom_target('formats_h', > output : 'formats.h', > command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > - install_dir : get_option('includedir') / include_dir) > + install_dir : get_option('includedir') / libcamera_include_dir) > libcamera_public_headers += formats_h > > # libcamera.h > @@ -68,7 +68,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') / include_dir) > + install_dir : get_option('includedir') / libcamera_include_dir) > > libcamera_public_headers += libcamera_h > > @@ -82,4 +82,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') / include_dir) > + install_dir : get_option('includedir') / libcamera_include_dir) > diff --git a/include/meson.build b/include/meson.build > index 2ac9a3a049f2..27ce2f41c534 100644 > --- a/include/meson.build > +++ b/include/meson.build > @@ -1,6 +1,4 @@ > # SPDX-License-Identifier: CC0-1.0 > > -libcamera_include_dir = 'libcamera' > - > subdir('android') > subdir('libcamera') > -- > Regards, > > Laurent Pinchart >
Hi Paul, On Mon, Sep 27, 2021 at 03:46:05PM +0900, paul.elder@ideasonboard.com wrote: > On Sun, Sep 26, 2021 at 07:45:53PM +0300, Laurent Pinchart wrote: > > All libcamera headers are meant to be installed in > > ${prefix}/include/libcamera/libcamera, with pkgconfig specifying the > > include directory as `-I ${prefix}/include/libcamera`. Applications then > > include the headers with `#include <libcamera/camera.h>`. > > > > The base and ipa headers are meant to be installed in subdirectories of > > the libcamera headers directory, but are mistakenly installed one level > > too high, in ${prefix}/include/libcamera. This leads to compilation > > failures when including the base or ipa header. > > > > Fix this by setting the meson libcamera_include_dir variable to the > > libcamera headers directory. All other header paths are derived from > > that variable and are now correct. > > > > Reported-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=79 > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/meson.build | 14 +++++++------- > > include/meson.build | 2 -- > > 2 files changed, 7 insertions(+), 9 deletions(-) > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > index 5b25ef847ed4..567782a66ba3 100644 > > --- a/include/libcamera/meson.build > > +++ b/include/libcamera/meson.build > > @@ -1,5 +1,7 @@ > > # SPDX-License-Identifier: CC0-1.0 > > > > +libcamera_include_dir = 'libcamera' / 'libcamera' > > + > > libcamera_public_headers = files([ > > 'camera.h', > > 'camera_manager.h', > > @@ -16,14 +18,12 @@ libcamera_public_headers = files([ > > 'transform.h', > > ]) > > > > -include_dir = libcamera_include_dir / 'libcamera' > > Am I missing something here? > > The old version has libcamera_include_dir = 'libcamera', so the subdir > that we feed below is 'libcamera' / 'libcamera'. > > The new version has 'libcamera' / 'libcamera' directly, so it seems to > me nothing has changed? In include/libcamera/, indeed, but the base/ and ipa/ subdirectories use the libcamera_include_dir variable, which is now 'libcamera/libcamera' when it was just 'libcamera'. > > - > > subdir('base') > > subdir('internal') > > subdir('ipa') > > > > install_headers(libcamera_public_headers, > > - subdir : include_dir) > > + subdir : libcamera_include_dir) > > > > # > > # Generate headers from templates. > > @@ -44,7 +44,7 @@ foreach header : control_source_files > > output : header + '.h', > > command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > > install : true, > > - install_dir : get_option('includedir') / include_dir) > > + install_dir : get_option('includedir') / libcamera_include_dir) > > endforeach > > > > libcamera_public_headers += control_headers > > @@ -59,7 +59,7 @@ formats_h = custom_target('formats_h', > > output : 'formats.h', > > command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'], > > install : true, > > - install_dir : get_option('includedir') / include_dir) > > + install_dir : get_option('includedir') / libcamera_include_dir) > > libcamera_public_headers += formats_h > > > > # libcamera.h > > @@ -68,7 +68,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') / include_dir) > > + install_dir : get_option('includedir') / libcamera_include_dir) > > > > libcamera_public_headers += libcamera_h > > > > @@ -82,4 +82,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') / include_dir) > > + install_dir : get_option('includedir') / libcamera_include_dir) > > diff --git a/include/meson.build b/include/meson.build > > index 2ac9a3a049f2..27ce2f41c534 100644 > > --- a/include/meson.build > > +++ b/include/meson.build > > @@ -1,6 +1,4 @@ > > # SPDX-License-Identifier: CC0-1.0 > > > > -libcamera_include_dir = 'libcamera' > > - > > subdir('android') > > subdir('libcamera')
Hi Laurent, On Mon, Sep 27, 2021 at 06:41:54PM +0300, Laurent Pinchart wrote: > Hi Paul, > > On Mon, Sep 27, 2021 at 03:46:05PM +0900, paul.elder@ideasonboard.com wrote: > > On Sun, Sep 26, 2021 at 07:45:53PM +0300, Laurent Pinchart wrote: > > > All libcamera headers are meant to be installed in > > > ${prefix}/include/libcamera/libcamera, with pkgconfig specifying the > > > include directory as `-I ${prefix}/include/libcamera`. Applications then > > > include the headers with `#include <libcamera/camera.h>`. > > > > > > The base and ipa headers are meant to be installed in subdirectories of > > > the libcamera headers directory, but are mistakenly installed one level > > > too high, in ${prefix}/include/libcamera. This leads to compilation > > > failures when including the base or ipa header. > > > > > > Fix this by setting the meson libcamera_include_dir variable to the > > > libcamera headers directory. All other header paths are derived from > > > that variable and are now correct. > > > > > > Reported-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=79 > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > include/libcamera/meson.build | 14 +++++++------- > > > include/meson.build | 2 -- > > > 2 files changed, 7 insertions(+), 9 deletions(-) > > > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > > index 5b25ef847ed4..567782a66ba3 100644 > > > --- a/include/libcamera/meson.build > > > +++ b/include/libcamera/meson.build > > > @@ -1,5 +1,7 @@ > > > # SPDX-License-Identifier: CC0-1.0 > > > > > > +libcamera_include_dir = 'libcamera' / 'libcamera' > > > + > > > libcamera_public_headers = files([ > > > 'camera.h', > > > 'camera_manager.h', > > > @@ -16,14 +18,12 @@ libcamera_public_headers = files([ > > > 'transform.h', > > > ]) > > > > > > -include_dir = libcamera_include_dir / 'libcamera' > > > > Am I missing something here? > > > > The old version has libcamera_include_dir = 'libcamera', so the subdir > > that we feed below is 'libcamera' / 'libcamera'. > > > > The new version has 'libcamera' / 'libcamera' directly, so it seems to > > me nothing has changed? > > In include/libcamera/, indeed, but the base/ and ipa/ subdirectories use > the libcamera_include_dir variable, which is now 'libcamera/libcamera' > when it was just 'libcamera'. Ah, I see. Thanks for the clarification. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > > - > > > subdir('base') > > > subdir('internal') > > > subdir('ipa') > > > > > > install_headers(libcamera_public_headers, > > > - subdir : include_dir) > > > + subdir : libcamera_include_dir) > > > > > > # > > > # Generate headers from templates. > > > @@ -44,7 +44,7 @@ foreach header : control_source_files > > > output : header + '.h', > > > command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > > > install : true, > > > - install_dir : get_option('includedir') / include_dir) > > > + install_dir : get_option('includedir') / libcamera_include_dir) > > > endforeach > > > > > > libcamera_public_headers += control_headers > > > @@ -59,7 +59,7 @@ formats_h = custom_target('formats_h', > > > output : 'formats.h', > > > command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'], > > > install : true, > > > - install_dir : get_option('includedir') / include_dir) > > > + install_dir : get_option('includedir') / libcamera_include_dir) > > > libcamera_public_headers += formats_h > > > > > > # libcamera.h > > > @@ -68,7 +68,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') / include_dir) > > > + install_dir : get_option('includedir') / libcamera_include_dir) > > > > > > libcamera_public_headers += libcamera_h > > > > > > @@ -82,4 +82,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') / include_dir) > > > + install_dir : get_option('includedir') / libcamera_include_dir) > > > diff --git a/include/meson.build b/include/meson.build > > > index 2ac9a3a049f2..27ce2f41c534 100644 > > > --- a/include/meson.build > > > +++ b/include/meson.build > > > @@ -1,6 +1,4 @@ > > > # SPDX-License-Identifier: CC0-1.0 > > > > > > -libcamera_include_dir = 'libcamera' > > > - > > > subdir('android') > > > subdir('libcamera') > > -- > Regards, > > Laurent Pinchart
On Sun, Sep 26, 2021 at 07:45:53PM +0300, Laurent Pinchart wrote: > All libcamera headers are meant to be installed in > ${prefix}/include/libcamera/libcamera, with pkgconfig specifying the The 'first' libcamera here is supposed to be the overall package, and we could perhaps envision being versioned if necessary later right? While the second libcamera is of course the 'libcamera' part. I think originally my aim was to have that first component out as a variable as it could be expected to change. however, this fixes things up for now - and so I don't mind it being hardcoded (in a better way than before). Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > include directory as `-I ${prefix}/include/libcamera`. Applications then > include the headers with `#include <libcamera/camera.h>`. > > The base and ipa headers are meant to be installed in subdirectories of > the libcamera headers directory, but are mistakenly installed one level > too high, in ${prefix}/include/libcamera. This leads to compilation > failures when including the base or ipa header. > > Fix this by setting the meson libcamera_include_dir variable to the > libcamera headers directory. All other header paths are derived from > that variable and are now correct. > > Reported-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=79 > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/meson.build | 14 +++++++------- > include/meson.build | 2 -- > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 5b25ef847ed4..567782a66ba3 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -1,5 +1,7 @@ > # SPDX-License-Identifier: CC0-1.0 > > +libcamera_include_dir = 'libcamera' / 'libcamera' > + > libcamera_public_headers = files([ > 'camera.h', > 'camera_manager.h', > @@ -16,14 +18,12 @@ libcamera_public_headers = files([ > 'transform.h', > ]) > > -include_dir = libcamera_include_dir / 'libcamera' > - > subdir('base') > subdir('internal') > subdir('ipa') > > install_headers(libcamera_public_headers, > - subdir : include_dir) > + subdir : libcamera_include_dir) > > # > # Generate headers from templates. > @@ -44,7 +44,7 @@ foreach header : control_source_files > output : header + '.h', > command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > - install_dir : get_option('includedir') / include_dir) > + install_dir : get_option('includedir') / libcamera_include_dir) > endforeach > > libcamera_public_headers += control_headers > @@ -59,7 +59,7 @@ formats_h = custom_target('formats_h', > output : 'formats.h', > command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > - install_dir : get_option('includedir') / include_dir) > + install_dir : get_option('includedir') / libcamera_include_dir) > libcamera_public_headers += formats_h > > # libcamera.h > @@ -68,7 +68,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') / include_dir) > + install_dir : get_option('includedir') / libcamera_include_dir) > > libcamera_public_headers += libcamera_h > > @@ -82,4 +82,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') / include_dir) > + install_dir : get_option('includedir') / libcamera_include_dir) > diff --git a/include/meson.build b/include/meson.build > index 2ac9a3a049f2..27ce2f41c534 100644 > --- a/include/meson.build > +++ b/include/meson.build > @@ -1,6 +1,4 @@ > # SPDX-License-Identifier: CC0-1.0 > > -libcamera_include_dir = 'libcamera' > - > subdir('android') > subdir('libcamera') > -- > Regards, > > Laurent Pinchart >
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@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? > # > # 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) > -- > Regards, > > Laurent Pinchart -- Kieran
Hi Kieran, On Tue, Sep 28, 2021 at 05:47:57PM +0100, Kieran Bingham wrote: > On Sun, Sep 26, 2021 at 07:45:53PM +0300, Laurent Pinchart wrote: > > All libcamera headers are meant to be installed in > > ${prefix}/include/libcamera/libcamera, with pkgconfig specifying the > > The 'first' libcamera here is supposed to be the overall package, and we > could perhaps envision being versioned if necessary later right? Correct. libcamera-1, libcamera-2, ... > While the second libcamera is of course the 'libcamera' part. > > I think originally my aim was to have that first component out as a > variable as it could be expected to change. It's a good point, but we can probably fix it on top. Appending the soname version should be simple enough. > however, this fixes things up for now - and so I don't mind it being > hardcoded (in a better way than before). > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > include directory as `-I ${prefix}/include/libcamera`. Applications then > > include the headers with `#include <libcamera/camera.h>`. > > > > The base and ipa headers are meant to be installed in subdirectories of > > the libcamera headers directory, but are mistakenly installed one level > > too high, in ${prefix}/include/libcamera. This leads to compilation > > failures when including the base or ipa header. > > > > Fix this by setting the meson libcamera_include_dir variable to the > > libcamera headers directory. All other header paths are derived from > > that variable and are now correct. > > > > Reported-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=79 > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/meson.build | 14 +++++++------- > > include/meson.build | 2 -- > > 2 files changed, 7 insertions(+), 9 deletions(-) > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > index 5b25ef847ed4..567782a66ba3 100644 > > --- a/include/libcamera/meson.build > > +++ b/include/libcamera/meson.build > > @@ -1,5 +1,7 @@ > > # SPDX-License-Identifier: CC0-1.0 > > > > +libcamera_include_dir = 'libcamera' / 'libcamera' > > + > > libcamera_public_headers = files([ > > 'camera.h', > > 'camera_manager.h', > > @@ -16,14 +18,12 @@ libcamera_public_headers = files([ > > 'transform.h', > > ]) > > > > -include_dir = libcamera_include_dir / 'libcamera' > > - > > subdir('base') > > subdir('internal') > > subdir('ipa') > > > > install_headers(libcamera_public_headers, > > - subdir : include_dir) > > + subdir : libcamera_include_dir) > > > > # > > # Generate headers from templates. > > @@ -44,7 +44,7 @@ foreach header : control_source_files > > output : header + '.h', > > command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > > install : true, > > - install_dir : get_option('includedir') / include_dir) > > + install_dir : get_option('includedir') / libcamera_include_dir) > > endforeach > > > > libcamera_public_headers += control_headers > > @@ -59,7 +59,7 @@ formats_h = custom_target('formats_h', > > output : 'formats.h', > > command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'], > > install : true, > > - install_dir : get_option('includedir') / include_dir) > > + install_dir : get_option('includedir') / libcamera_include_dir) > > libcamera_public_headers += formats_h > > > > # libcamera.h > > @@ -68,7 +68,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') / include_dir) > > + install_dir : get_option('includedir') / libcamera_include_dir) > > > > libcamera_public_headers += libcamera_h > > > > @@ -82,4 +82,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') / include_dir) > > + install_dir : get_option('includedir') / libcamera_include_dir) > > diff --git a/include/meson.build b/include/meson.build > > index 2ac9a3a049f2..27ce2f41c534 100644 > > --- a/include/meson.build > > +++ b/include/meson.build > > @@ -1,6 +1,4 @@ > > # SPDX-License-Identifier: CC0-1.0 > > > > -libcamera_include_dir = 'libcamera' > > - > > subdir('android') > > subdir('libcamera')
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@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 > > # > > # 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)
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@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@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) >