Re: [PATCH v2 1/8] Makefile: Ensure we don't run Sphinx in parallel for manpages

[Alex Bennée]

Peter Maydell <address@hidden> writes:

> Sphinx will corrupt its doctree cache if we run two copies
> of it in parallel. In commit 6bda415c10d966c8d3 we worked
> around this by having separate doctrees for 'html' vs 'manpage'
> runs. However now that we have more than one manpage produced
> from a single manual we can run into this again when trying
> to produce the two manpages.
>
> Use the trick described in 'Atomic Rules in GNU Make'
> https://www.cmcrossroads.com/article/atomic-rules-gnu-make
> to ensure that we only run the Sphinx manpage builder once
> for each manual, even if we're producing several manpages.
> This fixes doctree corruption in parallel builds and also
> avoids pointlessly running Sphinx more often than we need to.
>
> (In GNU Make 4.3 there is builtin support for this, via
> the "&:" syntax, but we can't wait for that to be available
> in all the distros we support...)
>
> The generic "one invocation for multiple output files"
> machinery is provided as a macro named 'atomic' in rules.mak;
> we then wrap this in a more specific macro for defining
> the rule and dependencies for the manpages in a Sphinx
> manual, to avoid excessive repetition.
>
> Signed-off-by: Peter Maydell <address@hidden>

I won't claim I fully follow the invocation but it works and I have
tested it.

Reviewed-by: Alex Bennée <address@hidden>
Tested-by: Alex Bennée <address@hidden>


> ---
>  Makefile  | 17 ++++++++++-------
>  rules.mak | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 04c77d3b962..9b7ff1dc82f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1028,6 +1028,14 @@ build-manual = $(call 
> quiet-command,CONFDIR="$(qemu_confdir)" sphinx-build $(if
>  manual-deps = $(wildcard $(SRC_PATH)/docs/$1/*.rst) \
>                $(wildcard $(SRC_PATH)/docs/$1/*.rst.inc) \
>                $(SRC_PATH)/docs/$1/conf.py $(SRC_PATH)/docs/conf.py
> +# Macro to write out the rule and dependencies for building manpages
> +# Usage: $(call define-manpage-rule,manualname,manpage1 
> manpage2...[,extradeps])
> +# 'extradeps' is optional, and specifies extra files (eg .hx files) that
> +# the manual page depends on.
> +define define-manpage-rule
> +$(call atomic,$(foreach manpage,$2,$(MANUAL_BUILDDIR)/$1/$(manpage)),$(call 
> manual-deps,$1) $3)
> +     $(call build-manual,$1,man)
> +endef
>  
>  $(MANUAL_BUILDDIR)/devel/index.html: $(call manual-deps,devel)
>       $(call build-manual,devel,html)
> @@ -1041,14 +1049,9 @@ $(MANUAL_BUILDDIR)/specs/index.html: $(call 
> manual-deps,specs)
>  $(MANUAL_BUILDDIR)/system/index.html: $(call manual-deps,system)
>       $(call build-manual,system,html)
>  
> -$(MANUAL_BUILDDIR)/interop/qemu-ga.8: $(call manual-deps,interop)
> -     $(call build-manual,interop,man)
> +$(call define-manpage-rule,interop,qemu-ga.8 qemu-nbd.8)
>  
> -$(MANUAL_BUILDDIR)/interop/qemu-nbd.8: $(call manual-deps,interop)
> -     $(call build-manual,interop,man)
> -
> -$(MANUAL_BUILDDIR)/system/qemu-block-drivers.7: $(call manual-deps,system)
> -     $(call build-manual,system,man)
> +$(call define-manpage-rule,system,qemu-block-drivers.7)
>  
>  $(MANUAL_BUILDDIR)/index.html: $(SRC_PATH)/docs/index.html.in qemu-version.h
>       @mkdir -p "$(MANUAL_BUILDDIR)"
> diff --git a/rules.mak b/rules.mak
> index 967295dd2b6..50f6776f529 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -399,3 +399,39 @@ GEN_SUBST = $(call quiet-command, \
>  
>  %.json: %.json.in
>       $(call GEN_SUBST)
> +
> +# Support for building multiple output files by atomically executing
> +# a single rule which depends on several input files (so the rule
> +# will be executed exactly once, not once per output file, and
> +# not multiple times in parallel.) For more explanation see:
> +# https://www.cmcrossroads.com/article/atomic-rules-gnu-make
> +
> +# Given a space-separated list of filenames, create the name of
> +# a 'sentinel' file to use to indicate that they have been built.
> +# We use fixed text on the end to avoid accidentally triggering
> +# automatic pattern rules, and . on the start to make the file
> +# not show up in ls output.
> +sentinel = .$(subst $(SPACE),_,$(subst /,_,$1)).sentinel.
> +
> +# Define an atomic rule that builds multiple outputs from multiple inputs.
> +# To use:
> +#    $(call atomic,out1 out2 ...,in1 in2 ...)
> +#    <TAB>rule to do the operation
> +#
> +# Make 4.3 will have native support for this, and you would be able
> +# to instead write:
> +#    out1 out2 ... &: in1 in2 ...
> +#    <TAB>rule to do the operation
> +#
> +# The way this works is that it creates a make rule
> +# "out1 out2 ... : sentinel-file ; @:" which says that the sentinel
> +# depends on the dependencies, and the rule to do that is "do nothing".
> +# Then we have a rule
> +# "sentinel-file : in1 in2 ..."
> +# whose commands start with "touch sentinel-file" and then continue
> +# with the rule text provided by the user of this 'atomic' function.
> +# The foreach... is there to delete the sentinel file if any of the
> +# output files don't exist, so that we correctly rebuild in that situation.
> +atomic = $(eval $1: $(call sentinel,$1) ; @:) \
> +         $(call sentinel,$1) : $2 ; @touch $$@ \
> +         $(foreach t,$1,$(if $(wildcard $t),,$(shell rm -f $(call 
> sentinel,$1))))


-- 
Alex Bennée