[v5,1/3] eal: disable function versioning on Windows
Checks
Commit Message
Function versioning implementation is not supported by Windows.
Function versioning was disabled on Windows.
Signed-off-by: Fady Bader <fady@mellanox.com>
---
lib/librte_eal/include/rte_function_versioning.h | 2 +-
lib/meson.build | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
Comments
On Sun, Jul 05, 2020 at 02:46:27PM +0300, Fady Bader wrote:
> Function versioning implementation is not supported by Windows.
> Function versioning was disabled on Windows.
>
> Signed-off-by: Fady Bader <fady@mellanox.com>
> ---
> lib/librte_eal/include/rte_function_versioning.h | 2 +-
> lib/meson.build | 5 +++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/include/rte_function_versioning.h b/lib/librte_eal/include/rte_function_versioning.h
> index f588f2643b..9890551ba1 100644
> --- a/lib/librte_eal/include/rte_function_versioning.h
> +++ b/lib/librte_eal/include/rte_function_versioning.h
> @@ -7,7 +7,7 @@
> #define _RTE_FUNCTION_VERSIONING_H_
> #include <rte_common.h>
>
> -#ifndef RTE_USE_FUNCTION_VERSIONING
> +#if !defined RTE_USE_FUNCTION_VERSIONING && !defined RTE_EXEC_ENV_WINDOWS
> #error Use of function versioning disabled, is "use_function_versioning=true" in meson.build?
> #endif
>
> diff --git a/lib/meson.build b/lib/meson.build
> index c1b9e1633f..a1ab582a51 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -107,6 +107,11 @@ foreach l:libraries
> shared_dep = declare_dependency(include_directories: includes)
> static_dep = shared_dep
> else
> + if is_windows and use_function_versioning
> + message('@0@: Function versioning is not supported by Windows.'
> + .format(name))
> + use_function_versioning = false
> + endif
>
> if use_function_versioning
> cflags += '-DRTE_USE_FUNCTION_VERSIONING'
> --
I wonder if this patch can be simplified as follows.
Presumably the use of the RTE_USE_FUNCTION_VERSIONING cflag doesn't cause
any problems building on windows, since it's basically nothing except a
flag to indicate the build is configured properly, so that block can be
left intact. Instead what is needed to be disabled is the
RTE_BUILD_SHARED_LIB setting so that we use the static macros. Therefore,
I think the same result can be got by just changing the following line
in lib/meson.build:
@@ -138,7 +138,7 @@ foreach l:libraries
include_directories: includes,
dependencies: static_deps)
- if not use_function_versioning
+ if not use_function_versioning or is_windows
# use pre-build objects to build shared lib
sources = []
objs += static_lib.extract_all_objects(recursive: false)
Then no error message disabling is needed in windows. I also don't think a
message about function versioning not being supported on windows is
necessary. It's not something the user can do anything about himself
anyway. If we want such a message I think it should just be printed once at
the start of the configuration process, rather than each time we hit a
versioned library.
Regards,
/Bruce
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Monday, July 6, 2020 11:20 AM
> To: Fady Bader <fady@mellanox.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Tasnim Bashar
> <tbashar@mellanox.com>; Tal Shnaiderman <talshn@mellanox.com>; Yohad Tor
> <yohadt@mellanox.com>; dmitry.kozliuk@gmail.com;
> harini.ramakrishnan@microsoft.com; ocardona@microsoft.com;
> pallavi.kadam@intel.com; ranjit.menon@intel.com; olivier.matz@6wind.com;
> arybchenko@solarflare.com; mdr@ashroe.eu; nhorman@tuxdriver.com
> Subject: Re: [dpdk-dev] [PATCH v5 1/3] eal: disable function versioning on
> Windows
>
> On Sun, Jul 05, 2020 at 02:46:27PM +0300, Fady Bader wrote:
> > Function versioning implementation is not supported by Windows.
> > Function versioning was disabled on Windows.
> >
> > Signed-off-by: Fady Bader <fady@mellanox.com>
> > ---
> > lib/librte_eal/include/rte_function_versioning.h | 2 +-
> > lib/meson.build | 5 +++++
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eal/include/rte_function_versioning.h
> > b/lib/librte_eal/include/rte_function_versioning.h
> > index f588f2643b..9890551ba1 100644
> > --- a/lib/librte_eal/include/rte_function_versioning.h
> > +++ b/lib/librte_eal/include/rte_function_versioning.h
> > @@ -7,7 +7,7 @@
> > #define _RTE_FUNCTION_VERSIONING_H_
> > #include <rte_common.h>
> >
> > -#ifndef RTE_USE_FUNCTION_VERSIONING
> > +#if !defined RTE_USE_FUNCTION_VERSIONING && !defined
> > +RTE_EXEC_ENV_WINDOWS
> > #error Use of function versioning disabled, is "use_function_versioning=true"
> in meson.build?
> > #endif
> >
> > diff --git a/lib/meson.build b/lib/meson.build index
> > c1b9e1633f..a1ab582a51 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -107,6 +107,11 @@ foreach l:libraries
> > shared_dep = declare_dependency(include_directories:
> includes)
> > static_dep = shared_dep
> > else
> > + if is_windows and use_function_versioning
> > + message('@0@: Function versioning is not
> supported by Windows.'
> > + .format(name))
> > + use_function_versioning = false
> > + endif
> >
> > if use_function_versioning
> > cflags += '-DRTE_USE_FUNCTION_VERSIONING'
> > --
>
> I wonder if this patch can be simplified as follows.
>
> Presumably the use of the RTE_USE_FUNCTION_VERSIONING cflag doesn't
> cause any problems building on windows, since it's basically nothing except a flag
> to indicate the build is configured properly, so that block can be left intact.
> Instead what is needed to be disabled is the RTE_BUILD_SHARED_LIB setting so
> that we use the static macros. Therefore, I think the same result can be got by
> just changing the following line in lib/meson.build:
>
> @@ -138,7 +138,7 @@ foreach l:libraries
> include_directories: includes,
> dependencies: static_deps)
>
> - if not use_function_versioning
> + if not use_function_versioning or is_windows
> # use pre-build objects to build shared lib
> sources = []
> objs += static_lib.extract_all_objects(recursive: false)
Good point, the patch is much simpler now.
I'll send a new version soon.
>
> Then no error message disabling is needed in windows. I also don't think a
> message about function versioning not being supported on windows is
> necessary. It's not something the user can do anything about himself anyway. If
> we want such a message I think it should just be printed once at the start of the
> configuration process, rather than each time we hit a versioned library.
It was added in order to avoid confusions in case Linux developers try to run the
project on windows. I'll print it once.
>
> Regards,
> /Bruce
@@ -7,7 +7,7 @@
#define _RTE_FUNCTION_VERSIONING_H_
#include <rte_common.h>
-#ifndef RTE_USE_FUNCTION_VERSIONING
+#if !defined RTE_USE_FUNCTION_VERSIONING && !defined RTE_EXEC_ENV_WINDOWS
#error Use of function versioning disabled, is "use_function_versioning=true" in meson.build?
#endif
@@ -107,6 +107,11 @@ foreach l:libraries
shared_dep = declare_dependency(include_directories: includes)
static_dep = shared_dep
else
+ if is_windows and use_function_versioning
+ message('@0@: Function versioning is not supported by Windows.'
+ .format(name))
+ use_function_versioning = false
+ endif
if use_function_versioning
cflags += '-DRTE_USE_FUNCTION_VERSIONING'