Re: [dpdk-dev] [PATCH v2 1/2] mk: allow use of environment var for make config

[Shreyansh Jain <shreyansh.jain@nxp.com>]

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