Re: [dpdk-dev] [PATCH v5 1/3] eal: disable function versioning on Windows

[Bruce Richardson <bruce.richardson@intel.com>]

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