[dpdk-dev,v2,16/17] build: add option to version libs using DPDK version
Checks
Commit Message
Normally, each library has it's own version number based on the ABI.
Add an option to have all libs just use the DPDK version number as the
.so version.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
---
drivers/meson.build | 8 +++++++-
lib/meson.build | 8 +++++++-
meson_options.txt | 1 +
3 files changed, 15 insertions(+), 2 deletions(-)
Comments
On Tue, 2017-09-12 at 11:38 +0100, Bruce Richardson wrote:
> Normally, each library has it's own version number based on the ABI.
> Add an option to have all libs just use the DPDK version number as
> the
> .so version.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
> drivers/meson.build | 8 +++++++-
> lib/meson.build | 8 +++++++-
> meson_options.txt | 1 +
> 3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/meson.build b/drivers/meson.build
> index f19da16fb..0c251bd61 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -92,6 +92,12 @@ foreach class:driver_classes
> depends:
> [pmdinfogen, tmp_lib])
> endforeach
>
> + if get_option('per_library_versions')
> + so_version = '@0@.1'.format(version)
> + else
> + so_version = meson.project_version()
> + endif
> +
> # now build the driver itself, and add to
> the drivers list
> lib_name = driver_name_fmt.format(name)
> version_map = '@0@/@1@/@2@_version.map'.form
> at(
> @@ -105,7 +111,7 @@ foreach class:driver_classes
> c_args: cflags,
> link_args: '-Wl,--version-script=' +
> version_map,
> link_depends: version_map,
> - version: '@0@.1'.format(version),
> + version: so_version,
> install: true,
> install_dir: driver_install_path)
>
> diff --git a/lib/meson.build b/lib/meson.build
> index d814721de..36652cfe1 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -76,6 +76,12 @@ foreach l:libraries
> dep_objs += [get_variable('dep_rte_' + d)]
> endforeach
>
> + if get_option('per_library_versions')
> + so_version = '@0@.1'.format(version)
> + else
> + so_version = meson.project_version()
> + endif
> +
> version_map = '@0@/@1@/rte_@2@_version.map'.format(
> meson.current_source_dir(),
> dir_name, name)
> libname = 'rte_' + name
> @@ -87,7 +93,7 @@ foreach l:libraries
> include_directories:
> include_directories(dir_name),
> link_args: '-Wl,--version-script=' +
> version_map,
> link_depends: version_map,
> - version: '@0@.1'.format(version),
> + version: so_version,
> install: true)
> dep = declare_dependency(link_with: lib,
> include_directories:
> include_directories(dir_name),
> diff --git a/meson_options.txt b/meson_options.txt
> index 9c45b8159..636226ce8 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -6,3 +6,4 @@ option('allow_invalid_socket_id', type: 'boolean',
> value: false,
> description: 'allow out-of-range NUMA socket id\'s for
> platforms that don\'t report the value correctly')
> option('enable_kmods', type: 'boolean', value: true, description:
> 'build kernel modules')
> option('kernel_dir', type: 'string', value: '', description: 'path
> to the kernel for building kernel modules')
> +option('per_library_versions', type: 'boolean', value: true,
> description: 'true: each lib gets its own version number, false: DPDK
> version used for each lib')
Hi Bruce,
First of all thanks for implementing this option, and sorry for the
delay.
A couple of minor things:
1) The project version has 3 digits, but the ABI version should have 2
- eg: should be 17.08 but right now it's 17.08.0 - given point releases
will be ABI compatible so they must not change the ABI version. In the
packaging code we use cut to chop it off:
DPDK_ABI := $(shell echo $(DEB_VERSION_UPSTREAM) | cut -d '.' -f1-2)
Not sure how to achieve the same in a Meson script though (can it shell
out? or are there string manipulation built-ins?), sorry - just
starting to look at it.
Right now the rte_config option allows the user to completely override
the version - I'm fine with having a single boolean provided this
change is applied, it simplifies packaging a bit, but if it's easier
for you to just accepted a version instead of a boolean it's fine as
well, as you prefer
2) When the option is true, it should not generate a symlink with the
ABI "revision", as the first digit is really not the revision, so it
does not make much sense. Worse, it will stop 2 versions being co-
installable (which was the whole point :-) ), as, eg, 17.02, 17.05 and
17.08 will all ship a libfoo.so.17 symlink. I can fix it in the
packaging, but I don't think it should be generated upstream at all
when this option is enabled.
IOW, the following is the current result:
$ ls -l debian/librte-ethdev17.08/usr/lib/x86_64-linux-gnu/
total 72
lrwxrwxrwx 1 lboccass lboccass 24 Sep 13 11:50 librte_ethdev.so.17 -> librte_ethdev.so.17.08.0
-rw-r--r-- 1 lboccass lboccass 71672 Sep 13 11:50 librte_ethdev.so.17.08.0
The expected result would be to create a single file "librte_ethdev.so.17.08" instead.
On Wed, Sep 13, 2017 at 12:32:24PM +0100, Luca Boccassi wrote:
> On Tue, 2017-09-12 at 11:38 +0100, Bruce Richardson wrote:
> > Normally, each library has it's own version number based on the ABI.
> > Add an option to have all libs just use the DPDK version number as
> > the
> > .so version.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
> > ---
> > drivers/meson.build | 8 +++++++-
> > lib/meson.build | 8 +++++++-
> > meson_options.txt | 1 +
> > 3 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/meson.build b/drivers/meson.build
> > index f19da16fb..0c251bd61 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -92,6 +92,12 @@ foreach class:driver_classes
> > depends:
> > [pmdinfogen, tmp_lib])
> > endforeach
> >
> > + if get_option('per_library_versions')
> > + so_version = '@0@.1'.format(version)
> > + else
> > + so_version = meson.project_version()
> > + endif
> > +
> > # now build the driver itself, and add to
> > the drivers list
> > lib_name = driver_name_fmt.format(name)
> > version_map = '@0@/@1@/@2@_version.map'.form
> > at(
> > @@ -105,7 +111,7 @@ foreach class:driver_classes
> > c_args: cflags,
> > link_args: '-Wl,--version-script=' +
> > version_map,
> > link_depends: version_map,
> > - version: '@0@.1'.format(version),
> > + version: so_version,
> > install: true,
> > install_dir: driver_install_path)
> >
> > diff --git a/lib/meson.build b/lib/meson.build
> > index d814721de..36652cfe1 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -76,6 +76,12 @@ foreach l:libraries
> > dep_objs += [get_variable('dep_rte_' + d)]
> > endforeach
> >
> > + if get_option('per_library_versions')
> > + so_version = '@0@.1'.format(version)
> > + else
> > + so_version = meson.project_version()
> > + endif
> > +
> > version_map = '@0@/@1@/rte_@2@_version.map'.format(
> > meson.current_source_dir(),
> > dir_name, name)
> > libname = 'rte_' + name
> > @@ -87,7 +93,7 @@ foreach l:libraries
> > include_directories:
> > include_directories(dir_name),
> > link_args: '-Wl,--version-script=' +
> > version_map,
> > link_depends: version_map,
> > - version: '@0@.1'.format(version),
> > + version: so_version,
> > install: true)
> > dep = declare_dependency(link_with: lib,
> > include_directories:
> > include_directories(dir_name),
> > diff --git a/meson_options.txt b/meson_options.txt
> > index 9c45b8159..636226ce8 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -6,3 +6,4 @@ option('allow_invalid_socket_id', type: 'boolean',
> > value: false,
> > description: 'allow out-of-range NUMA socket id\'s for
> > platforms that don\'t report the value correctly')
> > option('enable_kmods', type: 'boolean', value: true, description:
> > 'build kernel modules')
> > option('kernel_dir', type: 'string', value: '', description: 'path
> > to the kernel for building kernel modules')
> > +option('per_library_versions', type: 'boolean', value: true,
> > description: 'true: each lib gets its own version number, false: DPDK
> > version used for each lib')
>
> Hi Bruce,
>
> First of all thanks for implementing this option, and sorry for the
> delay.
>
> A couple of minor things:
>
> 1) The project version has 3 digits, but the ABI version should have 2
> - eg: should be 17.08 but right now it's 17.08.0 - given point releases
> will be ABI compatible so they must not change the ABI version. In the
> packaging code we use cut to chop it off:
>
> DPDK_ABI := $(shell echo $(DEB_VERSION_UPSTREAM) | cut -d '.' -f1-2)
>
> Not sure how to achieve the same in a Meson script though (can it shell
> out? or are there string manipulation built-ins?), sorry - just
> starting to look at it.
>
That should be easy enough to do. It's 3-digits because I'm just pulling
the project version unmodified from the initial project definition. I'd
say I can manage to strip off one digit as part of the build.
> Right now the rte_config option allows the user to completely override
> the version - I'm fine with having a single boolean provided this
> change is applied, it simplifies packaging a bit, but if it's easier
> for you to just accepted a version instead of a boolean it's fine as
> well, as you prefer
If there is a case where we might want to provide a particular version,
I'm happy to edit the option, but if not I'll keep it as-is, because it
will ensure all users of the option get the exact same behaviour, and
means I only have two possibilities to test.
>
> 2) When the option is true, it should not generate a symlink with the
> ABI "revision", as the first digit is really not the revision, so it
> does not make much sense. Worse, it will stop 2 versions being co-
> installable (which was the whole point :-) ), as, eg, 17.02, 17.05 and
> 17.08 will all ship a libfoo.so.17 symlink. I can fix it in the
> packaging, but I don't think it should be generated upstream at all
> when this option is enabled.
Not exactly sure how to fix this, but I think it may work by using
soversion as well as version in the library() calls.
>
> IOW, the following is the current result:
>
> $ ls -l debian/librte-ethdev17.08/usr/lib/x86_64-linux-gnu/
> total 72
> lrwxrwxrwx 1 lboccass lboccass 24 Sep 13 11:50 librte_ethdev.so.17 -> librte_ethdev.so.17.08.0
> -rw-r--r-- 1 lboccass lboccass 71672 Sep 13 11:50 librte_ethdev.so.17.08.0
>
> The expected result would be to create a single file "librte_ethdev.so.17.08" instead.
>
Let me see what I can do :-)
/Bruce
On Wed, 2017-09-13 at 14:11 +0100, Bruce Richardson wrote:
> On Wed, Sep 13, 2017 at 12:32:24PM +0100, Luca Boccassi wrote:
> > On Tue, 2017-09-12 at 11:38 +0100, Bruce Richardson wrote:
> > > Normally, each library has it's own version number based on the
> > > ABI.
> > > Add an option to have all libs just use the DPDK version number
> > > as
> > > the
> > > .so version.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
> > > ---
> > > drivers/meson.build | 8 +++++++-
> > > lib/meson.build | 8 +++++++-
> > > meson_options.txt | 1 +
> > > 3 files changed, 15 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/meson.build b/drivers/meson.build
> > > index f19da16fb..0c251bd61 100644
> > > --- a/drivers/meson.build
> > > +++ b/drivers/meson.build
> > > @@ -92,6 +92,12 @@ foreach class:driver_classes
> > > depends:
> > > [pmdinfogen, tmp_lib])
> > > endforeach
> > >
> > > + if get_option('per_library_versions')
> > > + so_version = '@0@.1'.format(vers
> > > ion)
> > > + else
> > > + so_version =
> > > meson.project_version()
> > > + endif
> > > +
> > > # now build the driver itself, and add
> > > to
> > > the drivers list
> > > lib_name = driver_name_fmt.format(name)
> > > version_map = '@0@/@1@/@2@_version.map'.
> > > form
> > > at(
> > > @@ -105,7 +111,7 @@ foreach class:driver_classes
> > > c_args: cflags,
> > > link_args: '-Wl,--version-
> > > script=' +
> > > version_map,
> > > link_depends: version_map,
> > > - version: '@0@.1'.format(version)
> > > ,
> > > + version: so_version,
> > > install: true,
> > > install_dir:
> > > driver_install_path)
> > >
> > > diff --git a/lib/meson.build b/lib/meson.build
> > > index d814721de..36652cfe1 100644
> > > --- a/lib/meson.build
> > > +++ b/lib/meson.build
> > > @@ -76,6 +76,12 @@ foreach l:libraries
> > > dep_objs += [get_variable('dep_rte_' +
> > > d)]
> > > endforeach
> > >
> > > + if get_option('per_library_versions')
> > > + so_version = '@0@.1'.format(version)
> > > + else
> > > + so_version = meson.project_version()
> > > + endif
> > > +
> > > version_map = '@0@/@1@/rte_@2@_version.map'.form
> > > at(
> > > meson.current_source_dir(),
> > > dir_name, name)
> > > libname = 'rte_' + name
> > > @@ -87,7 +93,7 @@ foreach l:libraries
> > > include_directories:
> > > include_directories(dir_name),
> > > link_args: '-Wl,--version-
> > > script=' +
> > > version_map,
> > > link_depends: version_map,
> > > - version: '@0@.1'.format(version)
> > > ,
> > > + version: so_version,
> > > install: true)
> > > dep = declare_dependency(link_with: lib,
> > > include_directories:
> > > include_directories(dir_name),
> > > diff --git a/meson_options.txt b/meson_options.txt
> > > index 9c45b8159..636226ce8 100644
> > > --- a/meson_options.txt
> > > +++ b/meson_options.txt
> > > @@ -6,3 +6,4 @@ option('allow_invalid_socket_id', type:
> > > 'boolean',
> > > value: false,
> > > description: 'allow out-of-range NUMA socket id\'s for
> > > platforms that don\'t report the value correctly')
> > > option('enable_kmods', type: 'boolean', value: true,
> > > description:
> > > 'build kernel modules')
> > > option('kernel_dir', type: 'string', value: '', description:
> > > 'path
> > > to the kernel for building kernel modules')
> > > +option('per_library_versions', type: 'boolean', value: true,
> > > description: 'true: each lib gets its own version number, false:
> > > DPDK
> > > version used for each lib')
> >
> > Hi Bruce,
> >
> > First of all thanks for implementing this option, and sorry for the
> > delay.
> >
> > A couple of minor things:
> >
> > 1) The project version has 3 digits, but the ABI version should
> > have 2
> > - eg: should be 17.08 but right now it's 17.08.0 - given point
> > releases
> > will be ABI compatible so they must not change the ABI version. In
> > the
> > packaging code we use cut to chop it off:
> >
> > DPDK_ABI := $(shell echo $(DEB_VERSION_UPSTREAM) | cut -d '.' -f1-
> > 2)
> >
> > Not sure how to achieve the same in a Meson script though (can it
> > shell
> > out? or are there string manipulation built-ins?), sorry - just
> > starting to look at it.
> >
>
> That should be easy enough to do. It's 3-digits because I'm just
> pulling
> the project version unmodified from the initial project definition.
> I'd
> say I can manage to strip off one digit as part of the build.
>
> > Right now the rte_config option allows the user to completely
> > override
> > the version - I'm fine with having a single boolean provided this
> > change is applied, it simplifies packaging a bit, but if it's
> > easier
> > for you to just accepted a version instead of a boolean it's fine
> > as
> > well, as you prefer
>
> If there is a case where we might want to provide a particular
> version,
> I'm happy to edit the option, but if not I'll keep it as-is, because
> it
> will ensure all users of the option get the exact same behaviour, and
> means I only have two possibilities to test.
I agree, it looks fine as it is right now - I'm not aware of any other
use case at the moment.
> >
> > 2) When the option is true, it should not generate a symlink with
> > the
> > ABI "revision", as the first digit is really not the revision, so
> > it
> > does not make much sense. Worse, it will stop 2 versions being co-
> > installable (which was the whole point :-) ), as, eg, 17.02, 17.05
> > and
> > 17.08 will all ship a libfoo.so.17 symlink. I can fix it in the
> > packaging, but I don't think it should be generated upstream at all
> > when this option is enabled.
>
> Not exactly sure how to fix this, but I think it may work by using
> soversion as well as version in the library() calls.
>
> >
> > IOW, the following is the current result:
> >
> > $ ls -l debian/librte-ethdev17.08/usr/lib/x86_64-linux-gnu/
> > total 72
> > lrwxrwxrwx 1 lboccass lboccass 24 Sep 13 11:50
> > librte_ethdev.so.17 -> librte_ethdev.so.17.08.0
> > -rw-r--r-- 1 lboccass lboccass 71672 Sep 13 11:50
> > librte_ethdev.so.17.08.0
> >
> > The expected result would be to create a single file
> > "librte_ethdev.so.17.08" instead.
> >
>
> Let me see what I can do :-)
>
> /Bruce
Now it works as expected, thanks!
@@ -92,6 +92,12 @@ foreach class:driver_classes
depends: [pmdinfogen, tmp_lib])
endforeach
+ if get_option('per_library_versions')
+ so_version = '@0@.1'.format(version)
+ else
+ so_version = meson.project_version()
+ endif
+
# now build the driver itself, and add to the drivers list
lib_name = driver_name_fmt.format(name)
version_map = '@0@/@1@/@2@_version.map'.format(
@@ -105,7 +111,7 @@ foreach class:driver_classes
c_args: cflags,
link_args: '-Wl,--version-script=' + version_map,
link_depends: version_map,
- version: '@0@.1'.format(version),
+ version: so_version,
install: true,
install_dir: driver_install_path)
@@ -76,6 +76,12 @@ foreach l:libraries
dep_objs += [get_variable('dep_rte_' + d)]
endforeach
+ if get_option('per_library_versions')
+ so_version = '@0@.1'.format(version)
+ else
+ so_version = meson.project_version()
+ endif
+
version_map = '@0@/@1@/rte_@2@_version.map'.format(
meson.current_source_dir(), dir_name, name)
libname = 'rte_' + name
@@ -87,7 +93,7 @@ foreach l:libraries
include_directories: include_directories(dir_name),
link_args: '-Wl,--version-script=' + version_map,
link_depends: version_map,
- version: '@0@.1'.format(version),
+ version: so_version,
install: true)
dep = declare_dependency(link_with: lib,
include_directories: include_directories(dir_name),
@@ -6,3 +6,4 @@ option('allow_invalid_socket_id', type: 'boolean', value: false,
description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
option('enable_kmods', type: 'boolean', value: true, description: 'build kernel modules')
option('kernel_dir', type: 'string', value: '', description: 'path to the kernel for building kernel modules')
+option('per_library_versions', type: 'boolean', value: true, description: 'true: each lib gets its own version number, false: DPDK version used for each lib')