Re: possible dependency error?

[Brian Norris <briannorris@chromium.org>]

Hi Masahiro,

Thanks for the reply. I've been away for a bit, but I've poked at a few
more things now.

On Sun, May 26, 2024 at 01:35:43AM +0900, Masahiro Yamada wrote:
> On Fri, May 24, 2024 at 2:54 AM Brian Norris <briannorris@chromium.org> wrote:
> > --- a/tools/lib/subcmd/Makefile
> > +++ b/tools/lib/subcmd/Makefile
> > @@ -76,7 +76,7 @@ include $(srctree)/tools/build/Makefile.include
> >
> >  all: fixdep $(LIBFILE)
> >
> > -$(SUBCMD_IN): FORCE
> > +$(SUBCMD_IN): fixdep FORCE
> >         @$(MAKE) $(build)=libsubcmd
> >
> >  $(LIBFILE): $(SUBCMD_IN)
> 
> 
> 
> I may not fully understand the design policy of the tools/ build system,
> but this fix is presumably correct because the 'fixdep' binary
> is needed in each sub-directory for it to work correctly.
> 
> tools/bpf/resolve_btfids/libsubcmd/.exec-cmd.o.cmd must
> be generated by tools/bpf/resolve_btfids/libsubcmd/fixdep
> instead of by tools/bpf/resolve_btfids/fixdep.
> 
> But, fixing tools/lib/subcmd/Makefile is not enough.
> 
> *.cmd files under tools/bpf/resolve_btfids/libbpf/staticobjs/
> are broken for the same reason.
> So, this is fundamentally broken in many places.

I think I hacked something that works there too. It gets a bit uglier,
but not too bad IMO.

> And, as you noted, there is no easy way to fix .fixdep.o.cmd

I haven't come up with a *great* solution, but I came up with something
that works for the most part, by circumventing the normal Build /
Makefile.build split. It's getting pretty ugly too though...

> Your description in
> https://issuetracker.google.com/issues/313508829#comment32
> is all correct.
> 
> 
> "can we just use Kbuild?" is a good question.
> I do not understand why they use fragile build systems,
> where obviously they cannot do it correctly.
> 
> 
> In fact, I submitted a patch to migrate objtool to Kbuild
> because that fixes all the issues cleanly.
> 
> The objtool maintainers rejected it.
> https://lore.kernel.org/linux-kbuild/1551764896-8453-3-git-send-email-yamada.masahiro@socionext.com/
> 
> 
> Not only the build system.
> He also refused to participate in the standard Documentation
> directory.
> tools/objtool/Documentation/objtool.txt still resides in its own directory.

While I don't love having to solve the same problems repeatedly (once in
Kbuild; potentially-many in tools/), I'm also OK with trying to hack
fixes into the current duplicate build system if it's not prohibitively
complex to do so.

Here's what I have for now -- I might submit some or all of this as a
proper patchset if I can fix a few rough edges.

diff --git a/tools/build/Makefile b/tools/build/Makefile
index 17cdf01e29a0..fad93f64035d 100644
--- a/tools/build/Makefile
+++ b/tools/build/Makefile
@@ -43,11 +43,8 @@ ifneq ($(wildcard $(TMP_O)),)
 	$(Q)$(MAKE) -C feature OUTPUT=$(TMP_O) clean >/dev/null
 endif
 
-$(OUTPUT)fixdep-in.o: FORCE
-	$(Q)$(MAKE) $(build)=fixdep
-
-$(OUTPUT)fixdep: $(OUTPUT)fixdep-in.o
-	$(QUIET_LINK)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
+$(OUTPUT)fixdep: $(srctree)/tools/build/fixdep.c
+	$(QUIET_CC)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
 
 FORCE:
 
diff --git a/tools/build/Makefile.include b/tools/build/Makefile.include
index 8dadaa0fbb43..27b2090cb293 100644
--- a/tools/build/Makefile.include
+++ b/tools/build/Makefile.include
@@ -1,8 +1,16 @@
 # SPDX-License-Identifier: GPL-2.0-only
 build := -f $(srctree)/tools/build/Makefile.build dir=. obj
 
+# More than just $(Q), we sometimes want to suppress all command output from a
+# recursive make -- even the 'up to date' printout.
+ifeq ($(V),1)
+  SILENT_MAKE = $(Q)$(MAKE)
+else
+  SILENT_MAKE = $(Q)$(MAKE) --silent
+endif
+
 fixdep:
-	$(Q)$(MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
+	$(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
 
 fixdep-clean:
 	$(Q)$(MAKE) -C $(srctree)/tools/build clean
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 2cf892774346..a8f34de1ca25 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -154,6 +154,8 @@ $(BPF_IN_SHARED): force $(BPF_GENERATED)
 	$(Q)$(MAKE) $(build)=libbpf OUTPUT=$(SHARED_OBJDIR) CFLAGS="$(CFLAGS) $(SHLIB_FLAGS)"
 
 $(BPF_IN_STATIC): force $(BPF_GENERATED)
+	$(call rule_mkdir)
+	$(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= OUTPUT=$(STATIC_OBJDIR) $(STATIC_OBJDIR)fixdep
 	$(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
 
 $(BPF_HELPER_DEFS): $(srctree)/tools/include/uapi/linux/bpf.h
diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index b87213263a5e..59b09f280e49 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -76,7 +76,7 @@ include $(srctree)/tools/build/Makefile.include
 
 all: fixdep $(LIBFILE)
 
-$(SUBCMD_IN): FORCE
+$(SUBCMD_IN): fixdep FORCE
 	@$(MAKE) $(build)=libsubcmd
 
 $(LIBFILE): $(SUBCMD_IN)