From: Brian Norris <briannorris@chromium.org>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
linux-kernel@vger.kernel.org,
Sami Tolvanen <samitolvanen@google.com>,
Nathan Chancellor <nathan@kernel.org>,
Nicolas Schier <nicolas@fjasle.eu>
Subject: Re: [PATCH] Makefile: add comment to discourage tools/* addition for kernel builds
Date: Thu, 20 Jun 2024 14:52:26 -0700 [thread overview]
Message-ID: <ZnSkmmpCY2Aj5VpU@google.com> (raw)
In-Reply-To: <20240619062145.3967720-1-masahiroy@kernel.org>
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
>
next prev parent reply other threads:[~2024-06-20 21:52 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-19 6:21 [PATCH] Makefile: add comment to discourage tools/* addition for kernel builds Masahiro Yamada
2024-06-19 11:22 ` Nicolas Schier
2024-06-20 21:52 ` Brian Norris [this message]
2024-06-26 19:02 ` Masahiro Yamada
2024-06-26 19:49 ` Nicolas Schier
2024-06-28 21:54 ` Brian Norris
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZnSkmmpCY2Aj5VpU@google.com \
--to=briannorris@chromium.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=masahiroy@kernel.org \
--cc=nathan@kernel.org \
--cc=nicolas@fjasle.eu \
--cc=peterz@infradead.org \
--cc=samitolvanen@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.