[PATCH] selftests/landlock: fix broken include of linux/landlock.h

[PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Guillaume Tucker]

Revert part of the earlier changes to fix the kselftest build when
using a sub-directory from the top of the tree as this broke the
landlock test build as a side-effect when building with "make -C
tools/testing/selftests/landlock".

Reported-by: Mickaël Salaün <mic@digikod.net>
Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 tools/testing/selftests/landlock/Makefile | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
index a6959df28eb0..02868ac3bc71 100644
--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
 TEST_GEN_PROGS_EXTENDED := true
 
 OVERRIDE_TARGETS := 1
+top_srcdir := ../../../..
 include ../lib.mk
 
+khdr_dir = $(top_srcdir)/usr/include
+
 $(OUTPUT)/true: true.c
 	$(LINK.c) $< $(LDLIBS) -o $@ -static
 
-$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
-	$(LINK.c) $< $(LDLIBS) -o $@ -lcap
+$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
+	$(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
-- 
2.30.2

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Shuah Khan]

On 8/3/22 2:13 PM, Guillaume Tucker wrote:
> Revert part of the earlier changes to fix the kselftest build when
> using a sub-directory from the top of the tree as this broke the
> landlock test build as a side-effect when building with "make -C
> tools/testing/selftests/landlock".
> 
> Reported-by: Mickaël Salaün <mic@digikod.net>
> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>   tools/testing/selftests/landlock/Makefile | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 

Thank you. Now applied to linux-kselftest next for the next pull request.

thanks,
-- Shuah

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Mickaël Salaün]

On 03/08/2022 22:13, Guillaume Tucker wrote:
> Revert part of the earlier changes to fix the kselftest build when
> using a sub-directory from the top of the tree as this broke the
> landlock test build as a side-effect when building with "make -C
> tools/testing/selftests/landlock".
> 
> Reported-by: Mickaël Salaün <mic@digikod.net>
> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>   tools/testing/selftests/landlock/Makefile | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
> index a6959df28eb0..02868ac3bc71 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>   TEST_GEN_PROGS_EXTENDED := true
>   
>   OVERRIDE_TARGETS := 1
> +top_srcdir := ../../../..

Not sure it changes much, but most other selftests Makefiles use 
"top_srcdir = ../../../.." (without ":="). Why this change?


>   include ../lib.mk
>   
> +khdr_dir = $(top_srcdir)/usr/include
> +
>   $(OUTPUT)/true: true.c
>   	$(LINK.c) $< $(LDLIBS) -o $@ -static
>   
> -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
> -	$(LINK.c) $< $(LDLIBS) -o $@ -lcap
> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
> +	$(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Guillaume Tucker]

On 04/08/2022 12:36, Mickaël Salaün wrote:
> 
> On 03/08/2022 22:13, Guillaume Tucker wrote:
>> Revert part of the earlier changes to fix the kselftest build when
>> using a sub-directory from the top of the tree as this broke the
>> landlock test build as a side-effect when building with "make -C
>> tools/testing/selftests/landlock".
>>
>> Reported-by: Mickaël Salaün <mic@digikod.net>
>> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
>> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>> ---
>>   tools/testing/selftests/landlock/Makefile | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
>> index a6959df28eb0..02868ac3bc71 100644
>> --- a/tools/testing/selftests/landlock/Makefile
>> +++ b/tools/testing/selftests/landlock/Makefile
>> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>>   TEST_GEN_PROGS_EXTENDED := true
>>     OVERRIDE_TARGETS := 1
>> +top_srcdir := ../../../..
> 
> Not sure it changes much, but most other selftests Makefiles use "top_srcdir = ../../../.." (without ":="). Why this change?

I didn't simply apply your diff but edited the file by hand to
test various combinations and see what side effects it might
have.  So when I added top_srcdir I typed it by hand and used :=
as a reflex since it's the standard way of assigning variables.
Using = instead only makes a difference when the r-value has
something dynamic as it will be re-evaluated every time it's
used.  So for constant values, I guess it's more of a question of
coding style and conventions.  Maybe all the top_srcdir variables
should be changed to := but that's unnecessary churn...  Either
way, it's benign.

Shuah, feel free to change this back to = in this particular case
if it's more consistent with other Makefiles.  Consistency is
often better than arbitrary rules.  Or conversely, change to :=
for the khdr_dir definition...  Entirely up to you I think.

Thanks,
Guillaume

>>   include ../lib.mk
>>   +khdr_dir = $(top_srcdir)/usr/include
>> +
>>   $(OUTPUT)/true: true.c
>>       $(LINK.c) $< $(LDLIBS) -o $@ -static
>>   -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
>> -    $(LINK.c) $< $(LDLIBS) -o $@ -lcap
>> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
>> +    $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Mickaël Salaün]

On 04/08/2022 21:38, Guillaume Tucker wrote:
> On 04/08/2022 12:36, Mickaël Salaün wrote:
>>
>> On 03/08/2022 22:13, Guillaume Tucker wrote:
>>> Revert part of the earlier changes to fix the kselftest build when
>>> using a sub-directory from the top of the tree as this broke the
>>> landlock test build as a side-effect when building with "make -C
>>> tools/testing/selftests/landlock".
>>>
>>> Reported-by: Mickaël Salaün <mic@digikod.net>
>>> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
>>> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
>>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>>> ---
>>>    tools/testing/selftests/landlock/Makefile | 7 +++++--
>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
>>> index a6959df28eb0..02868ac3bc71 100644
>>> --- a/tools/testing/selftests/landlock/Makefile
>>> +++ b/tools/testing/selftests/landlock/Makefile
>>> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>>>    TEST_GEN_PROGS_EXTENDED := true
>>>      OVERRIDE_TARGETS := 1
>>> +top_srcdir := ../../../..
>>
>> Not sure it changes much, but most other selftests Makefiles use "top_srcdir = ../../../.." (without ":="). Why this change?
> 
> I didn't simply apply your diff but edited the file by hand to
> test various combinations and see what side effects it might
> have.  So when I added top_srcdir I typed it by hand and used :=
> as a reflex since it's the standard way of assigning variables.
> Using = instead only makes a difference when the r-value has
> something dynamic as it will be re-evaluated every time it's
> used.  So for constant values, I guess it's more of a question of
> coding style and conventions.  Maybe all the top_srcdir variables
> should be changed to := but that's unnecessary churn...  Either
> way, it's benign.
> 
> Shuah, feel free to change this back to = in this particular case
> if it's more consistent with other Makefiles.  Consistency is
> often better than arbitrary rules.  Or conversely, change to :=
> for the khdr_dir definition...  Entirely up to you I think.

Looks good to me, thanks! Shuah, feel free to add
Signed-off-by: Mickaël Salaün <mic@digikod.net>

> 
> Thanks,
> Guillaume
> 
>>>    include ../lib.mk
>>>    +khdr_dir = $(top_srcdir)/usr/include
>>> +
>>>    $(OUTPUT)/true: true.c
>>>        $(LINK.c) $< $(LDLIBS) -o $@ -static
>>>    -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
>>> -    $(LINK.c) $< $(LDLIBS) -o $@ -lcap
>>> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
>>> +    $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
> 

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Mickaël Salaün]

Shuah, do you plan to send a pull request before merge window closes?

On 05/08/2022 19:16, Mickaël Salaün wrote:
> 
> On 04/08/2022 21:38, Guillaume Tucker wrote:
>> On 04/08/2022 12:36, Mickaël Salaün wrote:
>>>
>>> On 03/08/2022 22:13, Guillaume Tucker wrote:
>>>> Revert part of the earlier changes to fix the kselftest build when
>>>> using a sub-directory from the top of the tree as this broke the
>>>> landlock test build as a side-effect when building with "make -C
>>>> tools/testing/selftests/landlock".
>>>>
>>>> Reported-by: Mickaël Salaün <mic@digikod.net>
>>>> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
>>>> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
>>>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>>>> ---
>>>>     tools/testing/selftests/landlock/Makefile | 7 +++++--
>>>>     1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
>>>> index a6959df28eb0..02868ac3bc71 100644
>>>> --- a/tools/testing/selftests/landlock/Makefile
>>>> +++ b/tools/testing/selftests/landlock/Makefile
>>>> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>>>>     TEST_GEN_PROGS_EXTENDED := true
>>>>       OVERRIDE_TARGETS := 1
>>>> +top_srcdir := ../../../..
>>>
>>> Not sure it changes much, but most other selftests Makefiles use "top_srcdir = ../../../.." (without ":="). Why this change?
>>
>> I didn't simply apply your diff but edited the file by hand to
>> test various combinations and see what side effects it might
>> have.  So when I added top_srcdir I typed it by hand and used :=
>> as a reflex since it's the standard way of assigning variables.
>> Using = instead only makes a difference when the r-value has
>> something dynamic as it will be re-evaluated every time it's
>> used.  So for constant values, I guess it's more of a question of
>> coding style and conventions.  Maybe all the top_srcdir variables
>> should be changed to := but that's unnecessary churn...  Either
>> way, it's benign.
>>
>> Shuah, feel free to change this back to = in this particular case
>> if it's more consistent with other Makefiles.  Consistency is
>> often better than arbitrary rules.  Or conversely, change to :=
>> for the khdr_dir definition...  Entirely up to you I think.
> 
> Looks good to me, thanks! Shuah, feel free to add
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> 
>>
>> Thanks,
>> Guillaume
>>
>>>>     include ../lib.mk
>>>>     +khdr_dir = $(top_srcdir)/usr/include
>>>> +
>>>>     $(OUTPUT)/true: true.c
>>>>         $(LINK.c) $< $(LDLIBS) -o $@ -static
>>>>     -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
>>>> -    $(LINK.c) $< $(LDLIBS) -o $@ -lcap
>>>> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
>>>> +    $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
>>

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Shuah Khan]

On 8/12/22 9:29 AM, Mickaël Salaün wrote:
> Shuah, do you plan to send a pull request before merge window closes?
> 

I was hoping to do so and missed the window. Looks like more discussion
happening on this change. I will get this in for the next rc

thanks,
-- Shuah

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Mickaël Salaün]

On 15/08/2022 19:17, Shuah Khan wrote:
> On 8/12/22 9:29 AM, Mickaël Salaün wrote:
>> Shuah, do you plan to send a pull request before merge window closes?
>>
> 
> I was hoping to do so and missed the window. Looks like more discussion
> happening on this change. I will get this in for the next rc

Yes please, this is a regression fix. I cannot reproduce the issue 
reported by Anders Roxell.

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Anders Roxell]

On Wed, 3 Aug 2022 at 22:14, Guillaume Tucker
<guillaume.tucker@collabora.com> wrote:
>
> Revert part of the earlier changes to fix the kselftest build when
> using a sub-directory from the top of the tree as this broke the
> landlock test build as a side-effect when building with "make -C
> tools/testing/selftests/landlock".
>
> Reported-by: Mickaël Salaün <mic@digikod.net>
> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>

Building with this patch doesn't work, it gives this output:
make[3]: Entering directory
'/home/anders/src/kernel/next/tools/testing/selftests/landlock'
make[3]: Leaving directory
'/home/anders/src/kernel/next/tools/testing/selftests/landlock'
make[3]: *** No rule to make target
'/home/anders/.cache/tuxmake/builds/78/build/kselftest/landlock/base_test',
needed by 'all'.  Stop.

I'm building like this:
tuxmake --runtime podman --target-arch x86_64 --toolchain gcc-12
--kconfig defconfig kselftest

which translates into this make command:
make --silent --keep-going --jobs=32
O=/home/anders/.cache/tuxmake/builds/78/build
INSTALL_PATH=/home/anders/.cache/tuxmake/builds/78/build/kselftest_install
ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- kselftest-install

building without this patch works, see below:

make[3]: Entering directory
'/home/anders/src/kernel/next/tools/testing/selftests/landlock'
x86_64-linux-gnu-gcc -Wall -O2 -isystem
/home/anders/.cache/tuxmake/builds/77/build/usr/include    base_test.c
 -o /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/base_test
-lcap
x86_64-linux-gnu-gcc -Wall -O2 -isystem
/home/anders/.cache/tuxmake/builds/77/build/usr/include    fs_test.c
-o /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/fs_test
-lcap
x86_64-linux-gnu-gcc -Wall -O2 -isystem
/home/anders/.cache/tuxmake/builds/77/build/usr/include
ptrace_test.c  -o
/home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/ptrace_test
-lcap
x86_64-linux-gnu-gcc -Wall -O2 -isystem
/home/anders/.cache/tuxmake/builds/77/build/usr/include    true.c  -o
/home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/true
-static
make[3]: Leaving directory
'/home/anders/src/kernel/next/tools/testing/selftests/landlock'

Cheers,
Anders

> ---
>  tools/testing/selftests/landlock/Makefile | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
> index a6959df28eb0..02868ac3bc71 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>  TEST_GEN_PROGS_EXTENDED := true
>
>  OVERRIDE_TARGETS := 1
> +top_srcdir := ../../../..
>  include ../lib.mk
>
> +khdr_dir = $(top_srcdir)/usr/include
> +
>  $(OUTPUT)/true: true.c
>         $(LINK.c) $< $(LDLIBS) -o $@ -static
>
> -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
> -       $(LINK.c) $< $(LDLIBS) -o $@ -lcap
> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
> +       $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
> --
> 2.30.2
>

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Mickaël Salaün]

On 13/08/2022 12:01, Anders Roxell wrote:
> On Wed, 3 Aug 2022 at 22:14, Guillaume Tucker
> <guillaume.tucker@collabora.com> wrote:
>>
>> Revert part of the earlier changes to fix the kselftest build when
>> using a sub-directory from the top of the tree as this broke the
>> landlock test build as a side-effect when building with "make -C
>> tools/testing/selftests/landlock".
>>
>> Reported-by: Mickaël Salaün <mic@digikod.net>
>> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
>> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> 
> Building with this patch doesn't work, it gives this output:
> make[3]: Entering directory
> '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
> make[3]: Leaving directory
> '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
> make[3]: *** No rule to make target
> '/home/anders/.cache/tuxmake/builds/78/build/kselftest/landlock/base_test',
> needed by 'all'.  Stop.
> 
> I'm building like this:
> tuxmake --runtime podman --target-arch x86_64 --toolchain gcc-12
> --kconfig defconfig kselftest
> 
> which translates into this make command:
> make --silent --keep-going --jobs=32
> O=/home/anders/.cache/tuxmake/builds/78/build
> INSTALL_PATH=/home/anders/.cache/tuxmake/builds/78/build/kselftest_install
> ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- kselftest-install

This works well for me. Which commit is checkout?


> 
> building without this patch works, see below:
> 
> make[3]: Entering directory
> '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
> x86_64-linux-gnu-gcc -Wall -O2 -isystem
> /home/anders/.cache/tuxmake/builds/77/build/usr/include    base_test.c
>   -o /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/base_test
> -lcap
> x86_64-linux-gnu-gcc -Wall -O2 -isystem
> /home/anders/.cache/tuxmake/builds/77/build/usr/include    fs_test.c
> -o /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/fs_test
> -lcap
> x86_64-linux-gnu-gcc -Wall -O2 -isystem
> /home/anders/.cache/tuxmake/builds/77/build/usr/include
> ptrace_test.c  -o
> /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/ptrace_test
> -lcap
> x86_64-linux-gnu-gcc -Wall -O2 -isystem
> /home/anders/.cache/tuxmake/builds/77/build/usr/include    true.c  -o
> /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/true
> -static
> make[3]: Leaving directory
> '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
Does this work if you revert this patch, commit a917dd94b832 
("selftests/landlock: drop deprecated headers dependency") and commit 
f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")?

This patch mainly revert commit a917dd94b832, so I don't see the issue.


> 
> Cheers,
> Anders
> 
>> ---
>>   tools/testing/selftests/landlock/Makefile | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
>> index a6959df28eb0..02868ac3bc71 100644
>> --- a/tools/testing/selftests/landlock/Makefile
>> +++ b/tools/testing/selftests/landlock/Makefile
>> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>>   TEST_GEN_PROGS_EXTENDED := true
>>
>>   OVERRIDE_TARGETS := 1
>> +top_srcdir := ../../../..
>>   include ../lib.mk
>>
>> +khdr_dir = $(top_srcdir)/usr/include
>> +
>>   $(OUTPUT)/true: true.c
>>          $(LINK.c) $< $(LDLIBS) -o $@ -static
>>
>> -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
>> -       $(LINK.c) $< $(LDLIBS) -o $@ -lcap
>> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
>> +       $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
>> --
>> 2.30.2
>>

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Anders Roxell]

On Sat, 13 Aug 2022 at 14:31, Mickaël Salaün <mic@digikod.net> wrote:
>
>
> On 13/08/2022 12:01, Anders Roxell wrote:
> > On Wed, 3 Aug 2022 at 22:14, Guillaume Tucker
> > <guillaume.tucker@collabora.com> wrote:
> >>
> >> Revert part of the earlier changes to fix the kselftest build when
> >> using a sub-directory from the top of the tree as this broke the
> >> landlock test build as a side-effect when building with "make -C
> >> tools/testing/selftests/landlock".
> >>
> >> Reported-by: Mickaël Salaün <mic@digikod.net>
> >> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
> >> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
> >> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> >
> > Building with this patch doesn't work, it gives this output:
> > make[3]: Entering directory
> > '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
> > make[3]: Leaving directory
> > '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
> > make[3]: *** No rule to make target
> > '/home/anders/.cache/tuxmake/builds/78/build/kselftest/landlock/base_test',
> > needed by 'all'.  Stop.
> >
> > I'm building like this:
> > tuxmake --runtime podman --target-arch x86_64 --toolchain gcc-12
> > --kconfig defconfig kselftest
> >
> > which translates into this make command:
> > make --silent --keep-going --jobs=32
> > O=/home/anders/.cache/tuxmake/builds/78/build
> > INSTALL_PATH=/home/anders/.cache/tuxmake/builds/78/build/kselftest_install
> > ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- kselftest-install
>
> This works well for me.

Interesting

> Which commit is checkout?

I used the latest next tag, I tried to on todays tag as well
next-20220822 and I see
the same issue.
building without 'O=...' I can build the landlock tests...

>
>
> >
> > building without this patch works, see below:
> >
> > make[3]: Entering directory
> > '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
> > x86_64-linux-gnu-gcc -Wall -O2 -isystem
> > /home/anders/.cache/tuxmake/builds/77/build/usr/include    base_test.c
> >   -o /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/base_test
> > -lcap
> > x86_64-linux-gnu-gcc -Wall -O2 -isystem
> > /home/anders/.cache/tuxmake/builds/77/build/usr/include    fs_test.c
> > -o /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/fs_test
> > -lcap
> > x86_64-linux-gnu-gcc -Wall -O2 -isystem
> > /home/anders/.cache/tuxmake/builds/77/build/usr/include
> > ptrace_test.c  -o
> > /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/ptrace_test
> > -lcap
> > x86_64-linux-gnu-gcc -Wall -O2 -isystem
> > /home/anders/.cache/tuxmake/builds/77/build/usr/include    true.c  -o
> > /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/true
> > -static
> > make[3]: Leaving directory
> > '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
> Does this work if you revert this patch, commit a917dd94b832
> ("selftests/landlock: drop deprecated headers dependency") and commit
> f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")?
>
> This patch mainly revert commit a917dd94b832, so I don't see the issue.
>
>
> >
> > Cheers,
> > Anders
> >
> >> ---
> >>   tools/testing/selftests/landlock/Makefile | 7 +++++--
> >>   1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
> >> index a6959df28eb0..02868ac3bc71 100644
> >> --- a/tools/testing/selftests/landlock/Makefile
> >> +++ b/tools/testing/selftests/landlock/Makefile
> >> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
> >>   TEST_GEN_PROGS_EXTENDED := true
> >>
> >>   OVERRIDE_TARGETS := 1
> >> +top_srcdir := ../../../..
> >>   include ../lib.mk
> >>
> >> +khdr_dir = $(top_srcdir)/usr/include
> >> +
> >>   $(OUTPUT)/true: true.c
> >>          $(LINK.c) $< $(LDLIBS) -o $@ -static
> >>
> >> -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
> >> -       $(LINK.c) $< $(LDLIBS) -o $@ -lcap
> >> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
> >> +       $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
> >> --
> >> 2.30.2
> >>

Re: [PATCH] selftests/landlock: fix broken include of linux/landlock.h

[Mickaël Salaün]

On 22/08/2022 16:00, Anders Roxell wrote:
> On Sat, 13 Aug 2022 at 14:31, Mickaël Salaün <mic@digikod.net> wrote:
>>
>>
>> On 13/08/2022 12:01, Anders Roxell wrote:
>>> On Wed, 3 Aug 2022 at 22:14, Guillaume Tucker
>>> <guillaume.tucker@collabora.com> wrote:
>>>>
>>>> Revert part of the earlier changes to fix the kselftest build when
>>>> using a sub-directory from the top of the tree as this broke the
>>>> landlock test build as a side-effect when building with "make -C
>>>> tools/testing/selftests/landlock".
>>>>
>>>> Reported-by: Mickaël Salaün <mic@digikod.net>
>>>> Fixes: a917dd94b832 ("selftests/landlock: drop deprecated headers dependency")
>>>> Fixes: f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")
>>>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>>>
>>> Building with this patch doesn't work, it gives this output:
>>> make[3]: Entering directory
>>> '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
>>> make[3]: Leaving directory
>>> '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
>>> make[3]: *** No rule to make target
>>> '/home/anders/.cache/tuxmake/builds/78/build/kselftest/landlock/base_test',
>>> needed by 'all'.  Stop.
>>>
>>> I'm building like this:
>>> tuxmake --runtime podman --target-arch x86_64 --toolchain gcc-12
>>> --kconfig defconfig kselftest
>>>
>>> which translates into this make command:
>>> make --silent --keep-going --jobs=32
>>> O=/home/anders/.cache/tuxmake/builds/78/build
>>> INSTALL_PATH=/home/anders/.cache/tuxmake/builds/78/build/kselftest_install
>>> ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- kselftest-install
>>
>> This works well for me.
> 
> Interesting

I used this command (inspired by yours):

make --silent --keep-going --jobs=32 "O=${HOME}/build" 
"INSTALL_PATH=${HOME}/build/kselftest_install" ARCH=x86_64 
CROSS_COMPILE=x86_64-linux-gnu- kselftest-install

Can you run this command without using tuxmake?


> 
>> Which commit is checkout?
> 
> I used the latest next tag, I tried to on todays tag as well
> next-20220822 and I see
> the same issue.
> building without 'O=...' I can build the landlock tests...

Can you test it with Linux v5.19 and v6.0-rc2 and see if there is a 
difference?

Is your workspace clean?
What is the version of your make?

Can you replace this line from the Makefile with static names?
"src_test := $(wildcard *_test.c)"



> 
>>
>>
>>>
>>> building without this patch works, see below:
>>>
>>> make[3]: Entering directory
>>> '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
>>> x86_64-linux-gnu-gcc -Wall -O2 -isystem
>>> /home/anders/.cache/tuxmake/builds/77/build/usr/include    base_test.c
>>>    -o /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/base_test
>>> -lcap
>>> x86_64-linux-gnu-gcc -Wall -O2 -isystem
>>> /home/anders/.cache/tuxmake/builds/77/build/usr/include    fs_test.c
>>> -o /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/fs_test
>>> -lcap
>>> x86_64-linux-gnu-gcc -Wall -O2 -isystem
>>> /home/anders/.cache/tuxmake/builds/77/build/usr/include
>>> ptrace_test.c  -o
>>> /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/ptrace_test
>>> -lcap
>>> x86_64-linux-gnu-gcc -Wall -O2 -isystem
>>> /home/anders/.cache/tuxmake/builds/77/build/usr/include    true.c  -o
>>> /home/anders/.cache/tuxmake/builds/77/build/kselftest/landlock/true
>>> -static
>>> make[3]: Leaving directory
>>> '/home/anders/src/kernel/next/tools/testing/selftests/landlock'
>> Does this work if you revert this patch, commit a917dd94b832
>> ("selftests/landlock: drop deprecated headers dependency") and commit
>> f2745dc0ba3d ("selftests: stop using KSFT_KHDR_INSTALL")?
>>
>> This patch mainly revert commit a917dd94b832, so I don't see the issue.
>>
>>
>>>
>>> Cheers,
>>> Anders
>>>
>>>> ---
>>>>    tools/testing/selftests/landlock/Makefile | 7 +++++--
>>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
>>>> index a6959df28eb0..02868ac3bc71 100644
>>>> --- a/tools/testing/selftests/landlock/Makefile
>>>> +++ b/tools/testing/selftests/landlock/Makefile
>>>> @@ -9,10 +9,13 @@ TEST_GEN_PROGS := $(src_test:.c=)
>>>>    TEST_GEN_PROGS_EXTENDED := true
>>>>
>>>>    OVERRIDE_TARGETS := 1
>>>> +top_srcdir := ../../../..
>>>>    include ../lib.mk
>>>>
>>>> +khdr_dir = $(top_srcdir)/usr/include
>>>> +
>>>>    $(OUTPUT)/true: true.c
>>>>           $(LINK.c) $< $(LDLIBS) -o $@ -static
>>>>
>>>> -$(OUTPUT)/%_test: %_test.c ../kselftest_harness.h common.h
>>>> -       $(LINK.c) $< $(LDLIBS) -o $@ -lcap
>>>> +$(OUTPUT)/%_test: %_test.c $(khdr_dir)/linux/landlock.h ../kselftest_harness.h common.h
>>>> +       $(LINK.c) $< $(LDLIBS) -o $@ -lcap -I$(khdr_dir)
>>>> --
>>>> 2.30.2
>>>>