| Message ID | a459363217b1847c0f206a5dbdf181cb21cf3d0c.1659557290.git.guillaume.tucker@collabora.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | selftests/landlock: fix broken include of linux/landlock.h | expand |
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
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)
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)
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) >
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) >>
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 >
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 >>
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
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.
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 > >>
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 >>>>
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)
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(-)