Re: [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk.

[Bruce Richardson <bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote:
> 
> 
> On 4/30/15, 8:38 AM, "Olivier MATZ" <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> wrote:
> 
> >Hi Keith,
> >
> >On 04/30/2015 03:24 PM, Wiles, Keith wrote:
> >>
> >>
> >> On 4/30/15, 4:45 AM, "Olivier MATZ" <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> 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.htm
> >>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.

/Bruce