[PATCH] Makefile: add comment to discourage tools/* addition for kernel builds

[PATCH] Makefile: add comment to discourage tools/* addition for kernel builds

[Masahiro Yamada]

Kbuild provides scripts/Makefile.host to build host programs used for
building the kernel. Unfortunately, there are two exceptions that opt
out of Kbuild. The build system under tools/ is a cheesy replica, and
is always a disaster. I was recently poked about a problem in the tools
build issue, which I do not maintain (and nobody maintains). [1]

Without a comment, somebody might believe this is the right location
because that is where objtool lives, even when a more robust Kbuild
syntax satisfies their needs. [2]

[1]: https://lore.kernel.org/linux-kbuild/ZnIYWBgrJ-IJtqK8@google.com/T/#m8ece130dd0e23c6f2395ed89070161948dee8457
[2]: https://lore.kernel.org/all/20240618200501.GA1611012@google.com/

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index 471f2df86422..ba070596ad4e 100644
--- a/Makefile
+++ b/Makefile
@@ -1331,6 +1331,11 @@ prepare: tools/bpf/resolve_btfids
 endif
 endif
 
+# README
+# The tools build system is not a part of Kbuild. Before adding yet another
+# tools/* here, please consider if the standard "hostprogs" syntax satisfies
+# your needs.
+
 PHONY += resolve_btfids_clean
 
 resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
-- 
2.43.0

Re: [PATCH] Makefile: add comment to discourage tools/* addition for kernel builds

[Nicolas Schier]

On Wed, Jun 19, 2024 at 03:21:42PM +0900 Masahiro Yamada wrote:
> Kbuild provides scripts/Makefile.host to build host programs used for
> building the kernel. Unfortunately, there are two exceptions that opt
> out of Kbuild. The build system under tools/ is a cheesy replica, and
> is always a disaster. I was recently poked about a problem in the tools
> build issue, which I do not maintain (and nobody maintains). [1]
> 
> Without a comment, somebody might believe this is the right location
> because that is where objtool lives, even when a more robust Kbuild
> syntax satisfies their needs. [2]
> 
> [1]: https://lore.kernel.org/linux-kbuild/ZnIYWBgrJ-IJtqK8@google.com/T/#m8ece130dd0e23c6f2395ed89070161948dee8457
> [2]: https://lore.kernel.org/all/20240618200501.GA1611012@google.com/
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 471f2df86422..ba070596ad4e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1331,6 +1331,11 @@ prepare: tools/bpf/resolve_btfids
>  endif
>  endif
>  
> +# README
> +# The tools build system is not a part of Kbuild. Before adding yet another
> +# tools/* here, please consider if the standard "hostprogs" syntax satisfies
> +# your needs.
> +

Perhaps add a "See Documentation/kbuild/makefiles.rst for details." ?

I do understand the need for clarification.

Acked-by: Nicolas Schier <nicolas@fjasle.eu>

Re: [PATCH] Makefile: add comment to discourage tools/* addition for kernel builds

[Brian Norris]

On Wed, Jun 19, 2024 at 03:21:42PM +0900, Masahiro Yamada wrote:
> Kbuild provides scripts/Makefile.host to build host programs used for
> building the kernel. Unfortunately, there are two exceptions that opt
> out of Kbuild. The build system under tools/ is a cheesy replica, and
> is always a disaster. I was recently poked about a problem in the tools
> build issue, which I do not maintain (and nobody maintains). [1]

(Side note: I hope I haven't placed undue burden on you; I understood
you don't maintain tools/ and that it didn't use Kbuild. I only "poked"
you because the original bug report I was replying to had you and
linux-kbuild on CC already. And I appreciate your engagement, even if
the bugs are due to intentional forking.)

But anyway, I agree that clearer documentation and recommendations could
be helpful here. To that end, some dumb questions below, as I'm not sure
if this fully serves its purpose as-is:

> Without a comment, somebody might believe this is the right location
> because that is where objtool lives, even when a more robust Kbuild
> syntax satisfies their needs. [2]
> 
> [1]: https://lore.kernel.org/linux-kbuild/ZnIYWBgrJ-IJtqK8@google.com/T/#m8ece130dd0e23c6f2395ed89070161948dee8457
> [2]: https://lore.kernel.org/all/20240618200501.GA1611012@google.com/
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  Makefile | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 471f2df86422..ba070596ad4e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1331,6 +1331,11 @@ prepare: tools/bpf/resolve_btfids
>  endif
>  endif
>  
> +# README
> +# The tools build system is not a part of Kbuild. Before adding yet another
> +# tools/* here, please consider if the standard "hostprogs" syntax satisfies
> +# your needs.
> +

Some clarifying questions / statements-as-questions:

* nothing in tools/ uses Kbuild, right? (even stuff that uses KBUILD_*
  names is just an imitative port, right?)
* not everything in tools/ is actually promoted to a high-level target,
  that affects this top-level Makefile. Are you only concerned about
  stuff that pretends to be integrated in the top-level kernel Makefile?
  (If not, then it seems like placing the README comments only in this
  Makefile is a poor choice.)
* is the "standard hostprogs" recommendation a general recommendation,
  for all sorts of kept-in-the-kernel-tree host tools? Is the
  recommendation to "use Kbuild" or to "avoid putting your tool in
  tools/*"? Is it possible (recommended?) to plumb Kbuild stuff into
  tools/, even if other parts won't migrate?

As is, I can't tell if this is telling people to avoid adding new stuff
to tools/ entirely, or just to only add to tools/ if you're able to
remain completely isolated from the rest of the kernel build -- as soon
as you want to play some part in the Kbuild-covered part of the tree,
you need to use Kbuild.

If I'm inferring the right answers to the above, then maybe an improved
wording could be something like:

"The tools build system is not a part of Kbuild and tends to introduce
its own unique issues. If you need to integrate a new tool into Kbuild,
please consider locating that tool outside the tools/ tree and using the
standard Kbuild "hostprogs" syntax instead of adding a new tools/* entry
here."

It's possible I'm playing mental acrobatics here in my reading too.

Either way, I think this is a good trajectory:

Reviewed-by: Brian Norris <briannorris@chromium.org>

Regards,
Brian

>  PHONY += resolve_btfids_clean
>  
>  resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
> -- 
> 2.43.0
> 

Re: [PATCH] Makefile: add comment to discourage tools/* addition for kernel builds

[Masahiro Yamada]

On Fri, Jun 21, 2024 at 6:52 AM Brian Norris <briannorris@chromium.org> wrote:
>
> On Wed, Jun 19, 2024 at 03:21:42PM +0900, Masahiro Yamada wrote:
> > Kbuild provides scripts/Makefile.host to build host programs used for
> > building the kernel. Unfortunately, there are two exceptions that opt
> > out of Kbuild. The build system under tools/ is a cheesy replica, and
> > is always a disaster. I was recently poked about a problem in the tools
> > build issue, which I do not maintain (and nobody maintains). [1]
>
> (Side note: I hope I haven't placed undue burden on you; I understood
> you don't maintain tools/ and that it didn't use Kbuild. I only "poked"
> you because the original bug report I was replying to had you and
> linux-kbuild on CC already. And I appreciate your engagement, even if
> the bugs are due to intentional forking.)


I did not mean to express my complaint particularly with the previous thread.

It is not the first time that the tools/ build issue arose.


I will drop the references to the threads.



> But anyway, I agree that clearer documentation and recommendations could
> be helpful here. To that end, some dumb questions below, as I'm not sure
> if this fully serves its purpose as-is:
>
> > Without a comment, somebody might believe this is the right location
> > because that is where objtool lives, even when a more robust Kbuild
> > syntax satisfies their needs. [2]
> >
> > [1]: https://lore.kernel.org/linux-kbuild/ZnIYWBgrJ-IJtqK8@google.com/T/#m8ece130dd0e23c6f2395ed89070161948dee8457
> > [2]: https://lore.kernel.org/all/20240618200501.GA1611012@google.com/
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  Makefile | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 471f2df86422..ba070596ad4e 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1331,6 +1331,11 @@ prepare: tools/bpf/resolve_btfids
> >  endif
> >  endif
> >
> > +# README
> > +# The tools build system is not a part of Kbuild. Before adding yet another
> > +# tools/* here, please consider if the standard "hostprogs" syntax satisfies
> > +# your needs.
> > +
>
> Some clarifying questions / statements-as-questions:
>
> * nothing in tools/ uses Kbuild, right? (even stuff that uses KBUILD_*
>   names is just an imitative port, right?)


Correct.

You can build a tool from multiple directory locations.

For example, you can compile the 'perf' in multiple locations.


[1] From the top of the kernel tree

   $ make tools/perf


[2] From the tools/ directory

   $ cd tools
   $ make perf


[3] From the tools/perf/ directory

   $ cd tools/perf
   $ make



When you do [2] or [3], the top-level Makefile is not parsed.

If necessary, the tools build system copies code from Kbuild.




> * not everything in tools/ is actually promoted to a high-level target,
>   that affects this top-level Makefile. Are you only concerned about
>   stuff that pretends to be integrated in the top-level kernel Makefile?
>   (If not, then it seems like placing the README comments only in this
>   Makefile is a poor choice.)


The tool build is integrated as a pattern rule in the top Makefile.
(tools/%)


So, you can build other tools from the top Makefile.


See commit ea01fa9f63aef, which did not get Ack from any Kbuild
maintainer, and caused subsequent troubles, and the benefit
of which I still do not understand.


Supporting "make tools/perf" in addition to "make -C tools perf"
only saved a few characters to type.


So, the problem remains, unless I revert ea01fa9f63aef.

I decided to not care about it too much, as long as
such tools are not used during the kernel build.

I am really worried about objtool and resolve_btfids,
as these two are used for building the kernel.






> * is the "standard hostprogs" recommendation a general recommendation,
>   for all sorts of kept-in-the-kernel-tree host tools? Is the
>   recommendation to "use Kbuild" or to "avoid putting your tool in
>   tools/*"? Is it possible (recommended?) to plumb Kbuild stuff into
>   tools/, even if other parts won't migrate?


I do not know.

They are different build systems with different designs.

Kbuild always works in the top of the output directory.
Kbuild changes the working directory at most once if O= is given,
but otherwise, it never changes the working directory during the build.


The tools/ build system changes the working directory every time
it invokes a new Make, and compiles the tool in its source directory.


I do not know if all tools want to Kbuild.
(the same applied to kselftest)

I think I can convert objtool and resolve_btfids to the Kbuild way.


>
> As is, I can't tell if this is telling people to avoid adding new stuff
> to tools/ entirely, or just to only add to tools/ if you're able to
> remain completely isolated from the rest of the kernel build -- as soon
> as you want to play some part in the Kbuild-covered part of the tree,
> you need to use Kbuild.


See the code in the top Makefile.

'prepare' depends on tools/objtool and tools/bpf/resolve_btfids.

If other tools are not prerequisites of 'scripts',
Kbuild will not compile them.




>
> If I'm inferring the right answers to the above, then maybe an improved
> wording could be something like:
>
> "The tools build system is not a part of Kbuild and tends to introduce
> its own unique issues. If you need to integrate a new tool into Kbuild,
> please consider locating that tool outside the tools/ tree and using the
> standard Kbuild "hostprogs" syntax instead of adding a new tools/* entry
> here."



I am fine with this description.


Nicolas suggested a link to Documentation/kbuild/makefiles.rst

We can combine the two.


# The tools build system is not a part of Kbuild and tends to introduce
# its own unique issues. If you need to integrate a new tool into Kbuild,
# please consider locating that tool outside the tools/ tree and using the
# standard Kbuild "hostprogs" syntax instead of adding a new tools/* entry
# here. See Documentation/kbuild/makefiles.rst for details.




> It's possible I'm playing mental acrobatics here in my reading too.
>
> Either way, I think this is a good trajectory:
>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
>
> Regards,
> Brian
>
> >  PHONY += resolve_btfids_clean
> >
> >  resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
> > --
> > 2.43.0
> >



--
Best Regards


Masahiro Yamada

Re: [PATCH] Makefile: add comment to discourage tools/* addition for kernel builds

[Nicolas Schier]

On Thu, Jun 27, 2024 at 04:02:46AM +0900, Masahiro Yamada wrote:
> On Fri, Jun 21, 2024 at 6:52 AM Brian Norris <briannorris@chromium.org> wrote:
> >
> > On Wed, Jun 19, 2024 at 03:21:42PM +0900, Masahiro Yamada wrote:
> > > Kbuild provides scripts/Makefile.host to build host programs used for
> > > building the kernel. Unfortunately, there are two exceptions that opt
> > > out of Kbuild. The build system under tools/ is a cheesy replica, and
> > > is always a disaster. I was recently poked about a problem in the tools
> > > build issue, which I do not maintain (and nobody maintains). [1]
> >
> > (Side note: I hope I haven't placed undue burden on you; I understood
> > you don't maintain tools/ and that it didn't use Kbuild. I only "poked"
> > you because the original bug report I was replying to had you and
> > linux-kbuild on CC already. And I appreciate your engagement, even if
> > the bugs are due to intentional forking.)
> 
> 
> I did not mean to express my complaint particularly with the previous thread.
> 
> It is not the first time that the tools/ build issue arose.
> 
> 
> I will drop the references to the threads.
> 
> 
> 
> > But anyway, I agree that clearer documentation and recommendations could
> > be helpful here. To that end, some dumb questions below, as I'm not sure
> > if this fully serves its purpose as-is:
> >
> > > Without a comment, somebody might believe this is the right location
> > > because that is where objtool lives, even when a more robust Kbuild
> > > syntax satisfies their needs. [2]
> > >
> > > [1]: https://lore.kernel.org/linux-kbuild/ZnIYWBgrJ-IJtqK8@google.com/T/#m8ece130dd0e23c6f2395ed89070161948dee8457
> > > [2]: https://lore.kernel.org/all/20240618200501.GA1611012@google.com/
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > >  Makefile | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 471f2df86422..ba070596ad4e 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1331,6 +1331,11 @@ prepare: tools/bpf/resolve_btfids
> > >  endif
> > >  endif
> > >
> > > +# README
> > > +# The tools build system is not a part of Kbuild. Before adding yet another
> > > +# tools/* here, please consider if the standard "hostprogs" syntax satisfies
> > > +# your needs.
> > > +
> >
> > Some clarifying questions / statements-as-questions:
> >
> > * nothing in tools/ uses Kbuild, right? (even stuff that uses KBUILD_*
> >   names is just an imitative port, right?)
> 
> 
> Correct.
> 
> You can build a tool from multiple directory locations.
> 
> For example, you can compile the 'perf' in multiple locations.
> 
> 
> [1] From the top of the kernel tree
> 
>    $ make tools/perf
> 
> 
> [2] From the tools/ directory
> 
>    $ cd tools
>    $ make perf
> 
> 
> [3] From the tools/perf/ directory
> 
>    $ cd tools/perf
>    $ make
> 
> 
> 
> When you do [2] or [3], the top-level Makefile is not parsed.
> 
> If necessary, the tools build system copies code from Kbuild.
> 
> 
> 
> 
> > * not everything in tools/ is actually promoted to a high-level target,
> >   that affects this top-level Makefile. Are you only concerned about
> >   stuff that pretends to be integrated in the top-level kernel Makefile?
> >   (If not, then it seems like placing the README comments only in this
> >   Makefile is a poor choice.)
> 
> 
> The tool build is integrated as a pattern rule in the top Makefile.
> (tools/%)
> 
> 
> So, you can build other tools from the top Makefile.
> 
> 
> See commit ea01fa9f63aef, which did not get Ack from any Kbuild
> maintainer, and caused subsequent troubles, and the benefit
> of which I still do not understand.
> 
> 
> Supporting "make tools/perf" in addition to "make -C tools perf"
> only saved a few characters to type.
> 
> 
> So, the problem remains, unless I revert ea01fa9f63aef.
> 
> I decided to not care about it too much, as long as
> such tools are not used during the kernel build.
> 
> I am really worried about objtool and resolve_btfids,
> as these two are used for building the kernel.
> 
> 
> 
> 
> 
> 
> > * is the "standard hostprogs" recommendation a general recommendation,
> >   for all sorts of kept-in-the-kernel-tree host tools? Is the
> >   recommendation to "use Kbuild" or to "avoid putting your tool in
> >   tools/*"? Is it possible (recommended?) to plumb Kbuild stuff into
> >   tools/, even if other parts won't migrate?
> 
> 
> I do not know.
> 
> They are different build systems with different designs.
> 
> Kbuild always works in the top of the output directory.
> Kbuild changes the working directory at most once if O= is given,
> but otherwise, it never changes the working directory during the build.
> 
> 
> The tools/ build system changes the working directory every time
> it invokes a new Make, and compiles the tool in its source directory.
> 
> 
> I do not know if all tools want to Kbuild.
> (the same applied to kselftest)
> 
> I think I can convert objtool and resolve_btfids to the Kbuild way.
> 
> 
> >
> > As is, I can't tell if this is telling people to avoid adding new stuff
> > to tools/ entirely, or just to only add to tools/ if you're able to
> > remain completely isolated from the rest of the kernel build -- as soon
> > as you want to play some part in the Kbuild-covered part of the tree,
> > you need to use Kbuild.
> 
> 
> See the code in the top Makefile.
> 
> 'prepare' depends on tools/objtool and tools/bpf/resolve_btfids.
> 
> If other tools are not prerequisites of 'scripts',
> Kbuild will not compile them.
> 
> 
> 
> 
> >
> > If I'm inferring the right answers to the above, then maybe an improved
> > wording could be something like:
> >
> > "The tools build system is not a part of Kbuild and tends to introduce
> > its own unique issues. If you need to integrate a new tool into Kbuild,
> > please consider locating that tool outside the tools/ tree and using the
> > standard Kbuild "hostprogs" syntax instead of adding a new tools/* entry
> > here."
> 
> 
> 
> I am fine with this description.
> 
> 
> Nicolas suggested a link to Documentation/kbuild/makefiles.rst
> 
> We can combine the two.
> 
> 
> # The tools build system is not a part of Kbuild and tends to introduce
> # its own unique issues. If you need to integrate a new tool into Kbuild,
> # please consider locating that tool outside the tools/ tree and using the
> # standard Kbuild "hostprogs" syntax instead of adding a new tools/* entry
> # here. See Documentation/kbuild/makefiles.rst for details.

yeah, thanks. Sounds good to me, too.

Kind regards,
Nicolas

Re: [PATCH] Makefile: add comment to discourage tools/* addition for kernel builds

[Brian Norris]

Hi Masahiro,

On Thu, Jun 27, 2024 at 04:02:46AM +0900, Masahiro Yamada wrote:
> > (Side note: I hope I haven't placed undue burden on you; I understood
> > you don't maintain tools/ and that it didn't use Kbuild. I only "poked"
> > you because the original bug report I was replying to had you and
> > linux-kbuild on CC already. And I appreciate your engagement, even if
> > the bugs are due to intentional forking.)
> 
> I did not mean to express my complaint particularly with the previous thread.

Great!

> It is not the first time that the tools/ build issue arose.
> 
> 
> I will drop the references to the threads.

That's not necessary, IMO. I'm not offended at all by any link to my
post. Links are useful, to add color to the problems involved. I was
just hoping to clarify that I never hoped Kbuild folks to solve
non-Kbuild problems.

> See commit ea01fa9f63aef, which did not get Ack from any Kbuild
> maintainer, and caused subsequent troubles, and the benefit
> of which I still do not understand.

Benefit: if I'm reading right, you've explained it yourself below?
Without this commit, it'd be harder to integrate the non-Kbuild objtool
into the Kbuild system.

But I see that it has a lot more downsides and rough edges too.

> Supporting "make tools/perf" in addition to "make -C tools perf"
> only saved a few characters to type.
> 
> 
> So, the problem remains, unless I revert ea01fa9f63aef.
> 
> I decided to not care about it too much, as long as
> such tools are not used during the kernel build.
> 
> I am really worried about objtool and resolve_btfids,
> as these two are used for building the kernel.

And of course, we ($JOB) have 2 build-flake bugs open, one for each of
those...

> >   for all sorts of kept-in-the-kernel-tree host tools? Is the
> >   recommendation to "use Kbuild" or to "avoid putting your tool in
> >   tools/*"? Is it possible (recommended?) to plumb Kbuild stuff into
> >   tools/, even if other parts won't migrate?
> 
> 
> I do not know.
> 
> They are different build systems with different designs.
> 
> Kbuild always works in the top of the output directory.
> Kbuild changes the working directory at most once if O= is given,
> but otherwise, it never changes the working directory during the build.
> 
> 
> The tools/ build system changes the working directory every time
> it invokes a new Make, and compiles the tool in its source directory.
> 
> 
> I do not know if all tools want to Kbuild.
> (the same applied to kselftest)

Yeah, from what I've tried to read off old threads, there are competing
design goals. objtool folks claimed they want to be as self-contained as
possible, with a local 'make' target, and relatively easy ability to
pull their tree out for independent usage outside the kernel.

> I think I can convert objtool and resolve_btfids to the Kbuild way.

That would make sense to me. I suspect that vastly more people build the
kernel, compared to those that want to use these tools/
independently of the kernel build. And the rough edges between them
cause real trouble.

> > As is, I can't tell if this is telling people to avoid adding new stuff
> > to tools/ entirely, or just to only add to tools/ if you're able to
> > remain completely isolated from the rest of the kernel build -- as soon
> > as you want to play some part in the Kbuild-covered part of the tree,
> > you need to use Kbuild.
> 
> 
> See the code in the top Makefile.
> 
> 'prepare' depends on tools/objtool and tools/bpf/resolve_btfids.
> 
> If other tools are not prerequisites of 'scripts',
> Kbuild will not compile them.

Sure. So I surmise you're choosing more of my latter -- that it's OK to
use tools/ if you don't want to integrate in the top-level build.

Sounds good to me.

> Nicolas suggested a link to Documentation/kbuild/makefiles.rst
> 
> We can combine the two.
> 
> 
> # The tools build system is not a part of Kbuild and tends to introduce
> # its own unique issues. If you need to integrate a new tool into Kbuild,
> # please consider locating that tool outside the tools/ tree and using the
> # standard Kbuild "hostprogs" syntax instead of adding a new tools/* entry
> # here. See Documentation/kbuild/makefiles.rst for details.

Looks good, thanks! Repeated:

Reviewed-by: Brian Norris <briannorris@chromium.org>