[dpdk-dev] [PATCH] Windows: minor build fixes

[dpdk-dev] [PATCH] Windows: minor build fixes

[Nick Connolly]

Don't run symlink-drivers-solibs.sh as part of 'install' because
Windows doesn't support shell scripts, causing the install to
fail.

Meson versions >= 0.54.0 include support for handling /implib
with msvc link. Specifying it explicitly causes failures when
linking against the dll. Tested using Link 14.27.29112.0 and
Clang 11.0.0.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
---
 config/meson.build  | 8 +++++---
 drivers/meson.build | 6 ++++--
 lib/meson.build     | 6 ++++--
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 258b01d06..a29693b88 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -57,9 +57,11 @@ eal_pmd_path = join_paths(get_option('prefix'), driver_install_path)
 # driver .so files often depend upon the bus drivers for their connect bus,
 # e.g. ixgbe depends on librte_bus_pci. This means that the bus drivers need
 # to be in the library path, so symlink the drivers from the main lib directory.
-meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
-		get_option('libdir'),
-		pmd_subdir_opt)
+if not is_windows
+	meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
+			get_option('libdir'),
+			pmd_subdir_opt)
+endif
 
 # set the machine type and cflags for it
 if meson.is_cross_build()
diff --git a/drivers/meson.build b/drivers/meson.build
index 4bb7e9218..6b50f7238 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -186,8 +186,10 @@ foreach subpath:subdirs
 			lk_deps = [version_map, def_file, mingw_map]
 			if is_windows
 				if is_ms_linker
-					lk_args = ['-Wl,/def:' + def_file.full_path(),
-						'-Wl,/implib:drivers\\' + implib]
+					lk_args = ['-Wl,/def:' + def_file.full_path()]
+					if meson.version().version_compare('<0.54.0')
+						lk_args += ['-Wl,/implib:drivers\\' + implib]
+					endif
 				else
 					lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
 				endif
diff --git a/lib/meson.build b/lib/meson.build
index 1bb019720..ed00f8914 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -157,8 +157,10 @@ foreach l:libraries
 				output: '@0@_mingw.map'.format(libname))
 
 			if is_ms_linker
-				lk_args = ['-Wl,/def:' + def_file.full_path(),
-					'-Wl,/implib:lib\\' + implib]
+				lk_args = ['-Wl,/def:' + def_file.full_path()]
+				if meson.version().version_compare('<0.54.0')
+					lk_args += ['-Wl,/implib:lib\\' + implib]
+				endif
 			else
 				if is_windows
 					lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
-- 
2.25.1

[dpdk-dev] [PATCH] windows: minor build fixes

[Nick Connolly]

Don't run symlink-drivers-solibs.sh as part of 'install' because
Windows doesn't support shell scripts.

Meson versions >= 0.54.0 include support for handling /implib
with msvc link. Specifying it explicitly causes failures when
linking against the dll. Tested using Link 14.27.29112.0 and
Clang 11.0.0.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
---
 config/meson.build  | 8 +++++---
 drivers/meson.build | 6 ++++--
 lib/meson.build     | 6 ++++--
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 258b01d06..a29693b88 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -57,9 +57,11 @@ eal_pmd_path = join_paths(get_option('prefix'), driver_install_path)
 # driver .so files often depend upon the bus drivers for their connect bus,
 # e.g. ixgbe depends on librte_bus_pci. This means that the bus drivers need
 # to be in the library path, so symlink the drivers from the main lib directory.
-meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
-		get_option('libdir'),
-		pmd_subdir_opt)
+if not is_windows
+	meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
+			get_option('libdir'),
+			pmd_subdir_opt)
+endif
 
 # set the machine type and cflags for it
 if meson.is_cross_build()
diff --git a/drivers/meson.build b/drivers/meson.build
index 4bb7e9218..6b50f7238 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -186,8 +186,10 @@ foreach subpath:subdirs
 			lk_deps = [version_map, def_file, mingw_map]
 			if is_windows
 				if is_ms_linker
-					lk_args = ['-Wl,/def:' + def_file.full_path(),
-						'-Wl,/implib:drivers\\' + implib]
+					lk_args = ['-Wl,/def:' + def_file.full_path()]
+					if meson.version().version_compare('<0.54.0')
+						lk_args += ['-Wl,/implib:drivers\\' + implib]
+					endif
 				else
 					lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
 				endif
diff --git a/lib/meson.build b/lib/meson.build
index 1bb019720..ed00f8914 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -157,8 +157,10 @@ foreach l:libraries
 				output: '@0@_mingw.map'.format(libname))
 
 			if is_ms_linker
-				lk_args = ['-Wl,/def:' + def_file.full_path(),
-					'-Wl,/implib:lib\\' + implib]
+				lk_args = ['-Wl,/def:' + def_file.full_path()]
+				if meson.version().version_compare('<0.54.0')
+					lk_args += ['-Wl,/implib:lib\\' + implib]
+				endif
 			else
 				if is_windows
 					lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
-- 
2.25.1

Re: [dpdk-dev] [PATCH] windows: minor build fixes

[John Alexander]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Nick Connolly
> Sent: 30 October 2020 17:46
> To: Bruce Richardson <bruce.richardson@intel.com>
> Cc: dev@dpdk.org; Nick Connolly <nick.connolly@mayadata.io>
> Subject: [dpdk-dev] [PATCH] windows: minor build fixes
> 
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you recognize the sender and know the
> content is safe.
> 
> Don't run symlink-drivers-solibs.sh as part of 'install' because Windows
> doesn't support shell scripts.
> 
> Meson versions >= 0.54.0 include support for handling /implib with msvc
> link. Specifying it explicitly causes failures when linking against the dll. Tested
> using Link 14.27.29112.0 and Clang 11.0.0.
> 
> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> ---
>  config/meson.build  | 8 +++++---
>  drivers/meson.build | 6 ++++--
>  lib/meson.build     | 6 ++++--
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/config/meson.build b/config/meson.build index
> 258b01d06..a29693b88 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -57,9 +57,11 @@ eal_pmd_path = join_paths(get_option('prefix'),
> driver_install_path)  # driver .so files often depend upon the bus drivers for
> their connect bus,  # e.g. ixgbe depends on librte_bus_pci. This means that
> the bus drivers need  # to be in the library path, so symlink the drivers from
> the main lib directory.
> -meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
> -               get_option('libdir'),
> -               pmd_subdir_opt)
> +if not is_windows
> +       meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
> +                       get_option('libdir'),
> +                       pmd_subdir_opt)
> +endif
> 
>  # set the machine type and cflags for it  if meson.is_cross_build() diff --git
> a/drivers/meson.build b/drivers/meson.build index 4bb7e9218..6b50f7238
> 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -186,8 +186,10 @@ foreach subpath:subdirs
>                         lk_deps = [version_map, def_file, mingw_map]
>                         if is_windows
>                                 if is_ms_linker
> -                                       lk_args = ['-Wl,/def:' + def_file.full_path(),
> -                                               '-Wl,/implib:drivers\\' + implib]
> +                                       lk_args = ['-Wl,/def:' + def_file.full_path()]
> +                                       if meson.version().version_compare('<0.54.0')
> +                                               lk_args += ['-Wl,/implib:drivers\\' + implib]
> +                                       endif
>                                 else
>                                         lk_args = ['-Wl,--version-script=' +
> mingw_map.full_path()]
>                                 endif
> diff --git a/lib/meson.build b/lib/meson.build index 1bb019720..ed00f8914
> 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -157,8 +157,10 @@ foreach l:libraries
>                                 output: '@0@_mingw.map'.format(libname))
> 
>                         if is_ms_linker
> -                               lk_args = ['-Wl,/def:' + def_file.full_path(),
> -                                       '-Wl,/implib:lib\\' + implib]
> +                               lk_args = ['-Wl,/def:' + def_file.full_path()]
> +                               if meson.version().version_compare('<0.54.0')
> +                                       lk_args += ['-Wl,/implib:lib\\' + implib]
> +                               endif
>                         else
>                                 if is_windows
>                                         lk_args = ['-Wl,--version-script=' +
> mingw_map.full_path()]
> --
> 2.25.1

Hi,

Do we know what the motivation to rename the import library for the Windows build was in the first place?  I've tried removing the renaming of the import library completely in the Windows build and the build produces valid output (a valid DLL and paired import library is generated for each RTE lib using clang build); The meson version test addition may be unnecessary here?

Kind Regards,
John Alexander.

Re: [dpdk-dev] [PATCH] windows: minor build fixes

[Nick Connolly]

> Do we know what the motivation to rename the import library for the 
> Windows build was in the first place? I've tried removing the renaming 
> of the import library completely in the Windows build and the build 
> produces valid output (a valid DLL and paired import library is 
> generated for each RTE lib using clang build); The meson version test 
> addition may be unnecessary here?
>
> Kind Regards,
> John Alexander.

I looked back through the commit history for Meson and there were a 
number of changes to the way that import libraries are handled between 
0.47.1 and 0.54.0.  I installed the meson releases in order, but was 
unable to build successfully with anything before 0.54.0 - I was seeing 
complaints about unknown linker flags building with Clang 11.0.0 and 
Link 14.27.29112.0 from Visual Studio 2019.

Assuming that builds with meson 0.47.1 and Clang/Link do work, I took 
the conservative approach of only making the change for the version of 
Meson where I know that /implib is handled correctly, leaving the 
previous behaviour unchanged for earlier versions which I'm unable to 
verify.

Regards,
Nick

Re: [dpdk-dev] [PATCH] windows: minor build fixes

[Ranjit Menon]

On 10/30/2020 10:46 AM, Nick Connolly wrote:
> Don't run symlink-drivers-solibs.sh as part of 'install' because
> Windows doesn't support shell scripts.
>
> Meson versions >= 0.54.0 include support for handling /implib
> with msvc link. Specifying it explicitly causes failures when
> linking against the dll. Tested using Link 14.27.29112.0 and
> Clang 11.0.0.
>
> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> ---
>   config/meson.build  | 8 +++++---
>   drivers/meson.build | 6 ++++--
>   lib/meson.build     | 6 ++++--
>   3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/config/meson.build b/config/meson.build
> index 258b01d06..a29693b88 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -57,9 +57,11 @@ eal_pmd_path = join_paths(get_option('prefix'), driver_install_path)
>   # driver .so files often depend upon the bus drivers for their connect bus,
>   # e.g. ixgbe depends on librte_bus_pci. This means that the bus drivers need
>   # to be in the library path, so symlink the drivers from the main lib directory.
> -meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
> -		get_option('libdir'),
> -		pmd_subdir_opt)
> +if not is_windows
> +	meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
> +			get_option('libdir'),
> +			pmd_subdir_opt)
> +endif
>   
>   # set the machine type and cflags for it
>   if meson.is_cross_build()
> diff --git a/drivers/meson.build b/drivers/meson.build
> index 4bb7e9218..6b50f7238 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -186,8 +186,10 @@ foreach subpath:subdirs
>   			lk_deps = [version_map, def_file, mingw_map]
>   			if is_windows
>   				if is_ms_linker
> -					lk_args = ['-Wl,/def:' + def_file.full_path(),
> -						'-Wl,/implib:drivers\\' + implib]
> +					lk_args = ['-Wl,/def:' + def_file.full_path()]
> +					if meson.version().version_compare('<0.54.0')
> +						lk_args += ['-Wl,/implib:drivers\\' + implib]
> +					endif
>   				else
>   					lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
>   				endif
> diff --git a/lib/meson.build b/lib/meson.build
> index 1bb019720..ed00f8914 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -157,8 +157,10 @@ foreach l:libraries
>   				output: '@0@_mingw.map'.format(libname))
>   
>   			if is_ms_linker
> -				lk_args = ['-Wl,/def:' + def_file.full_path(),
> -					'-Wl,/implib:lib\\' + implib]
> +				lk_args = ['-Wl,/def:' + def_file.full_path()]
> +				if meson.version().version_compare('<0.54.0')
> +					lk_args += ['-Wl,/implib:lib\\' + implib]
> +				endif
>   			else
>   				if is_windows
>   					lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]

Thanks, Nick. This finally fixes this problem I've been having since I 
upgraded to meson v0.54.

Tested-by: Ranjit Menon <ranjit.menon@intel.com>
Acked-by: Ranjit Menon <ranjit.menon@intel.com>

Re: [dpdk-dev] [PATCH] windows: minor build fixes

[Tal Shnaiderman]

> Subject: Re: [dpdk-dev] [PATCH] windows: minor build fixes
>  
> 
> On 10/30/2020 10:46 AM, Nick Connolly wrote:
> > Don't run symlink-drivers-solibs.sh as part of 'install' because
> > Windows doesn't support shell scripts.
> >
> > Meson versions >= 0.54.0 include support for handling /implib with
> > msvc link. Specifying it explicitly causes failures when linking
> > against the dll. Tested using Link 14.27.29112.0 and Clang 11.0.0.
> >
> > Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> > ---
> >   config/meson.build  | 8 +++++---
> >   drivers/meson.build | 6 ++++--
> >   lib/meson.build     | 6 ++++--
> >   3 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/config/meson.build b/config/meson.build index
> > 258b01d06..a29693b88 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -57,9 +57,11 @@ eal_pmd_path = join_paths(get_option('prefix'),
> driver_install_path)
> >   # driver .so files often depend upon the bus drivers for their connect bus,
> >   # e.g. ixgbe depends on librte_bus_pci. This means that the bus drivers
> need
> >   # to be in the library path, so symlink the drivers from the main lib
> directory.
> > -meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
> > -             get_option('libdir'),
> > -             pmd_subdir_opt)
> > +if not is_windows
> > +     meson.add_install_script('../buildtools/symlink-drivers-solibs.sh',
> > +                     get_option('libdir'),
> > +                     pmd_subdir_opt)
> > +endif
> >
> >   # set the machine type and cflags for it
> >   if meson.is_cross_build()
> > diff --git a/drivers/meson.build b/drivers/meson.build index
> > 4bb7e9218..6b50f7238 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -186,8 +186,10 @@ foreach subpath:subdirs
> >                       lk_deps = [version_map, def_file, mingw_map]
> >                       if is_windows
> >                               if is_ms_linker
> > -                                     lk_args = ['-Wl,/def:' + def_file.full_path(),
> > -                                             '-Wl,/implib:drivers\\' + implib]
> > +                                     lk_args = ['-Wl,/def:' + def_file.full_path()]
> > +                                     if meson.version().version_compare('<0.54.0')
> > +                                             lk_args += ['-Wl,/implib:drivers\\' + implib]
> > +                                     endif
> >                               else
> >                                       lk_args = ['-Wl,--version-script=' +
> mingw_map.full_path()]
> >                               endif
> > diff --git a/lib/meson.build b/lib/meson.build index
> > 1bb019720..ed00f8914 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -157,8 +157,10 @@ foreach l:libraries
> >                               output: '@0@_mingw.map'.format(libname))
> >
> >                       if is_ms_linker
> > -                             lk_args = ['-Wl,/def:' + def_file.full_path(),
> > -                                     '-Wl,/implib:lib\\' + implib]
> > +                             lk_args = ['-Wl,/def:' + def_file.full_path()]
> > +                             if meson.version().version_compare('<0.54.0')
> > +                                     lk_args += ['-Wl,/implib:lib\\' + implib]
> > +                             endif
> >                       else
> >                               if is_windows
> >                                       lk_args =
> > ['-Wl,--version-script=' + mingw_map.full_path()]
> 
> Thanks, Nick. This finally fixes this problem I've been having since I upgraded
> to meson v0.54.

Do we know why meson changed its behavior of lib file location in versions > 0.54?
Was the old behavior of keeping lib files in the matching output folders incorrect?
	
BTW, although both are small changes,  you should move the symlink-drivers-solibs.sh change to a separated patch.

> 
> Tested-by: Ranjit Menon <ranjit.menon@intel.com>
> Acked-by: Ranjit Menon <ranjit.menon@intel.com>

Re: [dpdk-dev] [PATCH] windows: minor build fixes

[Nick Connolly]

On 30/10/2020 22:51, Tal Shnaiderman wrote:
> Do we know why meson changed its behavior of lib file location in versions > 0.54?
> Was the old behavior of keeping lib files in the matching output folders incorrect?
I'm not sure why meson was changed, but without the fix subsequent 
attempts to link with the
library fail.
> BTW, although both are small changes,  you should move the symlink-drivers-solibs.sh change to a separated patch.
>
Ok, will do.

Regards,
Nick

[dpdk-dev] [PATCH v2] windows: minor build fixes

[Nick Connolly]

Meson versions >= 0.54.0 include support for handling /implib
with msvc link. Specifying it explicitly causes failures when
linking against the dll. Tested using Link 14.27.29112.0 and
Clang 11.0.0.

There were a number of changes to the way that import libraries
are handled between 0.47.1 and 0.54.0. Only make the change
for >= 0.54.0, leaving the behaviour unchanged for earlier
versions.

Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
Tested-by: Ranjit Menon <ranjit.menon@intel.com>
Acked-by: Ranjit Menon <ranjit.menon@intel.com>
---
v2:
* split out config/meson.build change

 drivers/meson.build | 6 ++++--
 lib/meson.build     | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/meson.build b/drivers/meson.build
index 4bb7e9218..6b50f7238 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -186,8 +186,10 @@ foreach subpath:subdirs
 			lk_deps = [version_map, def_file, mingw_map]
 			if is_windows
 				if is_ms_linker
-					lk_args = ['-Wl,/def:' + def_file.full_path(),
-						'-Wl,/implib:drivers\\' + implib]
+					lk_args = ['-Wl,/def:' + def_file.full_path()]
+					if meson.version().version_compare('<0.54.0')
+						lk_args += ['-Wl,/implib:drivers\\' + implib]
+					endif
 				else
 					lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
 				endif
diff --git a/lib/meson.build b/lib/meson.build
index 1bb019720..ed00f8914 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -157,8 +157,10 @@ foreach l:libraries
 				output: '@0@_mingw.map'.format(libname))
 
 			if is_ms_linker
-				lk_args = ['-Wl,/def:' + def_file.full_path(),
-					'-Wl,/implib:lib\\' + implib]
+				lk_args = ['-Wl,/def:' + def_file.full_path()]
+				if meson.version().version_compare('<0.54.0')
+					lk_args += ['-Wl,/implib:lib\\' + implib]
+				endif
 			else
 				if is_windows
 					lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
-- 
2.25.1

Re: [dpdk-dev] [EXTERNAL] [PATCH v2] windows: minor build fixes

[Khoa To]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Nick Connolly
> Sent: Friday, October 30, 2020 11:45 PM
> Cc: dev@dpdk.org; bruce.richardson@intel.com; Nick Connolly
> <nick.connolly@mayadata.io>; ranjit.menon <ranjit.menon@intel.com>
> Subject: [EXTERNAL] [dpdk-dev] [PATCH v2] windows: minor build fixes
> 
> Meson versions >= 0.54.0 include support for handling /implib
> with msvc link. Specifying it explicitly causes failures when
> linking against the dll. Tested using Link 14.27.29112.0 and
> Clang 11.0.0.
> 
> There were a number of changes to the way that import libraries
> are handled between 0.47.1 and 0.54.0. Only make the change
> for >= 0.54.0, leaving the behaviour unchanged for earlier
> versions.
> 
> Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> Tested-by: Ranjit Menon <ranjit.menon@intel.com>
> Acked-by: Ranjit Menon <ranjit.menon@intel.com>
> ---
> v2:
> * split out config/meson.build change
> 

Acked-by: Khoa To <khot@microsoft.com>

Re: [dpdk-dev] [EXTERNAL] [PATCH v2] windows: minor build fixes

[Thomas Monjalon]

> > Meson versions >= 0.54.0 include support for handling /implib
> > with msvc link. Specifying it explicitly causes failures when
> > linking against the dll. Tested using Link 14.27.29112.0 and
> > Clang 11.0.0.
> > 
> > There were a number of changes to the way that import libraries
> > are handled between 0.47.1 and 0.54.0. Only make the change
> > for >= 0.54.0, leaving the behaviour unchanged for earlier
> > versions.
> > 
> > Signed-off-by: Nick Connolly <nick.connolly@mayadata.io>
> > Tested-by: Ranjit Menon <ranjit.menon@intel.com>
> > Acked-by: Ranjit Menon <ranjit.menon@intel.com>
> > ---
> > v2:
> > * split out config/meson.build change
> 
> Acked-by: Khoa To <khot@microsoft.com>

Applied, thanks