| Message ID | 20210430114331.151018-1-umang.jain@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, On 30/04/2021 12:43, Umang Jain wrote: > In most cases, file paths in meson files start with get_option(). > To maintain a consistent theme, use meson's get_option('includedir') > universal option over raw 'include'. This option is defaulted to > 'include' string value, hence this commit does not introduce any > functional changes. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > include/libcamera/meson.build | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 4a5b915e..086c958b 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -48,7 +48,7 @@ foreach header : control_source_files > output : header + '.h', > command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > - install_dir : 'include' / include_dir) > + install_dir : get_option('includedir') / include_dir) include_dir is currently defined as 'libcamera'. Could we make this use the same style as the 'define system paths' patch, and also update this variable to become libcamera_includedir = get_option('includedir') / 'libcamera' and then use that as the install dir? > endforeach > > libcamera_public_headers += control_headers > @@ -63,7 +63,7 @@ formats_h = custom_target('formats_h', > output : 'formats.h', > command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > - install_dir : 'include' / include_dir) > + install_dir : get_option('includedir') / include_dir) > libcamera_public_headers += formats_h > > # libcamera.h > @@ -72,7 +72,7 @@ libcamera_h = custom_target('gen-header', > output : 'libcamera.h', > command : [gen_header, meson.current_source_dir(), '@OUTPUT@'], > install : true, > - install_dir : 'include' / include_dir) > + install_dir : get_option('includedir') / include_dir) > > libcamera_public_headers += libcamera_h > > @@ -86,4 +86,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 : 'include' / include_dir) > + install_dir : get_option('includedir') / include_dir) >
Hi Umang, On 30/04/2021 12:43, Umang Jain wrote: > Since meson v0.49.0, join_paths() is equivalent to '/' hence, > drop and replace it with '/' short-hand in meson files. > > This commit does not introduce any functional changes. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > Documentation/meson.build | 4 ++-- > include/libcamera/ipa/meson.build | 2 +- > include/libcamera/meson.build | 10 +++++----- > meson.build | 3 +-- > src/ipa/meson.build | 14 +++++++------- > src/ipa/raspberrypi/data/meson.build | 2 +- > src/ipa/vimc/data/meson.build | 2 +- > src/libcamera/meson.build | 2 +- > src/libcamera/proxy/worker/meson.build | 4 ++-- > 9 files changed, 21 insertions(+), 22 deletions(-) > > diff --git a/Documentation/meson.build b/Documentation/meson.build > index 9950465d..ef3e1696 100644 > --- a/Documentation/meson.build > +++ b/Documentation/meson.build > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: CC0-1.0 > > -doc_install_dir = join_paths(get_option('datadir'), 'doc', > - 'libcamera-@0@'.format(libcamera_version)) > +doc_install_dir = get_option('datadir') / 'doc' / \ > + 'libcamera-@0@'.format(libcamera_version) I'm not sure which is better, A line continuation marker, which is really close to the 'join' operator (/ \) or 92 chars. 92 chars is still under 120 which is our full max line limit, so this could be one line. Either way is ok with me though. > > # > # Doxygen > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build > index 2d72c1fc..40c4e737 100644 > --- a/include/libcamera/ipa/meson.build > +++ b/include/libcamera/ipa/meson.build > @@ -7,7 +7,7 @@ libcamera_ipa_headers = files([ > ]) > > install_headers(libcamera_ipa_headers, > - subdir: join_paths(libcamera_include_dir, 'ipa')) > + subdir: libcamera_include_dir / 'ipa') Eeep - and I had just suggested we add a libcamera_includedir ... > > libcamera_generated_ipa_headers = [] > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index c7b8ee8e..4a5b915e 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -21,7 +21,7 @@ libcamera_public_headers = files([ > 'transform.h', > ]) > > -include_dir = join_paths(libcamera_include_dir, 'libcamera') > +include_dir = libcamera_include_dir / 'libcamera' Aha, which would have been here. So we have too many levels of libcamera_includedir ... > > subdir('internal') > subdir('ipa') > @@ -48,7 +48,7 @@ foreach header : control_source_files > output : header + '.h', > command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > - install_dir : join_paths('include', include_dir)) > + install_dir : 'include' / include_dir) > endforeach > > libcamera_public_headers += control_headers > @@ -63,7 +63,7 @@ formats_h = custom_target('formats_h', > output : 'formats.h', > command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > - install_dir : join_paths('include', include_dir)) > + install_dir : 'include' / include_dir) > libcamera_public_headers += formats_h > > # libcamera.h > @@ -72,7 +72,7 @@ libcamera_h = custom_target('gen-header', > output : 'libcamera.h', > command : [gen_header, meson.current_source_dir(), '@OUTPUT@'], > install : true, > - install_dir : join_paths('include', include_dir)) > + install_dir : 'include' / include_dir) > > libcamera_public_headers += libcamera_h > > @@ -86,4 +86,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 : join_paths('include', include_dir)) > + install_dir : 'include' / include_dir) > diff --git a/meson.build b/meson.build > index 13d88301..46eb1b46 100644 > --- a/meson.build > +++ b/meson.build > @@ -148,8 +148,7 @@ endif > # Create a symlink from the build root to the source root. This is used when > # running libcamera from the build directory to locate resources in the source > # directory (such as IPA configuration files). > -run_command('ln', '-fsT', meson.source_root(), > - join_paths(meson.build_root(), 'source')) > +run_command('ln', '-fsT', meson.source_root(), meson.build_root() / 'source') > > configure_file(output : 'config.h', configuration : config_h) > > diff --git a/src/ipa/meson.build b/src/ipa/meson.build > index df385eae..b1abfc74 100644 > --- a/src/ipa/meson.build > +++ b/src/ipa/meson.build > @@ -1,19 +1,19 @@ > # SPDX-License-Identifier: CC0-1.0 > > -ipa_install_dir = join_paths(get_option('libdir'), 'libcamera') > -ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa') > -ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa') > +ipa_install_dir = get_option('libdir') / 'libcamera' > +ipa_data_dir = get_option('datadir') / 'libcamera' / 'ipa' > +ipa_sysconf_dir = get_option('sysconfdir') /'libcamera' / 'ipa' > > ipa_includes = [ > libcamera_includes, > ] > > config_h.set('IPA_CONFIG_DIR', > - '"' + join_paths(get_option('prefix'), ipa_sysconf_dir) + > - ':' + join_paths(get_option('prefix'), ipa_data_dir) + '"') > + '"' + get_option('prefix') / ipa_sysconf_dir + > + ':' + get_option('prefix') / ipa_data_dir + '"') > > config_h.set('IPA_MODULE_DIR', > - '"' + join_paths(get_option('prefix'), ipa_install_dir) + '"') > + '"' + get_option('prefix') / ipa_install_dir + '"') > > subdir('libipa') > > @@ -25,7 +25,7 @@ ipa_names = [] > foreach pipeline : pipelines > if ipas.contains(pipeline) > subdir(pipeline) > - ipa_names += join_paths(ipa_install_dir, ipa_name + '.so') > + ipa_names += ipa_install_dir / ipa_name / '.so' Careful here. Previously this was ipa_name + '.so' not ipa_name, '.so'. We shouldn't use this as a join path - otherwise it might be creating ...libcamera/ipu3/.so instead of ...libcamera/ipu3.so > endif > endforeach > > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build > index 253fb9d7..92ad3272 100644 > --- a/src/ipa/raspberrypi/data/meson.build > +++ b/src/ipa/raspberrypi/data/meson.build > @@ -10,4 +10,4 @@ conf_files = files([ > ]) > > install_data(conf_files, > - install_dir : join_paths(ipa_data_dir, 'raspberrypi')) > + install_dir : ipa_data_dir / 'raspberrypi') > diff --git a/src/ipa/vimc/data/meson.build b/src/ipa/vimc/data/meson.build > index 6532662c..42ec651c 100644 > --- a/src/ipa/vimc/data/meson.build > +++ b/src/ipa/vimc/data/meson.build > @@ -5,4 +5,4 @@ conf_files = files([ > ]) > > install_data(conf_files, > - install_dir : join_paths(ipa_data_dir, 'vimc')) > + install_dir : ipa_data_dir / 'vimc') > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index e0a48aa2..99b09b3a 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -102,7 +102,7 @@ endforeach > > libcamera_sources += control_sources > > -gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh') > +gen_version = meson.source_root() / 'utils' / 'gen-version.sh' > > version_cpp = vcs_tag(command : [gen_version, meson.build_root()], > input : 'version.cpp.in', > diff --git a/src/libcamera/proxy/worker/meson.build b/src/libcamera/proxy/worker/meson.build > index 3796103e..40dada9d 100644 > --- a/src/libcamera/proxy/worker/meson.build > +++ b/src/libcamera/proxy/worker/meson.build > @@ -1,6 +1,6 @@ > # SPDX-License-Identifier: CC0-1.0 > > -proxy_install_dir = join_paths(get_option('libexecdir'), 'libcamera') > +proxy_install_dir = get_option('libexecdir') / 'libcamera' > > # generate {pipeline}_ipa_proxy_worker.cpp > foreach mojom : ipa_mojoms > @@ -25,4 +25,4 @@ foreach mojom : ipa_mojoms > endforeach > > config_h.set('IPA_PROXY_DIR', Ohh another one to add to my paths summary perhaps ... > - '"' + join_paths(get_option('prefix'), proxy_install_dir) + '"') > + '"' + get_option('prefix') / proxy_install_dir + '"') >
Hi Kieran, On 4/30/21 5:46 PM, Kieran Bingham wrote: > Hi Umang, > > On 30/04/2021 12:43, Umang Jain wrote: >> In most cases, file paths in meson files start with get_option(). >> To maintain a consistent theme, use meson's get_option('includedir') >> universal option over raw 'include'. This option is defaulted to >> 'include' string value, hence this commit does not introduce any >> functional changes. >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> include/libcamera/meson.build | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build >> index 4a5b915e..086c958b 100644 >> --- a/include/libcamera/meson.build >> +++ b/include/libcamera/meson.build >> @@ -48,7 +48,7 @@ foreach header : control_source_files >> output : header + '.h', >> command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], >> install : true, >> - install_dir : 'include' / include_dir) >> + install_dir : get_option('includedir') / include_dir) > include_dir is currently defined as 'libcamera'. No, in the same file - include/libcamera/meson.build include_dir = libcamera_include_dir / 'libcamera' and libcamera_include_dir = 'libcamera' from include/meson.build so install dir comes to (with --prefix=/usr) /usr/include/libcamera/libcamera/ Running a branch with these patches and running a master branch (with --prefix=/usr in both) confirms, the public headers are installed to /usr/include/libcamera/libcamera/ > > Could we make this use the same style as the 'define system paths' > patch, and also update this variable to become > > libcamera_includedir = get_option('includedir') / 'libcamera' > > and then use that as the install dir? Can it be done on top? This patch is just to bring in get_option() style in for file paths as Laurent suggested earlier? > > > > >> endforeach >> >> libcamera_public_headers += control_headers >> @@ -63,7 +63,7 @@ formats_h = custom_target('formats_h', >> output : 'formats.h', >> command : [gen_formats, '-o', '@OUTPUT@', '@INPUT@'], >> install : true, >> - install_dir : 'include' / include_dir) >> + install_dir : get_option('includedir') / include_dir) >> libcamera_public_headers += formats_h >> >> # libcamera.h >> @@ -72,7 +72,7 @@ libcamera_h = custom_target('gen-header', >> output : 'libcamera.h', >> command : [gen_header, meson.current_source_dir(), '@OUTPUT@'], >> install : true, >> - install_dir : 'include' / include_dir) >> + install_dir : get_option('includedir') / include_dir) >> >> libcamera_public_headers += libcamera_h >> >> @@ -86,4 +86,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 : 'include' / include_dir) >> + install_dir : get_option('includedir') / include_dir) >>
Hi [snip] On 4/30/21 5:55 PM, Kieran Bingham wrote: >> ] >> >> config_h.set('IPA_CONFIG_DIR', >> - '"' + join_paths(get_option('prefix'), ipa_sysconf_dir) + >> - ':' + join_paths(get_option('prefix'), ipa_data_dir) + '"') >> + '"' + get_option('prefix') / ipa_sysconf_dir + >> + ':' + get_option('prefix') / ipa_data_dir + '"') >> >> config_h.set('IPA_MODULE_DIR', >> - '"' + join_paths(get_option('prefix'), ipa_install_dir) + '"') >> + '"' + get_option('prefix') / ipa_install_dir + '"') >> >> subdir('libipa') >> >> @@ -25,7 +25,7 @@ ipa_names = [] >> foreach pipeline : pipelines >> if ipas.contains(pipeline) >> subdir(pipeline) >> - ipa_names += join_paths(ipa_install_dir, ipa_name + '.so') >> + ipa_names += ipa_install_dir / ipa_name / '.so' > Careful here. > > Previously this was ipa_name + '.so' > not > ipa_name, '.so'. > > We shouldn't use this as a join path - otherwise it might be creating > > ...libcamera/ipu3/.so > instead of > ...libcamera/ipu3.so oh, this was missed badly, thanks for spotting. Patches were mostly compiled test - > > > >> endif >> endforeach >> >> diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build >> index 253fb9d7..92ad3272 100644 >> --- a/src/ipa/raspberrypi/data/meson.build >> +++ b/src/ipa/raspberrypi/data/meson.build >> @@ -10,4 +10,4 @@ conf_files = files([ >> ]) >> >> install_data(conf_files, >> - install_dir : join_paths(ipa_data_dir, 'raspberrypi')) >> + install_dir : ipa_data_dir / 'raspberrypi') >> diff --git a/src/ipa/vimc/data/meson.build b/src/ipa/vimc/data/meson.build >> index 6532662c..42ec651c 100644 >> --- a/src/ipa/vimc/data/meson.build >> +++ b/src/ipa/vimc/data/meson.build >> @@ -5,4 +5,4 @@ conf_files = files([ >> ]) >> >> install_data(conf_files, >> - install_dir : join_paths(ipa_data_dir, 'vimc')) >> + install_dir : ipa_data_dir / 'vimc') >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build >> index e0a48aa2..99b09b3a 100644 >> --- a/src/libcamera/meson.build >> +++ b/src/libcamera/meson.build >> @@ -102,7 +102,7 @@ endforeach >> >> libcamera_sources += control_sources >> >> -gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh') >> +gen_version = meson.source_root() / 'utils' / 'gen-version.sh' >> >> version_cpp = vcs_tag(command : [gen_version, meson.build_root()], >> input : 'version.cpp.in', >> diff --git a/src/libcamera/proxy/worker/meson.build b/src/libcamera/proxy/worker/meson.build >> index 3796103e..40dada9d 100644 >> --- a/src/libcamera/proxy/worker/meson.build >> +++ b/src/libcamera/proxy/worker/meson.build >> @@ -1,6 +1,6 @@ >> # SPDX-License-Identifier: CC0-1.0 >> >> -proxy_install_dir = join_paths(get_option('libexecdir'), 'libcamera') >> +proxy_install_dir = get_option('libexecdir') / 'libcamera' >> >> # generate {pipeline}_ipa_proxy_worker.cpp >> foreach mojom : ipa_mojoms >> @@ -25,4 +25,4 @@ foreach mojom : ipa_mojoms >> endforeach >> >> config_h.set('IPA_PROXY_DIR', > Ohh another one to add to my paths summary perhaps ... > >> - '"' + join_paths(get_option('prefix'), proxy_install_dir) + '"') >> + '"' + get_option('prefix') / proxy_install_dir + '"') >>
The first two commits are clean ups that were identified during the RFC version review. The third commit is one of the pieces we require to build an IPA existing out-of-tree. Umang Jain (3): meson: Replace obselete join_paths() with '/' operator libcamera: Use get_option('includedir') instead of raw 'include' ipa: meson: Install mojom generated interface headers to include path Documentation/meson.build | 4 ++-- include/libcamera/ipa/meson.build | 8 +++++++- include/libcamera/meson.build | 10 +++++----- meson.build | 3 +-- src/ipa/meson.build | 14 +++++++------- src/ipa/raspberrypi/data/meson.build | 2 +- src/ipa/vimc/data/meson.build | 2 +- src/libcamera/meson.build | 2 +- src/libcamera/proxy/worker/meson.build | 4 ++-- 9 files changed, 27 insertions(+), 22 deletions(-)