From patchwork Wed Oct 18 14:20:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 30528 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 617274C57; Wed, 18 Oct 2017 16:20:30 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id ECD72325D for ; Wed, 18 Oct 2017 16:20:28 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Oct 2017 07:20:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,397,1503385200"; d="scan'208";a="161910377" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.32]) by orsmga005.jf.intel.com with SMTP; 18 Oct 2017 07:20:25 -0700 Received: by (sSMTP sendmail emulation); Wed, 18 Oct 2017 15:20:24 +0100 Date: Wed, 18 Oct 2017 15:20:24 +0100 From: Bruce Richardson To: Luca Boccassi Cc: dev@dpdk.org, thomas@monjalon.net, olivier.matz@6wind.com, sergio.gonzalez.monroy@intel.com Message-ID: <20171018142024.GA14652@bricha3-MOBL3.ger.corp.intel.com> References: <20171017161220.59941-1-bruce.richardson@intel.com> <20171017161220.59941-2-bruce.richardson@intel.com> <1508263898.32437.1.camel@debian.org> <1508264229.32437.3.camel@debian.org> <20171018093548.GA15216@bricha3-MOBL3.ger.corp.intel.com> <20171018095149.GA14720@bricha3-MOBL3.ger.corp.intel.com> <1508321647.25747.4.camel@debian.org> <20171018122453.GA5596@bricha3-MOBL3.ger.corp.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20171018122453.GA5596@bricha3-MOBL3.ger.corp.intel.com> Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.9.1 (2017-09-22) Subject: Re: [dpdk-dev] [PATCH 1/8] build: add maths library to libs in pkg-config file 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Oct 18, 2017 at 01:24:54PM +0100, Bruce Richardson wrote: > On Wed, Oct 18, 2017 at 11:14:07AM +0100, Luca Boccassi wrote: > > On Wed, 2017-10-18 at 10:51 +0100, Bruce Richardson wrote: > > > On Wed, Oct 18, 2017 at 10:35:48AM +0100, Bruce Richardson wrote: > > > > On Tue, Oct 17, 2017 at 07:17:09PM +0100, Luca Boccassi wrote: > > > > > On Tue, 2017-10-17 at 19:11 +0100, Luca Boccassi wrote: > > > > > > On Tue, 2017-10-17 at 17:12 +0100, Bruce Richardson wrote: > > > > > > > Since a number of libraries depend on the maths lib, as well > > > > > > > as > > > > > > > adding it > > > > > > > to the project args, we also need to add it to the pkgconfig > > > > > > > file > > > > > > > args. > > > > > > > > > > > > > > Signed-off-by: Bruce Richardson > > > > > > > --- > > > > > > >  config/meson.build | 1 + > > > > > > >  1 file changed, 1 insertion(+) > > > > > > > > > > > > > > diff --git a/config/meson.build b/config/meson.build > > > > > > > index db68a08d4..542fea4de 100644 > > > > > > > --- a/config/meson.build > > > > > > > +++ b/config/meson.build > > > > > > > @@ -35,6 +35,7 @@ dpdk_conf.set('RTE_MACHINE', machine) > > > > > > >  add_project_arguments('-march=@0@'.format(machine), > > > > > > > language: 'c') > > > > > > >  # some libs depend on maths lib > > > > > > >  add_project_link_arguments('-lm', language: 'c') > > > > > > > +dpdk_extra_ldflags += '-lm' > > > > > > >   > > > > > > >  # add -include rte_config to cflags > > > > > > >  add_project_arguments('-include', 'rte_config.h', language: > > > > > > > 'c') > > > > > > > > > > > > This is for static builds, right? If so it should go into the > > > > > > Libs.private section of the .pc file, so that it's only used > > > > > > when > > > > > > calling pkg-config --static --libs > > > > > > > > > > Bit of a brain fart - what I meant is, in order to have static > > > > > builds > > > > > work out of the box with pkg-config --static, -lm (and any other > > > > > dependency used internally) could also be added to Libs.private > > > > > in the > > > > > .pc > > > > > > > > Does that not assume that both static and dynamic libs are > > > > installed > > > > side-by-side? In DPDK case, we will either build static libs or > > > > shared > > > > libs, but not both. If we require applications to use different > > > > pkg-config commands depending on the type of DPDK build that was > > > > done, > > > > it makes things awkward for the apps. Right now by putting all libs > > > > and > > > > flags into the libs section of pkgconfig, and having the build > > > > system > > > > track whether it's static or dynamic and therefore what is actually > > > > necessary, we end up in a case where apps can be built against DPDK > > > > irrespective of the actual build type done. For this particular -lm > > > > flag, for instance, it only appears in the .pc file for static > > > > builds. > > > > > > > > See the patches for the sample app Makefiles. Not sure how that can > > > > be > > > > made to work if we use different pkg-config settings for different > > > > build > > > > types. > > > > > > > > Your input and suggestions here would be welcome. > > > > Yes that works nicely when the libraries are rebuilt locally - then it > > can be chosen whether to build the static or dynamic ones. > > > > In the packaging case though, at least in Debian and Ubuntu (not sure > > about RHEL/SUSE), we do ship both static and dynamic libraries at the > > same time, following best practices. So we'd have to choose and either > > cause applications linking dynamically to over-link (which is what we > > currently do in Debian/Ubuntu and many developers really don't like > > that) or to cause applications that use the static libraries to have to > > manually add the missing flags. > > > > From what I can see, the most common workflow for applications that > > want to do static builds when using packaged libraries is to use the -- > > static flag of pkgconfig. > > This has the advantage of being a ""standardised"" workflow, expected > > to work across any dependency, not just DPDK. Of course that's not > > always the case, but one can dream :-) > > > > If you prefer to optimise for the local build workflow that's fine, we > > can patch it in the distro, it's not too much overhead (I need to do it > > anyway for the old build system at some point). > > > > > > > > +Thomas, Olivier, Sergio to get more input > > > > > > Thinking about it some more, is there any reason why we can't or > > > shouldn't do both static and dynamic libs in all builds, and then let > > > apps use pkg-config to determine what they want to link against? It > > > wouldn't be a massive change to the new build system to do that, I > > > think. > > > > > > /Bruce > > > > Yes that would be ideal! Right now in Debian/Ubuntu we have to build > > twice, doubling the required time unfortunately. > > > > In theory the same objects could be packed into .ar and .so but sadly > > Meson doesn't support that yet like autotools/cmake do: > > > > https://github.com/mesonbuild/meson/issues/484 > > > > (please do add some feedback there as developers!) > > > > It looks like it could be possible with some extensive hacking, not > > sure if it would be worth it. > > > Actually, I think it's workable using extract_objects. Build the static > lib first, then extract_all_objects() and then use those to build the .so. > I can prototype it very easily by changing lib/meson.build and see what > happens. > Seems to work fine. This change makes all the libraries (bar EAL, which is special-cased) appear as both .a and .so. Testpmd links against the dynamic versions and still works. When I get the chance I'll see about doing the same for EAL and drivers, and then having the pkgconfig file reflect both possibilities. Then I can test with some examples linking both dynamically and statically and check all is as expected. /Bruce diff --git a/lib/meson.build b/lib/meson.build index f04cb2791..29c88548b 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -76,6 +76,20 @@ foreach l:libraries dep_objs += [get_variable('dep_rte_' + d)] endforeach + libname = 'rte_' + name + +# first build static library + static_lib = static_library(libname, sources, + objects: objs, + c_args: cflags, + dependencies: dep_objs, + include_directories: include_directories(dir_name), + install: true) + +# now use those objects to build dynamic library + sources = [] + objs += static_lib.extract_all_objects() + if get_option('per_library_versions') lib_version = '@0@.1'.format(version) so_version = '@0@'.format(version) @@ -87,8 +101,7 @@ foreach l:libraries version_map = '@0@/@1@/rte_@2@_version.map'.format( meson.current_source_dir(), dir_name, name) - libname = 'rte_' + name - lib = library(libname, + lib = shared_library(libname, sources, objects: objs, c_args: cflags,