Re: [PATCH v2] selftests: gpio: fix Makefile

[Fathi Boudra]

On 30 March 2017 at 16:14, Bamvor Zhang Jian
<bamvor.zhangjian@xxxxxxxxxx> wrote:
> Hi, Fathi
>
> Thanks for your patch.
>
> On 28 March 2017 at 13:36, Fathi Boudra <fathi.boudra@xxxxxxxxxx> wrote:
>> * Fix hardcoded and misplaced libmount headers. Use pkg-config instead to
>>   figure out CFLAGS/LDLIBS, fixing also their value for cross-compilation.
>>   Note: if pkg-config is missing (command not found), it will fail to
>>   build because headers can't be found or libmount library can't be
>>   linked.
>>
>> * Fix the clean target to clean up also gpio-utils.
>>
>> * Fix gpio-mockup-chardev installation by using TEST_PROGS_EXTENDED
>>   instead of BINARIES which is not supported by the top-level lib.mk.
>>
>> * Get rid of INSTALL_HDR_PATH. We don't need it since make -C is putting
>>   us in the right location.
>>
>> * Improve readibility:
>>   - introduce GPIODIR/GPIOOBJ/GPIOINC variables
>>   - split CFLAGS on multiple lines
>>
>> Signed-off-by: Fathi Boudra <fathi.boudra@xxxxxxxxxx>
>> ---
>>
>> in v2:
>>  * per Michael Ellerman request, revert to use exported headers instead of uapi.
>>
>>  tools/testing/selftests/gpio/Makefile | 31 ++++++++++++++++++-------------
>>  1 file changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
>> index 205e4d10e085..41826897af35 100644
>> --- a/tools/testing/selftests/gpio/Makefile
>> +++ b/tools/testing/selftests/gpio/Makefile
>> @@ -1,23 +1,28 @@
>> +CFLAGS += -O2 -g -std=gnu99 -Wall
>> +CFLAGS += -I../../../../usr/include/
>> +CFLAGS += $(shell pkg-config --cflags mount)
>> +LDLIBS += $(shell pkg-config --libs mount)
>>
>>  TEST_PROGS := gpio-mockup.sh
>> -TEST_FILES := gpio-mockup-sysfs.sh $(BINARIES)
>> -BINARIES := gpio-mockup-chardev
>> +TEST_PROGS_EXTENDED := gpio-mockup-chardev
>> +TEST_FILES := gpio-mockup-sysfs.sh
>> +
>> +GPIODIR := ../../../gpio
>> +GPIOOBJ := gpio-utils.o
>> +GPIOINC := gpio.h
>>
>>  include ../lib.mk
>>
>> -all: $(BINARIES)
>> +all: $(GPIOINC) $(TEST_PROGS_EXTENDED)
> All the TEST_PROGS_EXTENDED should be TEST_GEN_FILES which will output to
> KBUILD_OUTPUT/O directory when it enabled.
>>
>>  clean:
>> -       $(RM) $(BINARIES)
>> -
>> -CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
>> -LDLIBS += -lmount -I/usr/include/libmount
>> -
>> -$(BINARIES): ../../../gpio/gpio-utils.o ../../../../usr/include/linux/gpio.h
>> +       $(RM) $(TEST_PROGS_EXTENDED)
>> +       $(MAKE) -C $(GPIODIR) clean
> When I enable KBUILD_OUTPUT, it will affect the output directory in
> tools/gpio and
> output to wired directory:
>
> $ make KBUILD_OUTPUT=$PWD/2001 -C tools/testing/selftests/ TARGETS=gpio
> make: Entering directory
> `/home/bamvor/works/source/kernel/linux/tools/testing/selftests'
> for TARGET in gpio; do \
> BUILD_TARGET=$BUILD/$TARGET; \
> mkdir $BUILD_TARGET -p; \
> make OUTPUT=$BUILD_TARGET -C $TARGET;\
> done;
> make[1]: Entering directory
> `/home/bamvor/works/source/kernel/linux/tools/testing/selftests/gpio'
> Makefile:19: warning: overriding commands for target `clean'
> ../lib.mk:55: warning: ignoring old commands for target `clean'
> make -C ../../../gpio
> make[2]: Entering directory `/home/bamvor/works/source/kernel/linux/tools/gpio'
> mkdir -p /home/bamvor/works/source/kernel/linux/2001/gpioinclude/linux
> 2>&1 || true
> ln -sf /home/bamvor/works/source/kernel/linux/tools/gpio/../../include/uapi/linux/gpio.h
> /home/bamvor/works/source/kernel/linux/2001/gpioinclude/linux/gpio.h
> make -f /home/bamvor/works/source/kernel/linux/tools/build/Makefile.build
> dir=. obj=lsgpio
> make[3]: Entering directory `/home/bamvor/works/source/kernel/linux/tools/gpio'
> CC /home/bamvor/works/source/kernel/linux/2001/gpiolsgpio.o
> CC /home/bamvor/works/source/kernel/linux/2001/gpiogpio-utils.o
> LD /home/bamvor/works/source/kernel/linux/2001/gpiolsgpio-in.o
>
> It could be fixed by building the tools/gpio manually or passing the
> correct absolute path:
> $(MAKE) OUTPUT=$(realpath $(GPIODIR))/ -C $(GPIODIR) clean
>
> I looked for a bettter fix. But no progress so far. Loop Amaldo, hope could
> get some input.

Thanks for the review. I'll see if I can come with something better.
Otherwise, I prefer passing the correct absolute path over building
tools/gpio manually.

>>
>> -../../../gpio/gpio-utils.o:
>> -       make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
>> +$(TEST_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ)
>>
>> -../../../../usr/include/linux/gpio.h:
>> -       make -C ../../../.. headers_install INSTALL_HDR_PATH=$(shell pwd)/../../../../usr/
>> +$(GPIODIR)/$(GPIOOBJ):
>> +       $(MAKE) -C $(GPIODIR)
> Similiar to the above clean target.

ack, will fix in v3.

> Regards
>
> Bamvor
>>
>> +$(GPIOINC):
>> +       $(MAKE) -C ../../../.. headers_install
>> --
>> 2.11.0