[v7,01/10] config: change ABI versioning to global

  From: Marcin Baran <marcinx.baran@intel.com>

As per new ABI policy, all of the libraries are now versioned using
one global ABI version. Changes in this patch implement the
necessary steps to enable that.

Signed-off-by: Marcin Baran <marcinx.baran@intel.com>
Signed-off-by: Pawel Modrak <pawelx.modrak@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---

Notes:
    v6:
    - Silenced grep error message on trying to grep a directory
    
    v3:
    - Removed Windows support from Makefile changes
    - Removed unneeded path conversions from meson files

 buildtools/meson.build |  2 ++
 config/ABI_VERSION     |  1 +
 config/meson.build     |  4 +++-
 drivers/meson.build    | 20 ++++++++++++--------
 lib/meson.build        | 18 ++++++++++++------
 meson_options.txt      |  2 --
 mk/rte.lib.mk          | 13 ++++---------
 7 files changed, 34 insertions(+), 26 deletions(-)
 create mode 100644 config/ABI_VERSION
  

08/11/2019 17:25, Anatoly Burakov:
> From: Marcin Baran <marcinx.baran@intel.com>
> 
> As per new ABI policy, all of the libraries are now versioned using
> one global ABI version. Changes in this patch implement the
> necessary steps to enable that.

For the history, would be nice to describe the "why" of each change here.
Please do not be lazy :)

> --- a/buildtools/meson.build
> +++ b/buildtools/meson.build
> +is_experimental_cmd = [find_program('grep', 'findstr'), '^DPDK_']

A comment is missing to explain the relationship between
"experimental" and "^DPDK_".


> --- /dev/null
> +++ b/config/ABI_VERSION
> +20.0

Why in config/ directory and not in root as for VERSION file?


> +			if is_experimental != 0
> +				lib_version = '0.1'

Why 0.1 and not 0.0?
How do we increment the minor version of experimental libs?

> +				so_version = '0'

How so_version is incremented?
It would deserve a comment here.

>  			if not use_function_versioning
> -				# use pre-build objects to build shared lib
> +				# then use pre-build objects to build shared lib

Is this change relevant?

> -option('per_library_versions', type: 'boolean', value: true,
> -	description: 'true: each lib gets its own version number, false: DPDK version used for each lib')

Good to see this option removed.
  

On 19-Nov-19 1:53 PM, Thomas Monjalon wrote:
> 08/11/2019 17:25, Anatoly Burakov:
>> From: Marcin Baran <marcinx.baran@intel.com>
>>
>> As per new ABI policy, all of the libraries are now versioned using
>> one global ABI version. Changes in this patch implement the
>> necessary steps to enable that.
> 
> For the history, would be nice to describe the "why" of each change here.

Here and other review comments:

I never really touched this script because my meson-foo and ABI-foo is 
non-existent. I would really appreciate some help here on what the 
changes are supposed to do (esp. with regards to ABI versioning 
mechanics), because i understand very little of what this patch does.

> Please do not be lazy :)

Says the guy who waited until last moment to review it!
  

> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Friday 8 November 2019 16:25
> To: dev@dpdk.org
> Cc: Baran, MarcinX <marcinx.baran@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Richardson, Bruce <bruce.richardson@intel.com>;
> Mcnamara, John <john.mcnamara@intel.com>; Kinsella, Ray
> <ray.kinsella@intel.com>; david.marchand@redhat.com; Pawel Modrak
> <pawelx.modrak@intel.com>
> Subject: [PATCH v7 01/10] config: change ABI versioning to global
> 
> From: Marcin Baran <marcinx.baran@intel.com>
> 
> As per new ABI policy, all of the libraries are now versioned using one
> global ABI version. Changes in this patch implement the necessary steps
> to enable that.
> 
> Signed-off-by: Marcin Baran <marcinx.baran@intel.com>
> Signed-off-by: Pawel Modrak <pawelx.modrak@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> 
> Notes:
>     v6:
>     - Silenced grep error message on trying to grep a directory
> 
>     v3:
>     - Removed Windows support from Makefile changes
>     - Removed unneeded path conversions from meson files
> 
>  buildtools/meson.build |  2 ++
>  config/ABI_VERSION     |  1 +
>  config/meson.build     |  4 +++-
>  drivers/meson.build    | 20 ++++++++++++--------
>  lib/meson.build        | 18 ++++++++++++------
>  meson_options.txt      |  2 --
>  mk/rte.lib.mk          | 13 ++++---------
>  7 files changed, 34 insertions(+), 26 deletions(-)  create mode 100644
> config/ABI_VERSION
> 
> diff --git a/buildtools/meson.build b/buildtools/meson.build index
> 32c79c1308..78ce69977d 100644
> --- a/buildtools/meson.build
> +++ b/buildtools/meson.build
> @@ -12,3 +12,5 @@ if python3.found()
>  else
>  	map_to_def_cmd = ['meson', 'runpython', files('map_to_def.py')]
> endif
> +
> +is_experimental_cmd = [find_program('grep', 'findstr'), '^DPDK_']
> diff --git a/config/ABI_VERSION b/config/ABI_VERSION new file mode
> 100644 index 0000000000..9a7c1e503f
> --- /dev/null
> +++ b/config/ABI_VERSION
> @@ -0,0 +1 @@
> +20.0
> diff --git a/config/meson.build b/config/meson.build index
> 2b1cb92e7e..30aa2a5313 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -18,6 +18,8 @@ endforeach
>  # depending on the configuration options  pver =
> meson.project_version().split('.')
>  major_version = '@0@.@1@'.format(pver.get(0), pver.get(1))
> +abi_version = run_command(find_program('cat', 'more'),
> +	files('ABI_VERSION')).stdout().strip()
> 
>  # extract all version information into the build configuration
> dpdk_conf.set('RTE_VER_YEAR', pver.get(0).to_int()) @@ -37,7 +39,7 @@
> endif
> 
>  pmd_subdir_opt = get_option('drivers_install_subdir')
>  if pmd_subdir_opt.contains('<VERSION>')
> -	pmd_subdir_opt =
> major_version.join(pmd_subdir_opt.split('<VERSION>'))
> +	pmd_subdir_opt =
> abi_version.join(pmd_subdir_opt.split('<VERSION>'))
>  endif
>  driver_install_path = join_paths(get_option('libdir'), pmd_subdir_opt)
> eal_pmd_path = join_paths(get_option('prefix'), driver_install_path)
> diff --git a/drivers/meson.build b/drivers/meson.build index
> 156d2dc717..b03e0c3159 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -124,12 +124,19 @@ foreach class:dpdk_driver_classes
>  					output: out_filename,
>  					depends: [pmdinfogen, tmp_lib])
> 
> -			if get_option('per_library_versions')
> -				lib_version = '@0@.1'.format(version)
> -				so_version = '@0@'.format(version)
> +			version_map = '@0@/@1@/@2@_version.map'.format(
> +					meson.current_source_dir(),
> +					drv_path, lib_name)
> +
> +			is_experimental = run_command(is_experimental_cmd,
> +				files(version_map)).returncode()
> +
> +			if is_experimental != 0
> +				lib_version = '0.1'
[rk] This all makes sense - except this part.
[rk] I would expect the experimental major version to always be zero ...
[rk] However I would expect the minor version to increment with each new release or at the maintainers discretion. 

> +				so_version = '0'
>  			else
> -				lib_version = major_version
> -				so_version = major_version
> +				lib_version = abi_version
> +				so_version = abi_version
>  			endif
> 
>  			# now build the static driver
> @@ -142,9 +149,6 @@ foreach class:dpdk_driver_classes
>  				install: true)
> 
>  			# now build the shared driver
> -			version_map = '@0@/@1@/@2@_version.map'.format(
> -					meson.current_source_dir(),
> -					drv_path, lib_name)
>  			shared_lib = shared_library(lib_name,
>  				sources,
>  				objects: objs,
> diff --git a/lib/meson.build b/lib/meson.build index
> b2ec9c09a9..3cff2482b1 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -106,12 +106,18 @@ foreach l:libraries
>  				cflags += '-DRTE_USE_FUNCTION_VERSIONING'
>  			endif
> 
> -			if get_option('per_library_versions')
> -				lib_version = '@0@.1'.format(version)
> -				so_version = '@0@'.format(version)
> +			version_map = '@0@/@1@/rte_@2@_version.map'.format(
> +					meson.current_source_dir(), dir_name, name)
> +
> +			is_experimental = run_command(is_experimental_cmd,
> +					files(version_map)).returncode()
> +
> +			if is_experimental != 0
> +				lib_version = '0.1'
> +				so_version = '0'
>  			else
> -				lib_version = major_version
> -				so_version = major_version
> +				lib_version = abi_version
> +				so_version = abi_version
>  			endif
> 
>  			# first build static lib
> @@ -127,7 +133,7 @@ foreach l:libraries
>  					dependencies: static_deps)
> 
>  			if not use_function_versioning
> -				# use pre-build objects to build shared lib
> +				# then use pre-build objects to build shared lib
>  				sources = []
>  				objs += static_lib.extract_all_objects(recursive:
> false)
>  			else
> diff --git a/meson_options.txt b/meson_options.txt index
> 89650b0e9c..da6a7f0302 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -30,8 +30,6 @@ option('max_lcores', type: 'integer', value: 128,
>  	description: 'maximum number of cores/threads supported by EAL')
> option('max_numa_nodes', type: 'integer', value: 4,
>  	description: 'maximum number of NUMA nodes supported by EAL') -
> option('per_library_versions', type: 'boolean', value: true,
> -	description: 'true: each lib gets its own version number, false:
> DPDK version used for each lib')
>  option('tests', type: 'boolean', value: true,
>  	description: 'build unit tests')
>  option('use_hpet', type: 'boolean', value: false, diff --git
> a/mk/rte.lib.mk b/mk/rte.lib.mk index 4df8849a08..b49af9192b 100644
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -11,20 +11,15 @@ EXTLIB_BUILD ?= n
>  # VPATH contains at least SRCDIR
>  VPATH += $(SRCDIR)
> 
> -ifneq ($(CONFIG_RTE_MAJOR_ABI),)
> -ifneq ($(LIBABIVER),)
> -LIBABIVER := $(CONFIG_RTE_MAJOR_ABI)
> -endif
> +ifneq ($(shell grep -s "^DPDK_" $(SRCDIR)/$(EXPORT_MAP)),) LIBABIVER
> :=
> +$(shell cat $(RTE_SRCDIR)/config/ABI_VERSION) else LIBABIVER := 0
>  endif
> 
>  ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
>  LIB := $(patsubst %.a,%.so.$(LIBABIVER),$(LIB))  ifeq
> ($(EXTLIB_BUILD),n) -ifeq ($(CONFIG_RTE_MAJOR_ABI),) -ifeq
> ($(CONFIG_RTE_NEXT_ABI),y) -LIB := $(LIB).1 -endif -endif  CPU_LDFLAGS
> += --version-script=$(SRCDIR)/$(EXPORT_MAP)
>  endif
>  endif
> --
> 2.17.1
  

20/11/2019 13:10, Kinsella, Ray:
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > +			if is_experimental != 0
> > +				lib_version = '0.1'
> [rk] This all makes sense - except this part.
> [rk] I would expect the experimental major version to always be zero ...
> [rk] However I would expect the minor version to increment with each new release or at the maintainers discretion. 

Yes, the minor must be incremented with each new release
if we want to allow 2 DPDK versions to be installed in the same system.

This policy must be changed:
"
Experimental libraries always have a major version of 0
to indicate they exist outside of ABI Versioning,
with the minor version incremented with each ABI change to library.
"

I propose to re-use the global ABI version for experimental
by prefixing with "0.".
So for ABI 20.0, it could be 0.20.0 or 0.200? Which one?
  

+1 - that's a plan.

Ray K

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday 20 November 2019 13:32
> To: Kinsella, Ray <ray.kinsella@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Cc: dev@dpdk.org; Baran, MarcinX <marcinx.baran@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; david.marchand@redhat.com; Pawel Modrak
> <pawelx.modrak@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [PATCH v7 01/10] config: change ABI versioning to global
> 
> 20/11/2019 13:10, Kinsella, Ray:
> > From: Burakov, Anatoly <anatoly.burakov@intel.com>
> > > --- a/drivers/meson.build
> > > +++ b/drivers/meson.build
> > > +			if is_experimental != 0
> > > +				lib_version = '0.1'
> > [rk] This all makes sense - except this part.
> > [rk] I would expect the experimental major version to always be zero
> ...
> > [rk] However I would expect the minor version to increment with each
> new release or at the maintainers discretion.
> 
> Yes, the minor must be incremented with each new release if we want to
> allow 2 DPDK versions to be installed in the same system.
> 
> This policy must be changed:
> "
> Experimental libraries always have a major version of 0 to indicate
> they exist outside of ABI Versioning, with the minor version
> incremented with each ABI change to library.
> "
> 
> I propose to re-use the global ABI version for experimental by
> prefixing with "0.".
> So for ABI 20.0, it could be 0.20.0 or 0.200? Which one?
>