| Message ID | 20220223191016.1658728-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Makefile: Fix separate output directory build of kselftests | expand |
Hi, Any thoughts about this patch? On 2/24/22 12:10 AM, Muhammad Usama Anjum wrote: > Build of kselftests fail if kernel's top most Makefile is used for > running or building kselftests with separate output directory. The > absolute path is needed to reference other files during this kind of > build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It > fixes the following different types of errors: > > make kselftest-all O=/linux_mainline/build > Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory > > make kselftest-all O=build > Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > I've tested this patch on top of next-20220217. The latest next-20220222 > have missing patches. > --- > Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 86f633c2809ea..62b3eb8a102ab 100644 > --- a/Makefile > +++ b/Makefile > @@ -1411,10 +1411,10 @@ tools/%: FORCE > > PHONY += kselftest > kselftest: > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 run_tests > > kselftest-%: FORCE > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $* > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 $* > > PHONY += kselftest-merge > kselftest-merge:
On 2/23/22 12:10 PM, Muhammad Usama Anjum wrote: > Build of kselftests fail if kernel's top most Makefile is used for > running or building kselftests with separate output directory. The > absolute path is needed to reference other files during this kind of > build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It > fixes the following different types of errors: > > make kselftest-all O=/linux_mainline/build > Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory > > make kselftest-all O=build > Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > I've tested this patch on top of next-20220217. The latest next-20220222 > have missing patches. Can you give more details on the use-cases you tested? Did you test all the ways kselftest are built? > --- > Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 86f633c2809ea..62b3eb8a102ab 100644 > --- a/Makefile > +++ b/Makefile > @@ -1411,10 +1411,10 @@ tools/%: FORCE > > PHONY += kselftest > kselftest: > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 run_tests > > kselftest-%: FORCE > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $* > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 $* > > PHONY += kselftest-merge > kselftest-merge: > Change looks good to me as long as all the supported use-cases work correctly. Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah
On 3/4/22 2:32 AM, Shuah Khan wrote: > On 2/23/22 12:10 PM, Muhammad Usama Anjum wrote: >> Build of kselftests fail if kernel's top most Makefile is used for >> running or building kselftests with separate output directory. The >> absolute path is needed to reference other files during this kind of >> build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It >> fixes the following different types of errors: >> >> make kselftest-all O=/linux_mainline/build >> Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory >> >> make kselftest-all O=build >> Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory >> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >> --- >> I've tested this patch on top of next-20220217. The latest next-20220222 >> have missing patches. > > Can you give more details on the use-cases you tested? Did you test all > the ways kselftest are built? > Yeah, I've tried to test all the ways. Here are the different ways I've used to test it: 1) Same directory build of kselftest (this is already working) make kselftest make kselftest-all make kselftest-install make kselftest-clean make kselftest-gen_tar 2) These were failing when separate output directory is specified either as relative or absolute path. After adding this patch, these are also working. kselfetst.rst mentions separate output directory build in this way. make kselftest O=build make kselftest-all O=build make kselftest-install O=build make kselftest-clean O=build make kselftest-gen_tar O=build make kselftest O=/build make kselftest-all O=/build make kselftest-install O=/build make kselftest-clean O=/build make kselftest-gen_tar O=/build Tested on top of next-20220307 after applying this patch.
On 3/8/22 1:11 AM, Muhammad Usama Anjum wrote: > On 3/4/22 2:32 AM, Shuah Khan wrote: >> On 2/23/22 12:10 PM, Muhammad Usama Anjum wrote: >>> Build of kselftests fail if kernel's top most Makefile is used for >>> running or building kselftests with separate output directory. The >>> absolute path is needed to reference other files during this kind of >>> build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It >>> fixes the following different types of errors: >>> >>> make kselftest-all O=/linux_mainline/build >>> Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory >>> >>> make kselftest-all O=build >>> Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory >>> >>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >>> --- >>> I've tested this patch on top of next-20220217. The latest next-20220222 >>> have missing patches. >> >> Can you give more details on the use-cases you tested? Did you test all >> the ways kselftest are built? >> > Yeah, I've tried to test all the ways. Here are the different ways I've > used to test it: > 1) Same directory build of kselftest (this is already working) > make kselftest > make kselftest-all > make kselftest-install > make kselftest-clean > make kselftest-gen_tar > > 2) These were failing when separate output directory is specified either > as relative or absolute path. After adding this patch, these are also > working. kselfetst.rst mentions separate output directory build in this way. > make kselftest O=build > make kselftest-all O=build > make kselftest-install O=build > make kselftest-clean O=build > make kselftest-gen_tar O=build > > make kselftest O=/build > make kselftest-all O=/build > make kselftest-install O=/build > make kselftest-clean O=/build > make kselftest-gen_tar O=/build > > Tested on top of next-20220307 after applying this patch. > Thank you for testing all these use-cases. This is a good comprehensive list. Do you mind sending a doc patch for Documentation/dev-tools/kselftest.rst The text here could almost as is as a new section after Contributing new tests (details) with a new section that outlines the tests to run when adding a new test to selftests/Makefile and making changes to kselftest common frameowork: selftests/Makefile, selftests/lib.mk Let me know if you are unable to, I will send a patch in. thanks, -- Shuah
On 3/9/22 2:19 AM, Shuah Khan wrote: > On 3/8/22 1:11 AM, Muhammad Usama Anjum wrote: >> On 3/4/22 2:32 AM, Shuah Khan wrote: >>> On 2/23/22 12:10 PM, Muhammad Usama Anjum wrote: >>>> Build of kselftests fail if kernel's top most Makefile is used for >>>> running or building kselftests with separate output directory. The >>>> absolute path is needed to reference other files during this kind of >>>> build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It >>>> fixes the following different types of errors: >>>> >>>> make kselftest-all O=/linux_mainline/build >>>> Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory >>>> >>>> make kselftest-all O=build >>>> Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory >>>> >>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> >>>> --- >>>> I've tested this patch on top of next-20220217. The latest >>>> next-20220222 >>>> have missing patches. >>> >>> Can you give more details on the use-cases you tested? Did you test all >>> the ways kselftest are built? >>> >> Yeah, I've tried to test all the ways. Here are the different ways I've >> used to test it: >> 1) Same directory build of kselftest (this is already working) >> make kselftest >> make kselftest-all >> make kselftest-install >> make kselftest-clean >> make kselftest-gen_tar >> >> 2) These were failing when separate output directory is specified either >> as relative or absolute path. After adding this patch, these are also >> working. kselfetst.rst mentions separate output directory build in >> this way. >> make kselftest O=build >> make kselftest-all O=build >> make kselftest-install O=build >> make kselftest-clean O=build >> make kselftest-gen_tar O=build >> >> make kselftest O=/build >> make kselftest-all O=/build >> make kselftest-install O=/build >> make kselftest-clean O=/build >> make kselftest-gen_tar O=/build >> >> Tested on top of next-20220307 after applying this patch. >> > > Thank you for testing all these use-cases. This is a good comprehensive > list. Do you mind sending a doc patch for > > Documentation/dev-tools/kselftest.rst > > The text here could almost as is as a new section after > > Contributing new tests (details) with a new section that outlines > the tests to run when adding a new test to selftests/Makefile > and making changes to kselftest common frameowork: selftests/Makefile, > selftests/lib.mk Hi Shuah, Yeah, definitely I can write and contribute it. I've noted this. This is the last patch I want to get accepted, then I plan to update the kselftest documentation (with the things you have mentioned and a few more things) and write a blog about how KernelCI has helped in all this. > > Let me know if you are unable to, I will send a patch in. > > thanks, > -- Shuah
Reminder. Shuah is okay with this patch. Any thoughts? On 2/24/22 12:10 AM, Muhammad Usama Anjum wrote: > Build of kselftests fail if kernel's top most Makefile is used for > running or building kselftests with separate output directory. The > absolute path is needed to reference other files during this kind of > build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It > fixes the following different types of errors: > > make kselftest-all O=/linux_mainline/build > Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory > > make kselftest-all O=build > Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > I've tested this patch on top of next-20220217. The latest next-20220222 > have missing patches. > --- > Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 86f633c2809ea..62b3eb8a102ab 100644 > --- a/Makefile > +++ b/Makefile > @@ -1411,10 +1411,10 @@ tools/%: FORCE > > PHONY += kselftest > kselftest: > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 run_tests > > kselftest-%: FORCE > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $* > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 $* > > PHONY += kselftest-merge > kselftest-merge:
On Thu, Mar 17, 2022 at 7:49 PM Muhammad Usama Anjum <usama.anjum@collabora.com> wrote: > > Reminder. Shuah is okay with this patch. Any thoughts? I do not think this is the right fix, but something you just happen to find working. The Make is working in a wrong directory, that is why the relative path does not work (and you use the absolute path to work around it) > > On 2/24/22 12:10 AM, Muhammad Usama Anjum wrote: > > Build of kselftests fail if kernel's top most Makefile is used for > > running or building kselftests with separate output directory. The > > absolute path is needed to reference other files during this kind of > > build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It > > fixes the following different types of errors: > > > > make kselftest-all O=/linux_mainline/build > > Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory > > > > make kselftest-all O=build > > Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory > > > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > > --- > > I've tested this patch on top of next-20220217. The latest next-20220222 > > have missing patches. > > --- > > Makefile | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 86f633c2809ea..62b3eb8a102ab 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1411,10 +1411,10 @@ tools/%: FORCE > > > > PHONY += kselftest > > kselftest: > > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests > > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 run_tests > > > > kselftest-%: FORCE > > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $* > > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 $* > > > > PHONY += kselftest-merge > > kselftest-merge: > > -- > Muhammad Usama Anjum -- Best Regards Masahiro Yamada
From [Makefile](https://elixir.bootlin.com/linux/latest/source/Makefile):
```
ifeq ($(abs_srctree),$(abs_objtree))
# building in the source tree
srctree := .
building_out_of_srctree :=
else
ifeq ($(abs_srctree)/,$(dir $(abs_objtree)))
# building in a subdirectory of the source tree
srctree := ..
else
srctree := $(abs_srctree)
endif
building_out_of_srctree := 1
endif
```
`ifeq ($(abs_srctree)/,$(dir $(abs_objtree)))` condition is setting
`srctree` to `..`. This is wrong. This condition isn't considering that
`header_install` doesn't depend on `abs_srctree and abs_objtree`. This
condition needs to be tweaked or removed for the `install_headers` to
work fine and fix this issue. I've added `KBUILD_ABS_SRCTREE=1` to the
kselftest target which sets the `srctree` to `abs_srctree` and thus
forcefully affecting only kselftest targets. This seems like the clean
fix. Alternatively we should remove this condition `ifeq
($(abs_srctree)/,$(dir $(abs_objtree)))` but it'll affect other targets
as well.
Complete details of investigation can be found here:
https://github.com/kernelci/kernelci-project/issues/92#issuecomment-1087406222
On 3/17/22 11:08 PM, Masahiro Yamada wrote:
> On Thu, Mar 17, 2022 at 7:49 PM Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>>
>> Reminder. Shuah is okay with this patch. Any thoughts?
>
> I do not think this is the right fix,
> but something you just happen to find working.
>
>
> The Make is working in a wrong directory, that is why
> the relative path does not work
> (and you use the absolute path to work around it)
>
`ifeq ($(abs_srctree)/,$(dir $(abs_objtree))) \ srctree := ..` has
broken the `make headers_install` when called through
selftests/Makefile. We can remove it or use the absolute path each time.
Hi, At this time: make kselftest-all (works) make kselftest-all O=/tmp (works) make kselftest-all O=build (doesn't work, investigation was shared) make kselftest-all O=build/build2 (works) I'd shared my final thoughts in the last email. I don't see any other solution. If anybody has any other thoughts on how to cleanly fix `make kselftest-all O=build`, do share. Thanks, Usama On 4/4/22 4:09 PM, Muhammad Usama Anjum wrote: > From [Makefile](https://elixir.bootlin.com/linux/latest/source/Makefile): > ``` > ifeq ($(abs_srctree),$(abs_objtree)) > # building in the source tree > srctree := . > building_out_of_srctree := > else > ifeq ($(abs_srctree)/,$(dir $(abs_objtree))) > # building in a subdirectory of the source tree > srctree := .. > else > srctree := $(abs_srctree) > endif > building_out_of_srctree := 1 > endif > ``` > `ifeq ($(abs_srctree)/,$(dir $(abs_objtree)))` condition is setting > `srctree` to `..`. This is wrong. This condition isn't considering that > `header_install` doesn't depend on `abs_srctree and abs_objtree`. This > condition needs to be tweaked or removed for the `install_headers` to > work fine and fix this issue. I've added `KBUILD_ABS_SRCTREE=1` to the > kselftest target which sets the `srctree` to `abs_srctree` and thus > forcefully affecting only kselftest targets. This seems like the clean > fix. Alternatively we should remove this condition `ifeq > ($(abs_srctree)/,$(dir $(abs_objtree)))` but it'll affect other targets > as well. > > Complete details of investigation can be found here: > https://github.com/kernelci/kernelci-project/issues/92#issuecomment-1087406222 > > On 3/17/22 11:08 PM, Masahiro Yamada wrote: >> On Thu, Mar 17, 2022 at 7:49 PM Muhammad Usama Anjum >> <usama.anjum@collabora.com> wrote: >>> >>> Reminder. Shuah is okay with this patch. Any thoughts? >> >> I do not think this is the right fix, >> but something you just happen to find working. >> >> >> The Make is working in a wrong directory, that is why >> the relative path does not work >> (and you use the absolute path to work around it) >> > `ifeq ($(abs_srctree)/,$(dir $(abs_objtree))) \ srctree := ..` has > broken the `make headers_install` when called through > selftests/Makefile. We can remove it or use the absolute path each time. >
diff --git a/Makefile b/Makefile index 86f633c2809ea..62b3eb8a102ab 100644 --- a/Makefile +++ b/Makefile @@ -1411,10 +1411,10 @@ tools/%: FORCE PHONY += kselftest kselftest: - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 run_tests kselftest-%: FORCE - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $* + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests KBUILD_ABS_SRCTREE=1 $* PHONY += kselftest-merge kselftest-merge:
Build of kselftests fail if kernel's top most Makefile is used for running or building kselftests with separate output directory. The absolute path is needed to reference other files during this kind of build. Set KBUILD_ABS_SRCTREE to use absolute path during the build. It fixes the following different types of errors: make kselftest-all O=/linux_mainline/build Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory make kselftest-all O=build Makefile:1080: ../scripts/Makefile.extrawarn: No such file or directory Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- I've tested this patch on top of next-20220217. The latest next-20220222 have missing patches. --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)