| Message ID | 20220511082346.18972-1-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Tomi, Thank you for the patch. On Wed, May 11, 2022 at 11:23:46AM +0300, Tomi Valkeinen wrote: > Use files() for the input files for the custom_target(). I believe the > current code works, but perhaps it is safer to use files() here. And I think it's more in line with the meson recommendations. > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > src/py/libcamera/meson.build | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build > index 0cd7c75b..3f7ddf1d 100644 > --- a/src/py/libcamera/meson.build > +++ b/src/py/libcamera/meson.build > @@ -17,10 +17,10 @@ pycamera_sources = files([ > 'pymain.cpp', > ]) > > -gen_input_files = [ > - meson.project_source_root() / 'src' / 'libcamera' / 'control_ids.yaml', > +gen_input_files = files( > + '../../libcamera/control_ids.yaml', > 'pyenums_generated.cpp.in', > -] > +) Interesting, I didn't know files() could take a list of files, as opposed to an array. We always write gen_input_files = files([ '../../libcamera/control_ids.yaml', 'pyenums_generated.cpp.in', ]) I'd do the same here for consistency, and remove the [] in a global patch separately if desired. Is that OK with you ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > gen_py_control_enums = files('gen-py-control-enums.py')
On 11/05/2022 12:06, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Wed, May 11, 2022 at 11:23:46AM +0300, Tomi Valkeinen wrote: >> Use files() for the input files for the custom_target(). I believe the >> current code works, but perhaps it is safer to use files() here. > > And I think it's more in line with the meson recommendations. > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> >> --- >> src/py/libcamera/meson.build | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build >> index 0cd7c75b..3f7ddf1d 100644 >> --- a/src/py/libcamera/meson.build >> +++ b/src/py/libcamera/meson.build >> @@ -17,10 +17,10 @@ pycamera_sources = files([ >> 'pymain.cpp', >> ]) >> >> -gen_input_files = [ >> - meson.project_source_root() / 'src' / 'libcamera' / 'control_ids.yaml', >> +gen_input_files = files( >> + '../../libcamera/control_ids.yaml', >> 'pyenums_generated.cpp.in', >> -] >> +) > > Interesting, I didn't know files() could take a list of files, as > opposed to an array. We always write > > gen_input_files = files([ > '../../libcamera/control_ids.yaml', > 'pyenums_generated.cpp.in', > ]) > > I'd do the same here for consistency, and remove the [] in a global > patch separately if desired. Is that OK with you ? It is used without an array in include/libcamera/meson.build and src/libcamera/meson.build. I think I used one of those as a reference, and didn't even notice the array use elsewhere. Meson docs do not even mention that files() can take an array. But I can add the array, it's used in that way also a bit above when defining the source files. As a side note, using files() feels a bit silly to me. Afaics the point of files() is to create a file object when you want to use those files in some other meson build file, i.e. you want to "export" the file to other meson files. Here the list of files is "private" to this particular meson build file. Using files() for local source files does not provide any value that I can see. And if we use files() for the sources, shouldn't we also use files() for the output for the custom_target()? Tomi
Hi Tomi, On Wed, May 11, 2022 at 12:25:02PM +0300, Tomi Valkeinen wrote: > On 11/05/2022 12:06, Laurent Pinchart wrote: > > On Wed, May 11, 2022 at 11:23:46AM +0300, Tomi Valkeinen wrote: > >> Use files() for the input files for the custom_target(). I believe the > >> current code works, but perhaps it is safer to use files() here. > > > > And I think it's more in line with the meson recommendations. > > > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > >> --- > >> src/py/libcamera/meson.build | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build > >> index 0cd7c75b..3f7ddf1d 100644 > >> --- a/src/py/libcamera/meson.build > >> +++ b/src/py/libcamera/meson.build > >> @@ -17,10 +17,10 @@ pycamera_sources = files([ > >> 'pymain.cpp', > >> ]) > >> > >> -gen_input_files = [ > >> - meson.project_source_root() / 'src' / 'libcamera' / 'control_ids.yaml', > >> +gen_input_files = files( > >> + '../../libcamera/control_ids.yaml', > >> 'pyenums_generated.cpp.in', > >> -] > >> +) > > > > Interesting, I didn't know files() could take a list of files, as > > opposed to an array. We always write > > > > gen_input_files = files([ > > '../../libcamera/control_ids.yaml', > > 'pyenums_generated.cpp.in', > > ]) > > > > I'd do the same here for consistency, and remove the [] in a global > > patch separately if desired. Is that OK with you ? > > It is used without an array in include/libcamera/meson.build and > src/libcamera/meson.build. I think I used one of those as a reference, > and didn't even notice the array use elsewhere. Ah OK I haven't noticed that. We can mass-patch all the meson.build files if desired. > Meson docs do not even mention that files() can take an array. But I can > add the array, it's used in that way also a bit above when defining the > source files. > > As a side note, using files() feels a bit silly to me. Afaics the point > of files() is to create a file object when you want to use those files > in some other meson build file, i.e. you want to "export" the file to > other meson files. Here the list of files is "private" to this > particular meson build file. > > Using files() for local source files does not provide any value that I > can see. > > And if we use files() for the sources, shouldn't we also use files() for > the output for the custom_target()? My understanding is that files() help doing the right thing for input files as it removes the need to think about source vs. build directories. For outputs that won't work, your file object will reference a location in the source directory as far as I can tell, while the output is meant to be written to the build directory. I'm not even sure if the custom_target() function accepts a file object for the output.
diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build index 0cd7c75b..3f7ddf1d 100644 --- a/src/py/libcamera/meson.build +++ b/src/py/libcamera/meson.build @@ -17,10 +17,10 @@ pycamera_sources = files([ 'pymain.cpp', ]) -gen_input_files = [ - meson.project_source_root() / 'src' / 'libcamera' / 'control_ids.yaml', +gen_input_files = files( + '../../libcamera/control_ids.yaml', 'pyenums_generated.cpp.in', -] +) gen_py_control_enums = files('gen-py-control-enums.py')
Use files() for the input files for the custom_target(). I believe the current code works, but perhaps it is safer to use files() here. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- src/py/libcamera/meson.build | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)