Re: [PATCH v2 6/6] mk: add "make examples" target in root makefile

[Olivier MATZ <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>]

Hi Thomas,

Thank you for your comments.

On 05/14/2014 04:04 PM, Thomas Monjalon wrote:
>> +# Build directory is given with O=
>> +ifndef O
>> +O = $(RTE_SDK)/examples
>> +endif
>> +
>> +# Target for which examples should be built.
>> +ifndef T
>> +T = *
>> +endif
>
> Using "?=" should be cleaner than "ifndef" I think.

Agree, I'll fix that.

> [...]
>> +	$(Q)if [ ! -d "${RTE_SDK}/${*}" ]; then \
>> +		echo "Target ${*} does not exist in ${RTE_SDK}/${*}." ; \
>> +		echo -n "Please install DPDK first (make install) or use another " ;
> \
>> +		echo "target argument (T=target)." ; \
>
> You should stop make execution with "false" on such error.

The idea was to be able to build the examples for all installed
targets, skipping those which are not installed:

   make install T=x86_64-default-linuxapp-gcc
   # build for x86_64-default-linuxapp-gcc only
   make examples

But if you prefer, I can change it so the user has to specify
the target explicitly.

>> +	else \
>> +		$(MAKE) -C examples O=$(O) RTE_TARGET=$(*); \
>> +	fi
>
> We should use "abspath $O" here when changing directory.

Yes, I'll fix that too.

> Last comment: I cannot use
> 	make examples T=x86_64-default-linuxapp-gcc O=build
> because of
> 	RTE_SDK := $(CURDIR)
> in GNUmakefile

This works fine if you do:

   # install the x86_64-default-linuxapp-gcc target in RTE_SDK=.
   make install T=x86_64-default-linuxapp-gcc
   # compile examples in build, using the installed target
   # x86_64-default-linuxapp-gcc from RTE_SDK=.
   make examples T=x86_64-default-linuxapp-gcc O=build

Indeed, I would say that all makefile commands called from DPDK
root directory assume that RTE_SDK=. so that's the expected
behavior.

I'll send a v3 with the fixes tomorrow.

Regards,
Olivier