Message ID | 78bbf74c-9621-9094-a280-76ed9fc88393@redhat.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Delegated to: | Thomas Monjalon |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id C6DDD5954; Tue, 24 May 2016 11:57:03 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id E69F35953 for <dev@dpdk.org>; Tue, 24 May 2016 11:57:01 +0200 (CEST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3AB8381139; Tue, 24 May 2016 09:57:01 +0000 (UTC) Received: from sopuli.koti.laiskiainen.org (vpn1-4-146.ams2.redhat.com [10.36.4.146]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4O9uxCF014683; Tue, 24 May 2016 05:57:00 -0400 To: Thomas Monjalon <thomas.monjalon@6wind.com>, Christian Ehrhardt <christian.ehrhardt@canonical.com> References: <CAATJJ0KCLxdZjurkzsbps6BijkFFr2pHb6ysvCRQWuwRPLXWAw@mail.gmail.com> <1463763022-6673-1-git-send-email-christian.ehrhardt@canonical.com> <7920424.umDpp4ukym@xps13> Cc: dev@dpdk.org From: Panu Matilainen <pmatilai@redhat.com> Message-ID: <78bbf74c-9621-9094-a280-76ed9fc88393@redhat.com> Date: Tue, 24 May 2016 12:56:59 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <7920424.umDpp4ukym@xps13> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 24 May 2016 09:57:01 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH] mk: fix underlinking issues of most librte libraries X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Panu Matilainen
May 24, 2016, 9:56 a.m. UTC
On 05/20/2016 08:08 PM, Thomas Monjalon wrote: > 2016-05-20 18:50, Christian Ehrhardt: >> The individual libraries have various cross dependencies. >> This is already refelcted in the DEPDIR dependency, but not yet in >> proper DT_NEEDED flags in the .so's. >> This adds the -l flags so that is properly stored in the .so's ELF >> headers. > > Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk? > A fair question :) The thought has passed my mind too, but I thought it'd be too messy with differing library vs directory names and other exceptions. But in reality manually maintained separate LDLIBS is only going to get out of sync sooner or later, and turns out its not that bad anyway: I'm also sure somebody more familiar with gmake could improve it, but it seems to work quite fine here. I'll wait a bit to see if there are other suggestions before sending it as a proper patch. - Panu -
Comments
Hi Panu, I already agreed to Thomas on IRC but won't have time until next week. Thanks for making a patch that does that already - I'll give it a look and some test on my end next week then. Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd On Tue, May 24, 2016 at 11:56 AM, Panu Matilainen <pmatilai@redhat.com> wrote: > On 05/20/2016 08:08 PM, Thomas Monjalon wrote: > >> 2016-05-20 18:50, Christian Ehrhardt: >> >>> The individual libraries have various cross dependencies. >>> This is already refelcted in the DEPDIR dependency, but not yet in >>> proper DT_NEEDED flags in the .so's. >>> This adds the -l flags so that is properly stored in the .so's ELF >>> headers. >>> >> >> Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk? >> >> > A fair question :) The thought has passed my mind too, but I thought it'd > be too messy with differing library vs directory names and other > exceptions. But in reality manually maintained separate LDLIBS is only > going to get out of sync sooner or later, and turns out its not that bad > anyway: > > diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk > index b420280..88b4e98 100644 > --- a/mk/rte.lib.mk > +++ b/mk/rte.lib.mk > @@ -77,6 +77,12 @@ else > _CPU_LDFLAGS := $(CPU_LDFLAGS) > endif > > +# Translate DEPDIRS-y into LDLIBS > +IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat > +_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y)) > +_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS)) > +LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS)) > + > O_TO_A = $(AR) crDs $(LIB) $(OBJS-y) > O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight > O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)"," AR $(@)") > > I'm also sure somebody more familiar with gmake could improve it, but it > seems to work quite fine here. I'll wait a bit to see if there are other > suggestions before sending it as a proper patch. > > - Panu - >
2016-05-24 12:56, Panu Matilainen: > On 05/20/2016 08:08 PM, Thomas Monjalon wrote: > > 2016-05-20 18:50, Christian Ehrhardt: > >> The individual libraries have various cross dependencies. > >> This is already refelcted in the DEPDIR dependency, but not yet in > >> proper DT_NEEDED flags in the .so's. > >> This adds the -l flags so that is properly stored in the .so's ELF > >> headers. > > > > Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk? > > > > A fair question :) The thought has passed my mind too, but I thought > it'd be too messy with differing library vs directory names and other > exceptions. But in reality manually maintained separate LDLIBS is only > going to get out of sync sooner or later, and turns out its not that bad > anyway: > > diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk > index b420280..88b4e98 100644 > --- a/mk/rte.lib.mk > +++ b/mk/rte.lib.mk > @@ -77,6 +77,12 @@ else > _CPU_LDFLAGS := $(CPU_LDFLAGS) > endif > > +# Translate DEPDIRS-y into LDLIBS > +IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat We need some comments to explain why they are ignored. > +_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y)) > +_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS)) > +LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS)) > I'm also sure somebody more familiar with gmake could improve it, but it > seems to work quite fine here. I'll wait a bit to see if there are other > suggestions before sending it as a proper patch. I'm familiar with gmake and I do not think it can be improved more ;) Please send a patch. Thanks
On 06/07/2016 11:23 AM, Thomas Monjalon wrote: > 2016-05-24 12:56, Panu Matilainen: >> On 05/20/2016 08:08 PM, Thomas Monjalon wrote: >>> 2016-05-20 18:50, Christian Ehrhardt: >>>> The individual libraries have various cross dependencies. >>>> This is already refelcted in the DEPDIR dependency, but not yet in >>>> proper DT_NEEDED flags in the .so's. >>>> This adds the -l flags so that is properly stored in the .so's ELF >>>> headers. >>> >>> Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk? >>> >> >> A fair question :) The thought has passed my mind too, but I thought >> it'd be too messy with differing library vs directory names and other >> exceptions. But in reality manually maintained separate LDLIBS is only >> going to get out of sync sooner or later, and turns out its not that bad >> anyway: >> >> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk >> index b420280..88b4e98 100644 >> --- a/mk/rte.lib.mk >> +++ b/mk/rte.lib.mk >> @@ -77,6 +77,12 @@ else >> _CPU_LDFLAGS := $(CPU_LDFLAGS) >> endif >> >> +# Translate DEPDIRS-y into LDLIBS >> +IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat > > We need some comments to explain why they are ignored. Yup. It'd be more clear if this filtering was done before translating directories to -lrte_foo but this was the first variant I came across that worked :) The issue is that librte_eal/common, librte_net and librte_compat are common directory dependencies but no actual library is generated from them so they just need to be weeded out of the equation, the revised version makes this more clear. > >> +_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y)) >> +_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS)) >> +LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS)) > >> I'm also sure somebody more familiar with gmake could improve it, but it >> seems to work quite fine here. I'll wait a bit to see if there are other >> suggestions before sending it as a proper patch. > > I'm familiar with gmake and I do not think it can be improved more ;) > Please send a patch. Thanks > Ok, will do. - Panu -
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk index b420280..88b4e98 100644 --- a/mk/rte.lib.mk +++ b/mk/rte.lib.mk @@ -77,6 +77,12 @@ else _CPU_LDFLAGS := $(CPU_LDFLAGS) endif +# Translate DEPDIRS-y into LDLIBS +IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat +_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y)) +_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS)) +LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS)) + O_TO_A = $(AR) crDs $(LIB) $(OBJS-y) O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)"," AR $(@)")