[4/6] build: MinGW-w64 support for Meson
Checks
Commit Message
MinGW-w64 linker does not mimic MS linker options, so the build system
must differentiate between linkers on Windows. Use GNU linker options
with GCC and MS linker options with Clang.
MinGW-w64 by default uses MSVCRT stdio, which does not comply to ANSI,
most notably its formatting and string handling functions. MinGW-w64
support for the Universal CRT (UCRT) is ongoing, but the toolchain
provides its own standard-complying implementation of stdio. The latter
is used in the patch to support formatting in DPDK.
Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Comments
31/01/2020 04:07, Dmitry Kozlyuk:
> MinGW-w64 linker does not mimic MS linker options, so the build system
> must differentiate between linkers on Windows. Use GNU linker options
> with GCC and MS linker options with Clang.
>
> MinGW-w64 by default uses MSVCRT stdio, which does not comply to ANSI,
> most notably its formatting and string handling functions. MinGW-w64
> support for the Universal CRT (UCRT) is ongoing, but the toolchain
> provides its own standard-complying implementation of stdio. The latter
> is used in the patch to support formatting in DPDK.
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
I really like this patch.
So both GCC (with MinGW) and native clang are supported?
[...]
> +# MS linker requires special treatment.
> +# FIXME: use cc.get_linker_id() after upgrading to Meson >=0.53.
What does it mean? It won't work with meson 0.53?
> +is_ms_linker = is_windows and (cc.get_id() == 'clang')
[...]
> +if is_windows
> + # Require platform SDK for Windows 7 and above.
> + add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
Please explain. Why Windows 7 is needed? What this define is doing?
[...]
> - if is_windows
> +
> + if is_ms_linker
> lk_args = ['-Wl,/def:' + def_file.full_path(),
> '-Wl,/implib:lib\\' + implib]
> else
> lk_args = ['-Wl,--version-script=' + version_map]
> + endif
Looks good.
> I really like this patch.
> So both GCC (with MinGW) and native clang are supported?
Thanks. Yes, I tested both toolchains.
> [...]
> > +# MS linker requires special treatment.
> > +# FIXME: use cc.get_linker_id() after upgrading to Meson >=0.53.
>
> What does it mean? It won't work with meson 0.53?
It will work for any Meson version, I meant that Meson 0.54 will introduce a
better way to determine linker ID. Can you suggest a better wording?
>
> > +is_ms_linker = is_windows and (cc.get_id() == 'clang')
> [...]
> > +if is_windows
> > + # Require platform SDK for Windows 7 and above.
> > + add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
>
> Please explain. Why Windows 7 is needed? What this define is doing?
Yes, Windows 7 and above is need for already existing code in eal_lcore.c,
specifically for GetLogicalProcessorInformation() call.
When including <windows.h>, one must define minimum API version the
application is compiled against [0]. MSVC and Clang default to the version of
platform SDK (that is, maximum supported). MinGW defaults to Windows XP, so
this definition must be either in <rte_os.h> before #include <windows.h> or
here. Because other files may include <windows.h>, I'd prefer to have a
global definition via compiler command-line.
[0]:
https://docs.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers
05/02/2020 00:21, Dmitry Kozlyuk:
> > I really like this patch.
> > So both GCC (with MinGW) and native clang are supported?
>
> Thanks. Yes, I tested both toolchains.
>
> > [...]
> > > +# MS linker requires special treatment.
> > > +# FIXME: use cc.get_linker_id() after upgrading to Meson >=0.53.
> >
> > What does it mean? It won't work with meson 0.53?
>
> It will work for any Meson version, I meant that Meson 0.54 will introduce a
> better way to determine linker ID. Can you suggest a better wording?
FIXME: use cc.get_linker_id() with Meson >= 0.54
> > > +is_ms_linker = is_windows and (cc.get_id() == 'clang')
> > [...]
> > > +if is_windows
> > > + # Require platform SDK for Windows 7 and above.
> > > + add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
> >
> > Please explain. Why Windows 7 is needed? What this define is doing?
>
> Yes, Windows 7 and above is need for already existing code in eal_lcore.c,
> specifically for GetLogicalProcessorInformation() call.
>
> When including <windows.h>, one must define minimum API version the
> application is compiled against [0]. MSVC and Clang default to the version of
> platform SDK (that is, maximum supported). MinGW defaults to Windows XP, so
> this definition must be either in <rte_os.h> before #include <windows.h> or
> here. Because other files may include <windows.h>, I'd prefer to have a
> global definition via compiler command-line.
>
> [0]:
> https://docs.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers
OK, thanks.
Please reword the comment with something like
"Minimum supported API is Windows 7."
On Wed, Feb 05, 2020 at 01:41:24AM +0100, Thomas Monjalon wrote:
> 05/02/2020 00:21, Dmitry Kozlyuk:
> > > I really like this patch.
> > > So both GCC (with MinGW) and native clang are supported?
> >
> > Thanks. Yes, I tested both toolchains.
> >
> > > [...]
> > > > +# MS linker requires special treatment.
> > > > +# FIXME: use cc.get_linker_id() after upgrading to Meson >=0.53.
> > >
> > > What does it mean? It won't work with meson 0.53?
> >
> > It will work for any Meson version, I meant that Meson 0.54 will introduce a
> > better way to determine linker ID. Can you suggest a better wording?
>
> FIXME: use cc.get_linker_id() with Meson >= 0.54
>
>
> > > > +is_ms_linker = is_windows and (cc.get_id() == 'clang')
> > > [...]
> > > > +if is_windows
> > > > + # Require platform SDK for Windows 7 and above.
> > > > + add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
> > >
> > > Please explain. Why Windows 7 is needed? What this define is doing?
> >
> > Yes, Windows 7 and above is need for already existing code in eal_lcore.c,
> > specifically for GetLogicalProcessorInformation() call.
> >
> > When including <windows.h>, one must define minimum API version the
> > application is compiled against [0]. MSVC and Clang default to the version of
> > platform SDK (that is, maximum supported). MinGW defaults to Windows XP, so
> > this definition must be either in <rte_os.h> before #include <windows.h> or
> > here. Because other files may include <windows.h>, I'd prefer to have a
> > global definition via compiler command-line.
> >
> > [0]:
> > https://docs.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers
>
> OK, thanks.
> Please reword the comment with something like
> "Minimum supported API is Windows 7."
>
For this, as an alternative to putting it as a project argument, you can just
add it to dpdk_conf which means it will end up as a define in the global
rte_build_config.h and so be directly included in each compilation unit
ahead of any other headers. (rte_config.h includes rte_build_config.h)
/Bruce
> > > > > +if is_windows
> > > > > + # Require platform SDK for Windows 7 and above.
> > > > > + add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
> > > >
> > > > Please explain. Why Windows 7 is needed? What this define is doing?
> > >
> > > Yes, Windows 7 and above is need for already existing code in eal_lcore.c,
> > > specifically for GetLogicalProcessorInformation() call.
> > >
> > > When including <windows.h>, one must define minimum API version the
> > > application is compiled against [0]. MSVC and Clang default to the version of
> > > platform SDK (that is, maximum supported). MinGW defaults to Windows XP, so
> > > this definition must be either in <rte_os.h> before #include <windows.h> or
> > > here. Because other files may include <windows.h>, I'd prefer to have a
> > > global definition via compiler command-line.
> > >
> > > [0]:
> > > https://docs.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers
> >
> > OK, thanks.
> > Please reword the comment with something like
> > "Minimum supported API is Windows 7."
> >
> For this, as an alternative to putting it as a project argument, you can just
> add it to dpdk_conf which means it will end up as a define in the global
> rte_build_config.h and so be directly included in each compilation unit
> ahead of any other headers. (rte_config.h includes rte_build_config.h)
Can you please explain why using dpdk_conf is a better alternative? In
lib/meson.build I can see add_project_arguments('_D_GNU_SOURCE', ...), which
serves a similar purpose on POSIX systems. Compiler option also makes it
impossible to forget or redefine this constant in code by mistake.
On Wed, Feb 05, 2020 at 11:41:05PM +0300, Dmitry Kozlyuk wrote:
> > > > > > +if is_windows
> > > > > > + # Require platform SDK for Windows 7 and above.
> > > > > > + add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
> > > > >
> > > > > Please explain. Why Windows 7 is needed? What this define is doing?
> > > >
> > > > Yes, Windows 7 and above is need for already existing code in eal_lcore.c,
> > > > specifically for GetLogicalProcessorInformation() call.
> > > >
> > > > When including <windows.h>, one must define minimum API version the
> > > > application is compiled against [0]. MSVC and Clang default to the version of
> > > > platform SDK (that is, maximum supported). MinGW defaults to Windows XP, so
> > > > this definition must be either in <rte_os.h> before #include <windows.h> or
> > > > here. Because other files may include <windows.h>, I'd prefer to have a
> > > > global definition via compiler command-line.
> > > >
> > > > [0]:
> > > > https://docs.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers
> > >
> > > OK, thanks.
> > > Please reword the comment with something like
> > > "Minimum supported API is Windows 7."
> > >
> > For this, as an alternative to putting it as a project argument, you can just
> > add it to dpdk_conf which means it will end up as a define in the global
> > rte_build_config.h and so be directly included in each compilation unit
> > ahead of any other headers. (rte_config.h includes rte_build_config.h)
>
> Can you please explain why using dpdk_conf is a better alternative? In
> lib/meson.build I can see add_project_arguments('_D_GNU_SOURCE', ...), which
> serves a similar purpose on POSIX systems. Compiler option also makes it
> impossible to forget or redefine this constant in code by mistake.
>
I'm not necessarily saying it's better, it's just an alternative to
consider. :-) Having it in rte_config.h makes the define available to any
external apps using DPDK, which may or may not be desirable.
> On Wed, Feb 05, 2020 at 11:41:05PM +0300, Dmitry Kozlyuk wrote:
> > > > > > > +if is_windows
> > > > > > > + # Require platform SDK for Windows 7 and above.
> > > > > > > + add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
> > > > > >
> > > > > > Please explain. Why Windows 7 is needed? What this define is doing?
> > > > >
> > > > > Yes, Windows 7 and above is need for already existing code in eal_lcore.c,
> > > > > specifically for GetLogicalProcessorInformation() call.
> > > > >
> > > > > When including <windows.h>, one must define minimum API version the
> > > > > application is compiled against [0]. MSVC and Clang default to the version of
> > > > > platform SDK (that is, maximum supported). MinGW defaults to Windows XP, so
> > > > > this definition must be either in <rte_os.h> before #include <windows.h> or
> > > > > here. Because other files may include <windows.h>, I'd prefer to have a
> > > > > global definition via compiler command-line.
> > > > >
> > > > > [0]:
> > > > > https://docs.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers
> > > >
> > > > OK, thanks.
> > > > Please reword the comment with something like
> > > > "Minimum supported API is Windows 7."
> > > >
> > > For this, as an alternative to putting it as a project argument, you can just
> > > add it to dpdk_conf which means it will end up as a define in the global
> > > rte_build_config.h and so be directly included in each compilation unit
> > > ahead of any other headers. (rte_config.h includes rte_build_config.h)
> >
> > Can you please explain why using dpdk_conf is a better alternative? In
> > lib/meson.build I can see add_project_arguments('_D_GNU_SOURCE', ...), which
> > serves a similar purpose on POSIX systems. Compiler option also makes it
> > impossible to forget or redefine this constant in code by mistake.
> >
> I'm not necessarily saying it's better, it's just an alternative to
> consider. :-) Having it in rte_config.h makes the define available to any
> external apps using DPDK, which may or may not be desirable.
It's certainly *not* desirable, apps usually have this value
consciously and explicitly defined on their own, and it may differ from the
one required to compile DPDK. Thanks for the tip, but as I hope you can see,
it should stay a project argument.
@@ -14,6 +14,10 @@ foreach env:supported_exec_envs
set_variable('is_' + env, exec_env == env)
endforeach
+# MS linker requires special treatment.
+# FIXME: use cc.get_linker_id() after upgrading to Meson >=0.53.
+is_ms_linker = is_windows and (cc.get_id() == 'clang')
+
# set the major version, which might be used by drivers and libraries
# depending on the configuration options
pver = meson.project_version().split('.')
@@ -241,6 +245,16 @@ if is_freebsd
add_project_arguments('-D__BSD_VISIBLE', language: 'c')
endif
+if is_windows
+ # Require platform SDK for Windows 7 and above.
+ add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
+
+ # Use MinGW-w64 stdio, because DPDK assumes ANSI-compliant formatting.
+ if cc.get_id() == 'gcc'
+ add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
+ endif
+endif
+
if get_option('b_lto')
if cc.has_argument('-ffat-lto-objects')
add_project_arguments('-ffat-lto-objects', language: 'c')
@@ -20,6 +20,9 @@ endif
if cc.has_function('getentropy', prefix : '#include <unistd.h>')
cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
endif
+if cc.get_id() == 'gcc'
+ cflags += '-D__USE_MINGW_ANSI_STDIO'
+endif
sources = common_sources + env_sources
objs = common_objs + env_objs
headers = common_headers + env_headers
@@ -148,12 +148,16 @@ foreach l:libraries
command: [map_to_def_cmd, '@INPUT@', '@OUTPUT@'],
input: version_map,
output: 'rte_@0@_exports.def'.format(name))
- lk_deps = [version_map, def_file]
- if is_windows
+
+ if is_ms_linker
lk_args = ['-Wl,/def:' + def_file.full_path(),
'-Wl,/implib:lib\\' + implib]
else
lk_args = ['-Wl,--version-script=' + version_map]
+ endif
+
+ lk_deps = [version_map, def_file]
+ if not is_windows
# on unix systems check the output of the
# experimental syms script, using it as a
# dependency of the .so build