From: "Wiles, Keith" <keith.wiles@intel.com>
To: "Richardson, Bruce" <bruce.richardson@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.
Date: Thu, 30 Apr 2015 16:33:36 +0000 [thread overview]
Message-ID: <D167C366.1E6DE%keith.wiles@intel.com> (raw)
In-Reply-To: <20150430162255.GA5708@bricha3-MOBL3>
On 4/30/15, 11:22 AM, "Richardson, Bruce" <bruce.richardson@intel.com>
wrote:
>On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote:
>>
>>
>> On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>>
>> >Hi Keith,
>> >
>> >On 04/30/2015 03:24 PM, Wiles, Keith wrote:
>> >>
>> >>
>> >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz@6wind.com> wrote:
>> >>
>> >>> Hi Keith,
>> >>>
>> >>> Thank you for submitting a clean-up. Please see some comments below.
>> >>>
>> >>> On 04/29/2015 05:25 PM, Keith Wiles wrote:
>> >>>> Trying to simplify the ifdefs in rte.app.mk to make the code
>> >>>> more readable and maintainable by moving LDLIBS variable to use
>> >>>> the same style as LDLIBS-y being used in the rest of the code.
>> >>>>
>> >>>> Added a new variable called EXTRA_LDLIBS to be used by example apps
>> >>>> instead of using LDLIBS directly.
>> >>>
>> >>> If I understand well, the goal of this patch is only a cleanup in
>> >>> rte.app.mk, but at the end, it changes the makefile user "API",
>> >>> which could probably be a problem for applications using the
>> >>> dpdk makefile framework.
>> >>>
>> >>> Why not just having an temporary internal variable (let's say
>> >>> _LDLIBS-y) that would allow to do the clean-up without modifying
>> >>> the user interface?
>> >>>
>> >>> Also, with your patch, the approach for EXTRA_LDLIBS would be
>> >>> different than CFLAGS or LDFLAGS:
>> >>> - CFLAGS/LDFLAGS are in Makefiles only
>> >>> - EXTRA_CFLAGS/EXTRA_LDFLAGS are prefered in command line
>> >>> to add flags to the default ones
>> >>>
>> >>> I'm not opposed to add EXTRA_LDLIBS in addition to LDLIBS,
>> >>> keeping a compatibility with existing application Makefiles.
>> >>
>> >> The docs for DPDK 28.3.6 states they can be used for both command
>>line
>> >>and
>> >> Makefile, so I think I like the current solution unless everyone
>>wants
>> >>it
>> >> as you suggested.
>> >>
>> >>
>>
>>>>http://dpdk.readthedocs.org/en/v2.0.0/prog_guide/dev_kit_build_system.h
>>>>tm
>> >>l
>> >
>> > From the link you have sent:
>> >
>> >- About CFLAGS:
>> >
>> >"28.3.4. Variables that Can be Set/Overridden in a Makefile Only
>> >[...]
>> >CFLAGS: Flags to use for C compilation. The user should use += to
>>append
>> >data in this variable."
>> >
>> >nothing in 28.3.6
>> >
>> >
>> >- About EXTRA_CFLAGS:
>> >
>> >nothing in 28.3.4
>> >
>> >"28.3.6. Variables that Can be Set/Overridden by the User in a Makefile
>> >or Command Line
>> >[...]
>> >EXTRA_CFLAGS: The content of this variable is appended after CFLAGS
>>when
>> >compiling."
>>
>> The point was that EXTRA_XXX can be used for command line and Makefile
>>as
>> it was pointed out in a previous email the assumption was EXTRA_XXX was
>> only for the command line. (Just to make sure we understood EXTRA_XXX
>>was
>> not just for command line options.) This was the reason I sent the link
>>an
>> to point out using EXTRA_XXX is a much cleaner method then allowing
>> someone to modify what I believe is an internal variable.
>
>Just beware that setting EXTRA_* flags on the commandline can override
>their
>values in the makefiles, and cause unexpected compilation problems.
>Therefore,
>it tends to be best to avoid using the EXTRA_* variables for variables
>essential
>to compile. For example: putting "-g -O3" in EXTRA_CFLAGS is ok, as the
>if the
>useroverrides those with something else things should still work, but
>putting
>"-I/path/to/include" would not be.
On the command line and makefile you should be using += and not just = or
you run into this problem.
>
>/Bruce
>
>
next prev parent reply other threads:[~2015-04-30 16:34 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-29 15:25 Keith Wiles
2015-04-29 15:25 ` [dpdk-dev] [PATCH v4 2/2] Update Docs for new EXTRA_LDLIBS variable Keith Wiles
2015-04-29 17:17 ` [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk Thomas Monjalon
2015-04-29 17:49 ` Wiles, Keith
2015-04-30 9:45 ` Olivier MATZ
2015-04-30 13:24 ` Wiles, Keith
2015-04-30 13:38 ` Olivier MATZ
2015-04-30 14:31 ` Wiles, Keith
2015-04-30 15:56 ` Olivier MATZ
2015-04-30 16:22 ` Bruce Richardson
2015-04-30 16:33 ` Wiles, Keith [this message]
2015-05-01 9:09 ` Bruce Richardson
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=D167C366.1E6DE%keith.wiles@intel.com \
--to=keith.wiles@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
/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).