From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 9B2B5CCD2 for ; Thu, 30 Apr 2015 18:23:15 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP; 30 Apr 2015 09:22:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,677,1422950400"; d="scan'208";a="487748940" Received: from unknown ([10.237.220.106]) by FMSMGA003.fm.intel.com with SMTP; 30 Apr 2015 09:22:56 -0700 Received: by (sSMTP sendmail emulation); Thu, 30 Apr 2015 17:22:55 +0025 Date: Thu, 30 Apr 2015 17:22:55 +0100 From: Bruce Richardson To: "Wiles, Keith" Message-ID: <20150430162255.GA5708@bricha3-MOBL3> References: <1430321158-17805-1-git-send-email-keith.wiles@intel.com> <5541F9C1.9050701@6wind.com> <5542305D.9070505@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v4 1/2] Simplify the ifdefs in rte.app.mk. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Apr 2015 16:23:16 -0000 On Thu, Apr 30, 2015 at 02:31:13PM +0000, Wiles, Keith wrote: > > > On 4/30/15, 8:38 AM, "Olivier MATZ" wrote: > > >Hi Keith, > > > >On 04/30/2015 03:24 PM, Wiles, Keith wrote: > >> > >> > >> On 4/30/15, 4:45 AM, "Olivier MATZ" 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