Re: [PATCH v1 1/1] lib: allow libraries with no sources

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Mar 07, 2024 at 11:46:51AM +0100, David Marchand wrote:
> On Wed, Mar 6, 2024 at 11:17 PM Paul Szczepanek <paul.szczepanek@arm.com> wrote:
> >
> > Allow header only libraries.
> >
> > Signed-off-by: Paul Szczepanek <paul.szczepanek@arm.com>
> > Reviewed-by: Dhruv Tripathi <Dhruv.Tripathi@arm.com>
> 
> Don't forget to Cc: Bruce, he is the build system maintainer.
> 
> 
> >
> > ---
> >  lib/meson.build | 176 +++++++++++++++++++++++++-----------------------
> >  1 file changed, 91 insertions(+), 85 deletions(-)
> 
> Copy/pasting git show -w, it is easier to review:
> 
> 
> > diff --git a/lib/meson.build b/lib/meson.build
> > index 179a272932..293d65385d 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -222,6 +222,7 @@ foreach l:libraries
> >      includes += include_directories(l)
> >      dpdk_includes += include_directories(l)
> >
> > +    if sources.length() != 0
> >          if developer_mode and is_windows and use_function_versioning
> >              message('@0@: Function versioning is not supported by Windows.'.format(name))
> >          endif
> > @@ -321,6 +322,11 @@ foreach l:libraries
> >
> >          dpdk_libraries = [shared_lib] + dpdk_libraries
> >          dpdk_static_libraries = [static_lib] + dpdk_static_libraries
> > +    else # sources.length() == 0
> > +        # if no C files, just set a dependency on header path
> > +        shared_dep = declare_dependency(include_directories: includes)
> 
> In the hypothetical case that such a header only library would depend
> on some external library, I suspect we would need to pass external
> dependencies here.
> Like:
> 
> -        shared_dep = declare_dependency(include_directories: includes)
> +        shared_dep = declare_dependency(include_directories: includes,
> +                dependencies: shared_deps)
> 
> 
> > +        static_dep = shared_dep
> > +    endif
> >
> >      set_variable('shared_rte_' + name, shared_dep)
> >      set_variable('static_rte_' + name, static_dep)
> 

This is very similar to what was done previously in DPDK, but that
was done with the limitations of what early versions of meson supported.
Since meson 0.49 release we have now got "break" and "continue" keywords,
so rather than having to indent everything inside the if statement, I think
it would be better to invert "sources.length()" check at the start, handle
the len == 0 case, and then use "continue" to move on to the next library.
There should only be the need to duplicate a couple of lines in that case.

Thanks,
/Bruce