2018-01-05 16:33:41

by Anders Roxell

[permalink] [raw]
Subject: [PATCH] selftests: sync: missing CFLAGS while compiling

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.

Reported-by: Lei Yang <[email protected]>
Signed-off-by: Anders Roxell <[email protected]>
---
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)
-
-$(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)
--
2.11.0


2018-01-05 17:21:43

by Naresh Kamboju

[permalink] [raw]
Subject: Re: [PATCH] selftests: sync: missing CFLAGS while compiling

On 5 January 2018 at 22:03, Anders Roxell <[email protected]> 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.
>
> Reported-by: Lei Yang <[email protected]>
> Signed-off-by: Anders Roxell <[email protected]>

Tested-by: Naresh Kamboju <[email protected]>

> ---
> 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)
> -
> -$(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)
> --
> 2.11.0
>

2018-01-10 00:05:21

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH] selftests: sync: missing CFLAGS while compiling

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,

>
> Reported-by: Lei Yang <[email protected]>
> Signed-off-by: Anders Roxell <[email protected]>
> ---
> 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

thanks,
-- Shuah

2018-01-16 11:00:48

by Anders Roxell

[permalink] [raw]
Subject: Re: [PATCH] selftests: sync: missing CFLAGS while compiling

On 10 January 2018 at 01:05, Shuah Khan <[email protected]> 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 <[email protected]>
>> Signed-off-by: Anders Roxell <[email protected]>
>> ---
>> 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