DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] build and install only one library type on Windows
@ 2024-03-14 19:44 Tyler Retzlaff
  2024-03-14 19:44 ` [PATCH] build: build " Tyler Retzlaff
                   ` (4 more replies)
  0 siblings, 5 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-03-14 19:44 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant

MSVC does not guarantee any kind of compatibility for static libraries so
they should not be packaged and instead they should be rebuilt along with
the application using the same version of the compiler & linker.  Because
of this we have chosen to build only one library type at a time instead of
compiling twice to produce both types in a single pass.

Tyler Retzlaff (1):
  build: build only one library type on Windows

 config/meson.build                    | 20 +++++++
 drivers/meson.build                   | 51 ++++++++++--------
 drivers/net/octeontx/base/meson.build |  2 +-
 lib/meson.build                       | 98 +++++++++++++++++++----------------
 4 files changed, 101 insertions(+), 70 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] build: build only one library type on Windows
  2024-03-14 19:44 [PATCH] build and install only one library type on Windows Tyler Retzlaff
@ 2024-03-14 19:44 ` Tyler Retzlaff
  2024-03-15  6:30 ` [PATCH v2 0/2] build and install " Tyler Retzlaff
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-03-14 19:44 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 config/meson.build                    | 20 +++++++
 drivers/meson.build                   | 51 ++++++++++--------
 drivers/net/octeontx/base/meson.build |  2 +-
 lib/meson.build                       | 98 +++++++++++++++++++----------------
 4 files changed, 101 insertions(+), 70 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 8c8b019..2090846 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -516,4 +516,24 @@ if get_option('default_library') == 'both'
  NOTE: DPDK always builds both shared and static libraries.  Please set
  "default_library" to either "static" or "shared" to select default linkage
  for apps and any examples.''')
+elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
+    error( '''
+ Unsupported value "shared" for "default_library" option.
+
+ NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
+ "default_library" to either "static" or use MSVC.''')
+endif
+
+if is_windows and not is_ms_compiler
+    is_shared_enabled=false
+else
+    is_shared_enabled=true
+endif
+
+if is_windows
+    install_static = get_option('default_library') == 'static'
+    install_shared = get_option('default_library') == 'shared'
+else
+    install_static=true
+    install_shared=true
 endif
diff --git a/drivers/meson.build b/drivers/meson.build
index 66931d4..e7cfda6 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -80,7 +80,7 @@ foreach subpath:subdirs
         subdir(class)
         skip_class = false
         foreach d:std_deps
-            if not is_variable('shared_rte_' + d)
+            if not is_variable('static_rte_' + d)
                 skip_class = true
                 reason = 'missing internal dependency, "@0@"'.format(d)
                 if dpdk_libs_deprecated.contains(d)
@@ -173,7 +173,7 @@ foreach subpath:subdirs
                 if not build
                     break
                 endif
-                if not is_variable('shared_rte_' + d)
+                if not is_variable('static_rte_' + d)
                     build = false
                     reason = 'missing internal dependency, "@0@"'.format(d)
                     if dpdk_libs_deprecated.contains(d)
@@ -188,7 +188,9 @@ foreach subpath:subdirs
                                 +'\tPlease enable missing dependency "@0@"'.format(d))
                     endif
                 else
-                    shared_deps += [get_variable('shared_rte_' + d)]
+                    if is_shared_enabled
+                        shared_deps += [get_variable('shared_rte_' + d)]
+                    endif
                     static_deps += [get_variable('static_rte_' + d)]
                 endif
             endforeach
@@ -247,7 +249,7 @@ foreach subpath:subdirs
                 include_directories: includes,
                 dependencies: static_deps,
                 c_args: cflags,
-                install: true)
+                install: install_static)
 
         # now build the shared driver
         version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
@@ -271,7 +273,7 @@ foreach subpath:subdirs
             endif
         endif
 
-        if is_windows
+        if is_windows and is_shared_enabled
             if is_ms_linker
                 def_file = custom_target(lib_name + '_def',
                         command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
@@ -296,30 +298,33 @@ foreach subpath:subdirs
             lk_args = ['-Wl,--version-script=' + version_map]
         endif
 
-        shared_lib = shared_library(lib_name, sources,
-                objects: objs,
-                include_directories: includes,
-                dependencies: shared_deps,
-                c_args: cflags,
-                link_args: lk_args,
-                link_depends: lk_deps,
-                version: abi_version,
-                soversion: so_version,
-                install: true,
-                install_dir: driver_install_path)
-
-        # create a dependency object and add it to the global dictionary so
-        # testpmd or other built-in apps can find it if necessary
-        shared_dep = declare_dependency(link_with: shared_lib,
-                include_directories: includes,
-                dependencies: shared_deps)
+        if is_shared_enabled
+            shared_lib = shared_library(lib_name, sources,
+                    objects: objs,
+                    include_directories: includes,
+                    dependencies: shared_deps,
+                    c_args: cflags,
+                    link_args: lk_args,
+                    link_depends: lk_deps,
+                    version: abi_version,
+                    soversion: so_version,
+                    install: install_shared,
+                    install_dir: driver_install_path)
+
+            # create a dependency object and add it to the global dictionary so
+            # testpmd or other built-in apps can find it if necessary
+            shared_dep = declare_dependency(link_with: shared_lib,
+                    include_directories: includes,
+                    dependencies: shared_deps)
+
+            set_variable('shared_@0@'.format(lib_name), shared_dep)
+        endif
         static_dep = declare_dependency(
                 include_directories: includes,
                 dependencies: static_deps)
 
         dpdk_drivers += static_lib
 
-        set_variable('shared_@0@'.format(lib_name), shared_dep)
         set_variable('static_@0@'.format(lib_name), static_dep)
         # for drivers, we only need to add dependency objects for static libs,
         # shared lib drivers are not linked in
diff --git a/drivers/net/octeontx/base/meson.build b/drivers/net/octeontx/base/meson.build
index 8e5e8c1..a793c19 100644
--- a/drivers/net/octeontx/base/meson.build
+++ b/drivers/net/octeontx/base/meson.build
@@ -10,7 +10,7 @@ sources = [
 depends = ['ethdev', 'mempool_octeontx']
 static_objs = []
 foreach d: depends
-    if not is_variable('shared_rte_' + d)
+    if not is_variable('static_rte_' + d)
         subdir_done()
     endif
     static_objs += get_variable('static_rte_' + d)
diff --git a/lib/meson.build b/lib/meson.build
index 179a272..e92ba35 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -184,7 +184,7 @@ foreach l:libraries
         if not build
             break
         endif
-        if not is_variable('shared_rte_' + d)
+        if not is_variable('static_rte_' + d)
             build = false
             reason = 'missing internal dependency, "@0@"'.format(d)
             if dpdk_libs_deprecated.contains(d)
@@ -197,7 +197,9 @@ foreach l:libraries
                         + '\tPlease add missing dependency "@0@" to "enable_libs" option'.format(d))
             endif
         else
-            shared_deps += [get_variable('shared_rte_' + d)]
+            if is_shared_enabled
+                shared_deps += [get_variable('shared_rte_' + d)]
+            endif
             static_deps += [get_variable('static_rte_' + d)]
         endif
     endforeach
@@ -242,7 +244,7 @@ foreach l:libraries
             c_args: cflags,
             dependencies: static_deps,
             include_directories: includes,
-            install: true)
+            install: install_static)
     static_dep = declare_dependency(
             include_directories: includes,
             dependencies: static_deps)
@@ -260,35 +262,37 @@ foreach l:libraries
     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_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
-        if is_windows
-            mingw_map = custom_target(libname + '_mingw',
+    if is_shared_enabled
+        if is_ms_linker
+            def_file = custom_target(libname + '_def',
                     command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
                     input: version_map,
-                    output: '@0@_mingw.map'.format(libname))
-            lk_deps += [mingw_map]
+                    output: '@0@_exports.def'.format(libname))
+            lk_deps += [def_file]
 
-            lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+            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,--version-script=' + version_map]
+            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
     endif
 
@@ -304,27 +308,29 @@ foreach l:libraries
                 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)
+    if is_shared_enabled
+        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: install_shared)
+        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
+        set_variable('shared_rte_' + name, shared_dep)
+        dpdk_shared_lib_deps += shared_dep
+    endif
 
-    set_variable('shared_rte_' + name, shared_dep)
+    dpdk_static_libraries = [static_lib] + dpdk_static_libraries
     set_variable('static_rte_' + name, static_dep)
-    dpdk_shared_lib_deps += shared_dep
     dpdk_static_lib_deps += static_dep
     if developer_mode
         message('lib/@0@: Defining dependency "@1@"'.format(l, name))
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 0/2] build and install only one library type on Windows
  2024-03-14 19:44 [PATCH] build and install only one library type on Windows Tyler Retzlaff
  2024-03-14 19:44 ` [PATCH] build: build " Tyler Retzlaff
@ 2024-03-15  6:30 ` Tyler Retzlaff
  2024-03-15  6:30   ` [PATCH v2 1/2] build: build " Tyler Retzlaff
  2024-03-15  6:30   ` [PATCH v2 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  2024-04-12 22:52 ` [PATCH v3 0/2] build and install only one library type on Windows Tyler Retzlaff
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-03-15  6:30 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant

MSVC does not guarantee any kind of compatibility for static libraries so
they should not be packaged and instead they should be rebuilt along with
the application using the same version of the compiler & linker.  Because
of this we have chosen to build only one library type at a time instead of
compiling twice to produce both types in a single pass.

v2:
  * add patch to chkings to use static dependencies when building static
    cross build for windows target with mingw.
  * add missing msvc specific ldflags needed for shared builds.
  * define RTE_BUILD_SHARED_LIB for shared msvc builds.
  * fix is_shared_enabled conditional to check default_library=shared
    when building with msvc.
  * simplify linker flags when building for windows, we only need to
    provide flags for shared builds when building with msvc.
  * stop generating .def file for llvm and mingw builds, since no shared
    builds are performed the .def files would be unused.

Tyler Retzlaff (2):
  build: build only one library type on Windows
  buildtools: when building static library use static deps

 app/meson.build                       |  6 +++
 buildtools/chkincs/meson.build        | 18 +++++---
 config/meson.build                    | 27 ++++++++++++
 drivers/meson.build                   | 72 +++++++++++++++----------------
 drivers/net/octeontx/base/meson.build |  2 +-
 examples/meson.build                  |  6 +++
 lib/meson.build                       | 79 +++++++++++++++--------------------
 7 files changed, 121 insertions(+), 89 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 1/2] build: build only one library type on Windows
  2024-03-15  6:30 ` [PATCH v2 0/2] build and install " Tyler Retzlaff
@ 2024-03-15  6:30   ` Tyler Retzlaff
  2024-04-12 13:00     ` Bruce Richardson
  2024-03-15  6:30   ` [PATCH v2 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  1 sibling, 1 reply; 25+ messages in thread
From: Tyler Retzlaff @ 2024-03-15  6:30 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/meson.build                       |  6 +++
 config/meson.build                    | 27 ++++++++++++
 drivers/meson.build                   | 72 +++++++++++++++----------------
 drivers/net/octeontx/base/meson.build |  2 +-
 examples/meson.build                  |  6 +++
 lib/meson.build                       | 79 +++++++++++++++--------------------
 6 files changed, 108 insertions(+), 84 deletions(-)

diff --git a/app/meson.build b/app/meson.build
index 21b6da2..f4ed0f1 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
 default_ldflags = []
 if get_option('default_library') == 'static' and not is_windows
     default_ldflags += ['-Wl,--export-dynamic']
+elif get_option('default_library') == 'shared' and is_ms_compiler
+    default_ldflags += ['/experimental:tlsDllInterface']
 endif
 
 foreach app:apps
@@ -104,6 +106,10 @@ foreach app:apps
         link_libs = dpdk_static_libraries + dpdk_drivers
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     exec = executable('dpdk-' + name,
             sources,
             c_args: cflags,
diff --git a/config/meson.build b/config/meson.build
index 8c8b019..dd7971e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -53,6 +53,9 @@ if is_ms_compiler
 
     # enable statement expressions extension
     add_project_arguments('/experimental:statementExpressions', language: 'c')
+
+    # enable export of thread_local variables
+    add_project_arguments('/experimental:tlsDllInterface', language: 'c')
 endif
 
 # set the major version, which might be used by drivers and libraries
@@ -516,4 +519,28 @@ if get_option('default_library') == 'both'
  NOTE: DPDK always builds both shared and static libraries.  Please set
  "default_library" to either "static" or "shared" to select default linkage
  for apps and any examples.''')
+elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
+    error( '''
+ Unsupported value "shared" for "default_library" option.
+
+ NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
+ "default_library" to either "static" or use MSVC.''')
+endif
+
+if is_windows
+    if is_ms_compiler and get_option('default_library') == 'shared'
+        is_shared_enabled=true
+    else
+        is_shared_enabled=false
+    endif
+else
+    is_shared_enabled=true
+endif
+
+if is_windows
+    install_static = get_option('default_library') == 'static'
+    install_shared = get_option('default_library') == 'shared'
+else
+    install_static=true
+    install_shared=true
 endif
diff --git a/drivers/meson.build b/drivers/meson.build
index 66931d4..c8b2d13 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -80,7 +80,7 @@ foreach subpath:subdirs
         subdir(class)
         skip_class = false
         foreach d:std_deps
-            if not is_variable('shared_rte_' + d)
+            if not is_variable('static_rte_' + d)
                 skip_class = true
                 reason = 'missing internal dependency, "@0@"'.format(d)
                 if dpdk_libs_deprecated.contains(d)
@@ -173,7 +173,7 @@ foreach subpath:subdirs
                 if not build
                     break
                 endif
-                if not is_variable('shared_rte_' + d)
+                if not is_variable('static_rte_' + d)
                     build = false
                     reason = 'missing internal dependency, "@0@"'.format(d)
                     if dpdk_libs_deprecated.contains(d)
@@ -188,7 +188,9 @@ foreach subpath:subdirs
                                 +'\tPlease enable missing dependency "@0@"'.format(d))
                     endif
                 else
-                    shared_deps += [get_variable('shared_rte_' + d)]
+                    if is_shared_enabled
+                        shared_deps += [get_variable('shared_rte_' + d)]
+                    endif
                     static_deps += [get_variable('static_rte_' + d)]
                 endif
             endforeach
@@ -208,6 +210,9 @@ foreach subpath:subdirs
         enabled_drivers += name
         lib_name = '_'.join(['rte', class, name])
         cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
+        if is_windows and is_shared_enabled
+            cflags += '-DRTE_BUILD_SHARED_LIB'
+        endif
         if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
             cflags += '-DRTE_ANNOTATE_LOCKS'
             cflags += '-Wthread-safety'
@@ -247,7 +252,7 @@ foreach subpath:subdirs
                 include_directories: includes,
                 dependencies: static_deps,
                 c_args: cflags,
-                install: true)
+                install: install_static)
 
         # now build the shared driver
         version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
@@ -271,55 +276,46 @@ foreach subpath:subdirs
             endif
         endif
 
-        if is_windows
-            if is_ms_linker
+        if is_shared_enabled
+            if is_ms_compiler
                 def_file = custom_target(lib_name + '_def',
                         command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
                         input: version_map,
                         output: '@0@_exports.def'.format(lib_name))
                 lk_deps += [def_file]
-
-                lk_args = ['-Wl,/def:' + def_file.full_path()]
-                if meson.version().version_compare('<0.54.0')
-                    lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
-                endif
+                lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
             else
-                mingw_map = custom_target(lib_name + '_mingw',
-                        command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
-                        input: version_map,
-                        output: '@0@_mingw.map'.format(lib_name))
-                lk_deps += [mingw_map]
-
-                lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+                lk_args = ['-Wl,--version-script=' + version_map]
             endif
-        else
-            lk_args = ['-Wl,--version-script=' + version_map]
         endif
 
-        shared_lib = shared_library(lib_name, sources,
-                objects: objs,
-                include_directories: includes,
-                dependencies: shared_deps,
-                c_args: cflags,
-                link_args: lk_args,
-                link_depends: lk_deps,
-                version: abi_version,
-                soversion: so_version,
-                install: true,
-                install_dir: driver_install_path)
-
-        # create a dependency object and add it to the global dictionary so
-        # testpmd or other built-in apps can find it if necessary
-        shared_dep = declare_dependency(link_with: shared_lib,
-                include_directories: includes,
-                dependencies: shared_deps)
+        if is_shared_enabled
+            shared_lib = shared_library(lib_name, sources,
+                    objects: objs,
+                    include_directories: includes,
+                    dependencies: shared_deps,
+                    c_args: cflags,
+                    link_args: lk_args,
+                    link_depends: lk_deps,
+                    version: abi_version,
+                    soversion: so_version,
+                    install: install_shared,
+                    install_dir: driver_install_path)
+
+            # create a dependency object and add it to the global dictionary so
+            # testpmd or other built-in apps can find it if necessary
+            shared_dep = declare_dependency(link_with: shared_lib,
+                    include_directories: includes,
+                    dependencies: shared_deps)
+
+            set_variable('shared_@0@'.format(lib_name), shared_dep)
+        endif
         static_dep = declare_dependency(
                 include_directories: includes,
                 dependencies: static_deps)
 
         dpdk_drivers += static_lib
 
-        set_variable('shared_@0@'.format(lib_name), shared_dep)
         set_variable('static_@0@'.format(lib_name), static_dep)
         # for drivers, we only need to add dependency objects for static libs,
         # shared lib drivers are not linked in
diff --git a/drivers/net/octeontx/base/meson.build b/drivers/net/octeontx/base/meson.build
index 8e5e8c1..a793c19 100644
--- a/drivers/net/octeontx/base/meson.build
+++ b/drivers/net/octeontx/base/meson.build
@@ -10,7 +10,7 @@ sources = [
 depends = ['ethdev', 'mempool_octeontx']
 static_objs = []
 foreach d: depends
-    if not is_variable('shared_rte_' + d)
+    if not is_variable('static_rte_' + d)
         subdir_done()
     endif
     static_objs += get_variable('static_rte_' + d)
diff --git a/examples/meson.build b/examples/meson.build
index 8e8968a..e08a169 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -85,6 +85,8 @@ endif
 default_ldflags = dpdk_extra_ldflags
 if get_option('default_library') == 'static' and not is_windows
     default_ldflags += ['-Wl,--export-dynamic']
+elif is_ms_compiler
+    default_ldflags += ['/experimental:tlsDllInterface']
 endif
 
 foreach example: examples
@@ -121,6 +123,10 @@ foreach example: examples
         continue
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     if allow_experimental_apis
         cflags += '-DALLOW_EXPERIMENTAL_API'
     endif
diff --git a/lib/meson.build b/lib/meson.build
index 179a272..39014b4 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -184,7 +184,7 @@ foreach l:libraries
         if not build
             break
         endif
-        if not is_variable('shared_rte_' + d)
+        if not is_variable('static_rte_' + d)
             build = false
             reason = 'missing internal dependency, "@0@"'.format(d)
             if dpdk_libs_deprecated.contains(d)
@@ -197,7 +197,9 @@ foreach l:libraries
                         + '\tPlease add missing dependency "@0@" to "enable_libs" option'.format(d))
             endif
         else
-            shared_deps += [get_variable('shared_rte_' + d)]
+            if is_shared_enabled
+                shared_deps += [get_variable('shared_rte_' + d)]
+            endif
             static_deps += [get_variable('static_rte_' + d)]
         endif
     endforeach
@@ -235,6 +237,10 @@ foreach l:libraries
         cflags += '-Wthread-safety'
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     # first build static lib
     static_lib = static_library(libname,
             sources,
@@ -242,7 +248,7 @@ foreach l:libraries
             c_args: cflags,
             dependencies: static_deps,
             include_directories: includes,
-            install: true)
+            install: install_static)
     static_dep = declare_dependency(
             include_directories: includes,
             dependencies: static_deps)
@@ -260,33 +266,14 @@ foreach l:libraries
     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_shared_enabled
         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
-        if is_windows
-            mingw_map = custom_target(libname + '_mingw',
+            def_file = custom_target(libname + '_def',
                     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()]
+                    output: '@0@_exports.def'.format(libname))
+            lk_deps += [def_file]
+            lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
         else
             lk_args = ['-Wl,--version-script=' + version_map]
         endif
@@ -304,27 +291,29 @@ foreach l:libraries
                 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)
+    if is_shared_enabled
+        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: install_shared)
+        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
+        set_variable('shared_rte_' + name, shared_dep)
+        dpdk_shared_lib_deps += shared_dep
+    endif
 
-    set_variable('shared_rte_' + name, shared_dep)
+    dpdk_static_libraries = [static_lib] + dpdk_static_libraries
     set_variable('static_rte_' + name, static_dep)
-    dpdk_shared_lib_deps += shared_dep
     dpdk_static_lib_deps += static_dep
     if developer_mode
         message('lib/@0@: Defining dependency "@1@"'.format(l, name))
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/2] buildtools: when building static library use static deps
  2024-03-15  6:30 ` [PATCH v2 0/2] build and install " Tyler Retzlaff
  2024-03-15  6:30   ` [PATCH v2 1/2] build: build " Tyler Retzlaff
@ 2024-03-15  6:30   ` Tyler Retzlaff
  2024-03-15  8:28     ` Bruce Richardson
  2024-04-12 14:09     ` Bruce Richardson
  1 sibling, 2 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-03-15  6:30 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

Use static deps when default_library=static and use shared deps when
using default_library=shared.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 buildtools/chkincs/meson.build | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index f2dadca..66245a5 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -20,11 +20,19 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
 
 # some driver SDK headers depend on these two buses, which are mandatory in build
 # so we always include them in deps list
-deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
-# add the rest of the libs to the dependencies
-foreach l:dpdk_libs_enabled
-    deps += get_variable('shared_rte_' + l)
-endforeach
+if is_shared_enabled
+    deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
+    # add the rest of the libs to the dependencies
+    foreach l:dpdk_libs_enabled
+        deps += get_variable('shared_rte_' + l)
+    endforeach
+else
+    deps = [get_variable('static_rte_bus_vdev'), get_variable('static_rte_bus_pci')]
+    # add the rest of the libs to the dependencies
+    foreach l:dpdk_libs_enabled
+        deps += get_variable('static_rte_' + l)
+    endforeach
+endif
 
 executable('chkincs', sources,
         c_args: cflags,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/2] buildtools: when building static library use static deps
  2024-03-15  6:30   ` [PATCH v2 2/2] buildtools: when building static library use static deps Tyler Retzlaff
@ 2024-03-15  8:28     ` Bruce Richardson
  2024-04-04 18:47       ` Tyler Retzlaff
  2024-04-12 14:09     ` Bruce Richardson
  1 sibling, 1 reply; 25+ messages in thread
From: Bruce Richardson @ 2024-03-15  8:28 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev, Harman Kalra

On Thu, Mar 14, 2024 at 11:30:22PM -0700, Tyler Retzlaff wrote:
> Use static deps when default_library=static and use shared deps when
> using default_library=shared.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  buildtools/chkincs/meson.build | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> index f2dadca..66245a5 100644
> --- a/buildtools/chkincs/meson.build
> +++ b/buildtools/chkincs/meson.build
> @@ -20,11 +20,19 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
>  
>  # some driver SDK headers depend on these two buses, which are mandatory in build
>  # so we always include them in deps list
> -deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> -# add the rest of the libs to the dependencies
> -foreach l:dpdk_libs_enabled
> -    deps += get_variable('shared_rte_' + l)
> -endforeach
> +if is_shared_enabled
> +    deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> +    # add the rest of the libs to the dependencies
> +    foreach l:dpdk_libs_enabled
> +        deps += get_variable('shared_rte_' + l)
> +    endforeach
> +else
> +    deps = [get_variable('static_rte_bus_vdev'), get_variable('static_rte_bus_pci')]
> +    # add the rest of the libs to the dependencies
> +    foreach l:dpdk_libs_enabled
> +        deps += get_variable('static_rte_' + l)
> +    endforeach
> +endif
>  

Should the deps checks in lib and driver not be similarly conditional?

>  executable('chkincs', sources,
>          c_args: cflags,
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/2] buildtools: when building static library use static deps
  2024-03-15  8:28     ` Bruce Richardson
@ 2024-04-04 18:47       ` Tyler Retzlaff
  2024-04-05  9:11         ` Bruce Richardson
  0 siblings, 1 reply; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-04 18:47 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra

On Fri, Mar 15, 2024 at 08:28:55AM +0000, Bruce Richardson wrote:
> On Thu, Mar 14, 2024 at 11:30:22PM -0700, Tyler Retzlaff wrote:
> > Use static deps when default_library=static and use shared deps when
> > using default_library=shared.
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> >  buildtools/chkincs/meson.build | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> > index f2dadca..66245a5 100644
> > --- a/buildtools/chkincs/meson.build
> > +++ b/buildtools/chkincs/meson.build
> > @@ -20,11 +20,19 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
> >  
> >  # some driver SDK headers depend on these two buses, which are mandatory in build
> >  # so we always include them in deps list
> > -deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> > -# add the rest of the libs to the dependencies
> > -foreach l:dpdk_libs_enabled
> > -    deps += get_variable('shared_rte_' + l)
> > -endforeach
> > +if is_shared_enabled
> > +    deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> > +    # add the rest of the libs to the dependencies
> > +    foreach l:dpdk_libs_enabled
> > +        deps += get_variable('shared_rte_' + l)
> > +    endforeach
> > +else
> > +    deps = [get_variable('static_rte_bus_vdev'), get_variable('static_rte_bus_pci')]
> > +    # add the rest of the libs to the dependencies
> > +    foreach l:dpdk_libs_enabled
> > +        deps += get_variable('static_rte_' + l)
> > +    endforeach
> > +endif
> >  
> 
> Should the deps checks in lib and driver not be similarly conditional?

i might need more context about what you think is wrong. i think the
problem here was that there are not separate variables for shared_deps
vs static_deps.

for my changes to lib/meson.build there's already a separate shared_deps
and static_deps variables holding the deps that are used with
shared_library and static_library respectively.

ty


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/2] buildtools: when building static library use static deps
  2024-04-04 18:47       ` Tyler Retzlaff
@ 2024-04-05  9:11         ` Bruce Richardson
  0 siblings, 0 replies; 25+ messages in thread
From: Bruce Richardson @ 2024-04-05  9:11 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev, Harman Kalra

On Thu, Apr 04, 2024 at 11:47:49AM -0700, Tyler Retzlaff wrote:
> On Fri, Mar 15, 2024 at 08:28:55AM +0000, Bruce Richardson wrote:
> > On Thu, Mar 14, 2024 at 11:30:22PM -0700, Tyler Retzlaff wrote:
> > > Use static deps when default_library=static and use shared deps when
> > > using default_library=shared.
> > > 
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > ---
> > >  buildtools/chkincs/meson.build | 18 +++++++++++++-----
> > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> > > index f2dadca..66245a5 100644
> > > --- a/buildtools/chkincs/meson.build
> > > +++ b/buildtools/chkincs/meson.build
> > > @@ -20,11 +20,19 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
> > >  
> > >  # some driver SDK headers depend on these two buses, which are mandatory in build
> > >  # so we always include them in deps list
> > > -deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> > > -# add the rest of the libs to the dependencies
> > > -foreach l:dpdk_libs_enabled
> > > -    deps += get_variable('shared_rte_' + l)
> > > -endforeach
> > > +if is_shared_enabled
> > > +    deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> > > +    # add the rest of the libs to the dependencies
> > > +    foreach l:dpdk_libs_enabled
> > > +        deps += get_variable('shared_rte_' + l)
> > > +    endforeach
> > > +else
> > > +    deps = [get_variable('static_rte_bus_vdev'), get_variable('static_rte_bus_pci')]
> > > +    # add the rest of the libs to the dependencies
> > > +    foreach l:dpdk_libs_enabled
> > > +        deps += get_variable('static_rte_' + l)
> > > +    endforeach
> > > +endif
> > >  
> > 
> > Should the deps checks in lib and driver not be similarly conditional?
> 
> i might need more context about what you think is wrong. i think the
> problem here was that there are not separate variables for shared_deps
> vs static_deps.
> 
> for my changes to lib/meson.build there's already a separate shared_deps
> and static_deps variables holding the deps that are used with
> shared_library and static_library respectively.
> 
> ty
> 
Not sure I know the context of the question myself, but your answer
certainly seems reasonable. :-) Thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/2] build: build only one library type on Windows
  2024-03-15  6:30   ` [PATCH v2 1/2] build: build " Tyler Retzlaff
@ 2024-04-12 13:00     ` Bruce Richardson
  2024-04-12 23:04       ` Tyler Retzlaff
  0 siblings, 1 reply; 25+ messages in thread
From: Bruce Richardson @ 2024-04-12 13:00 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev, Harman Kalra

On Thu, Mar 14, 2024 at 11:30:21PM -0700, Tyler Retzlaff wrote:
> MSVC is the only compiler that can produce usable shared libraries for
> DPDK on Windows because of the use of exported TLS variables.
> 
> Disable building of shared libraries with LLVM and MinGW so that
> remaining __declspec macros needed for the functional libraries built by
> MSVC can be used without triggering errors in LLVM and MinGW builds.
> 
> For Windows only install the default_library type to avoid confusion.
> Windows builds cannot build both shared and static in a single pass so
> install only the functional variant.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Comments inline below.

/Bruce

> ---
>  app/meson.build                       |  6 +++
>  config/meson.build                    | 27 ++++++++++++
>  drivers/meson.build                   | 72 +++++++++++++++----------------
>  drivers/net/octeontx/base/meson.build |  2 +-
>  examples/meson.build                  |  6 +++
>  lib/meson.build                       | 79 +++++++++++++++--------------------
>  6 files changed, 108 insertions(+), 84 deletions(-)
> 
> diff --git a/app/meson.build b/app/meson.build
> index 21b6da2..f4ed0f1 100644
> --- a/app/meson.build
> +++ b/app/meson.build
> @@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
>  default_ldflags = []
>  if get_option('default_library') == 'static' and not is_windows
>      default_ldflags += ['-Wl,--export-dynamic']
> +elif get_option('default_library') == 'shared' and is_ms_compiler
> +    default_ldflags += ['/experimental:tlsDllInterface']
>  endif
>  

Is this necessary, given that you add this flag as a project-wide argument
below in config/meson.build?

>  foreach app:apps
> @@ -104,6 +106,10 @@ foreach app:apps
>          link_libs = dpdk_static_libraries + dpdk_drivers
>      endif
>  
> +    if is_windows and is_shared_enabled
> +        cflags += '-DRTE_BUILD_SHARED_LIB'
> +    endif
> +
>      exec = executable('dpdk-' + name,
>              sources,
>              c_args: cflags,
> diff --git a/config/meson.build b/config/meson.build
> index 8c8b019..dd7971e 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -53,6 +53,9 @@ if is_ms_compiler
>  
>      # enable statement expressions extension
>      add_project_arguments('/experimental:statementExpressions', language: 'c')
> +
> +    # enable export of thread_local variables
> +    add_project_arguments('/experimental:tlsDllInterface', language: 'c')
>  endif
>  
>  # set the major version, which might be used by drivers and libraries
> @@ -516,4 +519,28 @@ if get_option('default_library') == 'both'
>   NOTE: DPDK always builds both shared and static libraries.  Please set
>   "default_library" to either "static" or "shared" to select default linkage
>   for apps and any examples.''')
> +elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
> +    error( '''
> + Unsupported value "shared" for "default_library" option.
> +
> + NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
> + "default_library" to either "static" or use MSVC.''')
> +endif
> +
> +if is_windows
> +    if is_ms_compiler and get_option('default_library') == 'shared'
> +        is_shared_enabled=true
> +    else
> +        is_shared_enabled=false
> +    endif
> +else
> +    is_shared_enabled=true
> +endif
> +

This can be shortened to:

is_shared_enabled=true
if is_windows and (not is_ms_compiler or not get_option(...) == 'shared')
	is_shared_enabled=false
endif


> +if is_windows
> +    install_static = get_option('default_library') == 'static'
> +    install_shared = get_option('default_library') == 'shared'
> +else
> +    install_static=true
> +    install_shared=true
>  endif

Might be better merged with the block above:

is_shared_enabled = true
install_static = true
install_shared = true
if is_windows
    install_static = get_option('default_library') == 'static'
    install_shared = get_option('default_library') == 'shared'

    if not is_ms_compiler or not install_shared
        is_shared_enabled = false
    endif
endif

> diff --git a/drivers/meson.build b/drivers/meson.build
> index 66931d4..c8b2d13 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -80,7 +80,7 @@ foreach subpath:subdirs
>          subdir(class)
>          skip_class = false
>          foreach d:std_deps
> -            if not is_variable('shared_rte_' + d)
> +            if not is_variable('static_rte_' + d)
>                  skip_class = true
>                  reason = 'missing internal dependency, "@0@"'.format(d)
>                  if dpdk_libs_deprecated.contains(d)
> @@ -173,7 +173,7 @@ foreach subpath:subdirs
>                  if not build
>                      break
>                  endif
> -                if not is_variable('shared_rte_' + d)
> +                if not is_variable('static_rte_' + d)
>                      build = false
>                      reason = 'missing internal dependency, "@0@"'.format(d)
>                      if dpdk_libs_deprecated.contains(d)
> @@ -188,7 +188,9 @@ foreach subpath:subdirs
>                                  +'\tPlease enable missing dependency "@0@"'.format(d))
>                      endif
>                  else
> -                    shared_deps += [get_variable('shared_rte_' + d)]
> +                    if is_shared_enabled
> +                        shared_deps += [get_variable('shared_rte_' + d)]
> +                    endif
>                      static_deps += [get_variable('static_rte_' + d)]
>                  endif
>              endforeach
> @@ -208,6 +210,9 @@ foreach subpath:subdirs
>          enabled_drivers += name
>          lib_name = '_'.join(['rte', class, name])
>          cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
> +        if is_windows and is_shared_enabled
> +            cflags += '-DRTE_BUILD_SHARED_LIB'
> +        endif

Rather than setting this in cflags in the drivers or lib meson.build files,
can we set this once as a project-level cflag and be done with it?

>          if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
>              cflags += '-DRTE_ANNOTATE_LOCKS'
>              cflags += '-Wthread-safety'
> @@ -247,7 +252,7 @@ foreach subpath:subdirs
>                  include_directories: includes,
>                  dependencies: static_deps,
>                  c_args: cflags,
> -                install: true)
> +                install: install_static)
>  
>          # now build the shared driver
>          version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
> @@ -271,55 +276,46 @@ foreach subpath:subdirs
>              endif
>          endif
>  
> -        if is_windows
> -            if is_ms_linker
> +        if is_shared_enabled
> +            if is_ms_compiler
>                  def_file = custom_target(lib_name + '_def',
>                          command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
>                          input: version_map,
>                          output: '@0@_exports.def'.format(lib_name))
>                  lk_deps += [def_file]
> -
> -                lk_args = ['-Wl,/def:' + def_file.full_path()]
> -                if meson.version().version_compare('<0.54.0')
> -                    lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
> -                endif
> +                lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]

Again, the /experimental flag shouldn't be necessary, since it's set as
project option in config/meson.build.

>              else
> -                mingw_map = custom_target(lib_name + '_mingw',
> -                        command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> -                        input: version_map,
> -                        output: '@0@_mingw.map'.format(lib_name))
> -                lk_deps += [mingw_map]
> -
> -                lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
> +                lk_args = ['-Wl,--version-script=' + version_map]
>              endif
> -        else
> -            lk_args = ['-Wl,--version-script=' + version_map]
>          endif
>  
> -        shared_lib = shared_library(lib_name, sources,
> -                objects: objs,
> -                include_directories: includes,
> -                dependencies: shared_deps,
> -                c_args: cflags,
> -                link_args: lk_args,
> -                link_depends: lk_deps,
> -                version: abi_version,
> -                soversion: so_version,
> -                install: true,
> -                install_dir: driver_install_path)
> -
> -        # create a dependency object and add it to the global dictionary so
> -        # testpmd or other built-in apps can find it if necessary
> -        shared_dep = declare_dependency(link_with: shared_lib,
> -                include_directories: includes,
> -                dependencies: shared_deps)
> +        if is_shared_enabled
> +            shared_lib = shared_library(lib_name, sources,
> +                    objects: objs,
> +                    include_directories: includes,
> +                    dependencies: shared_deps,
> +                    c_args: cflags,
> +                    link_args: lk_args,
> +                    link_depends: lk_deps,
> +                    version: abi_version,
> +                    soversion: so_version,
> +                    install: install_shared,
> +                    install_dir: driver_install_path)
> +
> +            # create a dependency object and add it to the global dictionary so
> +            # testpmd or other built-in apps can find it if necessary
> +            shared_dep = declare_dependency(link_with: shared_lib,
> +                    include_directories: includes,
> +                    dependencies: shared_deps)
> +
> +            set_variable('shared_@0@'.format(lib_name), shared_dep)

Just a thought - could you avoid some of the changes throughout this set,
if you add an else-leg to the "is_shared_enabled" check, where, if shared
is not enabled, you assign shared_dep to be an empty string, or an empty
object or something.

That would mean that you wouldn't need to move the set_variable line as
here, you also wouldn't need to put conditionals around all the

Just a thought - could you avoid some of the changes throughout this set,
if you add an else-leg to the "is_shared_enabled" check, where, if shared
is not enabled, you assign shared_dep to be an empty string, or an empty
object or something.

That would mean that you wouldn't need to move the set_variable line as
here, you also wouldn't need to put conditionals around all the
'shared_deps +=' lines when processing deps for a component, and lastly,
you may not even need to change all the searches for shared_rte_ to
static_rte, since the variables would exist for shared - they'd just be
empty!

> +        endif
>          static_dep = declare_dependency(
>                  include_directories: includes,
>                  dependencies: static_deps)
>  
>          dpdk_drivers += static_lib

<snip for brevity>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/2] buildtools: when building static library use static deps
  2024-03-15  6:30   ` [PATCH v2 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  2024-03-15  8:28     ` Bruce Richardson
@ 2024-04-12 14:09     ` Bruce Richardson
  2024-04-12 22:52       ` Tyler Retzlaff
  1 sibling, 1 reply; 25+ messages in thread
From: Bruce Richardson @ 2024-04-12 14:09 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev, Harman Kalra

On Thu, Mar 14, 2024 at 11:30:22PM -0700, Tyler Retzlaff wrote:
> Use static deps when default_library=static and use shared deps when
> using default_library=shared.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  buildtools/chkincs/meson.build | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> index f2dadca..66245a5 100644
> --- a/buildtools/chkincs/meson.build
> +++ b/buildtools/chkincs/meson.build
> @@ -20,11 +20,19 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
>  
>  # some driver SDK headers depend on these two buses, which are mandatory in build
>  # so we always include them in deps list
> -deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> -# add the rest of the libs to the dependencies
> -foreach l:dpdk_libs_enabled
> -    deps += get_variable('shared_rte_' + l)
> -endforeach

shorter fix might be to use get_option('default_library') in place of the
"shared" string, rather than using a condition.

/Bruce

> +if is_shared_enabled
> +    deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> +    # add the rest of the libs to the dependencies
> +    foreach l:dpdk_libs_enabled
> +        deps += get_variable('shared_rte_' + l)
> +    endforeach
> +else
> +    deps = [get_variable('static_rte_bus_vdev'), get_variable('static_rte_bus_pci')]
> +    # add the rest of the libs to the dependencies
> +    foreach l:dpdk_libs_enabled
> +        deps += get_variable('static_rte_' + l)
> +    endforeach
> +endif
>  
>  executable('chkincs', sources,
>          c_args: cflags,
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 0/2] build and install only one library type on Windows
  2024-03-14 19:44 [PATCH] build and install only one library type on Windows Tyler Retzlaff
  2024-03-14 19:44 ` [PATCH] build: build " Tyler Retzlaff
  2024-03-15  6:30 ` [PATCH v2 0/2] build and install " Tyler Retzlaff
@ 2024-04-12 22:52 ` Tyler Retzlaff
  2024-04-12 22:52   ` [PATCH v3 1/2] build: build " Tyler Retzlaff
  2024-04-12 22:52   ` [PATCH v3 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  2024-04-15 16:45 ` [PATCH v4 0/2] build and install only one library type on Windows Tyler Retzlaff
  2024-04-15 17:12 ` [PATCH v5 0/2] build and install only one library type on Windows Tyler Retzlaff
  4 siblings, 2 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-12 22:52 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant

MSVC does not guarantee any kind of compatibility for static libraries so
they should not be packaged and instead they should be rebuilt along with
the application using the same version of the compiler & linker.  Because
of this we have chosen to build only one library type at a time instead of
compiling twice to produce both types in a single pass.

v3:
  * condense library, target and toolchain type logic determining values for
    is_shared_enabled,install_{static,shared} values.
  * define empty shared_dep object to allow unconditional evaluation of shared
    variables even when is_shared_enabled is false.
  * use default_library() string to determine deps for chkincs.

Tyler Retzlaff (2):
  build: build only one library type on Windows
  buildtools: when building static library use static deps

 app/meson.build                       |  6 +++
 buildtools/chkincs/meson.build        |  5 ++-
 config/meson.build                    | 20 ++++++++++
 drivers/meson.build                   | 64 +++++++++++++++----------------
 drivers/net/octeontx/base/meson.build |  2 +-
 examples/meson.build                  |  6 +++
 lib/meson.build                       | 72 +++++++++++++++--------------------
 7 files changed, 97 insertions(+), 78 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/2] build: build only one library type on Windows
  2024-04-12 22:52 ` [PATCH v3 0/2] build and install only one library type on Windows Tyler Retzlaff
@ 2024-04-12 22:52   ` Tyler Retzlaff
  2024-04-15  9:55     ` Bruce Richardson
  2024-04-12 22:52   ` [PATCH v3 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  1 sibling, 1 reply; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-12 22:52 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/meson.build                       |  6 +++
 config/meson.build                    | 20 ++++++++++
 drivers/meson.build                   | 64 +++++++++++++++----------------
 drivers/net/octeontx/base/meson.build |  2 +-
 examples/meson.build                  |  6 +++
 lib/meson.build                       | 72 +++++++++++++++--------------------
 6 files changed, 94 insertions(+), 76 deletions(-)

diff --git a/app/meson.build b/app/meson.build
index 21b6da2..f4ed0f1 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
 default_ldflags = []
 if get_option('default_library') == 'static' and not is_windows
     default_ldflags += ['-Wl,--export-dynamic']
+elif get_option('default_library') == 'shared' and is_ms_compiler
+    default_ldflags += ['/experimental:tlsDllInterface']
 endif
 
 foreach app:apps
@@ -104,6 +106,10 @@ foreach app:apps
         link_libs = dpdk_static_libraries + dpdk_drivers
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     exec = executable('dpdk-' + name,
             sources,
             c_args: cflags,
diff --git a/config/meson.build b/config/meson.build
index 8c8b019..e949240 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -53,6 +53,9 @@ if is_ms_compiler
 
     # enable statement expressions extension
     add_project_arguments('/experimental:statementExpressions', language: 'c')
+
+    # enable export of thread_local variables
+    add_project_arguments('/experimental:tlsDllInterface', language: 'c')
 endif
 
 # set the major version, which might be used by drivers and libraries
@@ -516,4 +519,21 @@ if get_option('default_library') == 'both'
  NOTE: DPDK always builds both shared and static libraries.  Please set
  "default_library" to either "static" or "shared" to select default linkage
  for apps and any examples.''')
+elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
+    error( '''
+ Unsupported value "shared" for "default_library" option.
+
+ NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
+ "default_library" to either "static" or use MSVC.''')
+endif
+
+is_shared_enabled=true
+install_static = true
+install_shared = true
+if is_windows
+    install_static = get_option('default_library') == 'static'
+    install_shared = get_option('default_library') == 'shared'
+    if not is_ms_compiler or not install_shared
+        is_shared_enabled = false
+    endif
 endif
diff --git a/drivers/meson.build b/drivers/meson.build
index 66931d4..751a295 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -208,6 +208,9 @@ foreach subpath:subdirs
         enabled_drivers += name
         lib_name = '_'.join(['rte', class, name])
         cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
+        if is_windows and is_shared_enabled
+            cflags += '-DRTE_BUILD_SHARED_LIB'
+        endif
         if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
             cflags += '-DRTE_ANNOTATE_LOCKS'
             cflags += '-Wthread-safety'
@@ -247,7 +250,7 @@ foreach subpath:subdirs
                 include_directories: includes,
                 dependencies: static_deps,
                 c_args: cflags,
-                install: true)
+                install: install_static)
 
         # now build the shared driver
         version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
@@ -271,48 +274,41 @@ foreach subpath:subdirs
             endif
         endif
 
-        if is_windows
-            if is_ms_linker
+        if is_shared_enabled
+            if is_ms_compiler
                 def_file = custom_target(lib_name + '_def',
                         command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
                         input: version_map,
                         output: '@0@_exports.def'.format(lib_name))
                 lk_deps += [def_file]
-
-                lk_args = ['-Wl,/def:' + def_file.full_path()]
-                if meson.version().version_compare('<0.54.0')
-                    lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
-                endif
+                lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
             else
-                mingw_map = custom_target(lib_name + '_mingw',
-                        command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
-                        input: version_map,
-                        output: '@0@_mingw.map'.format(lib_name))
-                lk_deps += [mingw_map]
-
-                lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+                lk_args = ['-Wl,--version-script=' + version_map]
             endif
-        else
-            lk_args = ['-Wl,--version-script=' + version_map]
         endif
 
-        shared_lib = shared_library(lib_name, sources,
-                objects: objs,
-                include_directories: includes,
-                dependencies: shared_deps,
-                c_args: cflags,
-                link_args: lk_args,
-                link_depends: lk_deps,
-                version: abi_version,
-                soversion: so_version,
-                install: true,
-                install_dir: driver_install_path)
-
-        # create a dependency object and add it to the global dictionary so
-        # testpmd or other built-in apps can find it if necessary
-        shared_dep = declare_dependency(link_with: shared_lib,
-                include_directories: includes,
-                dependencies: shared_deps)
+        if is_shared_enabled
+            shared_lib = shared_library(lib_name, sources,
+                    objects: objs,
+                    include_directories: includes,
+                    dependencies: shared_deps,
+                    c_args: cflags,
+                    link_args: lk_args,
+                    link_depends: lk_deps,
+                    version: abi_version,
+                    soversion: so_version,
+                    install: install_shared,
+                    install_dir: driver_install_path)
+
+            # create a dependency object and add it to the global dictionary so
+            # testpmd or other built-in apps can find it if necessary
+            shared_dep = declare_dependency(link_with: shared_lib,
+                    include_directories: includes,
+                    dependencies: shared_deps)
+
+        else
+            shared_dep = {}
+        endif
         static_dep = declare_dependency(
                 include_directories: includes,
                 dependencies: static_deps)
diff --git a/drivers/net/octeontx/base/meson.build b/drivers/net/octeontx/base/meson.build
index 8e5e8c1..a793c19 100644
--- a/drivers/net/octeontx/base/meson.build
+++ b/drivers/net/octeontx/base/meson.build
@@ -10,7 +10,7 @@ sources = [
 depends = ['ethdev', 'mempool_octeontx']
 static_objs = []
 foreach d: depends
-    if not is_variable('shared_rte_' + d)
+    if not is_variable('static_rte_' + d)
         subdir_done()
     endif
     static_objs += get_variable('static_rte_' + d)
diff --git a/examples/meson.build b/examples/meson.build
index 8e8968a..e08a169 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -85,6 +85,8 @@ endif
 default_ldflags = dpdk_extra_ldflags
 if get_option('default_library') == 'static' and not is_windows
     default_ldflags += ['-Wl,--export-dynamic']
+elif is_ms_compiler
+    default_ldflags += ['/experimental:tlsDllInterface']
 endif
 
 foreach example: examples
@@ -121,6 +123,10 @@ foreach example: examples
         continue
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     if allow_experimental_apis
         cflags += '-DALLOW_EXPERIMENTAL_API'
     endif
diff --git a/lib/meson.build b/lib/meson.build
index 179a272..3b0f632 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -235,6 +235,10 @@ foreach l:libraries
         cflags += '-Wthread-safety'
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     # first build static lib
     static_lib = static_library(libname,
             sources,
@@ -242,11 +246,13 @@ foreach l:libraries
             c_args: cflags,
             dependencies: static_deps,
             include_directories: includes,
-            install: true)
+            install: install_static)
     static_dep = declare_dependency(
             include_directories: includes,
             dependencies: static_deps)
 
+    dpdk_static_libraries = [static_lib] + dpdk_static_libraries
+
     if not use_function_versioning or is_windows
         # use pre-build objects to build shared lib
         sources = []
@@ -260,33 +266,14 @@ foreach l:libraries
     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_shared_enabled
         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
-        if is_windows
-            mingw_map = custom_target(libname + '_mingw',
+            def_file = custom_target(libname + '_def',
                     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()]
+                    output: '@0@_exports.def'.format(libname))
+            lk_deps += [def_file]
+            lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
         else
             lk_args = ['-Wl,--version-script=' + version_map]
         endif
@@ -304,23 +291,26 @@ foreach l:libraries
                 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)
+    if is_shared_enabled
+        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: install_shared)
+        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
+    else
+        shared_dep = {}
+    endif
 
     set_variable('shared_rte_' + name, shared_dep)
     set_variable('static_rte_' + name, static_dep)
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 2/2] buildtools: when building static library use static deps
  2024-04-12 22:52 ` [PATCH v3 0/2] build and install only one library type on Windows Tyler Retzlaff
  2024-04-12 22:52   ` [PATCH v3 1/2] build: build " Tyler Retzlaff
@ 2024-04-12 22:52   ` Tyler Retzlaff
  2024-04-15  9:56     ` Bruce Richardson
  1 sibling, 1 reply; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-12 22:52 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

Use the value of default_library and use it when building deps instead
of always using static deps so we get the correct static or shared deps.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 buildtools/chkincs/meson.build | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index f2dadca..5546489 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -20,10 +20,11 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
 
 # some driver SDK headers depend on these two buses, which are mandatory in build
 # so we always include them in deps list
-deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
+library_type = get_option('default_library')
+deps = [get_variable(library_type + '_rte_bus_vdev'), get_variable(library_type + '_rte_bus_pci')]
 # add the rest of the libs to the dependencies
 foreach l:dpdk_libs_enabled
-    deps += get_variable('shared_rte_' + l)
+    deps += get_variable(library_type + '_rte_' + l)
 endforeach
 
 executable('chkincs', sources,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/2] buildtools: when building static library use static deps
  2024-04-12 14:09     ` Bruce Richardson
@ 2024-04-12 22:52       ` Tyler Retzlaff
  0 siblings, 0 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-12 22:52 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra

On Fri, Apr 12, 2024 at 03:09:12PM +0100, Bruce Richardson wrote:
> On Thu, Mar 14, 2024 at 11:30:22PM -0700, Tyler Retzlaff wrote:
> > Use static deps when default_library=static and use shared deps when
> > using default_library=shared.
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> >  buildtools/chkincs/meson.build | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> > index f2dadca..66245a5 100644
> > --- a/buildtools/chkincs/meson.build
> > +++ b/buildtools/chkincs/meson.build
> > @@ -20,11 +20,19 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
> >  
> >  # some driver SDK headers depend on these two buses, which are mandatory in build
> >  # so we always include them in deps list
> > -deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> > -# add the rest of the libs to the dependencies
> > -foreach l:dpdk_libs_enabled
> > -    deps += get_variable('shared_rte_' + l)
> > -endforeach
> 
> shorter fix might be to use get_option('default_library') in place of the
> "shared" string, rather than using a condition.

done in v3.

> 
> /Bruce
> 
> > +if is_shared_enabled
> > +    deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> > +    # add the rest of the libs to the dependencies
> > +    foreach l:dpdk_libs_enabled
> > +        deps += get_variable('shared_rte_' + l)
> > +    endforeach
> > +else
> > +    deps = [get_variable('static_rte_bus_vdev'), get_variable('static_rte_bus_pci')]
> > +    # add the rest of the libs to the dependencies
> > +    foreach l:dpdk_libs_enabled
> > +        deps += get_variable('static_rte_' + l)
> > +    endforeach
> > +endif
> >  
> >  executable('chkincs', sources,
> >          c_args: cflags,
> > -- 
> > 1.8.3.1
> > 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/2] build: build only one library type on Windows
  2024-04-12 13:00     ` Bruce Richardson
@ 2024-04-12 23:04       ` Tyler Retzlaff
  2024-04-15  9:42         ` Bruce Richardson
  0 siblings, 1 reply; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-12 23:04 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra

On Fri, Apr 12, 2024 at 02:00:43PM +0100, Bruce Richardson wrote:
> On Thu, Mar 14, 2024 at 11:30:21PM -0700, Tyler Retzlaff wrote:
> > MSVC is the only compiler that can produce usable shared libraries for
> > DPDK on Windows because of the use of exported TLS variables.
> > 
> > Disable building of shared libraries with LLVM and MinGW so that
> > remaining __declspec macros needed for the functional libraries built by
> > MSVC can be used without triggering errors in LLVM and MinGW builds.
> > 
> > For Windows only install the default_library type to avoid confusion.
> > Windows builds cannot build both shared and static in a single pass so
> > install only the functional variant.
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> 
> Comments inline below.
> 
> /Bruce
> 
> > ---
> >  app/meson.build                       |  6 +++
> >  config/meson.build                    | 27 ++++++++++++
> >  drivers/meson.build                   | 72 +++++++++++++++----------------
> >  drivers/net/octeontx/base/meson.build |  2 +-
> >  examples/meson.build                  |  6 +++
> >  lib/meson.build                       | 79 +++++++++++++++--------------------
> >  6 files changed, 108 insertions(+), 84 deletions(-)
> > 
> > diff --git a/app/meson.build b/app/meson.build
> > index 21b6da2..f4ed0f1 100644
> > --- a/app/meson.build
> > +++ b/app/meson.build
> > @@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
> >  default_ldflags = []
> >  if get_option('default_library') == 'static' and not is_windows
> >      default_ldflags += ['-Wl,--export-dynamic']
> > +elif get_option('default_library') == 'shared' and is_ms_compiler
> > +    default_ldflags += ['/experimental:tlsDllInterface']
> >  endif
> >  
> 
> Is this necessary, given that you add this flag as a project-wide argument
> below in config/meson.build?

yes, i need to provide tlsDllInterface as an argument to both the
compiler and the linker commands. project-wide argument from
config/meson.build only adds the argument to the compiler command line
but not the linker. there doesn't appear to be a global variable that
can be used as the same for the linker so it has been added to the
various ldflags/lkflags variables in individual meson.build files.

> >  foreach app:apps
> > @@ -104,6 +106,10 @@ foreach app:apps
> >          link_libs = dpdk_static_libraries + dpdk_drivers
> >      endif
> >  
> > +    if is_windows and is_shared_enabled
> > +        cflags += '-DRTE_BUILD_SHARED_LIB'
> > +    endif
> > +
> >      exec = executable('dpdk-' + name,
> >              sources,
> >              c_args: cflags,
> > diff --git a/config/meson.build b/config/meson.build
> > index 8c8b019..dd7971e 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -53,6 +53,9 @@ if is_ms_compiler
> >  
> >      # enable statement expressions extension
> >      add_project_arguments('/experimental:statementExpressions', language: 'c')
> > +
> > +    # enable export of thread_local variables
> > +    add_project_arguments('/experimental:tlsDllInterface', language: 'c')
> >  endif

as per above, this only causes the argument to be added to the compiler
not the linker.

> >  
> >  # set the major version, which might be used by drivers and libraries
> > @@ -516,4 +519,28 @@ if get_option('default_library') == 'both'
> >   NOTE: DPDK always builds both shared and static libraries.  Please set
> >   "default_library" to either "static" or "shared" to select default linkage
> >   for apps and any examples.''')
> > +elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
> > +    error( '''
> > + Unsupported value "shared" for "default_library" option.
> > +
> > + NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
> > + "default_library" to either "static" or use MSVC.''')
> > +endif
> > +
> > +if is_windows
> > +    if is_ms_compiler and get_option('default_library') == 'shared'
> > +        is_shared_enabled=true
> > +    else
> > +        is_shared_enabled=false
> > +    endif
> > +else
> > +    is_shared_enabled=true
> > +endif
> > +
> 
> This can be shortened to:
> 
> is_shared_enabled=true
> if is_windows and (not is_ms_compiler or not get_option(...) == 'shared')
> 	is_shared_enabled=false
> endif
> 
> 
> > +if is_windows
> > +    install_static = get_option('default_library') == 'static'
> > +    install_shared = get_option('default_library') == 'shared'
> > +else
> > +    install_static=true
> > +    install_shared=true
> >  endif
> 
> Might be better merged with the block above:
> 
> is_shared_enabled = true
> install_static = true
> install_shared = true
> if is_windows
>     install_static = get_option('default_library') == 'static'
>     install_shared = get_option('default_library') == 'shared'
> 
>     if not is_ms_compiler or not install_shared
>         is_shared_enabled = false
>     endif
> endif

done in v3, much nicer thanks.

> 
> > diff --git a/drivers/meson.build b/drivers/meson.build
> > index 66931d4..c8b2d13 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -80,7 +80,7 @@ foreach subpath:subdirs
> >          subdir(class)
> >          skip_class = false
> >          foreach d:std_deps
> > -            if not is_variable('shared_rte_' + d)
> > +            if not is_variable('static_rte_' + d)
> >                  skip_class = true
> >                  reason = 'missing internal dependency, "@0@"'.format(d)
> >                  if dpdk_libs_deprecated.contains(d)
> > @@ -173,7 +173,7 @@ foreach subpath:subdirs
> >                  if not build
> >                      break
> >                  endif
> > -                if not is_variable('shared_rte_' + d)
> > +                if not is_variable('static_rte_' + d)
> >                      build = false
> >                      reason = 'missing internal dependency, "@0@"'.format(d)
> >                      if dpdk_libs_deprecated.contains(d)
> > @@ -188,7 +188,9 @@ foreach subpath:subdirs
> >                                  +'\tPlease enable missing dependency "@0@"'.format(d))
> >                      endif
> >                  else
> > -                    shared_deps += [get_variable('shared_rte_' + d)]
> > +                    if is_shared_enabled
> > +                        shared_deps += [get_variable('shared_rte_' + d)]
> > +                    endif
> >                      static_deps += [get_variable('static_rte_' + d)]
> >                  endif
> >              endforeach
> > @@ -208,6 +210,9 @@ foreach subpath:subdirs
> >          enabled_drivers += name
> >          lib_name = '_'.join(['rte', class, name])
> >          cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
> > +        if is_windows and is_shared_enabled
> > +            cflags += '-DRTE_BUILD_SHARED_LIB'
> > +        endif
> 
> Rather than setting this in cflags in the drivers or lib meson.build files,
> can we set this once as a project-level cflag and be done with it?

i don't think so, there are drivers (notably mellanox) that appear to
assume it is not always set by default even with a shared build. i think
this one might best be cleaned up and clarified in a separate series.

> 
> >          if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
> >              cflags += '-DRTE_ANNOTATE_LOCKS'
> >              cflags += '-Wthread-safety'
> > @@ -247,7 +252,7 @@ foreach subpath:subdirs
> >                  include_directories: includes,
> >                  dependencies: static_deps,
> >                  c_args: cflags,
> > -                install: true)
> > +                install: install_static)
> >  
> >          # now build the shared driver
> >          version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
> > @@ -271,55 +276,46 @@ foreach subpath:subdirs
> >              endif
> >          endif
> >  
> > -        if is_windows
> > -            if is_ms_linker
> > +        if is_shared_enabled
> > +            if is_ms_compiler
> >                  def_file = custom_target(lib_name + '_def',
> >                          command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> >                          input: version_map,
> >                          output: '@0@_exports.def'.format(lib_name))
> >                  lk_deps += [def_file]
> > -
> > -                lk_args = ['-Wl,/def:' + def_file.full_path()]
> > -                if meson.version().version_compare('<0.54.0')
> > -                    lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
> > -                endif
> > +                lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
> 
> Again, the /experimental flag shouldn't be necessary, since it's set as
> project option in config/meson.build.

as per above, let me know if what i said seems crazy.

> 
> >              else
> > -                mingw_map = custom_target(lib_name + '_mingw',
> > -                        command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> > -                        input: version_map,
> > -                        output: '@0@_mingw.map'.format(lib_name))
> > -                lk_deps += [mingw_map]
> > -
> > -                lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
> > +                lk_args = ['-Wl,--version-script=' + version_map]
> >              endif
> > -        else
> > -            lk_args = ['-Wl,--version-script=' + version_map]
> >          endif
> >  
> > -        shared_lib = shared_library(lib_name, sources,
> > -                objects: objs,
> > -                include_directories: includes,
> > -                dependencies: shared_deps,
> > -                c_args: cflags,
> > -                link_args: lk_args,
> > -                link_depends: lk_deps,
> > -                version: abi_version,
> > -                soversion: so_version,
> > -                install: true,
> > -                install_dir: driver_install_path)
> > -
> > -        # create a dependency object and add it to the global dictionary so
> > -        # testpmd or other built-in apps can find it if necessary
> > -        shared_dep = declare_dependency(link_with: shared_lib,
> > -                include_directories: includes,
> > -                dependencies: shared_deps)
> > +        if is_shared_enabled
> > +            shared_lib = shared_library(lib_name, sources,
> > +                    objects: objs,
> > +                    include_directories: includes,
> > +                    dependencies: shared_deps,
> > +                    c_args: cflags,
> > +                    link_args: lk_args,
> > +                    link_depends: lk_deps,
> > +                    version: abi_version,
> > +                    soversion: so_version,
> > +                    install: install_shared,
> > +                    install_dir: driver_install_path)
> > +
> > +            # create a dependency object and add it to the global dictionary so
> > +            # testpmd or other built-in apps can find it if necessary
> > +            shared_dep = declare_dependency(link_with: shared_lib,
> > +                    include_directories: includes,
> > +                    dependencies: shared_deps)
> > +
> > +            set_variable('shared_@0@'.format(lib_name), shared_dep)
> 
> Just a thought - could you avoid some of the changes throughout this set,
> if you add an else-leg to the "is_shared_enabled" check, where, if shared
> is not enabled, you assign shared_dep to be an empty string, or an empty
> object or something.
> 
> That would mean that you wouldn't need to move the set_variable line as
> here, you also wouldn't need to put conditionals around all the
> 
> Just a thought - could you avoid some of the changes throughout this set,
> if you add an else-leg to the "is_shared_enabled" check, where, if shared
> is not enabled, you assign shared_dep to be an empty string, or an empty
> object or something.
> 
> That would mean that you wouldn't need to move the set_variable line as
> here, you also wouldn't need to put conditionals around all the
> 'shared_deps +=' lines when processing deps for a component, and lastly,
> you may not even need to change all the searches for shared_rte_ to
> static_rte, since the variables would exist for shared - they'd just be
> empty!

did this in v3, still requires a diff -b to read what actually changed
but appears to have worked out as you anticipated. so i think probably
it was a good improvement.

> 
> > +        endif
> >          static_dep = declare_dependency(
> >                  include_directories: includes,
> >                  dependencies: static_deps)
> >  
> >          dpdk_drivers += static_lib
> 
> <snip for brevity>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/2] build: build only one library type on Windows
  2024-04-12 23:04       ` Tyler Retzlaff
@ 2024-04-15  9:42         ` Bruce Richardson
  0 siblings, 0 replies; 25+ messages in thread
From: Bruce Richardson @ 2024-04-15  9:42 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev, Harman Kalra

On Fri, Apr 12, 2024 at 04:04:12PM -0700, Tyler Retzlaff wrote:
> On Fri, Apr 12, 2024 at 02:00:43PM +0100, Bruce Richardson wrote:
> > On Thu, Mar 14, 2024 at 11:30:21PM -0700, Tyler Retzlaff wrote:
> > > MSVC is the only compiler that can produce usable shared libraries for
> > > DPDK on Windows because of the use of exported TLS variables.
> > > 
> > > Disable building of shared libraries with LLVM and MinGW so that
> > > remaining __declspec macros needed for the functional libraries built by
> > > MSVC can be used without triggering errors in LLVM and MinGW builds.
> > > 
> > > For Windows only install the default_library type to avoid confusion.
> > > Windows builds cannot build both shared and static in a single pass so
> > > install only the functional variant.
> > > 
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > 
> > Comments inline below.
> > 
> > /Bruce
> > 
> > > ---
> > >  app/meson.build                       |  6 +++
> > >  config/meson.build                    | 27 ++++++++++++
> > >  drivers/meson.build                   | 72 +++++++++++++++----------------
> > >  drivers/net/octeontx/base/meson.build |  2 +-
> > >  examples/meson.build                  |  6 +++
> > >  lib/meson.build                       | 79 +++++++++++++++--------------------
> > >  6 files changed, 108 insertions(+), 84 deletions(-)
> > > 
> > > diff --git a/app/meson.build b/app/meson.build
> > > index 21b6da2..f4ed0f1 100644
> > > --- a/app/meson.build
> > > +++ b/app/meson.build
> > > @@ -46,6 +46,8 @@ default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
> > >  default_ldflags = []
> > >  if get_option('default_library') == 'static' and not is_windows
> > >      default_ldflags += ['-Wl,--export-dynamic']
> > > +elif get_option('default_library') == 'shared' and is_ms_compiler
> > > +    default_ldflags += ['/experimental:tlsDllInterface']
> > >  endif
> > >  
> > 
> > Is this necessary, given that you add this flag as a project-wide argument
> > below in config/meson.build?
> 
> yes, i need to provide tlsDllInterface as an argument to both the
> compiler and the linker commands. project-wide argument from
> config/meson.build only adds the argument to the compiler command line
> but not the linker. there doesn't appear to be a global variable that
> can be used as the same for the linker so it has been added to the
> various ldflags/lkflags variables in individual meson.build files.
> 

The add_project_link_arguments() function should be what you are looking
for, I think.

https://mesonbuild.com/Reference-manual_functions.html#add_project_link_arguments

> > >  foreach app:apps
> > > @@ -104,6 +106,10 @@ foreach app:apps
> > >          link_libs = dpdk_static_libraries + dpdk_drivers
> > >      endif
> > >  
> > > +    if is_windows and is_shared_enabled
> > > +        cflags += '-DRTE_BUILD_SHARED_LIB'
> > > +    endif
> > > +
> > >      exec = executable('dpdk-' + name,
> > >              sources,
> > >              c_args: cflags,
> > > diff --git a/config/meson.build b/config/meson.build
> > > index 8c8b019..dd7971e 100644
> > > --- a/config/meson.build
> > > +++ b/config/meson.build
> > > @@ -53,6 +53,9 @@ if is_ms_compiler
> > >  
> > >      # enable statement expressions extension
> > >      add_project_arguments('/experimental:statementExpressions', language: 'c')
> > > +
> > > +    # enable export of thread_local variables
> > > +    add_project_arguments('/experimental:tlsDllInterface', language: 'c')
> > >  endif
> 
> as per above, this only causes the argument to be added to the compiler
> not the linker.
> 
> > >  
> > >  # set the major version, which might be used by drivers and libraries
> > > @@ -516,4 +519,28 @@ if get_option('default_library') == 'both'
> > >   NOTE: DPDK always builds both shared and static libraries.  Please set
> > >   "default_library" to either "static" or "shared" to select default linkage
> > >   for apps and any examples.''')
> > > +elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
> > > +    error( '''
> > > + Unsupported value "shared" for "default_library" option.
> > > +
> > > + NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
> > > + "default_library" to either "static" or use MSVC.''')
> > > +endif
> > > +
> > > +if is_windows
> > > +    if is_ms_compiler and get_option('default_library') == 'shared'
> > > +        is_shared_enabled=true
> > > +    else
> > > +        is_shared_enabled=false
> > > +    endif
> > > +else
> > > +    is_shared_enabled=true
> > > +endif
> > > +
> > 
> > This can be shortened to:
> > 
> > is_shared_enabled=true
> > if is_windows and (not is_ms_compiler or not get_option(...) == 'shared')
> > 	is_shared_enabled=false
> > endif
> > 
> > 
> > > +if is_windows
> > > +    install_static = get_option('default_library') == 'static'
> > > +    install_shared = get_option('default_library') == 'shared'
> > > +else
> > > +    install_static=true
> > > +    install_shared=true
> > >  endif
> > 
> > Might be better merged with the block above:
> > 
> > is_shared_enabled = true
> > install_static = true
> > install_shared = true
> > if is_windows
> >     install_static = get_option('default_library') == 'static'
> >     install_shared = get_option('default_library') == 'shared'
> > 
> >     if not is_ms_compiler or not install_shared
> >         is_shared_enabled = false
> >     endif
> > endif
> 
> done in v3, much nicer thanks.
> 
> > 
> > > diff --git a/drivers/meson.build b/drivers/meson.build
> > > index 66931d4..c8b2d13 100644
> > > --- a/drivers/meson.build
> > > +++ b/drivers/meson.build
> > > @@ -80,7 +80,7 @@ foreach subpath:subdirs
> > >          subdir(class)
> > >          skip_class = false
> > >          foreach d:std_deps
> > > -            if not is_variable('shared_rte_' + d)
> > > +            if not is_variable('static_rte_' + d)
> > >                  skip_class = true
> > >                  reason = 'missing internal dependency, "@0@"'.format(d)
> > >                  if dpdk_libs_deprecated.contains(d)
> > > @@ -173,7 +173,7 @@ foreach subpath:subdirs
> > >                  if not build
> > >                      break
> > >                  endif
> > > -                if not is_variable('shared_rte_' + d)
> > > +                if not is_variable('static_rte_' + d)
> > >                      build = false
> > >                      reason = 'missing internal dependency, "@0@"'.format(d)
> > >                      if dpdk_libs_deprecated.contains(d)
> > > @@ -188,7 +188,9 @@ foreach subpath:subdirs
> > >                                  +'\tPlease enable missing dependency "@0@"'.format(d))
> > >                      endif
> > >                  else
> > > -                    shared_deps += [get_variable('shared_rte_' + d)]
> > > +                    if is_shared_enabled
> > > +                        shared_deps += [get_variable('shared_rte_' + d)]
> > > +                    endif
> > >                      static_deps += [get_variable('static_rte_' + d)]
> > >                  endif
> > >              endforeach
> > > @@ -208,6 +210,9 @@ foreach subpath:subdirs
> > >          enabled_drivers += name
> > >          lib_name = '_'.join(['rte', class, name])
> > >          cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
> > > +        if is_windows and is_shared_enabled
> > > +            cflags += '-DRTE_BUILD_SHARED_LIB'
> > > +        endif
> > 
> > Rather than setting this in cflags in the drivers or lib meson.build files,
> > can we set this once as a project-level cflag and be done with it?
> 
> i don't think so, there are drivers (notably mellanox) that appear to
> assume it is not always set by default even with a shared build. i think
> this one might best be cleaned up and clarified in a separate series.
> 

Oh, how does this assumption show itself or what impact has it? Using git
grep in the source code for the macro only shows the expected usage in
function_versioning header, and in lib/meson.build.

> > 
> > >          if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
> > >              cflags += '-DRTE_ANNOTATE_LOCKS'
> > >              cflags += '-Wthread-safety'
> > > @@ -247,7 +252,7 @@ foreach subpath:subdirs
> > >                  include_directories: includes,
> > >                  dependencies: static_deps,
> > >                  c_args: cflags,
> > > -                install: true)
> > > +                install: install_static)
> > >  
> > >          # now build the shared driver
> > >          version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
> > > @@ -271,55 +276,46 @@ foreach subpath:subdirs
> > >              endif
> > >          endif
> > >  
> > > -        if is_windows
> > > -            if is_ms_linker
> > > +        if is_shared_enabled
> > > +            if is_ms_compiler
> > >                  def_file = custom_target(lib_name + '_def',
> > >                          command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> > >                          input: version_map,
> > >                          output: '@0@_exports.def'.format(lib_name))
> > >                  lk_deps += [def_file]
> > > -
> > > -                lk_args = ['-Wl,/def:' + def_file.full_path()]
> > > -                if meson.version().version_compare('<0.54.0')
> > > -                    lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
> > > -                endif
> > > +                lk_args = ['/experimental:tlsDllInterface', '/def:' + def_file.full_path()]
> > 
> > Again, the /experimental flag shouldn't be necessary, since it's set as
> > project option in config/meson.build.
> 
> as per above, let me know if what i said seems crazy.
> 
> > 
> > >              else
> > > -                mingw_map = custom_target(lib_name + '_mingw',
> > > -                        command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
> > > -                        input: version_map,
> > > -                        output: '@0@_mingw.map'.format(lib_name))
> > > -                lk_deps += [mingw_map]
> > > -
> > > -                lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
> > > +                lk_args = ['-Wl,--version-script=' + version_map]
> > >              endif
> > > -        else
> > > -            lk_args = ['-Wl,--version-script=' + version_map]
> > >          endif
> > >  
> > > -        shared_lib = shared_library(lib_name, sources,
> > > -                objects: objs,
> > > -                include_directories: includes,
> > > -                dependencies: shared_deps,
> > > -                c_args: cflags,
> > > -                link_args: lk_args,
> > > -                link_depends: lk_deps,
> > > -                version: abi_version,
> > > -                soversion: so_version,
> > > -                install: true,
> > > -                install_dir: driver_install_path)
> > > -
> > > -        # create a dependency object and add it to the global dictionary so
> > > -        # testpmd or other built-in apps can find it if necessary
> > > -        shared_dep = declare_dependency(link_with: shared_lib,
> > > -                include_directories: includes,
> > > -                dependencies: shared_deps)
> > > +        if is_shared_enabled
> > > +            shared_lib = shared_library(lib_name, sources,
> > > +                    objects: objs,
> > > +                    include_directories: includes,
> > > +                    dependencies: shared_deps,
> > > +                    c_args: cflags,
> > > +                    link_args: lk_args,
> > > +                    link_depends: lk_deps,
> > > +                    version: abi_version,
> > > +                    soversion: so_version,
> > > +                    install: install_shared,
> > > +                    install_dir: driver_install_path)
> > > +
> > > +            # create a dependency object and add it to the global dictionary so
> > > +            # testpmd or other built-in apps can find it if necessary
> > > +            shared_dep = declare_dependency(link_with: shared_lib,
> > > +                    include_directories: includes,
> > > +                    dependencies: shared_deps)
> > > +
> > > +            set_variable('shared_@0@'.format(lib_name), shared_dep)
> > 
> > Just a thought - could you avoid some of the changes throughout this set,
> > if you add an else-leg to the "is_shared_enabled" check, where, if shared
> > is not enabled, you assign shared_dep to be an empty string, or an empty
> > object or something.
> > 
> > That would mean that you wouldn't need to move the set_variable line as
> > here, you also wouldn't need to put conditionals around all the
> > 
> > Just a thought - could you avoid some of the changes throughout this set,
> > if you add an else-leg to the "is_shared_enabled" check, where, if shared
> > is not enabled, you assign shared_dep to be an empty string, or an empty
> > object or something.
> > 
> > That would mean that you wouldn't need to move the set_variable line as
> > here, you also wouldn't need to put conditionals around all the
> > 'shared_deps +=' lines when processing deps for a component, and lastly,
> > you may not even need to change all the searches for shared_rte_ to
> > static_rte, since the variables would exist for shared - they'd just be
> > empty!
> 
> did this in v3, still requires a diff -b to read what actually changed
> but appears to have worked out as you anticipated. so i think probably
> it was a good improvement.
> 

Great, thanks for trying it!

> > 
> > > +        endif
> > >          static_dep = declare_dependency(
> > >                  include_directories: includes,
> > >                  dependencies: static_deps)
> > >  
> > >          dpdk_drivers += static_lib
> > 
> > <snip for brevity>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 1/2] build: build only one library type on Windows
  2024-04-12 22:52   ` [PATCH v3 1/2] build: build " Tyler Retzlaff
@ 2024-04-15  9:55     ` Bruce Richardson
  0 siblings, 0 replies; 25+ messages in thread
From: Bruce Richardson @ 2024-04-15  9:55 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev, Harman Kalra

On Fri, Apr 12, 2024 at 03:52:12PM -0700, Tyler Retzlaff wrote:
> MSVC is the only compiler that can produce usable shared libraries for
> DPDK on Windows because of the use of exported TLS variables.
> 
> Disable building of shared libraries with LLVM and MinGW so that
> remaining __declspec macros needed for the functional libraries built by
> MSVC can be used without triggering errors in LLVM and MinGW builds.
> 
> For Windows only install the default_library type to avoid confusion.
> Windows builds cannot build both shared and static in a single pass so
> install only the functional variant.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  app/meson.build                       |  6 +++
>  config/meson.build                    | 20 ++++++++++
>  drivers/meson.build                   | 64 +++++++++++++++----------------
>  drivers/net/octeontx/base/meson.build |  2 +-
>  examples/meson.build                  |  6 +++
>  lib/meson.build                       | 72 +++++++++++++++--------------------
>  6 files changed, 94 insertions(+), 76 deletions(-)
> 
<snip>
> -
> -        # create a dependency object and add it to the global dictionary so
> -        # testpmd or other built-in apps can find it if necessary
> -        shared_dep = declare_dependency(link_with: shared_lib,
> -                include_directories: includes,
> -                dependencies: shared_deps)
> +        if is_shared_enabled
> +            shared_lib = shared_library(lib_name, sources,
> +                    objects: objs,
> +                    include_directories: includes,
> +                    dependencies: shared_deps,
> +                    c_args: cflags,
> +                    link_args: lk_args,
> +                    link_depends: lk_deps,
> +                    version: abi_version,
> +                    soversion: so_version,
> +                    install: install_shared,
> +                    install_dir: driver_install_path)
> +
> +            # create a dependency object and add it to the global dictionary so
> +            # testpmd or other built-in apps can find it if necessary
> +            shared_dep = declare_dependency(link_with: shared_lib,
> +                    include_directories: includes,
> +                    dependencies: shared_deps)
> +
> +        else
> +            shared_dep = {}
> +        endif

Very minor nit, but it's generally easier to follow code where you put the
short leg of the condition first, and then the longer leg. Having the short
leg (in this case the one-line) first means the reader has no difficulty
remembering what the original condition was, once they get to the "else".
The longer leg requires more attention and having it last means the reader
doesn't need to worry about maintaining the context of the "if" in memory.
[To a certain extent this is just user-preference, so feel free to ignore
this comment if it's too much work! :-)]

>          static_dep = declare_dependency(
>                  include_directories: includes,
>                  dependencies: static_deps)
> diff --git a/drivers/net/octeontx/base/meson.build b/drivers/net/octeontx/base/meson.build
> index 8e5e8c1..a793c19 100644
> --- a/drivers/net/octeontx/base/meson.build
> +++ b/drivers/net/octeontx/base/meson.build
> @@ -10,7 +10,7 @@ sources = [
>  depends = ['ethdev', 'mempool_octeontx']
>  static_objs = []
>  foreach d: depends
> -    if not is_variable('shared_rte_' + d)
> +    if not is_variable('static_rte_' + d)

Since we have empty objects defined for the shared vars in the no-shared
case, you can drop this change.

>          subdir_done()
>      endif
>      static_objs += get_variable('static_rte_' + d)
<snip>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 2/2] buildtools: when building static library use static deps
  2024-04-12 22:52   ` [PATCH v3 2/2] buildtools: when building static library use static deps Tyler Retzlaff
@ 2024-04-15  9:56     ` Bruce Richardson
  0 siblings, 0 replies; 25+ messages in thread
From: Bruce Richardson @ 2024-04-15  9:56 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev, Harman Kalra

On Fri, Apr 12, 2024 at 03:52:13PM -0700, Tyler Retzlaff wrote:
> Use the value of default_library and use it when building deps instead
> of always using static deps so we get the correct static or shared deps.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v4 0/2] build and install only one library type on Windows
  2024-03-14 19:44 [PATCH] build and install only one library type on Windows Tyler Retzlaff
                   ` (2 preceding siblings ...)
  2024-04-12 22:52 ` [PATCH v3 0/2] build and install only one library type on Windows Tyler Retzlaff
@ 2024-04-15 16:45 ` Tyler Retzlaff
  2024-04-15 16:45   ` [PATCH v4 1/2] build: build " Tyler Retzlaff
  2024-04-15 16:45   ` [PATCH v4 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  2024-04-15 17:12 ` [PATCH v5 0/2] build and install only one library type on Windows Tyler Retzlaff
  4 siblings, 2 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-15 16:45 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant

MSVC does not guarantee any kind of compatibility for static libraries so
they should not be packaged and instead they should be rebuilt along with
the application using the same version of the compiler & linker.  Because
of this we have chosen to build only one library type at a time instead of
compiling twice to produce both types in a single pass.

v4:
  * use add_project_link_arguments() and move addition of linker and compiler
    arguments required only dynamic linking to block where it is determined
    is_shared_enabled=true and don't specify when is_shared_enabled=false.
  * unconditionally initialize shared_deps = {} and get rid of else leg of
    conditional evaluation in drivers/meson.build and lib/meson.build.
  * drop change to octeontx meson.build.

v3:
  * condense library, target and toolchain type logic determining values for
    is_shared_enabled,install_{static,shared} values.
  * define empty shared_dep object to allow unconditional evaluation of shared
    variables even when is_shared_enabled is false.
  * use default_library() string to determine deps for chkincs.

Tyler Retzlaff (2):
  build: build only one library type on Windows
  buildtools: when building static library use static deps

 app/meson.build                |  4 +++
 buildtools/chkincs/meson.build |  5 +--
 config/meson.build             | 21 +++++++++++++
 drivers/meson.build            | 62 +++++++++++++++++-------------------
 examples/meson.build           |  4 +++
 lib/meson.build                | 71 ++++++++++++++++++------------------------
 6 files changed, 90 insertions(+), 77 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v4 1/2] build: build only one library type on Windows
  2024-04-15 16:45 ` [PATCH v4 0/2] build and install only one library type on Windows Tyler Retzlaff
@ 2024-04-15 16:45   ` Tyler Retzlaff
  2024-04-15 16:45   ` [PATCH v4 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  1 sibling, 0 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-15 16:45 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 app/meson.build      |  4 +++
 config/meson.build   | 21 ++++++++++++++++
 drivers/meson.build  | 62 +++++++++++++++++++++------------------------
 examples/meson.build |  4 +++
 lib/meson.build      | 71 ++++++++++++++++++++++------------------------------
 5 files changed, 87 insertions(+), 75 deletions(-)

diff --git a/app/meson.build b/app/meson.build
index 21b6da2..d94378e 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -104,6 +104,10 @@ foreach app:apps
         link_libs = dpdk_static_libraries + dpdk_drivers
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     exec = executable('dpdk-' + name,
             sources,
             c_args: cflags,
diff --git a/config/meson.build b/config/meson.build
index 8c8b019..c74899e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -516,4 +516,25 @@ if get_option('default_library') == 'both'
  NOTE: DPDK always builds both shared and static libraries.  Please set
  "default_library" to either "static" or "shared" to select default linkage
  for apps and any examples.''')
+elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
+    error( '''
+ Unsupported value "shared" for "default_library" option.
+
+ NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
+ "default_library" to either "static" or use MSVC.''')
+endif
+
+is_shared_enabled=true
+install_static = true
+install_shared = true
+if is_windows
+    install_static = get_option('default_library') == 'static'
+    install_shared = get_option('default_library') == 'shared'
+    if not is_ms_compiler or not install_shared
+        is_shared_enabled = false
+    else
+        # enable export / import of thread_local variables
+        add_project_arguments('/experimental:tlsDllInterface', language: 'c')
+        add_project_link_arguments('/experimental:tlsDllInterface', language: 'c')
+    endif
 endif
diff --git a/drivers/meson.build b/drivers/meson.build
index 66931d4..92a85f8 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -208,6 +208,9 @@ foreach subpath:subdirs
         enabled_drivers += name
         lib_name = '_'.join(['rte', class, name])
         cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name])
+        if is_windows and is_shared_enabled
+            cflags += '-DRTE_BUILD_SHARED_LIB'
+        endif
         if annotate_locks and cc.get_id() == 'clang' and cc.version().version_compare('>=3.5.0')
             cflags += '-DRTE_ANNOTATE_LOCKS'
             cflags += '-Wthread-safety'
@@ -247,7 +250,7 @@ foreach subpath:subdirs
                 include_directories: includes,
                 dependencies: static_deps,
                 c_args: cflags,
-                install: true)
+                install: install_static)
 
         # now build the shared driver
         version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
@@ -271,48 +274,39 @@ foreach subpath:subdirs
             endif
         endif
 
-        if is_windows
-            if is_ms_linker
+        if is_shared_enabled
+            if is_ms_compiler
                 def_file = custom_target(lib_name + '_def',
                         command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
                         input: version_map,
                         output: '@0@_exports.def'.format(lib_name))
                 lk_deps += [def_file]
-
-                lk_args = ['-Wl,/def:' + def_file.full_path()]
-                if meson.version().version_compare('<0.54.0')
-                    lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
-                endif
+                lk_args = ['/def:' + def_file.full_path()]
             else
-                mingw_map = custom_target(lib_name + '_mingw',
-                        command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
-                        input: version_map,
-                        output: '@0@_mingw.map'.format(lib_name))
-                lk_deps += [mingw_map]
-
-                lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+                lk_args = ['-Wl,--version-script=' + version_map]
             endif
-        else
-            lk_args = ['-Wl,--version-script=' + version_map]
         endif
 
-        shared_lib = shared_library(lib_name, sources,
-                objects: objs,
-                include_directories: includes,
-                dependencies: shared_deps,
-                c_args: cflags,
-                link_args: lk_args,
-                link_depends: lk_deps,
-                version: abi_version,
-                soversion: so_version,
-                install: true,
-                install_dir: driver_install_path)
-
-        # create a dependency object and add it to the global dictionary so
-        # testpmd or other built-in apps can find it if necessary
-        shared_dep = declare_dependency(link_with: shared_lib,
-                include_directories: includes,
-                dependencies: shared_deps)
+        shared_dep = {}
+        if is_shared_enabled
+            shared_lib = shared_library(lib_name, sources,
+                    objects: objs,
+                    include_directories: includes,
+                    dependencies: shared_deps,
+                    c_args: cflags,
+                    link_args: lk_args,
+                    link_depends: lk_deps,
+                    version: abi_version,
+                    soversion: so_version,
+                    install: install_shared,
+                    install_dir: driver_install_path)
+
+            # create a dependency object and add it to the global dictionary so
+            # testpmd or other built-in apps can find it if necessary
+            shared_dep = declare_dependency(link_with: shared_lib,
+                    include_directories: includes,
+                    dependencies: shared_deps)
+        endif
         static_dep = declare_dependency(
                 include_directories: includes,
                 dependencies: static_deps)
diff --git a/examples/meson.build b/examples/meson.build
index 8e8968a..9ca2cb9 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -121,6 +121,10 @@ foreach example: examples
         continue
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     if allow_experimental_apis
         cflags += '-DALLOW_EXPERIMENTAL_API'
     endif
diff --git a/lib/meson.build b/lib/meson.build
index 179a272..f2894c6 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -235,6 +235,10 @@ foreach l:libraries
         cflags += '-Wthread-safety'
     endif
 
+    if is_windows and is_shared_enabled
+        cflags += '-DRTE_BUILD_SHARED_LIB'
+    endif
+
     # first build static lib
     static_lib = static_library(libname,
             sources,
@@ -242,11 +246,13 @@ foreach l:libraries
             c_args: cflags,
             dependencies: static_deps,
             include_directories: includes,
-            install: true)
+            install: install_static)
     static_dep = declare_dependency(
             include_directories: includes,
             dependencies: static_deps)
 
+    dpdk_static_libraries = [static_lib] + dpdk_static_libraries
+
     if not use_function_versioning or is_windows
         # use pre-build objects to build shared lib
         sources = []
@@ -260,33 +266,14 @@ foreach l:libraries
     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_shared_enabled
         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
-        if is_windows
-            mingw_map = custom_target(libname + '_mingw',
+            def_file = custom_target(libname + '_def',
                     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()]
+                    output: '@0@_exports.def'.format(libname))
+            lk_deps += [def_file]
+            lk_args = ['/def:' + def_file.full_path()]
         else
             lk_args = ['-Wl,--version-script=' + version_map]
         endif
@@ -304,23 +291,25 @@ foreach l:libraries
                 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_dep = {}
+    if is_shared_enabled
+        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: install_shared)
+        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
+    endif
 
     set_variable('shared_rte_' + name, shared_dep)
     set_variable('static_rte_' + name, static_dep)
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v4 2/2] buildtools: when building static library use static deps
  2024-04-15 16:45 ` [PATCH v4 0/2] build and install only one library type on Windows Tyler Retzlaff
  2024-04-15 16:45   ` [PATCH v4 1/2] build: build " Tyler Retzlaff
@ 2024-04-15 16:45   ` Tyler Retzlaff
  1 sibling, 0 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-15 16:45 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

Use the value of default_library and use it when building deps instead
of always using static deps so we get the correct static or shared deps.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 buildtools/chkincs/meson.build | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index f2dadca..5546489 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -20,10 +20,11 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
 
 # some driver SDK headers depend on these two buses, which are mandatory in build
 # so we always include them in deps list
-deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
+library_type = get_option('default_library')
+deps = [get_variable(library_type + '_rte_bus_vdev'), get_variable(library_type + '_rte_bus_pci')]
 # add the rest of the libs to the dependencies
 foreach l:dpdk_libs_enabled
-    deps += get_variable('shared_rte_' + l)
+    deps += get_variable(library_type + '_rte_' + l)
 endforeach
 
 executable('chkincs', sources,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v5 0/2] build and install only one library type on Windows
  2024-03-14 19:44 [PATCH] build and install only one library type on Windows Tyler Retzlaff
                   ` (3 preceding siblings ...)
  2024-04-15 16:45 ` [PATCH v4 0/2] build and install only one library type on Windows Tyler Retzlaff
@ 2024-04-15 17:12 ` Tyler Retzlaff
  2024-04-15 17:12   ` [PATCH v5 1/2] build: build " Tyler Retzlaff
  2024-04-15 17:12   ` [PATCH v5 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  4 siblings, 2 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-15 17:12 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.
                                                                                                                             For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant

MSVC does not guarantee any kind of compatibility for static libraries so
they should not be packaged and instead they should be rebuilt along with
the application using the same version of the compiler & linker.  Because
of this we have chosen to build only one library type at a time instead of
compiling twice to produce both types in a single pass.

thanks Bruce for all the discussion/feedback!

v5: set RTE_BUILD_SHARED_LIB in a single location when building with msvc
    and is_shared_enabled instead of scattered over various meson.build files.

v4:
  * use add_project_link_arguments() and move addition of linker and compiler
    arguments required only dynamic linking to block where it is determined
    is_shared_enabled=true and don't specify when is_shared_enabled=false.
  * unconditionally initialize shared_deps = {} and get rid of else leg of
    conditional evaluation in drivers/meson.build and lib/meson.build.
  * drop change to octeontx meson.build.

v3:
  * condense library, target and toolchain type logic determining values for
    is_shared_enabled,install_{static,shared} values.
  * define empty shared_dep object to allow unconditional evaluation of shared
    variables even when is_shared_enabled is false.
  * use default_library() string to determine deps for chkincs.

Tyler Retzlaff (2):
  build: build only one library type on Windows
  buildtools: when building static library use static deps

 buildtools/chkincs/meson.build |  5 ++--
 config/meson.build             | 24 +++++++++++++++
 drivers/meson.build            | 59 ++++++++++++++++---------------------
 lib/meson.build                | 67 ++++++++++++++++--------------------------
 4 files changed, 78 insertions(+), 77 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v5 1/2] build: build only one library type on Windows
  2024-04-15 17:12 ` [PATCH v5 0/2] build and install only one library type on Windows Tyler Retzlaff
@ 2024-04-15 17:12   ` Tyler Retzlaff
  2024-04-15 17:24     ` Bruce Richardson
  2024-04-15 17:12   ` [PATCH v5 2/2] buildtools: when building static library use static deps Tyler Retzlaff
  1 sibling, 1 reply; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-15 17:12 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

MSVC is the only compiler that can produce usable shared libraries for
DPDK on Windows because of the use of exported TLS variables.

Disable building of shared libraries with LLVM and MinGW so that
remaining __declspec macros needed for the functional libraries built by
MSVC can be used without triggering errors in LLVM and MinGW builds.

For Windows only install the default_library type to avoid confusion.
Windows builds cannot build both shared and static in a single pass so
install only the functional variant.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 config/meson.build  | 24 +++++++++++++++++++
 drivers/meson.build | 59 ++++++++++++++++++++--------------------------
 lib/meson.build     | 67 +++++++++++++++++++++--------------------------------
 3 files changed, 75 insertions(+), 75 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 8c8b019..9882d33 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -516,4 +516,28 @@ if get_option('default_library') == 'both'
  NOTE: DPDK always builds both shared and static libraries.  Please set
  "default_library" to either "static" or "shared" to select default linkage
  for apps and any examples.''')
+elif get_option('default_library') == 'shared' and is_windows and not is_ms_compiler
+    error( '''
+ Unsupported value "shared" for "default_library" option.
+
+ NOTE: DPDK Windows shared is only supported when building with MSVC. Please set
+ "default_library" to either "static" or use MSVC.''')
+endif
+
+is_shared_enabled=true
+install_static = true
+install_shared = true
+if is_windows
+    install_static = get_option('default_library') == 'static'
+    install_shared = get_option('default_library') == 'shared'
+    if not is_ms_compiler or not install_shared
+        is_shared_enabled = false
+    else
+        # building shared libraries
+        dpdk_conf.set('RTE_BUILD_SHARED_LIB', true)
+
+        # enable export / import of thread_local variables
+        add_project_arguments('/experimental:tlsDllInterface', language: 'c')
+        add_project_link_arguments('/experimental:tlsDllInterface', language: 'c')
+    endif
 endif
diff --git a/drivers/meson.build b/drivers/meson.build
index 66931d4..173403a 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -247,7 +247,7 @@ foreach subpath:subdirs
                 include_directories: includes,
                 dependencies: static_deps,
                 c_args: cflags,
-                install: true)
+                install: install_static)
 
         # now build the shared driver
         version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
@@ -271,48 +271,39 @@ foreach subpath:subdirs
             endif
         endif
 
-        if is_windows
-            if is_ms_linker
+        if is_shared_enabled
+            if is_ms_compiler
                 def_file = custom_target(lib_name + '_def',
                         command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
                         input: version_map,
                         output: '@0@_exports.def'.format(lib_name))
                 lk_deps += [def_file]
-
-                lk_args = ['-Wl,/def:' + def_file.full_path()]
-                if meson.version().version_compare('<0.54.0')
-                    lk_args += ['-Wl,/implib:drivers\\lib' + lib_name + '.dll.a']
-                endif
+                lk_args = ['/def:' + def_file.full_path()]
             else
-                mingw_map = custom_target(lib_name + '_mingw',
-                        command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
-                        input: version_map,
-                        output: '@0@_mingw.map'.format(lib_name))
-                lk_deps += [mingw_map]
-
-                lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+                lk_args = ['-Wl,--version-script=' + version_map]
             endif
-        else
-            lk_args = ['-Wl,--version-script=' + version_map]
         endif
 
-        shared_lib = shared_library(lib_name, sources,
-                objects: objs,
-                include_directories: includes,
-                dependencies: shared_deps,
-                c_args: cflags,
-                link_args: lk_args,
-                link_depends: lk_deps,
-                version: abi_version,
-                soversion: so_version,
-                install: true,
-                install_dir: driver_install_path)
-
-        # create a dependency object and add it to the global dictionary so
-        # testpmd or other built-in apps can find it if necessary
-        shared_dep = declare_dependency(link_with: shared_lib,
-                include_directories: includes,
-                dependencies: shared_deps)
+        shared_dep = {}
+        if is_shared_enabled
+            shared_lib = shared_library(lib_name, sources,
+                    objects: objs,
+                    include_directories: includes,
+                    dependencies: shared_deps,
+                    c_args: cflags,
+                    link_args: lk_args,
+                    link_depends: lk_deps,
+                    version: abi_version,
+                    soversion: so_version,
+                    install: install_shared,
+                    install_dir: driver_install_path)
+
+            # create a dependency object and add it to the global dictionary so
+            # testpmd or other built-in apps can find it if necessary
+            shared_dep = declare_dependency(link_with: shared_lib,
+                    include_directories: includes,
+                    dependencies: shared_deps)
+        endif
         static_dep = declare_dependency(
                 include_directories: includes,
                 dependencies: static_deps)
diff --git a/lib/meson.build b/lib/meson.build
index 179a272..8b28314 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -242,11 +242,13 @@ foreach l:libraries
             c_args: cflags,
             dependencies: static_deps,
             include_directories: includes,
-            install: true)
+            install: install_static)
     static_dep = declare_dependency(
             include_directories: includes,
             dependencies: static_deps)
 
+    dpdk_static_libraries = [static_lib] + dpdk_static_libraries
+
     if not use_function_versioning or is_windows
         # use pre-build objects to build shared lib
         sources = []
@@ -260,33 +262,14 @@ foreach l:libraries
     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_shared_enabled
         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
-        if is_windows
-            mingw_map = custom_target(libname + '_mingw',
+            def_file = custom_target(libname + '_def',
                     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()]
+                    output: '@0@_exports.def'.format(libname))
+            lk_deps += [def_file]
+            lk_args = ['/def:' + def_file.full_path()]
         else
             lk_args = ['-Wl,--version-script=' + version_map]
         endif
@@ -304,23 +287,25 @@ foreach l:libraries
                 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_dep = {}
+    if is_shared_enabled
+        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: install_shared)
+        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
+    endif
 
     set_variable('shared_rte_' + name, shared_dep)
     set_variable('static_rte_' + name, static_dep)
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v5 2/2] buildtools: when building static library use static deps
  2024-04-15 17:12 ` [PATCH v5 0/2] build and install only one library type on Windows Tyler Retzlaff
  2024-04-15 17:12   ` [PATCH v5 1/2] build: build " Tyler Retzlaff
@ 2024-04-15 17:12   ` Tyler Retzlaff
  1 sibling, 0 replies; 25+ messages in thread
From: Tyler Retzlaff @ 2024-04-15 17:12 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Harman Kalra, Tyler Retzlaff

Use the value of default_library and use it when building deps instead
of always using static deps so we get the correct static or shared deps.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/chkincs/meson.build | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index f2dadca..5546489 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -20,10 +20,11 @@ sources += gen_c_files.process(dpdk_chkinc_headers)
 
 # some driver SDK headers depend on these two buses, which are mandatory in build
 # so we always include them in deps list
-deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
+library_type = get_option('default_library')
+deps = [get_variable(library_type + '_rte_bus_vdev'), get_variable(library_type + '_rte_bus_pci')]
 # add the rest of the libs to the dependencies
 foreach l:dpdk_libs_enabled
-    deps += get_variable('shared_rte_' + l)
+    deps += get_variable(library_type + '_rte_' + l)
 endforeach
 
 executable('chkincs', sources,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v5 1/2] build: build only one library type on Windows
  2024-04-15 17:12   ` [PATCH v5 1/2] build: build " Tyler Retzlaff
@ 2024-04-15 17:24     ` Bruce Richardson
  0 siblings, 0 replies; 25+ messages in thread
From: Bruce Richardson @ 2024-04-15 17:24 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev, Harman Kalra

On Mon, Apr 15, 2024 at 10:12:47AM -0700, Tyler Retzlaff wrote:
> MSVC is the only compiler that can produce usable shared libraries for
> DPDK on Windows because of the use of exported TLS variables.
> 
> Disable building of shared libraries with LLVM and MinGW so that
> remaining __declspec macros needed for the functional libraries built by
> MSVC can be used without triggering errors in LLVM and MinGW builds.
> 
> For Windows only install the default_library type to avoid confusion.
> Windows builds cannot build both shared and static in a single pass so
> install only the functional variant.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---

Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2024-04-15 17:24 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-14 19:44 [PATCH] build and install only one library type on Windows Tyler Retzlaff
2024-03-14 19:44 ` [PATCH] build: build " Tyler Retzlaff
2024-03-15  6:30 ` [PATCH v2 0/2] build and install " Tyler Retzlaff
2024-03-15  6:30   ` [PATCH v2 1/2] build: build " Tyler Retzlaff
2024-04-12 13:00     ` Bruce Richardson
2024-04-12 23:04       ` Tyler Retzlaff
2024-04-15  9:42         ` Bruce Richardson
2024-03-15  6:30   ` [PATCH v2 2/2] buildtools: when building static library use static deps Tyler Retzlaff
2024-03-15  8:28     ` Bruce Richardson
2024-04-04 18:47       ` Tyler Retzlaff
2024-04-05  9:11         ` Bruce Richardson
2024-04-12 14:09     ` Bruce Richardson
2024-04-12 22:52       ` Tyler Retzlaff
2024-04-12 22:52 ` [PATCH v3 0/2] build and install only one library type on Windows Tyler Retzlaff
2024-04-12 22:52   ` [PATCH v3 1/2] build: build " Tyler Retzlaff
2024-04-15  9:55     ` Bruce Richardson
2024-04-12 22:52   ` [PATCH v3 2/2] buildtools: when building static library use static deps Tyler Retzlaff
2024-04-15  9:56     ` Bruce Richardson
2024-04-15 16:45 ` [PATCH v4 0/2] build and install only one library type on Windows Tyler Retzlaff
2024-04-15 16:45   ` [PATCH v4 1/2] build: build " Tyler Retzlaff
2024-04-15 16:45   ` [PATCH v4 2/2] buildtools: when building static library use static deps Tyler Retzlaff
2024-04-15 17:12 ` [PATCH v5 0/2] build and install only one library type on Windows Tyler Retzlaff
2024-04-15 17:12   ` [PATCH v5 1/2] build: build " Tyler Retzlaff
2024-04-15 17:24     ` Bruce Richardson
2024-04-15 17:12   ` [PATCH v5 2/2] buildtools: when building static library use static deps Tyler Retzlaff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).