| Message ID | cover-0.5-00000000000-20210721T231900Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Makefile: "make tags" fixes & cleanup | expand |
On Thu, Jul 22, 2021 at 01:23:01AM +0200, Ævar Arnfjörð Bjarmason wrote: > The big win here is that none of the tags targets depend on "FORCE" > anymore, so we'll only re-generate them if our sources change. > > For v2, see > https://lore.kernel.org/git/cover-0.5-0000000000-20210629T110837Z-avarab@gmail.com/ > > This fixes the series per feedback from Jeff King and Ramsay Jones, > i.e: > > * In v2 the 3/5 broke things in a way that 4/5 fixed, that's now > re-arranged and fixed. Thanks. Aside from some cscope appending arcana I found, these look good to me. I have no opinion on "cscope*" versus "cscope.out", except that it is not worth anybody's time to argue about. :) -Peff
Jeff King <peff@peff.net> writes: > Thanks. Aside from some cscope appending arcana I found, these look good > to me. I have no opinion on "cscope*" versus "cscope.out", except that > it is not worth anybody's time to argue about. :) Thanks, sorry and I agree with both counts.
Jeff King wrote: > On Thu, Jul 22, 2021 at 01:23:01AM +0200, Ævar Arnfjörð Bjarmason wrote: > > > The big win here is that none of the tags targets depend on "FORCE" > > anymore, so we'll only re-generate them if our sources change. > > > > For v2, see > > https://lore.kernel.org/git/cover-0.5-0000000000-20210629T110837Z-avarab@gmail.com/ > > > > This fixes the series per feedback from Jeff King and Ramsay Jones, > > i.e: > > > > * In v2 the 3/5 broke things in a way that 4/5 fixed, that's now > > re-arranged and fixed. > > Thanks. Aside from some cscope appending arcana I found, these look good > to me. I have no opinion on "cscope*" versus "cscope.out", except that > it is not worth anybody's time to argue about. :) I agree with that as well.
Ævar Arnfjörð Bjarmason wrote: > The big win here is that none of the tags targets depend on "FORCE" > anymore, so we'll only re-generate them if our sources change. Very nice. I think most of the nitpicking in the comments is not really worth the trouble (either for or against), there is one comment from Jeff King regarding multiple runs of the xargs command that I think is valid, but too hypothetical for me to care about (ARG_MAX is 2097152 on my system), so it would be nice to fix, but not necessary. Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
The big win here is that none of the tags targets depend on "FORCE" anymore, so we'll only re-generate them if our sources change. For v2, see https://lore.kernel.org/git/cover-0.5-0000000000-20210629T110837Z-avarab@gmail.com/ This fixes the series per feedback from Jeff King and Ramsay Jones, i.e: * In v2 the 3/5 broke things in a way that 4/5 fixed, that's now re-arranged and fixed. * I now use $(FOUND_SOURCE_FILES) instead of $(FIND_SOURCE_FILES) consistently. I was redundantly re-running the "find" command. Ævar Arnfjörð Bjarmason (5): Makefile: move ".PHONY: cscope" near its target Makefile: add QUIET_GEN to "cscope" target Makefile: don't use "FORCE" for tags targets Makefile: the "cscope" target always creates a "cscope.out" Makefile: normalize clobbering & xargs for tags targets .gitignore | 2 +- Makefile | 34 ++++++++++++++++++++-------------- 2 files changed, 21 insertions(+), 15 deletions(-) Range-diff against v2: 1: dd6cfd6022c = 1: 6b4ddc126d9 Makefile: move ".PHONY: cscope" near its target 2: 56daa09738f = 2: d3d5d332e92 Makefile: add QUIET_GEN to "cscope" target 4: b924cc3f566 ! 3: 9dd69d68178 Makefile: don't use "FORCE" for tags targets @@ Metadata ## Commit message ## Makefile: don't use "FORCE" for tags targets - Remove the "FORCE" dependency from the "tags", "TAGS" and "cscope.out" + Remove the "FORCE" dependency from the "tags", "TAGS" and "cscope" targets, instead make them depend on whether or not the relevant source files have changed. + For the cscope target we need to change it to depend on the actual + generated file while we generate while we're at it, as the next commit + will discuss we always generate a cscope.out file. + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Makefile ## @@ Makefile: FIND_SOURCE_FILES = ( \ + +$(ETAGS_TARGET): $(FOUND_SOURCE_FILES) $(QUIET_GEN)$(RM) "$(ETAGS_TARGET)+" && \ - $(FIND_SOURCE_FILES) | xargs etags -a -o "$(ETAGS_TARGET)+" && \ +- $(FIND_SOURCE_FILES) | xargs etags -a -o "$(ETAGS_TARGET)+" && \ ++ echo $(FOUND_SOURCE_FILES) | xargs etags -a -o "$(ETAGS_TARGET)+" && \ mv "$(ETAGS_TARGET)+" "$(ETAGS_TARGET)" -tags: FORCE +tags: $(FOUND_SOURCE_FILES) $(QUIET_GEN)$(RM) tags+ && \ - $(FIND_SOURCE_FILES) | xargs ctags -a -o tags+ && \ +- $(FIND_SOURCE_FILES) | xargs ctags -a -o tags+ && \ ++ echo $(FOUND_SOURCE_FILES) | xargs ctags -a -o tags+ && \ mv tags+ tags --cscope.out: +-.PHONY: cscope +-cscope: +cscope.out: $(FOUND_SOURCE_FILES) - $(QUIET_GEN)$(RM) cscope.out && \ -- $(FIND_SOURCE_FILES) | xargs cscope -f$@ -b -+ echo $(FOUND_SOURCE_FILES) | xargs cscope -f$@ -b + $(QUIET_GEN)$(RM) cscope* && \ +- $(FIND_SOURCE_FILES) | xargs cscope -b ++ echo $(FOUND_SOURCE_FILES) | xargs cscope -b ++ ++.PHONY: cscope ++cscope: cscope.out - .PHONY: cscope - cscope: cscope.out + ### Detect prefix changes + TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\ @@ Makefile: check: config-list.h command-list.h exit 1; \ fi 3: 35c8b839048 ! 4: f8d151f1f6a Makefile: fix "cscope" target to refer to cscope.out @@ Metadata Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Commit message ## - Makefile: fix "cscope" target to refer to cscope.out + Makefile: the "cscope" target always creates a "cscope.out" - The cscope target added in a2a9150bf06 (makefile: Add a cscope target, - 2007-10-06) has for some reason been referring to cscope* instead of + In the preceding commit the "cscope" target was changed to be a phony + alias for the "cscope.out" target. + + The cscope target was added in a2a9150bf06 (makefile: Add a cscope + target, 2007-10-06), and has always referred to cscope* instead of to cscope.out. - Let's generate the cscope.out file directly so we don't need to - speculate. The "-fcscope.out" (note, no whitespace) argument is - enabled by default on my system's cscope 15.9, but let's provide it - explicitly for good measure. + As far as I can tell this ambiguity was never needed. The + "-fcscope.out" (note, no whitespace) argument is enabled by default, + but let's provide it explicitly for good measure. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> @@ .gitignore *.obj ## Makefile ## -@@ Makefile: tags: FORCE - $(FIND_SOURCE_FILES) | xargs ctags -a -o tags+ && \ +@@ Makefile: tags: $(FOUND_SOURCE_FILES) mv tags+ tags -+cscope.out: -+ $(QUIET_GEN)$(RM) cscope.out && \ -+ $(FIND_SOURCE_FILES) | xargs cscope -f$@ -b -+ - .PHONY: cscope --cscope: + cscope.out: $(FOUND_SOURCE_FILES) - $(QUIET_GEN)$(RM) cscope* && \ -- $(FIND_SOURCE_FILES) | xargs cscope -b -+cscope: cscope.out +- echo $(FOUND_SOURCE_FILES) | xargs cscope -b ++ $(QUIET_GEN)$(RM) cscope.out && \ ++ echo $(FOUND_SOURCE_FILES) | xargs cscope -f$@ -b - ### Detect prefix changes - TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\ + .PHONY: cscope + cscope: cscope.out @@ Makefile: clean: profile-clean coverage-clean cocciclean $(RM) $(HCC) $(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json 5: 5195d99e25c ! 5: f3ff76d0e98 Makefile: normalize clobbering & xargs for tags targets @@ Metadata ## Commit message ## Makefile: normalize clobbering & xargs for tags targets - Since the "tags", "TAGS" and "cscope.out" targets rely on ping into + 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 recent changes of mine to make use of ".DELETE_ON_ERROR" we - really do need the "rm $@+" at the beginning (note, not "rm $@"). + 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 on multiple invocations - of the program. We need to make sure we've got a union of its results + 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 @@ Makefile: FIND_SOURCE_FILES = ( \ $(ETAGS_TARGET): $(FOUND_SOURCE_FILES) - $(QUIET_GEN)$(RM) "$(ETAGS_TARGET)+" && \ -- $(FIND_SOURCE_FILES) | xargs etags -a -o "$(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 $@+ && \ @@ Makefile: FIND_SOURCE_FILES = ( \ tags: $(FOUND_SOURCE_FILES) - $(QUIET_GEN)$(RM) tags+ && \ -- $(FIND_SOURCE_FILES) | xargs ctags -a -o tags+ && \ +- echo $(FOUND_SOURCE_FILES) | xargs ctags -a -o tags+ && \ - mv tags+ tags + $(QUIET_GEN)$(RM) $@+ && \ + echo $(FOUND_SOURCE_FILES) | xargs ctags -a -o $@+ && \