[libcamera-devel] [PATCH] py: meson: use files() for custom_target input files

[Laurent Pinchart]

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 at 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.

-- 
Regards,

Laurent Pinchart