From mboxrd@z Thu Jan 1 00:00:00 1970 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) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-transfer-encoding:mime-version; bh=kW0IYkb0iO5O8Qmkomx3Q1uCuM9ona/O4HSCXVH/ozM=; b=SONjVgzGHTdtfw7srvDbpkNen5V4OEoB5Z6PRw0Fpe4Bl7ig6kHKtkSHJA6vxDNRYE +Up4i8T9nBZGWuRM/uYeY595Kf4oo1DOTX0JGbfeAmzbPiTSDUAJMzgRe3fEBD8J157w ysSYU/LlW7TGDnrkkpcAxvPwMrLO0/NP86m2in0z4FuRzrTwBcww1SD26CTSsoenPbEn Yth2B49jGLbkfDXn4TeTKPt4LBpTtK5BCC7VDA4eLFx90eYRIrlfmbX1St2NEIBYZ+gI dWMzlLEKDBnWNjLs0G0p9Wa1QsY9otkyZySotY2Vxzb/9AObxzlyF/W0i9VOKrkHL09x hIXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:mime-version; bh=kW0IYkb0iO5O8Qmkomx3Q1uCuM9ona/O4HSCXVH/ozM=; b=ruru7hZzqTYxa6Gj2HoD9AjyQyk8B1gSj+j+yDoRsGpFnMFL/eNqXZqQrmAdljT2eG MRBru+NmbjvAqkoWnICpptwE4tIu3wgxpWiKroGfNmB1ZpY023va5cFEEVTP1uvp2SLs uyYqWN1JtN41JTMfqqzNK/np4gGC8Co68XbFcGmmy8hTRpv01f75Xt7zpnEQ7yYyKuz5 ka0fMHbVcAjShsWG2CDFrOQhErucg45HWH9gQd5qCTQLXyGYKN5buiejGNqBBP+EDmr3 uRm4bhKrmfMthA2HH6Hqsfpf4S09cxmvCmVXrvMY0Q1PQKzekW/4SNXVPmeNq/kQuao7 O9Iw== X-Gm-Message-State: AHPjjUiRyzunMyyHBPCUmkBkG+XOJ7hoaVcUcFHSzmvrJcfpkZCHnhLh CevMmTWI0DynGzah2t0= X-Google-Smtp-Source: ADKCNb4iEFk7c5SmePfTeb8f+v4lWqLT0w4zIUAEJ0P+O+fKv5NrlPIthwBU/vejs9Bkv0trUJ0pJA== X-Received: by 10.223.164.74 with SMTP id e10mr17651736wra.19.1505322160986; Wed, 13 Sep 2017 10:02:40 -0700 (PDT) Received: from localhost ([2a00:23c5:bef3:400:4a51:b7ff:fe0b:4749]) by smtp.gmail.com with ESMTPSA id c56sm12972841wrc.1.2017.09.13.10.02.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 13 Sep 2017 10:02:39 -0700 (PDT) Message-ID: <1505322159.20547.4.camel@gmail.com> From: Luca Boccassi To: Bruce Richardson Cc: dev@dpdk.org, nhorman@tuxdriver.com, harry.van.haaren@intel.com, Christian Ehrhardt Date: Wed, 13 Sep 2017 18:02:39 +0100 In-Reply-To: <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> <20170913131103.GA41520@bricha3-MOBL3.ger.corp.intel.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.22.6-1 Mime-Version: 1.0 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 17:02:41 -0000 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