From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 74665374E for ; Wed, 13 Sep 2017 15:11:08 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga104.jf.intel.com with ESMTP; 13 Sep 2017 06:11:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,388,1500966000"; d="scan'208";a="311209438" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.24]) by fmsmga004.fm.intel.com with SMTP; 13 Sep 2017 06:11:04 -0700 Received: by (sSMTP sendmail emulation); Wed, 13 Sep 2017 14:11:04 +0100 Date: Wed, 13 Sep 2017 14:11:03 +0100 From: Bruce Richardson To: Luca Boccassi Cc: dev@dpdk.org, nhorman@tuxdriver.com, harry.van.haaren@intel.com, Christian Ehrhardt Message-ID: <20170913131103.GA41520@bricha3-MOBL3.ger.corp.intel.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> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1505302344.31233.3.camel@gmail.com> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.8.3 (2017-05-23) Subject: Re: [dpdk-dev] [PATCH v2 16/17] build: add option to version libs using DPDK version X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Sep 2017 13:11:09 -0000 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 > > Reviewed-by: Harry van Haaren > > --- > >  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