[dpdk-dev] mk: fix app linking for combined libs

  Building combined shared libraries results in applications being linked
against separeted/individual and combined libs altogether.

Link only against combined lib when the config option is enabled.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 mk/rte.app.mk | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez
> Monroy
> Sent: Thursday, October 23, 2014 4:37 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] mk: fix app linking for combined libs
> 
> Building combined shared libraries results in applications being linked
> against separeted/individual and combined libs altogether.

Minor typo in the commit message: "separeted".
> 
> Link only against combined lib when the config option is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy
> <sergio.gonzalez.monroy@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
  

2014-10-23 16:36, Sergio Gonzalez Monroy:
> Building combined shared libraries results in applications being linked
> against separeted/individual and combined libs altogether.
> 
> Link only against combined lib when the config option is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
[...]
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -217,6 +217,12 @@ endif
>  
>  endif # plugins
>  
> +ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> +LDLIBS = --whole-archive

You are resetting LDLIBS here.
It's not easy to read and probably not desired.
I think it would be better to explicitly disable separated libs
in this case.

> +LDLIBS += --start-group
> +LDLIBS += -l$(RTE_LIBNAME)
> +endif

Thanks
  

> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, November 28, 2014 3:56 PM
> 
> 2014-10-23 16:36, Sergio Gonzalez Monroy:
> > Building combined shared libraries results in applications being
> > linked against separeted/individual and combined libs altogether.
> >
> > Link only against combined lib when the config option is enabled.
> >
> > Signed-off-by: Sergio Gonzalez Monroy
> > <sergio.gonzalez.monroy@intel.com>
> [...]
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -217,6 +217,12 @@ endif
> >
> >  endif # plugins
> >
> > +ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> > +LDLIBS = --whole-archive
> 
> You are resetting LDLIBS here.
> It's not easy to read and probably not desired.
> I think it would be better to explicitly disable separated libs in this case.
> 
Yes, I am resetting LDLIBS so we just link against the combined lib instead of all the previous separated libs.
I am not sure I understand what you mean with 'disable separated libs in this case'.

Thanks,
Sergio

> > +LDLIBS += --start-group
> > +LDLIBS += -l$(RTE_LIBNAME)
> > +endif
> 
> Thanks
> --
> Thomas
  

2014-12-01 09:57, Gonzalez Monroy, Sergio:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-23 16:36, Sergio Gonzalez Monroy:
> > > Building combined shared libraries results in applications being
> > > linked against separeted/individual and combined libs altogether.
> > >
> > > Link only against combined lib when the config option is enabled.
> > >
> > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > [...]
> > > --- a/mk/rte.app.mk
> > > +++ b/mk/rte.app.mk
> > > @@ -217,6 +217,12 @@ endif
> > >
> > >  endif # plugins
> > >
> > > +ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> > > +LDLIBS = --whole-archive
> > 
> > You are resetting LDLIBS here.
> > It's not easy to read and probably not desired.
> > I think it would be better to explicitly disable separated libs in this case.
> > 
> Yes, I am resetting LDLIBS so we just link against the combined lib instead of all the previous separated libs.
> I am not sure I understand what you mean with 'disable separated libs in this case'.

By "disable separated libs", I mean enclose the LDLIBS lines for separated libs
with ifneq ($(RTE_BUILD_COMBINE_LIBS),y)
Using ifeq is more explicit than inserting LDLIBS= in the middle of LDLIBS+= lines.
Do you agree?
  

> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, December 1, 2014 10:35 AM
> 
> By "disable separated libs", I mean enclose the LDLIBS lines for separated libs
> with ifneq ($(RTE_BUILD_COMBINE_LIBS),y) Using ifeq is more explicit than
> inserting LDLIBS= in the middle of LDLIBS+= lines.
> Do you agree?
> 
That makes sense, I agree it would be easier to read that way.
I'll work on a v2.

Thanks,
Sergio

> --
> Thomas
  

On Thu, Oct 23, 2014 at 04:36:44PM +0100, Sergio Gonzalez Monroy wrote:
> Building combined shared libraries results in applications being linked
> against separeted/individual and combined libs altogether.
> 
> Link only against combined lib when the config option is enabled.
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> ---
> mk/rte.app.mk | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 285b65c..45444af 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -217,6 +217,12 @@ endif
>  
>  endif # plugins
>  
> +ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> +LDLIBS = --whole-archive
> +LDLIBS += --start-group
> +LDLIBS += -l$(RTE_LIBNAME)
> +endif
> +
>  LDLIBS += $(EXECENV_LDLIBS)
>  
>  LDLIBS += --end-group
> @@ -240,10 +246,6 @@ build: _postbuild
>  
>  exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
>  
> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> -LDLIBS += -l$(RTE_LIBNAME)
> -endif
> -
>  ifeq ($(LINK_USING_CC),1)
>  LDLIBS := $(call linkerprefix,$(LDLIBS))
>  LDFLAGS := $(call linkerprefix,$(LDFLAGS))
Acked-by: Neil Horman <nhorman@tuxdriver.com>
  

2014-12-08 09:53, Neil Horman:
> On Thu, Oct 23, 2014 at 04:36:44PM +0100, Sergio Gonzalez Monroy wrote:
> > Building combined shared libraries results in applications being linked
> > against separeted/individual and combined libs altogether.
> > 
> > Link only against combined lib when the config option is enabled.
> > 
> > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
[...]
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Neil, I didn't noticed your ack which happened after a discussion I had with
Sergio. He agreed to make a v2.
Actually Hiroshi did it:
	http://dpdk.org/ml/archives/dev/2014-December/009847.html
I'm going to apply Hiroshi's one.
  

> Subject: Re: [dpdk-dev] mk: fix app linking for combined libs
> 
> 2014-12-08 09:53, Neil Horman:
> > On Thu, Oct 23, 2014 at 04:36:44PM +0100, Sergio Gonzalez Monroy wrote:
> > > Building combined shared libraries results in applications being linked
> > > against separeted/individual and combined libs altogether.
> > >
> > > Link only against combined lib when the config option is enabled.
> > >
> > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > > Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> [...]
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> 
> Neil, I didn't noticed your ack which happened after a discussion I had with
> Sergio. He agreed to make a v2.
> Actually Hiroshi did it:
> 	http://dpdk.org/ml/archives/dev/2014-December/009847.html
> I'm going to apply Hiroshi's one.

Ah, I haven't noticed there was the patch to address this issue.

thanks,
Hiroshi