On Wed, Jun 19, 2024 at 8:29 AM Brian Norris <briannorris@xxxxxxxxxxxx> wrote: > > 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@xxxxxxxxxxxx> 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@xxxxxxxxxxxxx/ > > > > > > 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. I do not see any maintainer for tools/build/, but at least you can find who is picking up the patches. masahiro@zoe:~/ref/linux(master)$ ./scripts/get_maintainer.pl -f tools/build/ Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> (commit_signer:7/10=70%,authored:1/10=10%) Namhyung Kim <namhyung@xxxxxxxxxx> (commit_signer:5/10=50%,authored:2/10=20%) Ian Rogers <irogers@xxxxxxxxxx> (commit_signer:3/10=30%,authored:1/10=10%) Thomas Richter <tmricht@xxxxxxxxxxxxx> (commit_signer:2/10=20%,authored:2/10=20%) Quentin Monnet <qmo@xxxxxxxxxx> (commit_signer:2/10=20%) Jiri Olsa <jolsa@xxxxxxxxxx> (authored:1/10=10%) linux-kernel@xxxxxxxxxxxxxxx (open list) > 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 $@ $< OK, you bypassed fixdep for fixdep itself. fixdep will not be rebuilt when the command line changes, but it may not be a big deal. It is not working that way already. BTW, did you have a chance to test your code with the -j<N> option? I quickly tested your change, and I observed new "jobserver unavailable" warnings. masahiro@zoe:~/ref/linux(master)$ make -j24 mkdir -p /home/masahiro/ref/linux/tools/objtool && make O=/home/masahiro/ref/linux subdir=tools/objtool --no-print-directory -C objtool make[4]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule. make[5]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule. The first line: mkdir -p /home/masahiro/ref/linux/tools/objtool && make O=/home/masahiro/ref/linux subdir=tools/objtool --no-print-directory -C objtool is an existing noise regardless of your change. (I do not know if anybody cares about this either) > 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) > -- Best Regards Masahiro Yamada
