diff mbox

[1/2] kbuild: create built-in.o automatically if parent directory wants it

Message ID 1510072307-16819-2-git-send-email-yamada.masahiro@socionext.com (mailing list archive)
State New, archived
Headers show

Commit Message

Masahiro Yamada Nov. 7, 2017, 4:31 p.m. UTC
"obj-y += foo/" syntax requires Kbuild to visit the "foo" subdirectory
and link built-in.o from that directory.  This means foo/Makefile is
responsible for creating built-in.o even if there is no object to
link (in this case, built-in.o is an empty archive).

We have had several fixups like commit 4b024242e8a4 ("kbuild: Fix
linking error built-in.o no such file or directory"), then ended up
with a complex condition as follows:

  ifneq ($(strip $(obj-y) $(obj-m) $(obj-) $(subdir-m) $(lib-target)),)
  builtin-target := $(obj)/built-in.o
  endif

We still have more cases not covered by the above, so we need to add
  obj- := dummy.o
in several places just for creating empty built-in.o.

A key point is, the parent Makefile knows whether built-in.o is needed
or not.  If a subdirectory needs to create built-in.o, its parent can
tell the fact when Kbuild descends into it.

If non-empty $(need-builtin) flag is passed from the parent, built-in.o
should be created.  $(obj-y) should be still checked to support the
single target "%/".  All of ugly tricks will go away.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Makefile               | 2 +-
 scripts/Makefile.build | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Sam Ravnborg Nov. 9, 2017, 5:35 a.m. UTC | #1
Hi Masahiro.

Thanks for picking this up.

> A key point is, the parent Makefile knows whether built-in.o is needed
> or not.  If a subdirectory needs to create built-in.o, its parent can
> tell the fact when Kbuild descends into it.
Good observation!
> 
> diff --git a/Makefile b/Makefile
> index 008a4e5..cc0b618 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1003,7 +1003,7 @@ $(sort $(vmlinux-deps)): $(vmlinux-dirs) ;
>  
>  PHONY += $(vmlinux-dirs)
>  $(vmlinux-dirs): prepare scripts
> -	$(Q)$(MAKE) $(build)=$@
> +	$(Q)$(MAKE) $(build)=$@ need-builtin=1

The need-bultin may also be required for the shortcuts
that allows one to use:

	make <dir>/

    example:

	make net/

And maybe selftest, documentation shortcuts too?
Other than that - looks good.

Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Masahiro Yamada Nov. 9, 2017, 5:53 a.m. UTC | #2
Hi Sam,

Thanks for your review.

2017-11-09 14:35 GMT+09:00 Sam Ravnborg <sam@ravnborg.org>:
> Hi Masahiro.
>
> Thanks for picking this up.
>
>> A key point is, the parent Makefile knows whether built-in.o is needed
>> or not.  If a subdirectory needs to create built-in.o, its parent can
>> tell the fact when Kbuild descends into it.
> Good observation!
>>
>> diff --git a/Makefile b/Makefile
>> index 008a4e5..cc0b618 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1003,7 +1003,7 @@ $(sort $(vmlinux-deps)): $(vmlinux-dirs) ;
>>
>>  PHONY += $(vmlinux-dirs)
>>  $(vmlinux-dirs): prepare scripts
>> -     $(Q)$(MAKE) $(build)=$@
>> +     $(Q)$(MAKE) $(build)=$@ need-builtin=1
>
> The need-bultin may also be required for the shortcuts
> that allows one to use:
>
>         make <dir>/
>
>     example:
>
>         make net/


I do not want to add need-builtin=1 for single targets.


make scripts/
would create false scripts/built-in.o
This is odd.

I wrote the solution in the commit log:
  $(obj-y) should be still checked to support the single target "%/".


If net/Makefile contains at least one obj-y,
"make net/" will create built-in.o





> And maybe selftest, documentation shortcuts too?
> Other than that - looks good.
>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>
>         Sam
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg Nov. 9, 2017, 4:08 p.m. UTC | #3
Hi Masahiro.

On Thu, Nov 09, 2017 at 02:53:04PM +0900, Masahiro Yamada wrote:
> Hi Sam,
> 
> Thanks for your review.
> 
> 2017-11-09 14:35 GMT+09:00 Sam Ravnborg <sam@ravnborg.org>:
> > Hi Masahiro.
> >
> > Thanks for picking this up.
> >
> >> A key point is, the parent Makefile knows whether built-in.o is needed
> >> or not.  If a subdirectory needs to create built-in.o, its parent can
> >> tell the fact when Kbuild descends into it.
> > Good observation!
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 008a4e5..cc0b618 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -1003,7 +1003,7 @@ $(sort $(vmlinux-deps)): $(vmlinux-dirs) ;
> >>
> >>  PHONY += $(vmlinux-dirs)
> >>  $(vmlinux-dirs): prepare scripts
> >> -     $(Q)$(MAKE) $(build)=$@
> >> +     $(Q)$(MAKE) $(build)=$@ need-builtin=1
> >
> > The need-bultin may also be required for the shortcuts
> > that allows one to use:
> >
> >         make <dir>/
> >
> >     example:
> >
> >         make net/
> 
> 
> I do not want to add need-builtin=1 for single targets.
> 
> 
> make scripts/
> would create false scripts/built-in.o
> This is odd.
> 
> I wrote the solution in the commit log:
>   $(obj-y) should be still checked to support the single target "%/".

I missed this bit in the commit log,
but I almost commented on this in the patch itself.
Good way to do it.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Masahiro Yamada Nov. 18, 2017, 4:06 a.m. UTC | #4
2017-11-08 1:31 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> "obj-y += foo/" syntax requires Kbuild to visit the "foo" subdirectory
> and link built-in.o from that directory.  This means foo/Makefile is
> responsible for creating built-in.o even if there is no object to
> link (in this case, built-in.o is an empty archive).
>
> We have had several fixups like commit 4b024242e8a4 ("kbuild: Fix
> linking error built-in.o no such file or directory"), then ended up
> with a complex condition as follows:
>
>   ifneq ($(strip $(obj-y) $(obj-m) $(obj-) $(subdir-m) $(lib-target)),)
>   builtin-target := $(obj)/built-in.o
>   endif
>
> We still have more cases not covered by the above, so we need to add
>   obj- := dummy.o
> in several places just for creating empty built-in.o.
>
> A key point is, the parent Makefile knows whether built-in.o is needed
> or not.  If a subdirectory needs to create built-in.o, its parent can
> tell the fact when Kbuild descends into it.
>
> If non-empty $(need-builtin) flag is passed from the parent, built-in.o
> should be created.  $(obj-y) should be still checked to support the
> single target "%/".  All of ugly tricks will go away.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>

Applied to linux-kbuild/kbuild.
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 008a4e5..cc0b618 100644
--- a/Makefile
+++ b/Makefile
@@ -1003,7 +1003,7 @@  $(sort $(vmlinux-deps)): $(vmlinux-dirs) ;
 
 PHONY += $(vmlinux-dirs)
 $(vmlinux-dirs): prepare scripts
-	$(Q)$(MAKE) $(build)=$@
+	$(Q)$(MAKE) $(build)=$@ need-builtin=1
 
 define filechk_kernel.release
 	echo "$(KERNELVERSION)$$($(CONFIG_SHELL) $(srctree)/scripts/setlocalversion $(srctree))"
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 061d0c3..e1c6efd 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -84,7 +84,7 @@  lib-target := $(obj)/lib.a
 obj-y += $(obj)/lib-ksyms.o
 endif
 
-ifneq ($(strip $(obj-y) $(obj-m) $(obj-) $(subdir-m) $(lib-target)),)
+ifneq ($(strip $(obj-y) $(need-builtin)),)
 builtin-target := $(obj)/built-in.o
 endif
 
@@ -569,7 +569,7 @@  targets += $(multi-used-y) $(multi-used-m)
 
 PHONY += $(subdir-ym)
 $(subdir-ym):
-	$(Q)$(MAKE) $(build)=$@
+	$(Q)$(MAKE) $(build)=$@ need-builtin=$(if $(findstring $@,$(subdir-obj-y)),1)
 
 # Add FORCE to the prequisites of a target to force it to be always rebuilt.
 # ---------------------------------------------------------------------------