Re: [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2014-12-17 15:01, Thomas Monjalon:
> 2014-12-17 10:41, Gonzalez Monroy, Sergio:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-12-16 19:48, Thomas Monjalon:
> > > > 2014-10-28 15:48, Sergio Gonzalez Monroy:
> > > > > Fix: link combined shared lib using CC if LINK_USING_CC is enabled.
> > > > >
> > > > > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > > > > ---
> > > > >  mk/rte.lib.mk      |  1 -
> > > > >  mk/rte.sharelib.mk | 12 +++++++++++-
> > > > [...]
> > > > > --- a/mk/rte.lib.mk
> > > > > +++ b/mk/rte.lib.mk
> > > > > @@ -63,7 +63,6 @@ ifeq ($(LINK_USING_CC),1)
> > > > >  # Override the definition of LD here, since we're linking with CC
> > > > >  LD := $(CC)
> > > > >  LD_MULDEFS := $(call linkerprefix,-z$(comma)muldefs)
> > > > > -CPU_LDFLAGS := $(call linkerprefix,$(CPU_LDFLAGS))
> > > > >  endif
> > > >
> > > > Why are you removing this line?
> > > 
> > Hi Thomas,
> > 
> > Basically the problem is when CPU_LDFLAGS has a value, which in the
> > currently only happens if we set CPU_LDFLAGS in the command line or
> > build i686 (setting CPU_LDFLAGS=-melf_i386).
> > In those cases, given the way the makefiles are being included,
> 
> These 2 files don't include each other.
> rte.sharelib.mk is included only in lib/Makefile
> and rte.lib.mk is included in each lib Makefile.
> 
> I think the problem is not about how the files are included
> but how this variable is exported.
> CPU_LDFLAGS is modified in lib/Makefile via rte.sharelib.mk,
> then exported (see mk/arch/*/rte.vars.mk) then modified
> again in libs via rte.lib.mk.
> 
> > CPU_LDFLAGS is being 'linkerprefixed' twice, resulting in build error.
> > Removing that line avoided the second linker prefix of the CPU_LDFLAGS
> > (note that we reset the original var instead of using a different var name).
> > 
> > There probably is a better way but I could not see an straightforward fix for it.
> 
> I suggest 2 possible fixes:
> - do not export this variable (why is it exported?)
> - or do not override the variable, i.e. use a different variable for prefixing

I'm preparing a patch with the second approach.
It raised other problems like error when linking examples with combined library.
Patchset to come.

> > I planned to improve the current build system (already sent an RFC)
> > and was waiting until 1.8 release to resume the work on it.
> 
> I agree we should work on improvements after 1.8.
> 
> > > If it's really needed, it could be another patch.
> [...]
> > > 
> > > Applied without the change to rte.lib.mk.
> 
> It definitely deserves another patch with a proper fix.

-- 
Thomas