Re: [PATCH 1/2] drivers: suggestion on meson without version file

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

The title of this patch needs an update - I would suggest something
like:

"build: make version file optional for drivers"

More comments inline below.

On Thu, Oct 06, 2022 at 10:19:22AM +0300, Abdullah Ömer Yamaç wrote:
> Most of the drivers don't have a special version.map file. They just
> included due to the compilation issue and needs to be updated for each
> release.
> 
> These version.map files include:
> DPDK_23 {
>   local: *;
> };
> 
> In this patch, we removed the necessity of the version files and
> you don't need to update these files for each release, you can just
> remove them.
> 
> Signed-off-by: Abdullah Ömer Yamaç <omer.yamac@ceng.metu.edu.tr>
> Suggested-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 
> ---
> Depends on: patch-116222 ("build: increase minimum meson version to 0.53")
> ---

Thanks for splitting the patch. It is a lot easier to review now,
especially if we apply and use "diff -w".

For any other reviewers, the "diff -w" for this patch is:

--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017-2019 Intel Corporation
 
+fs = import('fs')
+
 # Defines the order of dependencies evaluation
 subdirs = [
         'common',
@@ -193,6 +195,7 @@ foreach subpath:subdirs
         version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
         implib = 'lib' + lib_name + '.dll.a'
 
+        if fs.is_file(version_map)
             def_file = custom_target(lib_name + '_def',
                     command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
                     input: version_map,
@@ -227,6 +230,8 @@ foreach subpath:subdirs
                 endif
             endif
 
+        endif
+
         shared_lib = shared_library(lib_name, sources,
                 objects: objs,
                 include_directories: includes,


>  drivers/meson.build | 63 ++++++++++++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/meson.build b/drivers/meson.build
> index f6ba5ba4fb..6ef03e14c7 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -1,6 +1,8 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2017-2019 Intel Corporation
>  
> +fs = import('fs')
> +
>  # Defines the order of dependencies evaluation
>  subdirs = [
>          'common',
> @@ -193,38 +195,41 @@ foreach subpath:subdirs
>          version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
>          implib = 'lib' + lib_name + '.dll.a'
>  
<snip>
> +                            capture: true,
> +                            input: static_lib,
> +                            output: lib_name + '.sym_chk')
> +                endif
>              endif
> +
>          endif
>  
>          shared_lib = shared_library(lib_name, sources,

Beware that the shared_lib calls use both lk_deps and lk_args parameters,
which are only set inside the "if" block you added.
This will cause problems in that:
1. If the first driver doesn't have a version.map file, these variables
   will be undefined and you'll get a build error.
2. For any subsequent drivers that don't have a version.map file, the old
   values of the variables from the previous driver will be used.

Therefore, at the start of processing each driver, you need to assign empty
values to these variable.

Regards,
/Bruce