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

Message ID 20240306221709.166722-2-paul.szczepanek@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series allow libraries with no sources |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Paul Szczepanek March 6, 2024, 10:17 p.m. UTC
  Allow header only libraries.

Signed-off-by: Paul Szczepanek <paul.szczepanek@arm.com>
Reviewed-by: Dhruv Tripathi <Dhruv.Tripathi@arm.com>

---
 lib/meson.build | 176 +++++++++++++++++++++++++-----------------------
 1 file changed, 91 insertions(+), 85 deletions(-)

--
2.25.1
  

Comments

David Marchand March 7, 2024, 10:46 a.m. UTC | #1
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)
  
Bruce Richardson March 7, 2024, 11:42 a.m. UTC | #2
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
  

Patch

diff --git a/lib/meson.build b/lib/meson.build
index 4fb01f059b..0fcf3336d1 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -220,105 +220,111 @@  foreach l:libraries
     includes += include_directories(l)
     dpdk_includes += include_directories(l)

-    if developer_mode and is_windows and use_function_versioning
-        message('@0@: Function versioning is not supported by Windows.'.format(name))
-    endif
+    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

-    if use_function_versioning
-        cflags += '-DRTE_USE_FUNCTION_VERSIONING'
-    endif
-    cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=lib.' + l
-    if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
-        cflags += '-DRTE_ANNOTATE_LOCKS'
-        cflags += '-Wthread-safety'
-    endif
+        if use_function_versioning
+            cflags += '-DRTE_USE_FUNCTION_VERSIONING'
+        endif
+        cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=lib.' + l
+        if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
+            cflags += '-DRTE_ANNOTATE_LOCKS'
+            cflags += '-Wthread-safety'
+        endif

-    # first build static lib
-    static_lib = static_library(libname,
-            sources,
-            objects: objs,
-            c_args: cflags,
-            dependencies: static_deps,
-            include_directories: includes,
-            install: true)
-    static_dep = declare_dependency(
-            include_directories: includes,
-            dependencies: static_deps)
+        # first build static lib
+        static_lib = static_library(libname,
+                sources,
+                objects: objs,
+                c_args: cflags,
+                dependencies: static_deps,
+                include_directories: includes,
+                install: true)
+        static_dep = declare_dependency(
+                include_directories: includes,
+                dependencies: static_deps)

-    if not use_function_versioning or is_windows
-        # use pre-build objects to build shared lib
-        sources = []
-        objs += static_lib.extract_all_objects(recursive: false)
-    else
-        # for compat we need to rebuild with
-        # RTE_BUILD_SHARED_LIB defined
-        cflags += '-DRTE_BUILD_SHARED_LIB'
-    endif
+        if not use_function_versioning or is_windows
+            # use pre-build objects to build shared lib
+            sources = []
+            objs += static_lib.extract_all_objects(recursive: false)
+        else
+            # for compat we need to rebuild with
+            # RTE_BUILD_SHARED_LIB defined
+            cflags += '-DRTE_BUILD_SHARED_LIB'
+        endif

-    version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), l)
-    lk_deps = [version_map]
+        version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), l)
+        lk_deps = [version_map]

-    if is_ms_linker
-        def_file = custom_target(libname + '_def',
-                command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
-                input: version_map,
-                output: '@0@_exports.def'.format(libname))
-        lk_deps += [def_file]
+        if is_ms_linker
+            def_file = custom_target(libname + '_def',
+                    command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
+                    input: version_map,
+                    output: '@0@_exports.def'.format(libname))
+            lk_deps += [def_file]

-        if is_ms_compiler
-            lk_args = ['/def:' + def_file.full_path()]
-            if meson.version().version_compare('<0.54.0')
-                lk_args += ['/implib:lib\\librte_' + l + '.dll.a']
+            if is_ms_compiler
+                lk_args = ['/def:' + def_file.full_path()]
+                if meson.version().version_compare('<0.54.0')
+                    lk_args += ['/implib:lib\\librte_' + l + '.dll.a']
+                endif
+            else
+                lk_args = ['-Wl,/def:' + def_file.full_path()]
+                if meson.version().version_compare('<0.54.0')
+                    lk_args += ['-Wl,/implib:lib\\librte_' + l + '.dll.a']
+                endif
             endif
         else
-            lk_args = ['-Wl,/def:' + def_file.full_path()]
-            if meson.version().version_compare('<0.54.0')
-                lk_args += ['-Wl,/implib:lib\\librte_' + l + '.dll.a']
+            if is_windows
+                mingw_map = custom_target(libname + '_mingw',
+                        command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
+                        input: version_map,
+                        output: '@0@_mingw.map'.format(libname))
+                lk_deps += [mingw_map]
+
+                lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+            else
+                lk_args = ['-Wl,--version-script=' + version_map]
             endif
         endif
-    else
-        if is_windows
-            mingw_map = custom_target(libname + '_mingw',
-                    command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
-                    input: version_map,
-                    output: '@0@_mingw.map'.format(libname))
-            lk_deps += [mingw_map]

-            lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
-        else
-            lk_args = ['-Wl,--version-script=' + version_map]
+        if developer_mode and not is_windows
+            # on unix systems check the output of the
+            # check-symbols.sh script, using it as a
+            # dependency of the .so build
+            lk_deps += custom_target(name + '.sym_chk',
+                    command: [check_symbols,
+                        version_map, '@INPUT@'],
+                    capture: true,
+                    input: static_lib,
+                    output: name + '.sym_chk')
         endif
-    endif
-
-    if developer_mode and not is_windows
-        # on unix systems check the output of the
-        # check-symbols.sh script, using it as a
-        # dependency of the .so build
-        lk_deps += custom_target(name + '.sym_chk',
-                command: [check_symbols,
-                    version_map, '@INPUT@'],
-                capture: true,
-                input: static_lib,
-                output: name + '.sym_chk')
-    endif

-    shared_lib = shared_library(libname,
-            sources,
-            objects: objs,
-            c_args: cflags,
-            dependencies: shared_deps,
-            include_directories: includes,
-            link_args: lk_args,
-            link_depends: lk_deps,
-            version: abi_version,
-            soversion: so_version,
-            install: true)
-    shared_dep = declare_dependency(link_with: shared_lib,
-            include_directories: includes,
-            dependencies: shared_deps)
+        shared_lib = shared_library(libname,
+                sources,
+                objects: objs,
+                c_args: cflags,
+                dependencies: shared_deps,
+                include_directories: includes,
+                link_args: lk_args,
+                link_depends: lk_deps,
+                version: abi_version,
+                soversion: so_version,
+                install: true)
+        shared_dep = declare_dependency(link_with: shared_lib,
+                include_directories: includes,
+                dependencies: shared_deps)

-    dpdk_libraries = [shared_lib] + dpdk_libraries
-    dpdk_static_libraries = [static_lib] + dpdk_static_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)
+        static_dep = shared_dep
+    endif

     set_variable('shared_rte_' + name, shared_dep)
     set_variable('static_rte_' + name, static_dep)