From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shreyansh Jain <shreyansh.jain@nxp.com>
Subject: Re: [PATCH v2 1/2] mk: allow use of environment var for
	make config
Date: Wed, 7 Jun 2017 15:06:05 +0530
Message-ID: <ef07f4e9-6a88-5ac6-8f00-04a573dfaebd@nxp.com>
References: <1495535304-159542-3-git-send-email-david.hunt@intel.com>
 <1495788764-37652-1-git-send-email-david.hunt@intel.com>
 <1495788764-37652-2-git-send-email-david.hunt@intel.com>
 <dffa95fe-e251-5fc5-0a59-3b41d71f56e7@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Cc: <dev@dpdk.org>, <thomas@monjalon.net>
To: "Hunt, David" <david.hunt@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from NAM01-SN1-obe.outbound.protection.outlook.com
 (mail-sn1nam01on0089.outbound.protection.outlook.com [104.47.32.89])
 by dpdk.org (Postfix) with ESMTP id 923A223B
 for <dev@dpdk.org>; Wed,  7 Jun 2017 11:27:59 +0200 (CEST)
In-Reply-To: <dffa95fe-e251-5fc5-0a59-3b41d71f56e7@intel.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hello David,

On Wednesday 07 June 2017 02:09 PM, Hunt, David wrote:
> Shreyansh,
> 
>      I found an issue (or two) with this part of the patch, and have a 
> proposed solution.
> 
> 1. RTE_TARGET originally had a different meaning. It was used for making 
> examples, specifying the target directory of where the SDK was built. 
> It's not good to re-purpose this for something else, as I'm doing in 
> this patch. (even though I'm not sure that variable is suitably named in 
> the first place, but that's a different issue).

Even I didn't realize this until you highlighted here.

> 2. If we set RTE_TARGET on the environment, we will break the 'make -C 
> examples/<app>', unless we set RTE_TARGET to be something else (i.e. 
> 'make -C examples/<app> RTE_TARGET=build'). One value for making DPDK, 
> and another for building examples. It's confusing to the user.

Agree about re-using RTE_TARGET is breaking existing assumption about
its use.

> 
> An alternative patch would be as follows:
> 
>   RTE_CONFIG_TEMPLATE :=
>   ifdef T
> *-ifeq ("$(origin T)", "command line")*
>   RTE_CONFIG_TEMPLATE := $(RTE_SRCDIR)/config/defconfig_$(T)
> *-endif**
> *endif
>   export RTE_CONFIG_TEMPLATE
So, that would mean, user would do either of the following:

make T=<template> config

or

export T=<template>
make config

Is that correct? (I tried it and it seems to be working fine)
First method is same as today. For the second, I am just skeptical
whether we should use such a small identifier ("T") or we have a new
RTE_TEMPLATE.

Either way, I am OK. [export T=<template>] looks fine to me - in fact,
on a second though, IMO, if T=<template> is provided as command line, it 
should also be acceptable as env variable.

> 
> So instead of setting 'RTE_TARGET' on in the environment, we set 'T' 
> instead. This allows 'T' to come from the command line OR an environment 
> variable. It resolves the 'make examples' issue, and everything else 
> works as it did before, 'make install', etc. It seems to me to be a 
> cleaner solution for this. What do you think? If it's OK with you, I'll 
> submit a v3 (the 'make defconfig' part of the patchset will remain the 
> same as v2).
> 
> Rgds,
> Dave.
> 
> 
> 
> On 26/5/2017 9:52 AM, David Hunt wrote:
>> Users can now set RTE_TARGET in their environment and use
>> 'make config' without T=template.
>>
>> If RTE_TARGET is set in the user's environment, and if T=
>> is not used, 'make config' will use $RTE_TARGET.
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
>> ---
>>   mk/rte.sdkroot.mk | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/mk/rte.sdkroot.mk b/mk/rte.sdkroot.mk
>> index 2843b7d..9bdaf20 100644
>> --- a/mk/rte.sdkroot.mk
>> +++ b/mk/rte.sdkroot.mk
>> @@ -63,6 +63,8 @@ ifdef T
>>   ifeq ("$(origin T)", "command line")
>>   RTE_CONFIG_TEMPLATE := $(RTE_SRCDIR)/config/defconfig_$(T)
>>   endif
>> +else ifdef RTE_TARGET
>> +RTE_CONFIG_TEMPLATE := $(RTE_SRCDIR)/config/defconfig_$(RTE_TARGET)
>>   endif
>>   export RTE_CONFIG_TEMPLATE
> 
>