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.richard...@intel.com>
> > Reviewed-by: Harry van Haaren <harry.van.haa...@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
