| Message ID | patch-5.5-f3ff76d0e98-20210721T231900Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Makefile: "make tags" fixes & cleanup | expand |
On Thu, Jul 22, 2021 at 01:23:06AM +0200, Ævar Arnfjörð Bjarmason wrote: > This is because the xargs command may decide to invoke the program > multiple times. We need to make sure we've got a union of its results > at the end. > > For "ctags" and "etags" we used the "-a" flag for this, for cscope > that behavior is the default. Its "-u" flag disables its equivalent of > an implicit "-a" flag. Hrm, that's not the experience I get with cscope. E.g.: $ cscope -b wt-status.c $ grep -m1 wt-status.c cscope.out @wt-status.c $ cscope -b git.c $ grep -m1 wt-status.c cscope.out [no output] I wondered if I was being too hacky with my grep there. But if I simulate more extreme cmdline-splitting like so: diff --git a/Makefile b/Makefile index c7c46c017d..8a1ec00938 100644 --- a/Makefile +++ b/Makefile @@ -2751,7 +2751,7 @@ tags: FORCE cscope: $(RM) cscope* - $(FIND_SOURCE_FILES) | xargs cscope -b + $(FIND_SOURCE_FILES) | xargs -n1 cscope -b ### Detect prefix changes TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\ then I get a file that is much smaller (1MB versus 9MB), and fails to find lots of things: $ cscope -d -L1cmd_pack_objects [no output] (by the way, I confused myself several times while testing this because without "-d", it will actually rebuild the index using files in the current directory. So something like " cscope -L1main" gives the same result in both cases, or even if you don't have a cscope.out file at all!) But it really seems like cscope is not appending as we'd want. From skimming the manpage, I think replacing xargs with "cscope -b -i -" should work (and seems to for me). -Peff
On Fri, Jul 23, 2021 at 06:43:13AM -0400, Jeff King wrote: > On Thu, Jul 22, 2021 at 01:23:06AM +0200, Ævar Arnfjörð Bjarmason wrote: > > > This is because the xargs command may decide to invoke the program > > multiple times. We need to make sure we've got a union of its results > > at the end. > > > > For "ctags" and "etags" we used the "-a" flag for this, for cscope > > that behavior is the default. Its "-u" flag disables its equivalent of > > an implicit "-a" flag. > > Hrm, that's not the experience I get with cscope. E.g.: Just to make it extra clear: this is not a problem introduced by your series, since we were already using xargs with cscope. But this seems like a good time to fix it. -Peff
diff --git a/Makefile b/Makefile index 730ff23b923..295ac359cea 100644 --- a/Makefile +++ b/Makefile @@ -2742,18 +2742,19 @@ FIND_SOURCE_FILES = ( \ FOUND_SOURCE_FILES = $(shell $(FIND_SOURCE_FILES)) $(ETAGS_TARGET): $(FOUND_SOURCE_FILES) - $(QUIET_GEN)$(RM) "$(ETAGS_TARGET)+" && \ - echo $(FOUND_SOURCE_FILES) | xargs etags -a -o "$(ETAGS_TARGET)+" && \ - mv "$(ETAGS_TARGET)+" "$(ETAGS_TARGET)" + $(QUIET_GEN)$(RM) $@+ && \ + echo $(FOUND_SOURCE_FILES) | xargs etags -a -o $@+ && \ + mv $@+ $@ tags: $(FOUND_SOURCE_FILES) - $(QUIET_GEN)$(RM) tags+ && \ - echo $(FOUND_SOURCE_FILES) | xargs ctags -a -o tags+ && \ - mv tags+ tags + $(QUIET_GEN)$(RM) $@+ && \ + echo $(FOUND_SOURCE_FILES) | xargs ctags -a -o $@+ && \ + mv $@+ $@ cscope.out: $(FOUND_SOURCE_FILES) - $(QUIET_GEN)$(RM) cscope.out && \ - echo $(FOUND_SOURCE_FILES) | xargs cscope -f$@ -b + $(QUIET_GEN)$(RM) $@+ && \ + echo $(FOUND_SOURCE_FILES) | xargs cscope -f$@+ -b && \ + mv $@+ $@ .PHONY: cscope cscope: cscope.out
Since the "tags", "TAGS" and "cscope.out" targets rely on piping into xargs with an "echo <list> | xargs" pattern, we need to make sure we're in an append mode. Unlike my recent change to make use of ".DELETE_ON_ERROR" in 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR" flag, 2021-06-29), we really do need the "rm $@+" at the beginning (note, not "rm $@"). This is because the xargs command may decide to invoke the program multiple times. We need to make sure we've got a union of its results at the end. For "ctags" and "etags" we used the "-a" flag for this, for cscope that behavior is the default. Its "-u" flag disables its equivalent of an implicit "-a" flag. Let's also consistently use the $@ and $@+ names instead of needlessly hardcoding or referring to more verbose names in the "tags" and "TAGS" rules. These targets could perhaps be improved in the future by factoring this "echo <list> | xargs" pattern so that we make intermediate tags files for each source file, and then assemble them into one "tags" file at the end. The etags manual page suggests that doing that (or perhaps just --update) might be counter-productive, in any case, the tag building is fast enough for me, so I'm leaving that for now. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)