windows: minor build fixes

Message ID 20201030174617.1958-2-nick.connolly@mayadata.io (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series windows: minor build fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-testing fail Testing issues
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Nick Connolly Oct. 30, 2020, 5:46 p.m. UTC
  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(-)
  

Comments

John Alexander Oct. 30, 2020, 7:57 p.m. UTC | #1
> -----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.
  
Nick Connolly Oct. 30, 2020, 8:21 p.m. UTC | #2
> 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
  
Menon, Ranjit Oct. 30, 2020, 9:41 p.m. UTC | #3
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>
  
Tal Shnaiderman Oct. 30, 2020, 10:51 p.m. UTC | #4
> 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>
  
Nick Connolly Oct. 31, 2020, 6:16 a.m. UTC | #5
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
  

Patch

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()]