From: Felipe Contreras <hidden> Date: 2021-05-14 11:56:38
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
From: Felipe Contreras <hidden> Date: 2021-05-14 11:56:42
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 <redacted>
---
Documentation/Makefile | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
From: Felipe Contreras <hidden> Date: 2021-05-14 11:56:48
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 <redacted>
Signed-off-by: Felipe Contreras <redacted>
---
Documentation/Makefile | 61 +++++++++++++++---------------------------
1 file changed, 21 insertions(+), 40 deletions(-)
From: Jeff King <hidden> Date: 2021-05-15 09:24:24
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
From: Felipe Contreras <hidden> Date: 2021-05-15 12:04:10
Jeff King wrote:
On Fri, May 14, 2021 at 06:56:29AM -0500, Felipe Contreras wrote:
quoted
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
From: Jeff King <hidden> Date: 2021-05-17 08:53:20
On Sat, May 15, 2021 at 07:04:05AM -0500, Felipe Contreras wrote:
Jeff King wrote:
quoted
On Fri, May 14, 2021 at 06:56:29AM -0500, Felipe Contreras wrote:
quoted
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
From: Felipe Contreras <hidden> Date: 2021-05-17 10:42:37
Jeff King wrote:
On Sat, May 15, 2021 at 07:04:05AM -0500, Felipe Contreras wrote:
quoted
Jeff King wrote:
quoted
quoted
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