From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luca Boccassi Subject: Re: [PATCH v2 16/17] build: add option to version libs using DPDK version Date: Wed, 13 Sep 2017 18:02:39 +0100 Message-ID: <1505322159.20547.4.camel@gmail.com> References: <20170901100416.80264-1-bruce.richardson@intel.com> <20170912103809.140473-1-bruce.richardson@intel.com> <20170912103809.140473-17-bruce.richardson@intel.com> <1505302344.31233.3.camel@gmail.com> <20170913131103.GA41520@bricha3-MOBL3.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org, nhorman@tuxdriver.com, harry.van.haaren@intel.com, Christian Ehrhardt To: Bruce Richardson Return-path: Received: from mail-wr0-f195.google.com (mail-wr0-f195.google.com [209.85.128.195]) by dpdk.org (Postfix) with ESMTP id 53595325B for ; Wed, 13 Sep 2017 19:02:41 +0200 (CEST) Received: by mail-wr0-f195.google.com with SMTP id p37so402624wrb.5 for ; Wed, 13 Sep 2017 10:02:41 -0700 (PDT) In-Reply-To: <20170913131103.GA41520@bricha3-MOBL3.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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. > > >=20 > > > Signed-off-by: Bruce Richardson > > > Reviewed-by: Harry van Haaren > > > --- > > > =C2=A0drivers/meson.build | 8 +++++++- > > > =C2=A0lib/meson.build=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0| 8 +++++++- > > > =C2=A0meson_options.txt=C2=A0=C2=A0=C2=A0| 1 + > > > =C2=A03 files changed, 15 insertions(+), 2 deletions(-) > > >=20 > > > 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 > > > =C2=A0 depends: > > > [pmdinfogen, tmp_lib]) > > > =C2=A0 endforeach > > > =C2=A0 > > > + if get_option('per_library_versions') > > > + so_version =3D '@0@.1'.format(vers > > > ion) > > > + else > > > + so_version =3D > > > meson.project_version() > > > + endif > > > + > > > =C2=A0 # now build the driver itself, and add > > > to > > > the drivers list > > > =C2=A0 lib_name =3D driver_name_fmt.format(name) > > > =C2=A0 version_map =3D '@0@/@1@/@2@_version.map'. > > > form > > > at( > > > @@ -105,7 +111,7 @@ foreach class:driver_classes > > > =C2=A0 c_args: cflags, > > > =C2=A0 link_args: '-Wl,--version- > > > script=3D' + > > > version_map, > > > =C2=A0 link_depends: version_map, > > > - version: '@0@.1'.format(version) > > > , > > > + version: so_version, > > > =C2=A0 install: true, > > > =C2=A0 install_dir: > > > driver_install_path) > > > =C2=A0 > > > 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 > > > =C2=A0 dep_objs +=3D [get_variable('dep_rte_' + > > > d)] > > > =C2=A0 endforeach > > > =C2=A0 > > > + if get_option('per_library_versions') > > > + so_version =3D '@0@.1'.format(version) > > > + else > > > + so_version =3D meson.project_version() > > > + endif > > > + > > > =C2=A0 version_map =3D '@0@/@1@/rte_@2@_version.map'.form > > > at( > > > =C2=A0 meson.current_source_dir(), > > > dir_name, name) > > > =C2=A0 libname =3D 'rte_' + name > > > @@ -87,7 +93,7 @@ foreach l:libraries > > > =C2=A0 include_directories: > > > include_directories(dir_name), > > > =C2=A0 link_args: '-Wl,--version- > > > script=3D' + > > > version_map, > > > =C2=A0 link_depends: version_map, > > > - version: '@0@.1'.format(version) > > > , > > > + version: so_version, > > > =C2=A0 install: true) > > > =C2=A0 dep =3D declare_dependency(link_with: lib, > > > =C2=A0 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, > > > =C2=A0 description: 'allow out-of-range NUMA socket id\'s for > > > platforms that don\'t report the value correctly') > > > =C2=A0option('enable_kmods', type: 'boolean', value: true, > > > description: > > > 'build kernel modules') > > > =C2=A0option('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') > >=20 > > Hi Bruce, > >=20 > > First of all thanks for implementing this option, and sorry for the > > delay. > >=20 > > A couple of minor things: > >=20 > > 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: > >=20 > > DPDK_ABI :=3D $(shell echo $(DEB_VERSION_UPSTREAM) | cut -d '.'=C2=A0= =C2=A0-f1- > > 2) > >=20 > > 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. > >=20 >=20 > 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. >=20 > > 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 >=20 > 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. > >=20 > > 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. >=20 > Not exactly sure how to fix this, but I think it may work by using > soversion as well as version in the library() calls. >=20 > >=20 > > IOW, the following is the current result: > >=20 > > $ ls -l debian/librte-ethdev17.08/usr/lib/x86_64-linux-gnu/ > > total 72 > > lrwxrwxrwx 1 lboccass lboccass=C2=A0=C2=A0=C2=A0=C2=A024 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 > >=20 > > The expected result would be to create a single file > > "librte_ethdev.so.17.08" instead. > >=20 >=20 > Let me see what I can do :-) >=20 > /Bruce Now it works as expected, thanks! --=20 Kind regards, Luca Boccassi