[PATCH v2 0/5] doc: asciidoc cleanups

[PATCH v2 0/5] doc: asciidoc cleanups

[Felipe Contreras]

This patch series is an attempt to cleanup the Makefile of the documentation.

It's quie different from v1 mainly because now the .DELETE_ON_ERROR approach is being used as Aevar
suggested, however it tries to do the same thing.

It does not enable asciidoctor direct man page creation, that's in a separate patch.

Felipe Contreras (5):
  doc: refactor common asciidoc dependencies
  doc: improve asciidoc dependencies
  doc: remove unnecessary rm instances
  doc: simplify Makefile using .DELETE_ON_ERROR
  doc: avoid using rm directly

 Documentation/Makefile | 77 ++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 48 deletions(-)

Range-diff against v1:
1:  62d76c126a < -:  ---------- doc: standardize asciidoc calls
2:  0677725926 < -:  ---------- doc: add an asciidoc helper
3:  ca69c75596 < -:  ---------- doc: disable asciidoc-helper for asciidoctor
4:  f379515577 < -:  ---------- doc: simplify the handling of interruptions
5:  d2d10b34f3 < -:  ---------- doc: remove redundant rm
6:  d78e08aa2a < -:  ---------- doc: refactor common dependencies
7:  450a79d36f < -:  ---------- doc: improve asciidoc dependencies
8:  5be9efaa11 < -:  ---------- doc: join xml and man rules
-:  ---------- > 1:  55b188c8ad doc: refactor common asciidoc dependencies
-:  ---------- > 2:  e69d0a5b89 doc: improve asciidoc dependencies
-:  ---------- > 3:  4f18675ce9 doc: remove unnecessary rm instances
-:  ---------- > 4:  935675e070 doc: simplify Makefile using .DELETE_ON_ERROR
-:  ---------- > 5:  b621f3b8e9 doc: avoid using rm directly
-- 
2.31.1

[PATCH v2 1/5] doc: refactor common asciidoc dependencies

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2aae4c9cbb..46d9b98dac 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -139,6 +139,7 @@ ASCIIDOC_CONF = -f asciidoc.conf
 ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \
 		-amanversion=$(GIT_VERSION) \
 		-amanmanual='Git Manual' -amansource='Git'
+ASCIIDOC_DEPS = asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
 TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)
 TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK)
 MANPAGE_XSL = manpage-normal.xsl
@@ -354,12 +355,12 @@ clean:
 	$(RM) manpage-base-url.xsl
 	$(RM) GIT-ASCIIDOCFLAGS
 
-$(MAN_HTML): %.html : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+$(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS)
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_HTML) -d manpage -o $@+ $< && \
 	mv $@+ $@
 
-$(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+$(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_HTML) -o $@+ $< && \
 	mv $@+ $@
@@ -371,7 +372,7 @@ manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_XMLTO)$(RM) $@ && \
 	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
 
-%.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+%.xml : %.txt $(ASCIIDOC_DEPS)
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_XML) -d manpage -o $@+ $< && \
 	mv $@+ $@
-- 
2.31.1

[PATCH v2 2/5] doc: improve asciidoc dependencies

[Felipe Contreras]

asciidoc needs asciidoc.conf, asciidoctor asciidoctor-extensions.rb.

Neither needs the other.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 46d9b98dac..0f59cc0853 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -139,7 +139,7 @@ ASCIIDOC_CONF = -f asciidoc.conf
 ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \
 		-amanversion=$(GIT_VERSION) \
 		-amanmanual='Git Manual' -amansource='Git'
-ASCIIDOC_DEPS = asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+ASCIIDOC_DEPS = asciidoc.conf GIT-ASCIIDOCFLAGS
 TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)
 TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK)
 MANPAGE_XSL = manpage-normal.xsl
@@ -194,6 +194,7 @@ ASCIIDOC_DOCBOOK = docbook5
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
 ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
+ASCIIDOC_DEPS = asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
 DBLATEX_COMMON =
 XMLTO_EXTRA += --skip-validation
 XMLTO_EXTRA += -x manpage.xsl
-- 
2.31.1

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

[Felipe Contreras]

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.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0f59cc0853..aae8803e4c 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -318,8 +318,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 +326,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 && \
@@ -370,8 +369,7 @@ 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) $@+ $@ && \
-- 
2.31.1

[PATCH v2 4/5] doc: simplify Makefile using .DELETE_ON_ERROR

[Felipe Contreras]

Currently GNU make already removes files when catching an interruption
signal, however, in order to deal with other kinds of errors a
workaround is in place to store target output to a temporary file, and
only move it to its right place on success.

By enabling the built-in .DELETE_ON_ERROR we let make do this task, so
we don't have to.

This way the rules can be simplified a lot.

Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 61 +++++++++++++++---------------------------
 1 file changed, 21 insertions(+), 40 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index aae8803e4c..eaff97dcb8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -296,9 +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) && \
-	mv $@+ $@
+	$(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@ $(QUIET_STDERR)
 
 ifneq ($(MAKECMDGOALS),clean)
 -include doc.dep
@@ -356,14 +354,10 @@ clean:
 	$(RM) GIT-ASCIIDOCFLAGS
 
 $(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-	$(TXT_TO_HTML) -d manpage -o $@+ $< && \
-	mv $@+ $@
+	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@ $<
 
 $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-	$(TXT_TO_HTML) -o $@+ $< && \
-	mv $@+ $@
+	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -o $@ $<
 
 manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
@@ -372,14 +366,10 @@ manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
 
 %.xml : %.txt $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-	$(TXT_TO_XML) -d manpage -o $@+ $< && \
-	mv $@+ $@
+	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@ $<
 
 user-manual.xml: user-manual.txt user-manual.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-	$(TXT_TO_XML) -d book -o $@+ $< && \
-	mv $@+ $@
+	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d book -o $@ $<
 
 technical/api-index.txt: technical/api-index-skel.txt \
 	technical/api-index.sh $(patsubst %,%.txt,$(API_DOCS))
@@ -400,46 +390,35 @@ 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) $< && \
-	mv $@+ $@
+	$(QUIET_XSLTPROC)xsltproc $(XSLTOPTS) -o $@ $(XSLT) $<
 
 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 >$@++ && \
-	$(PERL_PATH) fix-texi.perl <$@++ >$@+ && \
-	rm $@++ && \
-	mv $@+ $@
+	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@+ && \
+	$(PERL_PATH) fix-texi.perl <$@+ >$@ && \
+	rm $@+
 
 user-manual.pdf: user-manual.xml
-	$(QUIET_DBLATEX)$(RM) $@+ $@ && \
-	$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
-	mv $@+ $@
+	$(QUIET_DBLATEX)$(DBLATEX) -o $@ $(DBLATEX_COMMON) $<
 
 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) > $@++ && \
-	$(PERL_PATH) cat-texi.perl $@ <$@++ >$@+ && \
-	rm $@++ && \
-	mv $@+ $@
+		rm $(xml)+ &&) true) > $@+ && \
+	$(PERL_PATH) cat-texi.perl $@ <$@+ >$@ && \
+	rm $@+
 
 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 >$@+ && \
-	mv $@+ $@
+	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@
 
 howto-index.txt: howto-index.sh $(HOWTO_TXT)
-	$(QUIET_GEN)$(RM) $@+ $@ && \
-	'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
-	mv $@+ $@
+	$(QUIET_GEN)'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@
 
 $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
 	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) $*.txt
@@ -448,10 +427,9 @@ 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 $@+ $@
+	$(TXT_TO_HTML) - >$@
 
 install-webdoc : html
 	'$(SHELL_PATH_SQ)' ./install-webdoc.sh $(WEBDOC_DEST)
@@ -492,4 +470,7 @@ doc-l10n install-l10n::
 	$(MAKE) -C po $@
 endif
 
+# Delete the target file on error
+.DELETE_ON_ERROR:
+
 .PHONY: FORCE
-- 
2.31.1

[PATCH v2 5/5] doc: avoid using rm directly

[Felipe Contreras]

That's what we have $(RM) for.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index eaff97dcb8..f5605b7767 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -398,7 +398,7 @@ git.info: user-manual.texi
 user-manual.texi: user-manual.xml
 	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@+ && \
 	$(PERL_PATH) fix-texi.perl <$@+ >$@ && \
-	rm $@+
+	$(RM) $@+
 
 user-manual.pdf: user-manual.xml
 	$(QUIET_DBLATEX)$(DBLATEX) -o $@ $(DBLATEX_COMMON) $<
@@ -407,9 +407,9 @@ gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl
 	$(QUIET_DB2TEXI) \
 	($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \
 		$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \
-		rm $(xml)+ &&) true) > $@+ && \
+		$(RM) $(xml)+ &&) true) > $@+ && \
 	$(PERL_PATH) cat-texi.perl $@ <$@+ >$@ && \
-	rm $@+
+	$(RM) $@+
 
 gitman.info: gitman.texi
 	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi
-- 
2.31.1

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

[Jeff King]

On Fri, May 14, 2021 at 06:56:29AM -0500, Felipe Contreras wrote:

> 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/.

Aren't there tons more that you end up removing in the next patch? E.g.:

   doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
  -       $(QUIET_GEN)$(RM) $@+ $@ && \
  -       $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
  -       mv $@+ $@
  +       $(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@ $(QUIET_STDERR)

That does differ in that it removes $@+, too, but the premise is the
same (we know that $@+ could not be a problem, as we're about to
clobber it anyway).

I'm OK with getting rid of all of them, but it seems like it ought to be
happening all in this patch.

(And in general the rest of the series looks OK to me).

-Peff

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

[Felipe Contreras]

Jeff King wrote:
> On Fri, May 14, 2021 at 06:56:29AM -0500, Felipe Contreras wrote:
> 
> > 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/.
> 
> Aren't there tons more that you end up removing in the next patch? E.g.:
> 
>    doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
>   -       $(QUIET_GEN)$(RM) $@+ $@ && \
>   -       $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
>   -       mv $@+ $@
>   +       $(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@ $(QUIET_STDERR)
> 
> That does differ in that it removes $@+, too, but the premise is the
> same (we know that $@+ could not be a problem, as we're about to
> clobber it anyway).
> 
> I'm OK with getting rid of all of them, but it seems like it ought to be
> happening all in this patch.

Yeah, but the rationale is different.

  1. $(RM) $@: these remove the target file because of permissions
     (i.e. root owned)
  2. $(RM) $@+ $@ && $(CODE) && mv $@+ $@: these are for interrupted builds

To get rid of #2 we need an alternative solution, like .DELETE_ON_ERROR,
to get rid of #1 we don't, we can just do it.

-- 
Felipe Contreras

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

[Jeff King]

On Sat, May 15, 2021 at 07:04:05AM -0500, Felipe Contreras wrote:

> Jeff King wrote:
> > On Fri, May 14, 2021 at 06:56:29AM -0500, Felipe Contreras wrote:
> > 
> > > 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/.
> > 
> > Aren't there tons more that you end up removing in the next patch? E.g.:
> > 
> >    doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
> >   -       $(QUIET_GEN)$(RM) $@+ $@ && \
> >   -       $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
> >   -       mv $@+ $@
> >   +       $(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@ $(QUIET_STDERR)
> > 
> > That does differ in that it removes $@+, too, but the premise is the
> > same (we know that $@+ could not be a problem, as we're about to
> > clobber it anyway).
> > 
> > I'm OK with getting rid of all of them, but it seems like it ought to be
> > happening all in this patch.
> 
> Yeah, but the rationale is different.
> 
>   1. $(RM) $@: these remove the target file because of permissions
>      (i.e. root owned)
>   2. $(RM) $@+ $@ && $(CODE) && mv $@+ $@: these are for interrupted builds
> 
> To get rid of #2 we need an alternative solution, like .DELETE_ON_ERROR,
> to get rid of #1 we don't, we can just do it.

To get rid of the "mv", you need something like .DELETE_ON_ERROR. But
the "rm" in the second case has nothing to do with interrupted builds.
It is is doing the same nothing that the ones you are getting rid of
here are.

I.e., I was suggesting to get rid of the "rm" call in the hunk I showed
above, but leave the "mv" for the follow-on patch.

-Peff

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

[Felipe Contreras]

Jeff King wrote:
> On Sat, May 15, 2021 at 07:04:05AM -0500, Felipe Contreras wrote:
> > Jeff King wrote:

> > > That does differ in that it removes $@+, too, but the premise is the
> > > same (we know that $@+ could not be a problem, as we're about to
> > > clobber it anyway).
> > > 
> > > I'm OK with getting rid of all of them, but it seems like it ought to be
> > > happening all in this patch.
> > 
> > Yeah, but the rationale is different.
> > 
> >   1. $(RM) $@: these remove the target file because of permissions
> >      (i.e. root owned)
> >   2. $(RM) $@+ $@ && $(CODE) && mv $@+ $@: these are for interrupted builds
> > 
> > To get rid of #2 we need an alternative solution, like .DELETE_ON_ERROR,
> > to get rid of #1 we don't, we can just do it.
> 
> To get rid of the "mv", you need something like .DELETE_ON_ERROR. But
> the "rm" in the second case has nothing to do with interrupted builds.
> It is is doing the same nothing that the ones you are getting rid of
> here are.
> 
> I.e., I was suggesting to get rid of the "rm" call in the hunk I showed
> above, but leave the "mv" for the follow-on patch.

Ahh, I see. It's quite a bit more work, but sure, I can do that.

-- 
Felipe Contreras

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

[Jeff King]

On Mon, May 17, 2021 at 05:42:31AM -0500, Felipe Contreras wrote:

> > I.e., I was suggesting to get rid of the "rm" call in the hunk I showed
> > above, but leave the "mv" for the follow-on patch.
> 
> Ahh, I see. It's quite a bit more work, but sure, I can do that.

If it's a lot more work, I can live with the split as you have here. I
didn't think it would be a lot, though.

-Peff