| Message ID | 20200213175647.17628-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Convert QAPI doc comments to generate rST instead of texinfo | expand |
Does not work out of the box on my Fedora 30 build host, where
sphinx-build gives me sphinx-build-2. I have to specify
--sphinx-build=/usr/bin/sphinx-build-3 to unbreak it. Which of course
breaks things when I try to build anything before this commit
The appended patch makes it work out of the box. Please consider
squashing it in.
diff --git a/configure b/configure
index 14172909f0..a9d175c400 100755
--- a/configure
+++ b/configure
@@ -584,7 +584,6 @@ query_pkg_config() {
}
pkg_config=query_pkg_config
sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
-sphinx_build=sphinx-build
# If the user hasn't specified ARFLAGS, default to 'rv', just as make does.
ARFLAGS="${ARFLAGS-rv}"
@@ -903,6 +902,7 @@ fi
: ${make=${MAKE-make}}
: ${install=${INSTALL-install}}
+
# We prefer python 3.x. A bare 'python' is traditionally
# python 2.x, but some distros have it as python 3.x, so
# we check that too
@@ -915,6 +915,19 @@ do
break
fi
done
+
+set -x
+sphinx_build=
+for binary in sphinx-build-3 sphinx-build
+do
+ if has "$binary"
+ then
+ sphinx_build=$(command -v "$binary")
+ break
+ fi
+done
+set +x
+
: ${smbd=${SMBD-/usr/sbin/smbd}}
# Default objcc to clang if available, otherwise use CC
@@ -4803,7 +4816,7 @@ has_sphinx_build() {
# sphinx-build doesn't exist at all or if it is too old.
mkdir -p "$TMPDIR1/sphinx"
touch "$TMPDIR1/sphinx/index.rst"
- $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
+ "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
}
# Check if tools are available to build documentation.
On Fri, 14 Feb 2020 at 06:33, Markus Armbruster <armbru@redhat.com> wrote: > > Does not work out of the box on my Fedora 30 build host, where > sphinx-build gives me sphinx-build-2. I have to specify > --sphinx-build=/usr/bin/sphinx-build-3 to unbreak it. Which of course > breaks things when I try to build anything before this commit > > The appended patch makes it work out of the box. Please consider > squashing it in. > > diff --git a/configure b/configure > index 14172909f0..a9d175c400 100755 > --- a/configure > +++ b/configure > @@ -584,7 +584,6 @@ query_pkg_config() { > } > pkg_config=query_pkg_config > sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" > -sphinx_build=sphinx-build > > # If the user hasn't specified ARFLAGS, default to 'rv', just as make does. > ARFLAGS="${ARFLAGS-rv}" > @@ -903,6 +902,7 @@ fi > > : ${make=${MAKE-make}} > : ${install=${INSTALL-install}} > + > # We prefer python 3.x. A bare 'python' is traditionally > # python 2.x, but some distros have it as python 3.x, so > # we check that too > @@ -915,6 +915,19 @@ do > break > fi > done > + > +set -x I guess the set -x / set +x here are accidentally left in debug printing? > +sphinx_build= > +for binary in sphinx-build-3 sphinx-build > +do > + if has "$binary" > + then > + sphinx_build=$(command -v "$binary") > + break > + fi > +done > +set +x > + > : ${smbd=${SMBD-/usr/sbin/smbd}} > > # Default objcc to clang if available, otherwise use CC > @@ -4803,7 +4816,7 @@ has_sphinx_build() { > # sphinx-build doesn't exist at all or if it is too old. > mkdir -p "$TMPDIR1/sphinx" > touch "$TMPDIR1/sphinx/index.rst" > - $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 > + "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 > } This change isn't related to trying sphinx-build-3 -- did you actually need it ? I did think about quoting when I wrote the patch, but looking at existing practice we are all over the place on whether we bother to quote variables containing program names in configure. I think I ended up following the same thing we do for $python, which doesn't quote. Other than that, I'm happy to squash this in, or for you to squash it in if you are otherwise OK taking the first chunk of the patchset via your tree now. (Do you have a preference for whether you take these patches via your tree or I send them in a docs pullreq?) thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Fri, 14 Feb 2020 at 06:33, Markus Armbruster <armbru@redhat.com> wrote: >> >> Does not work out of the box on my Fedora 30 build host, where >> sphinx-build gives me sphinx-build-2. I have to specify >> --sphinx-build=/usr/bin/sphinx-build-3 to unbreak it. Which of course >> breaks things when I try to build anything before this commit >> >> The appended patch makes it work out of the box. Please consider >> squashing it in. >> >> diff --git a/configure b/configure >> index 14172909f0..a9d175c400 100755 >> --- a/configure >> +++ b/configure >> @@ -584,7 +584,6 @@ query_pkg_config() { >> } >> pkg_config=query_pkg_config >> sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" >> -sphinx_build=sphinx-build >> >> # If the user hasn't specified ARFLAGS, default to 'rv', just as make does. >> ARFLAGS="${ARFLAGS-rv}" >> @@ -903,6 +902,7 @@ fi >> >> : ${make=${MAKE-make}} >> : ${install=${INSTALL-install}} >> + >> # We prefer python 3.x. A bare 'python' is traditionally >> # python 2.x, but some distros have it as python 3.x, so >> # we check that too >> @@ -915,6 +915,19 @@ do >> break >> fi >> done >> + >> +set -x > > I guess the set -x / set +x here are accidentally left in > debug printing? Mispasted. I just double-checked these two lines are the only crap I left in. >> +sphinx_build= >> +for binary in sphinx-build-3 sphinx-build >> +do >> + if has "$binary" >> + then >> + sphinx_build=$(command -v "$binary") >> + break >> + fi >> +done >> +set +x >> + >> : ${smbd=${SMBD-/usr/sbin/smbd}} >> >> # Default objcc to clang if available, otherwise use CC >> @@ -4803,7 +4816,7 @@ has_sphinx_build() { >> # sphinx-build doesn't exist at all or if it is too old. >> mkdir -p "$TMPDIR1/sphinx" >> touch "$TMPDIR1/sphinx/index.rst" >> - $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 >> + "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 >> } > > This change isn't related to trying sphinx-build-3 -- > did you actually need it ? If the for loop finds nothing, $sphinx_build remains empty. Quoting the variable seems cleaner. Oh, and if the user passes '--sphinx-build=', $sphinx_build becomes empty. Precedes my fixup. Admittedly a rather silly thing to do. > I did think about quoting when I wrote the patch, > but looking at existing practice we are all over the > place on whether we bother to quote variables containing > program names in configure. I think I ended up following > the same thing we do for $python, which doesn't quote. I tend to omit quotes when it's obvious the variable's value can only be harmless. An empty value isn't. > Other than that, I'm happy to squash this in, or for > you to squash it in if you are otherwise OK taking > the first chunk of the patchset via your tree now. > (Do you have a preference for whether you take these > patches via your tree or I send them in a docs pullreq?) I can do the pull request.
On Fri, 14 Feb 2020 at 12:20, Markus Armbruster <armbru@redhat.com> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > >> # Default objcc to clang if available, otherwise use CC > >> @@ -4803,7 +4816,7 @@ has_sphinx_build() { > >> # sphinx-build doesn't exist at all or if it is too old. > >> mkdir -p "$TMPDIR1/sphinx" > >> touch "$TMPDIR1/sphinx/index.rst" > >> - $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 > >> + "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 > >> } > > > > This change isn't related to trying sphinx-build-3 -- > > did you actually need it ? > > If the for loop finds nothing, $sphinx_build remains empty. Quoting the > variable seems cleaner. Oh, I see. Anyway, yes, happy to have quotes here. thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Fri, 14 Feb 2020 at 12:20, Markus Armbruster <armbru@redhat.com> wrote: >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >> # Default objcc to clang if available, otherwise use CC >> >> @@ -4803,7 +4816,7 @@ has_sphinx_build() { >> >> # sphinx-build doesn't exist at all or if it is too old. >> >> mkdir -p "$TMPDIR1/sphinx" >> >> touch "$TMPDIR1/sphinx/index.rst" >> >> - $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 >> >> + "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 >> >> } >> > >> > This change isn't related to trying sphinx-build-3 -- >> > did you actually need it ? >> >> If the for loop finds nothing, $sphinx_build remains empty. Quoting the >> variable seems cleaner. > > Oh, I see. Anyway, yes, happy to have quotes here. I decided I prefer this as a separate patch, between PATCH 01 and 02. Hmm, maybe I should squash the last hunk into PATCH 01. From 10d174a9f811708807fb60a610e88084f282c222 Mon Sep 17 00:00:00 2001 From: Markus Armbruster <armbru@redhat.com> Date: Fri, 14 Feb 2020 07:33:43 +0100 Subject: [PATCH] configure: Pick sphinx-build-3 when available The next commit will require a sphinx-build that uses Python 3. On some systems, sphinx-build is fine, on others you need to use sphinx-build-3. To keep things working out of the box on both kinds of systems, try sphinx-build-3, then sphinx-build. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- configure | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 14172909f0..4cbeb06b86 100755 --- a/configure +++ b/configure @@ -584,7 +584,6 @@ query_pkg_config() { } pkg_config=query_pkg_config sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" -sphinx_build=sphinx-build # If the user hasn't specified ARFLAGS, default to 'rv', just as make does. ARFLAGS="${ARFLAGS-rv}" @@ -903,6 +902,7 @@ fi : ${make=${MAKE-make}} : ${install=${INSTALL-install}} + # We prefer python 3.x. A bare 'python' is traditionally # python 2.x, but some distros have it as python 3.x, so # we check that too @@ -915,6 +915,17 @@ do break fi done + +sphinx_build= +for binary in sphinx-build-3 sphinx-build +do + if has "$binary" + then + sphinx_build=$(command -v "$binary") + break + fi +done + : ${smbd=${SMBD-/usr/sbin/smbd}} # Default objcc to clang if available, otherwise use CC @@ -4803,7 +4814,7 @@ has_sphinx_build() { # sphinx-build doesn't exist at all or if it is too old. mkdir -p "$TMPDIR1/sphinx" touch "$TMPDIR1/sphinx/index.rst" - $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 + "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 } # Check if tools are available to build documentation. -- 2.21.1
On Fri, 14 Feb 2020 at 17:18, Markus Armbruster <armbru@redhat.com> wrote: > I decided I prefer this as a separate patch, between PATCH 01 and 02. > > Hmm, maybe I should squash the last hunk into PATCH 01. > > > From 10d174a9f811708807fb60a610e88084f282c222 Mon Sep 17 00:00:00 2001 > From: Markus Armbruster <armbru@redhat.com> > Date: Fri, 14 Feb 2020 07:33:43 +0100 > Subject: [PATCH] configure: Pick sphinx-build-3 when available > > The next commit will require a sphinx-build that uses Python 3. On > some systems, sphinx-build is fine, on others you need to use > sphinx-build-3. To keep things working out of the box on both kinds > of systems, try sphinx-build-3, then sphinx-build. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > configure | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/configure b/configure > index 14172909f0..4cbeb06b86 100755 > --- a/configure > +++ b/configure > @@ -584,7 +584,6 @@ query_pkg_config() { > } > pkg_config=query_pkg_config > sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" > -sphinx_build=sphinx-build > > # If the user hasn't specified ARFLAGS, default to 'rv', just as make does. > ARFLAGS="${ARFLAGS-rv}" > @@ -903,6 +902,7 @@ fi > > : ${make=${MAKE-make}} > : ${install=${INSTALL-install}} > + > # We prefer python 3.x. A bare 'python' is traditionally > # python 2.x, but some distros have it as python 3.x, so > # we check that too Stray whitespace change. > @@ -915,6 +915,17 @@ do > break > fi > done > + > +sphinx_build= > +for binary in sphinx-build-3 sphinx-build > +do > + if has "$binary" > + then > + sphinx_build=$(command -v "$binary") > + break > + fi > +done > + > : ${smbd=${SMBD-/usr/sbin/smbd}} > > # Default objcc to clang if available, otherwise use CC > @@ -4803,7 +4814,7 @@ has_sphinx_build() { > # sphinx-build doesn't exist at all or if it is too old. > mkdir -p "$TMPDIR1/sphinx" > touch "$TMPDIR1/sphinx/index.rst" > - $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 > + "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 > } > > # Check if tools are available to build documentation. > -- > 2.21.1 Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Fri, 14 Feb 2020 at 17:18, Markus Armbruster <armbru@redhat.com> wrote: >> I decided I prefer this as a separate patch, between PATCH 01 and 02. >> >> Hmm, maybe I should squash the last hunk into PATCH 01. >> >> >> From 10d174a9f811708807fb60a610e88084f282c222 Mon Sep 17 00:00:00 2001 >> From: Markus Armbruster <armbru@redhat.com> >> Date: Fri, 14 Feb 2020 07:33:43 +0100 >> Subject: [PATCH] configure: Pick sphinx-build-3 when available >> >> The next commit will require a sphinx-build that uses Python 3. On >> some systems, sphinx-build is fine, on others you need to use >> sphinx-build-3. To keep things working out of the box on both kinds >> of systems, try sphinx-build-3, then sphinx-build. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> configure | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/configure b/configure >> index 14172909f0..4cbeb06b86 100755 >> --- a/configure >> +++ b/configure >> @@ -584,7 +584,6 @@ query_pkg_config() { >> } >> pkg_config=query_pkg_config >> sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" >> -sphinx_build=sphinx-build >> >> # If the user hasn't specified ARFLAGS, default to 'rv', just as make does. >> ARFLAGS="${ARFLAGS-rv}" >> @@ -903,6 +902,7 @@ fi >> >> : ${make=${MAKE-make}} >> : ${install=${INSTALL-install}} >> + >> # We prefer python 3.x. A bare 'python' is traditionally >> # python 2.x, but some distros have it as python 3.x, so >> # we check that too > > > Stray whitespace change. I added the blank line to separate the Python check from its surroundings on both sides. I'll drop it. >> @@ -915,6 +915,17 @@ do >> break >> fi >> done >> + >> +sphinx_build= >> +for binary in sphinx-build-3 sphinx-build >> +do >> + if has "$binary" >> + then >> + sphinx_build=$(command -v "$binary") >> + break >> + fi >> +done >> + >> : ${smbd=${SMBD-/usr/sbin/smbd}} >> >> # Default objcc to clang if available, otherwise use CC >> @@ -4803,7 +4814,7 @@ has_sphinx_build() { >> # sphinx-build doesn't exist at all or if it is too old. >> mkdir -p "$TMPDIR1/sphinx" >> touch "$TMPDIR1/sphinx/index.rst" >> - $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 >> + "$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 >> } >> >> # Check if tools are available to build documentation. >> -- >> 2.21.1 > > Otherwise > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Thanks!
diff --git a/configure b/configure index 115dc38085f..0aceb8e50db 100755 --- a/configure +++ b/configure @@ -584,6 +584,7 @@ query_pkg_config() { } pkg_config=query_pkg_config sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" +sphinx_build=sphinx-build # If the user hasn't specified ARFLAGS, default to 'rv', just as make does. ARFLAGS="${ARFLAGS-rv}" @@ -975,6 +976,8 @@ for opt do ;; --python=*) python="$optarg" ;; + --sphinx-build=*) sphinx_build="$optarg" + ;; --gcov=*) gcov_tool="$optarg" ;; --smbd=*) smbd="$optarg" @@ -1677,6 +1680,7 @@ Advanced options (experts only): --make=MAKE use specified make [$make] --install=INSTALL use specified install [$install] --python=PYTHON use specified python [$python] + --sphinx-build=SPHINX use specified sphinx-build [$sphinx_build] --smbd=SMBD use specified smbd [$smbd] --with-git=GIT use specified git [$git] --static enable static build [$static] @@ -4799,7 +4803,7 @@ has_sphinx_build() { # sphinx-build doesn't exist at all or if it is too old. mkdir -p "$TMPDIR1/sphinx" touch "$TMPDIR1/sphinx/index.rst" - sphinx-build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 + $sphinx_build -c "$source_path/docs" -b html "$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1 } # Check if tools are available to build documentation. @@ -6474,6 +6478,9 @@ echo "QEMU_LDFLAGS $QEMU_LDFLAGS" echo "make $make" echo "install $install" echo "python $python ($python_version)" +if test "$docs" != "no"; then + echo "sphinx-build $sphinx_build" +fi echo "slirp support $slirp $(echo_version $slirp $slirp_version)" if test "$slirp" != "no" ; then echo "smbd $smbd" @@ -7503,6 +7510,7 @@ echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak echo "PYTHON=$python" >> $config_host_mak +echo "SPHINX_BUILD=$sphinx_build" >> $config_host_mak echo "CC=$cc" >> $config_host_mak if $iasl -h > /dev/null 2>&1; then echo "IASL=$iasl" >> $config_host_mak diff --git a/Makefile b/Makefile index f0e1a2fc1dc..430bbad0557 100644 --- a/Makefile +++ b/Makefile @@ -1030,7 +1030,7 @@ sphinxdocs: $(MANUAL_BUILDDIR)/devel/index.html \ # Note the use of different doctree for each (manual, builder) tuple; # this works around Sphinx not handling parallel invocation on # a single doctree: https://github.com/sphinx-doc/sphinx/issues/2946 -build-manual = $(call quiet-command,CONFDIR="$(qemu_confdir)" sphinx-build $(if $(V),,-q) -W -b $2 -D version=$(VERSION) -D release="$(FULL_VERSION)" -d .doctrees/$1-$2 $(SRC_PATH)/docs/$1 $(MANUAL_BUILDDIR)/$1 ,"SPHINX","$(MANUAL_BUILDDIR)/$1") +build-manual = $(call quiet-command,CONFDIR="$(qemu_confdir)" $(SPHINX_BUILD) $(if $(V),,-q) -W -b $2 -D version=$(VERSION) -D release="$(FULL_VERSION)" -d .doctrees/$1-$2 $(SRC_PATH)/docs/$1 $(MANUAL_BUILDDIR)/$1 ,"SPHINX","$(MANUAL_BUILDDIR)/$1") # We assume all RST files in the manual's directory are used in it manual-deps = $(wildcard $(SRC_PATH)/docs/$1/*.rst) \ $(wildcard $(SRC_PATH)/docs/$1/*.rst.inc) \