DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] mk: link combined shared lib using CC
Date: Wed, 17 Dec 2014 15:01:59 +0100	[thread overview]
Message-ID: <3025851.uNGPVxDmSR@xps13> (raw)
In-Reply-To: <91383E96CE459D47BCE92EFBF5CE73B004F0B593@IRSMSX107.ger.corp.intel.com>

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 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

  reply	other threads:[~2014-12-17 14:02 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-23 15:35 [dpdk-dev] [PATCH] mk: link combined " Sergio Gonzalez Monroy
2014-10-28 11:45 ` De Lara Guarch, Pablo
2014-10-28 14:51   ` Sergio Gonzalez Monroy
2014-10-28 15:33     ` Thomas Monjalon
2014-10-28 15:39       ` Sergio Gonzalez Monroy
2014-10-28 15:48 ` [dpdk-dev] [PATCH v2] mk: link combined shared " Sergio Gonzalez Monroy
2014-10-28 16:09   ` De Lara Guarch, Pablo
2014-12-16 18:48   ` Thomas Monjalon
2014-12-16 23:42     ` Thomas Monjalon
2014-12-17 10:41       ` Gonzalez Monroy, Sergio
2014-12-17 14:01         ` Thomas Monjalon [this message]
2014-12-17 18:49           ` Thomas Monjalon
2014-12-17 21:59             ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon
2014-12-17 21:59               ` [dpdk-dev] [PATCH 1/3] mk: fix link examples to combined library Thomas Monjalon
2014-12-17 21:59               ` [dpdk-dev] [PATCH 2/3] mk: forbid multiple definitions Thomas Monjalon
2014-12-17 21:59               ` [dpdk-dev] [PATCH 3/3] mk: fix link with CC Thomas Monjalon
2014-12-17 23:35               ` [dpdk-dev] [PATCH 0/3] mk: fix link options Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3025851.uNGPVxDmSR@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    --cc=sergio.gonzalez.monroy@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).