diff mbox series

[1/2] selftests: cgroup: build error multiple outpt files

Message ID 20211105162530.3307666-1-anders.roxell@linaro.org (mailing list archive)
State New
Headers show
Series [1/2] selftests: cgroup: build error multiple outpt files | expand

Commit Message

Anders Roxell Nov. 5, 2021, 4:25 p.m. UTC
When building selftests/cgroup: with clang the following error are seen:

clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
clang: error: cannot specify -o when generating multiple output files
make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1

Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
file lib.mk.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
 tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
 tools/testing/selftests/lib.mk          |  2 +-
 2 files changed, 8 insertions(+), 6 deletions(-)

Comments

Nick Desaulniers Nov. 5, 2021, 8:10 p.m. UTC | #1
On Fri, Nov 5, 2021 at 9:25 AM Anders Roxell <anders.roxell@linaro.org> wrote:
>
> When building selftests/cgroup: with clang the following error are seen:

Thanks for the patches!

typo in subject/oneline, and the `:` above can be dropped.

Andrew reported similar failures throughout selftests:
https://lore.kernel.org/linux-kselftest/20211005222739.2491124-1-adelg@google.com/

Both patches touch the same part of tools/testing/selftests/lib.mk.
This approach looks cleaner to me, but it should reconcile the changes
to tools/testing/selftests/filesystems/binderfs/Makefile that Andrew
made in the link above.

>
> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> clang: error: cannot specify -o when generating multiple output files
> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
>
> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> file lib.mk.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>  tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
>  tools/testing/selftests/lib.mk          |  2 +-
>  2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> index 59e222460581..745fe25fa0b9 100644
> --- a/tools/testing/selftests/cgroup/Makefile
> +++ b/tools/testing/selftests/cgroup/Makefile
> @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
>  TEST_GEN_PROGS += test_freezer
>  TEST_GEN_PROGS += test_kill
>
> +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> +
>  include ../lib.mk
>
> -$(OUTPUT)/test_memcontrol: cgroup_util.c ../clone3/clone3_selftests.h
> -$(OUTPUT)/test_kmem: cgroup_util.c ../clone3/clone3_selftests.h
> -$(OUTPUT)/test_core: cgroup_util.c ../clone3/clone3_selftests.h
> -$(OUTPUT)/test_freezer: cgroup_util.c ../clone3/clone3_selftests.h
> -$(OUTPUT)/test_kill: cgroup_util.c ../clone3/clone3_selftests.h ../pidfd/pidfd.h
> +$(OUTPUT)/test_memcontrol: cgroup_util.c
> +$(OUTPUT)/test_kmem: cgroup_util.c
> +$(OUTPUT)/test_core: cgroup_util.c
> +$(OUTPUT)/test_freezer: cgroup_util.c
> +$(OUTPUT)/test_kill: cgroup_util.c
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index fe7ee2b0f29c..a40add31a2e3 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -141,7 +141,7 @@ endif
>  # Selftest makefiles can override those targets by setting
>  # OVERRIDE_TARGETS = 1.
>  ifeq ($(OVERRIDE_TARGETS),)
> -LOCAL_HDRS := $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
> +LOCAL_HDRS += $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
>  $(OUTPUT)/%:%.c $(LOCAL_HDRS)
>         $(LINK.c) $(filter-out $(LOCAL_HDRS),$^) $(LDLIBS) -o $@
>
> --
> 2.33.0
>
Christian Brauner Nov. 9, 2021, 3:01 p.m. UTC | #2
On Fri, Nov 05, 2021 at 05:25:29PM +0100, Anders Roxell wrote:
> When building selftests/cgroup: with clang the following error are seen:
> 
> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> clang: error: cannot specify -o when generating multiple output files
> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
> 
> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> file lib.mk.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---

Thanks!
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Shuah Khan Nov. 20, 2021, 12:22 a.m. UTC | #3
On 11/5/21 10:25 AM, Anders Roxell wrote:
> When building selftests/cgroup: with clang the following error are seen:
> 
> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> clang: error: cannot specify -o when generating multiple output files
> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
> 
> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> file lib.mk.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
>   tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
>   tools/testing/selftests/lib.mk          |  2 +-
>   2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> index 59e222460581..745fe25fa0b9 100644
> --- a/tools/testing/selftests/cgroup/Makefile
> +++ b/tools/testing/selftests/cgroup/Makefile
> @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
>   TEST_GEN_PROGS += test_freezer
>   TEST_GEN_PROGS += test_kill
>   
> +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> +

This looks odd to me. Why are we introducing dependencies between tests?
clone3 includes in cgroup? Looks odd to me.

thanks,
-- Shuah
Christian Brauner Nov. 23, 2021, 2:26 p.m. UTC | #4
On Fri, Nov 19, 2021 at 05:22:20PM -0700, Shuah Khan wrote:
> On 11/5/21 10:25 AM, Anders Roxell wrote:
> > When building selftests/cgroup: with clang the following error are seen:
> > 
> > clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> > clang: error: cannot specify -o when generating multiple output files
> > make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
> > 
> > Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> > since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> > file lib.mk.
> > 
> > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > ---
> >   tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
> >   tools/testing/selftests/lib.mk          |  2 +-
> >   2 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> > index 59e222460581..745fe25fa0b9 100644
> > --- a/tools/testing/selftests/cgroup/Makefile
> > +++ b/tools/testing/selftests/cgroup/Makefile
> > @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
> >   TEST_GEN_PROGS += test_freezer
> >   TEST_GEN_PROGS += test_kill
> > +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> > +
> 
> This looks odd to me. Why are we introducing dependencies between tests?
> clone3 includes in cgroup? Looks odd to me.

The cgroup tests need access to clone3() functionality in order to test
CLONE_INTO_CGROUP which is more suited to be placed alongside the cgroup
tests. There are a few other tests that include the clone3 header.
Shuah Khan Nov. 30, 2021, 4:41 p.m. UTC | #5
On 11/23/21 7:26 AM, Christian Brauner wrote:
> On Fri, Nov 19, 2021 at 05:22:20PM -0700, Shuah Khan wrote:
>> On 11/5/21 10:25 AM, Anders Roxell wrote:
>>> When building selftests/cgroup: with clang the following error are seen:
>>>
>>> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
>>> clang: error: cannot specify -o when generating multiple output files
>>> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
>>>
>>> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
>>> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
>>> file lib.mk.
>>>
>>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>>> ---
>>>    tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
>>>    tools/testing/selftests/lib.mk          |  2 +-
>>>    2 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
>>> index 59e222460581..745fe25fa0b9 100644
>>> --- a/tools/testing/selftests/cgroup/Makefile
>>> +++ b/tools/testing/selftests/cgroup/Makefile
>>> @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
>>>    TEST_GEN_PROGS += test_freezer
>>>    TEST_GEN_PROGS += test_kill
>>> +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
>>> +
>>
>> This looks odd to me. Why are we introducing dependencies between tests?
>> clone3 includes in cgroup? Looks odd to me.
> 
> The cgroup tests need access to clone3() functionality in order to test
> CLONE_INTO_CGROUP which is more suited to be placed alongside the cgroup
> tests. There are a few other tests that include the clone3 header.
> 

If other tests are also including this header, we could move it up under
selftests level. Might have to add include directory.

thanks,
-- Shuah
Christian Brauner Dec. 1, 2021, 1:06 p.m. UTC | #6
On Tue, Nov 30, 2021 at 09:41:49AM -0700, Shuah Khan wrote:
> On 11/23/21 7:26 AM, Christian Brauner wrote:
> > On Fri, Nov 19, 2021 at 05:22:20PM -0700, Shuah Khan wrote:
> > > On 11/5/21 10:25 AM, Anders Roxell wrote:
> > > > When building selftests/cgroup: with clang the following error are seen:
> > > > 
> > > > clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
> > > > clang: error: cannot specify -o when generating multiple output files
> > > > make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
> > > > 
> > > > Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
> > > > since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
> > > > file lib.mk.
> > > > 
> > > > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> > > > ---
> > > >    tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
> > > >    tools/testing/selftests/lib.mk          |  2 +-
> > > >    2 files changed, 8 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
> > > > index 59e222460581..745fe25fa0b9 100644
> > > > --- a/tools/testing/selftests/cgroup/Makefile
> > > > +++ b/tools/testing/selftests/cgroup/Makefile
> > > > @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
> > > >    TEST_GEN_PROGS += test_freezer
> > > >    TEST_GEN_PROGS += test_kill
> > > > +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
> > > > +
> > > 
> > > This looks odd to me. Why are we introducing dependencies between tests?
> > > clone3 includes in cgroup? Looks odd to me.
> > 
> > The cgroup tests need access to clone3() functionality in order to test
> > CLONE_INTO_CGROUP which is more suited to be placed alongside the cgroup
> > tests. There are a few other tests that include the clone3 header.
> > 
> 
> If other tests are also including this header, we could move it up under
> selftests level. Might have to add include directory.

No objection from me if that's useful. I won't have time for that in the
near future. (This might be of interest for one of the LF programs that
help get new folks interested in kernel development started.)

Christian
Shuah Khan Dec. 3, 2021, 5:08 p.m. UTC | #7
On 12/1/21 6:06 AM, Christian Brauner wrote:
> On Tue, Nov 30, 2021 at 09:41:49AM -0700, Shuah Khan wrote:
>> On 11/23/21 7:26 AM, Christian Brauner wrote:
>>> On Fri, Nov 19, 2021 at 05:22:20PM -0700, Shuah Khan wrote:
>>>> On 11/5/21 10:25 AM, Anders Roxell wrote:
>>>>> When building selftests/cgroup: with clang the following error are seen:
>>>>>
>>>>> clang -Wall -pthread    test_memcontrol.c cgroup_util.c ../clone3/clone3_selftests.h  -o /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol
>>>>> clang: error: cannot specify -o when generating multiple output files
>>>>> make[3]: *** [../lib.mk:146: /home/anders/.cache/tuxmake/builds/current/kselftest/cgroup/test_memcontrol] Error 1
>>>>>
>>>>> Rework to add the header files to LOCAL_HDRS before including ../lib.mk,
>>>>> since the dependency is evaluated in '$(OUTPUT)/%:%.c $(LOCAL_HDRS)' in
>>>>> file lib.mk.
>>>>>
>>>>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>>>>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
>>>>> ---
>>>>>     tools/testing/selftests/cgroup/Makefile | 12 +++++++-----
>>>>>     tools/testing/selftests/lib.mk          |  2 +-
>>>>>     2 files changed, 8 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
>>>>> index 59e222460581..745fe25fa0b9 100644
>>>>> --- a/tools/testing/selftests/cgroup/Makefile
>>>>> +++ b/tools/testing/selftests/cgroup/Makefile
>>>>> @@ -11,10 +11,12 @@ TEST_GEN_PROGS += test_core
>>>>>     TEST_GEN_PROGS += test_freezer
>>>>>     TEST_GEN_PROGS += test_kill
>>>>> +LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
>>>>> +
>>>>
>>>> This looks odd to me. Why are we introducing dependencies between tests?
>>>> clone3 includes in cgroup? Looks odd to me.
>>>
>>> The cgroup tests need access to clone3() functionality in order to test
>>> CLONE_INTO_CGROUP which is more suited to be placed alongside the cgroup
>>> tests. There are a few other tests that include the clone3 header.
>>>
>>
>> If other tests are also including this header, we could move it up under
>> selftests level. Might have to add include directory.
> 
> No objection from me if that's useful. I won't have time for that in the
> near future. (This might be of interest for one of the LF programs that
> help get new folks interested in kernel development started.)
> 
> Christian
> 

Thanks. I will apply this fix for now.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index 59e222460581..745fe25fa0b9 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -11,10 +11,12 @@  TEST_GEN_PROGS += test_core
 TEST_GEN_PROGS += test_freezer
 TEST_GEN_PROGS += test_kill
 
+LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
+
 include ../lib.mk
 
-$(OUTPUT)/test_memcontrol: cgroup_util.c ../clone3/clone3_selftests.h
-$(OUTPUT)/test_kmem: cgroup_util.c ../clone3/clone3_selftests.h
-$(OUTPUT)/test_core: cgroup_util.c ../clone3/clone3_selftests.h
-$(OUTPUT)/test_freezer: cgroup_util.c ../clone3/clone3_selftests.h
-$(OUTPUT)/test_kill: cgroup_util.c ../clone3/clone3_selftests.h ../pidfd/pidfd.h
+$(OUTPUT)/test_memcontrol: cgroup_util.c
+$(OUTPUT)/test_kmem: cgroup_util.c
+$(OUTPUT)/test_core: cgroup_util.c
+$(OUTPUT)/test_freezer: cgroup_util.c
+$(OUTPUT)/test_kill: cgroup_util.c
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index fe7ee2b0f29c..a40add31a2e3 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -141,7 +141,7 @@  endif
 # Selftest makefiles can override those targets by setting
 # OVERRIDE_TARGETS = 1.
 ifeq ($(OVERRIDE_TARGETS),)
-LOCAL_HDRS := $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
+LOCAL_HDRS += $(selfdir)/kselftest_harness.h $(selfdir)/kselftest.h
 $(OUTPUT)/%:%.c $(LOCAL_HDRS)
 	$(LINK.c) $(filter-out $(LOCAL_HDRS),$^) $(LDLIBS) -o $@