[PATCH] Makefile: Fix separate output directory build of kselftests

[PATCH] Makefile: Fix separate output directory build of kselftests

[Muhammad Usama Anjum]

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:
-- 
2.30.2

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Muhammad Usama Anjum]

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:

-- 
--
Muhammad Usama Anjum

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Shuah Khan]

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

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Muhammad Usama Anjum]

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.

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Shuah Khan]

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

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Muhammad Usama Anjum]

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

-- 
Muhammad Usama Anjum

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Muhammad Usama Anjum]

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:

-- 
Muhammad Usama Anjum

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Masahiro Yamada]

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

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Muhammad Usama Anjum]

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.

-- 
Muhammad Usama Anjum

Re: [PATCH] Makefile: Fix separate output directory build of kselftests

[Muhammad Usama Anjum]

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.
> 

-- 
Muhammad Usama Anjum