| Message ID | 20221127224251.2508200-2-psmith@gnu.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9f95c7aefa8419b697972abd9d25ce9062fac2bf |
| Headers | show |
| Series | Avoid multiple patterns when recipes generate one file | expand |
On Sun, Nov 27 2022, Paul Smith wrote: > A GNU make pattern rule with multiple targets has always meant that > a single invocation of the recipe will build all the targets. > However in older versions of GNU make a recipe that did not really > build all the targets would be tolerated. > > Starting with GNU make 4.4 this behavior is deprecated and pattern > rules are expected to generate files to match all the patterns. > If not all targets are created then GNU make will not consider any > target up to date and will re-run the recipe when it is run again. > > Modify Documentation/Makefile to split the man page-creating pattern > rule into a separate pattern rule for each pattern. > > Reported-by: Alexander Kanavin <alex.kanavin@gmail.com> > Signed-off-by: Paul Smith <psmith@gnu.org> > --- Thanks for fixing downstream, and for working on GNU make. > Documentation/Makefile | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/Documentation/Makefile b/Documentation/Makefile > index d47acb2e25..21375cd3f2 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -351,8 +351,16 @@ $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS) > 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)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< > + > +manpage-prereqs := manpage-base-url.xsl $(wildcard manpage*.xsl) > +manpage-cmd = $(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< > + > +%.1 : %.xml $(manpage-prereqs) > + $(manpage-cmd) > +%.5 : %.xml $(manpage-prereqs) > + $(manpage-cmd) > +%.7 : %.xml $(manpage-prereqs) > + $(manpage-cmd) > > %.xml : %.txt $(ASCIIDOC_DEPS) > $(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@ $< Whether we use eval/define or not (I just tried to avoid the repetition) I think referring to $(DOC_MAN[157]) here probably makes more sense if we're poking at these rules. I.e. in this case the rest of the Makefile is carrying forward what manpages we're generating exactly, so rather than a wildcard %.1 to %.xml we can narrow it down to just the %.1 files we're going to b generating (but maybe that's best left for later...): diff --git a/Documentation/Makefile b/Documentation/Makefile index 5e1a7f655c2..7404cead084 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -351,8 +351,12 @@ $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS) 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)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< +define doc-man-tmpl +$$(DOC_MAN$(1)): %.$(1) : %.xml manpage-base-url.xsl $$(wildcard manpage*.xsl) + $$(QUIET_XMLTO)$$(XMLTO) -m $$(MANPAGE_XSL) $$(XMLTO_EXTRA) man $$< + +endef +$(eval $(foreach n,1 5 7,$(call doc-man-tmpl,$(n)))) %.xml : %.txt $(ASCIIDOC_DEPS) $(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@ $<
On Mon, 2022-11-28 at 14:08 +0100, Ævar Arnfjörð Bjarmason wrote: > Whether we use eval/define or not (I just tried to avoid the > repetition) I think referring to $(DOC_MAN[157]) here probably makes > more sense if we're poking at these rules. > > I.e. in this case the rest of the Makefile is carrying forward what > manpages we're generating exactly, so rather than a wildcard %.1 to > %.xml we can narrow it down to just the %.1 files we're going to b > generating (but maybe that's best left for later...): I have no opinion on which is better :). I'm not sure what the above comment is asking for though: are you going to take over pushing this change? Or do you want me to reroll the commit with these changes instead? Or are we waiting for more opinions? > diff --git a/Documentation/Makefile b/Documentation/Makefile > index 5e1a7f655c2..7404cead084 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -351,8 +351,12 @@ $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS) > 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)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< > +define doc-man-tmpl > +$$(DOC_MAN$(1)): %.$(1) : %.xml manpage-base-url.xsl $$(wildcard manpage*.xsl) > + $$(QUIET_XMLTO)$$(XMLTO) -m $$(MANPAGE_XSL) $$(XMLTO_EXTRA) man $$< > + > +endef > +$(eval $(foreach n,1 5 7,$(call doc-man-tmpl,$(n)))) > > %.xml : %.txt $(ASCIIDOC_DEPS) > $(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@ $<
On Mon, Nov 28 2022, Paul Smith wrote: > On Mon, 2022-11-28 at 14:08 +0100, Ævar Arnfjörð Bjarmason wrote: >> Whether we use eval/define or not (I just tried to avoid the >> repetition) I think referring to $(DOC_MAN[157]) here probably makes >> more sense if we're poking at these rules. >> >> I.e. in this case the rest of the Makefile is carrying forward what >> manpages we're generating exactly, so rather than a wildcard %.1 to >> %.xml we can narrow it down to just the %.1 files we're going to b >> generating (but maybe that's best left for later...): > > I have no opinion on which is better :). > > I'm not sure what the above comment is asking for though: are you going > to take over pushing this change? Or do you want me to reroll the > commit with these changes instead? Or are we waiting for more > opinions? Just a suggestion in case you thought it helped, but I think we can just go for your version.
diff --git a/Documentation/Makefile b/Documentation/Makefile index d47acb2e25..21375cd3f2 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -351,8 +351,16 @@ $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS) 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)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< + +manpage-prereqs := manpage-base-url.xsl $(wildcard manpage*.xsl) +manpage-cmd = $(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $< + +%.1 : %.xml $(manpage-prereqs) + $(manpage-cmd) +%.5 : %.xml $(manpage-prereqs) + $(manpage-cmd) +%.7 : %.xml $(manpage-prereqs) + $(manpage-cmd) %.xml : %.txt $(ASCIIDOC_DEPS) $(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@ $<
A GNU make pattern rule with multiple targets has always meant that a single invocation of the recipe will build all the targets. However in older versions of GNU make a recipe that did not really build all the targets would be tolerated. Starting with GNU make 4.4 this behavior is deprecated and pattern rules are expected to generate files to match all the patterns. If not all targets are created then GNU make will not consider any target up to date and will re-run the recipe when it is run again. Modify Documentation/Makefile to split the man page-creating pattern rule into a separate pattern rule for each pattern. Reported-by: Alexander Kanavin <alex.kanavin@gmail.com> Signed-off-by: Paul Smith <psmith@gnu.org> --- Documentation/Makefile | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)