Re: [PATCH] selftests: sync: missing CFLAGS while compiling

[Anders Roxell]

On 10 January 2018 at 01:05, Shuah Khan <shuah@xxxxxxxxxx> wrote:
> On 01/05/2018 09:33 AM, Anders Roxell wrote:
>> Based on patch: https://patchwork.kernel.org/patch/10042045/
>>
>> arch64-linux-gnu-gcc -c sync.c -o sync/sync.o
>> sync.c:42:29: fatal error: linux/sync_file.h: No such file or directory
>>  #include <linux/sync_file.h>
>>                              ^
>> CFLAGS is not used during the compile step, so the system instead of
>> kernel headers are used.  Fix this by using lib.mk's compile rules and
>> remove CFLAGS from the linking step.
>
> Hmm. The changes don't match the change log. It odes more than just
> removing LDFLAGS from compile step,

oh, you are correct. I'm sorry.
I will re-spin the patch and make the changelog reflect the actual change
next time.

>
>>
>> Reported-by: Lei Yang <Lei.Yang@xxxxxxxxxxxxx>
>> Signed-off-by: Anders Roxell <anders.roxell@xxxxxxxxxx>
>> ---
>>  tools/testing/selftests/sync/Makefile | 8 +-------
>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/sync/Makefile b/tools/testing/selftests/sync/Makefile
>> index b3c8ba3cb668..58b9336d6c84 100644
>> --- a/tools/testing/selftests/sync/Makefile
>> +++ b/tools/testing/selftests/sync/Makefile
>> @@ -27,12 +27,6 @@ OBJS := $(patsubst %,$(OUTPUT)/%,$(OBJS))
>>  TESTS := $(patsubst %,$(OUTPUT)/%,$(TESTS))
>>
>>  $(TEST_CUSTOM_PROGS): $(TESTS) $(OBJS)
>> -     $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(CFLAGS) $(LDFLAGS)
>
> So why not just delete $(LDFLAGS)??
> $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(CFLAGS)
>
>> -
>> -$(OBJS): $(OUTPUT)/%.o: %.c
>> -     $(CC) -c $^ -o $@
>> -
>> -$(TESTS): $(OUTPUT)/%.o: %.c
>> -     $(CC) -c $^ -o $@
>> +     $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(LDFLAGS)
>>
>>  EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS)
>>
>
> I can't take this patch the way it is. It breaks the following
> use-case:
>
> make O=/tmp/kselftest TARGETS=sync kselftest

Oh, I've missed that use-case. Will fix that in the re-spin of the patch.

To fix this build issue when cross compiling

--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -30,7 +30,7 @@ $(TEST_CUSTOM_PROGS): $(TESTS) $(OBJS)
        $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(CFLAGS) $(LDFLAGS)

 $(OBJS): $(OUTPUT)/%.o: %.c
-       $(CC) -c $^ -o $@
+       $(CC) -c $^ -o $@ $(CFLAGS)

 $(TESTS): $(OUTPUT)/%.o: %.c
        $(CC) -c $^ -o $@


The other issue that needs to be addressed somehow would be make the
build system do
headers_install or if that should be a prerequisite.
I've seen ways to install the headers: "make -C ../../../..
headers_install" if the headers
aren't there, in
tools/testing/selftests/gpio/Makefile
tools/testing/selftests/vm/Makefile

Is that the preferred way or should an update in
Documentation/dev-tools/kselftest.rst be done to say do "make
headers_install" before
building kselftests?


Cheers,
Anders

>
> thanks,
> -- Shuah