git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

From
Junio C Hamano <gitster@pobox.com>
Date
May 24, 2021, 17:51 UTC
Message-ID
<xmqqcztgj9za.fsf@gitster.g>
In-Reply-To
<20210521222941.525901-4-felipe.contreras@gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 12 quoted lines
> Commits 50cff52f1a (When generating manpages, delete outdated targets
> first., 2007-08-02) and f9286765b2 (Documentation/Makefile: remove
> cmd-list.made before redirecting to it., 2007-08-06) created these rm
> instances for a very rare corner-case: building as root by mistake.
>
> It's odd to have workarounds here, but nowhere else in the Makefile--
> which already fails in this stuation, starting from
> Documentation/technical/.
>
> We gain nothing but complexity, so let's remove them.
>
> Comments-by: Jeff King <peff@peff.net>

This is curious. I understand what other kinds of trailers like Helped-by and Suggested-by people sometimes use mean, but this one does not tell anything other than that this person had some comments on an earlier rounds, does not tell us what kind of comments they were and if something was done to address those comments or the comments were totally ignored or what.

The patch text itself, and the justification given in the second paragraph, both make perfect sense to me.

Show 131 quoted lines
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  Documentation/Makefile | 42 +++++++++++++++---------------------------
>  1 file changed, 15 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 0f59cc0853..73b88c3aad 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -296,8 +296,7 @@ docdep_prereqs = \
>  	cmd-list.made $(cmds_txt)
>  
>  doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
> -	$(QUIET_GEN)$(RM) $@+ $@ && \
> -	$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
> +	$(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
>  	mv $@+ $@
>  
>  ifneq ($(MAKECMDGOALS),clean)
> @@ -318,8 +317,7 @@ cmds_txt = cmds-ancillaryinterrogators.txt \
>  $(cmds_txt): cmd-list.made
>  
>  cmd-list.made: cmd-list.perl ../command-list.txt $(MAN1_TXT)
> -	$(QUIET_GEN)$(RM) $@ && \
> -	$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(cmds_txt) $(QUIET_STDERR) && \
> +	$(QUIET_GEN)$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(cmds_txt) $(QUIET_STDERR) && \
>  	date >$@
>  
>  mergetools_txt = mergetools-diff.txt mergetools-merge.txt
> @@ -327,7 +325,7 @@ mergetools_txt = mergetools-diff.txt mergetools-merge.txt
>  $(mergetools_txt): mergetools-list.made
>  
>  mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
> -	$(QUIET_GEN)$(RM) $@ && \
> +	$(QUIET_GEN) \
>  	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
>  		. ../git-mergetool--lib.sh && \
>  		show_tool_names can_diff "* " || :' >mergetools-diff.txt && \
> @@ -357,30 +355,25 @@ clean:
>  	$(RM) GIT-ASCIIDOCFLAGS
>  
>  $(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS)
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> -	$(TXT_TO_HTML) -d manpage -o $@+ $< && \
> +	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@+ $< && \
>  	mv $@+ $@
>  
>  $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> -	$(TXT_TO_HTML) -o $@+ $< && \
> +	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -o $@+ $< && \
>  	mv $@+ $@
>  
>  manpage-base-url.xsl: manpage-base-url.xsl.in
>  	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
>  
>  %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)
> -	$(QUIET_XMLTO)$(RM) $@ && \
> -	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
> +	$(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
>  
>  %.xml : %.txt $(ASCIIDOC_DEPS)
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> -	$(TXT_TO_XML) -d manpage -o $@+ $< && \
> +	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@+ $< && \
>  	mv $@+ $@
>  
>  user-manual.xml: user-manual.txt user-manual.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> -	$(TXT_TO_XML) -d book -o $@+ $< && \
> +	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d book -o $@+ $< && \
>  	mv $@+ $@
>  
>  technical/api-index.txt: technical/api-index-skel.txt \
> @@ -402,27 +395,24 @@ XSLTOPTS += --stringparam html.stylesheet docbook-xsl.css
>  XSLTOPTS += --param generate.consistent.ids 1
>  
>  user-manual.html: user-manual.xml $(XSLT)
> -	$(QUIET_XSLTPROC)$(RM) $@+ $@ && \
> -	xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
> +	$(QUIET_XSLTPROC)xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
>  	mv $@+ $@
>  
>  git.info: user-manual.texi
>  	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split -o $@ user-manual.texi
>  
>  user-manual.texi: user-manual.xml
> -	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
> -	$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
> +	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
>  	$(PERL_PATH) fix-texi.perl <$@++ >$@+ && \
>  	rm $@++ && \
>  	mv $@+ $@
>  
>  user-manual.pdf: user-manual.xml
> -	$(QUIET_DBLATEX)$(RM) $@+ $@ && \
> -	$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
> +	$(QUIET_DBLATEX)$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
>  	mv $@+ $@
>  
>  gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl
> -	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
> +	$(QUIET_DB2TEXI) \
>  	($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \
>  		$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \
>  		rm $(xml)+ &&) true) > $@++ && \
> @@ -434,13 +424,11 @@ gitman.info: gitman.texi
>  	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi
>  
>  $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml
> -	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
> -	$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
> +	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
>  	mv $@+ $@
>  
>  howto-index.txt: howto-index.sh $(HOWTO_TXT)
> -	$(QUIET_GEN)$(RM) $@+ $@ && \
> -	'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
> +	$(QUIET_GEN)'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
>  	mv $@+ $@
>  
>  $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
> @@ -450,7 +438,7 @@ WEBDOC_DEST = /pub/software/scm/git/docs
>  
>  howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
>  $(patsubst %.txt,%.html,$(HOWTO_TXT)): %.html : %.txt GIT-ASCIIDOCFLAGS
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> +	$(QUIET_ASCIIDOC) \
>  	sed -e '1,/^$$/d' $< | \
>  	$(TXT_TO_HTML) - >$@+ && \
>  	mv $@+ $@
Previous: Felipe ContrerasNext: Felipe Contreras
Message 5 of 21 in “doc: asciidoc cleanups”
  1. 0/5 doc: asciidoc cleanupsFelipe Contreras, May 21, 2021
  2. 1/5 doc: refactor common asciidoc dependenciesFelipe Contreras, May 21, 2021
  3. 2/5 doc: improve asciidoc dependenciesFelipe Contreras, May 21, 2021
  4. 3/5 doc: remove unnecessary rm instancesFelipe Contreras, May 21, 2021
  5. Junio C HamanoMay 24, 2021
  6. Felipe ContrerasMay 24, 2021
  7. Felipe ContrerasMay 24, 2021
  8. Junio C HamanoMay 25, 2021
  9. Felipe ContrerasMay 25, 2021
  10. Junio C HamanoMay 25, 2021
  11. Felipe ContrerasMay 25, 2021
  12. Felipe ContrerasMay 26, 2021
  13. 4/5 doc: simplify Makefile using .DELETE_ON_ERRORFelipe Contreras, May 21, 2021
  14. 5/5 doc: avoid using rm directlyFelipe Contreras, May 21, 2021
  15. Junio C HamanoMay 24, 2021
  16. Jeff KingMay 27, 2021
  17. Felipe ContrerasMay 27, 2021
  18. Jeff KingMay 27, 2021
  19. Felipe ContrerasMay 27, 2021
  20. Junio C HamanoMay 28, 2021
  21. Felipe ContrerasMay 28, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.