[4/4] gdb/doc: fix parallel build of pdf and dvi files

Message ID c75d7a4482fa6407339be90cb1321466498616df.1715420013.git.aburgess@redhat.com
State New
Headers
Series gdb/doc: build fixes and improvements |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Andrew Burgess May 11, 2024, 9:37 a.m. UTC
  When building with 'make -j20 -C gdb/doc all-doc' I often see problems
caused from trying to build some dvi files in parallel with some pdf
files.  The problem files are: gdb.dvi and gdb.pdf; stabs.dvi and
stabs.pdf; and annotate.dvi and annotate.pdf.

The problem is that building these files create temporary files in the
local directory.  There's already a race here that two make threads
might try to create these files at the same time.

But it gets worse, to avoid issues where a failed build could leave
these temporary files in a corrupted state, and so prevent the next
build from succeeding, the recipe for each of these files delete all
the temporary files first, this obviously causes problems if some
other thread has already started the build and is relying on these
temporary files.

To work around this problem I propose we start using the --build flag
for texi2dvi (which is the same tool used to create the pdf files).
The --build flag allows for the temporary files to be placed into a
sub-directory, e.g. creating gdb.pdf will create the temporary files
in gdb.t2d/pdf/ and gdb.dvi uses gdb.t2d/dvi/, in this way the
temporary files will never clash, and we can easily delete the
specific sub-directory at the start of the recipe to ensure a
successful build.

Very old versions of texi2dvi don't support --build, so I've added a
configure check for this option.  If the option is not supported then
we don't use it.  This means we fall back to placing temporary files
in the local directory, which means parallel builds will remain
broken.  The --build option is definitely present in texi2dvi version
6.1, from 2016, so any version after that should be fine.

For me, with this fix in place, I can now run 'make -j20 -C gdb/doc
all-doc' without seeing any build problems.
---
 gdb/configure       | 33 +++++++++++++++++++++++++++++++--
 gdb/configure.ac    | 21 +++++++++++++++++++++
 gdb/doc/Makefile.in | 42 +++++++++++++++++++++++++++++-------------
 3 files changed, 81 insertions(+), 15 deletions(-)
  

Comments

Tom Tromey May 13, 2024, 4:18 p.m. UTC | #1
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> When building with 'make -j20 -C gdb/doc all-doc' I often see problems
Andrew> caused from trying to build some dvi files in parallel with some pdf
Andrew> files.  The problem files are: gdb.dvi and gdb.pdf; stabs.dvi and
Andrew> stabs.pdf; and annotate.dvi and annotate.pdf.

Andrew> The problem is that building these files create temporary files in the
Andrew> local directory.  There's already a race here that two make threads
Andrew> might try to create these files at the same time.

Does this mean that without --build the tools here use fixed temporary
file names?  That seems bad.

Anyway the patch seems ok to me.  The configure check is maybe overkill
IMO but considering that it exists it also seems ok.

Approved-By: Tom Tromey <tom@tromey.com>

Tom
  
Andrew Burgess May 14, 2024, 1:40 p.m. UTC | #2
Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> When building with 'make -j20 -C gdb/doc all-doc' I often see problems
> Andrew> caused from trying to build some dvi files in parallel with some pdf
> Andrew> files.  The problem files are: gdb.dvi and gdb.pdf; stabs.dvi and
> Andrew> stabs.pdf; and annotate.dvi and annotate.pdf.
>
> Andrew> The problem is that building these files create temporary files in the
> Andrew> local directory.  There's already a race here that two make threads
> Andrew> might try to create these files at the same time.
>
> Does this mean that without --build the tools here use fixed temporary
> file names?  That seems bad.

Yes, building gdb.dvi produces files matching:

  gdb.aux gdb.cp* gdb.fn* gdb.ky* gdb.log gdb.pg* gdb.toc gdb.tp* gdb.vr*

And building gdb.pdf produces the same set of files.  These two targets
use the same tool with different flags (to create dvi vs pdf).

Indeed this isn't great, which is why (I guess) the tool gained the
--build option.

The other option would be to use the same trick I used with the refcard,
copy the input file to some unique name, e.g. gdb.texi -> gdb_dvi.texi,
then the temporary files will not clash between dvi/pdf targets.

The reason I used that trick for refcard, is that is invoking
(basically) LaTeX directly, which doesn't have a --build option or
equivalent (that I could find).  For these targets (e.g. gdb.{dvi,pdf})
we're using "texi2dvi" or "texi2dvi --pdf", which does have an
"official" way to solve this problem, so I figured using that was
better.

> Anyway the patch seems ok to me.  The configure check is maybe overkill
> IMO but considering that it exists it also seems ok.

It seemed cheap to add, and there are some folk who build on older
systems.  I was worried they might not have this option.

>
> Approved-By: Tom Tromey <tom@tromey.com>

Thanks,
Andrew
  

Patch

diff --git a/gdb/configure b/gdb/configure
index 98cd488a737..f6e8a387172 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -765,6 +765,7 @@  ENABLE_BFD_64_BIT_TRUE
 subdirs
 GDB_DATADIR
 DEBUGDIR
+TEXI2DVI_EXTRA_FLAGS
 MAKEINFO_EXTRA_FLAGS
 MAKEINFOFLAGS
 MAKEINFO
@@ -11499,7 +11500,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11502 "configure"
+#line 11503 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11605,7 +11606,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11608 "configure"
+#line 11609 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -24601,6 +24602,34 @@  if test x"$gdb_cv_have_makeinfo_click" = xyes; then
 fi
 
 
+## Figure out if texi2dvi supports the --build option.  Older versions
+## didn't.  If this option is not supported then parallel building in
+## the docs/ directory is probably going to be broken.  But single
+## threaded build should be fine.
+TEXI2DVI=texi2dvi
+TEXI2DVI_EXTRA_FLAGS=""
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $TEXI2DVI supports --build" >&5
+$as_echo_n "checking whether $TEXI2DVI supports --build... " >&6; }
+if ${gdb_cv_have_texi2dvi_build_opt+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  echo '\input texinfo' >conftest.texi
+   echo 'abc' >>conftest.texi
+   echo '@bye' >>conftest.texi
+   cp conftest.texi /home/andrew/tmp/
+   if eval "$TEXI2DVI --build=tidy conftest.texi >&5 2>&5"; then
+     gdb_cv_have_texi2dvi_build_opt=yes
+   else
+     gdb_cv_have_texi2dvi_build_opt=no
+   fi
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gdb_cv_have_texi2dvi_build_opt" >&5
+$as_echo "$gdb_cv_have_texi2dvi_build_opt" >&6; }
+if test x"$gdb_cv_have_texi2dvi_build_opt" = xyes; then
+  TEXI2DVI_EXTRA_FLAGS="$TEXI2DVI_EXTRA_FLAGS --build=tidy"
+fi
+
+
 
 
 # Check whether --with-separate-debug-dir was given.
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 62ff09cea20..3b726e5f0aa 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -117,6 +117,27 @@  if test x"$gdb_cv_have_makeinfo_click" = xyes; then
 fi
 AC_SUBST(MAKEINFO_EXTRA_FLAGS)
 
+## Figure out if texi2dvi supports the --build option.  Older versions
+## didn't.  If this option is not supported then parallel building in
+## the docs/ directory is probably going to be broken.  But single
+## threaded build should be fine.
+TEXI2DVI=texi2dvi
+TEXI2DVI_EXTRA_FLAGS=""
+AC_CACHE_CHECK([whether $TEXI2DVI supports --build], gdb_cv_have_texi2dvi_build_opt,
+  [echo '\input texinfo' >conftest.texi
+   echo 'abc' >>conftest.texi
+   echo '@bye' >>conftest.texi
+   cp conftest.texi /home/andrew/tmp/
+   if eval "$TEXI2DVI --build=tidy conftest.texi >&5 2>&5"; then
+     gdb_cv_have_texi2dvi_build_opt=yes
+   else
+     gdb_cv_have_texi2dvi_build_opt=no
+   fi])
+if test x"$gdb_cv_have_texi2dvi_build_opt" = xyes; then
+  TEXI2DVI_EXTRA_FLAGS="$TEXI2DVI_EXTRA_FLAGS --build=tidy"
+fi
+AC_SUBST(TEXI2DVI_EXTRA_FLAGS)
+
 GDB_AC_WITH_DIR(DEBUGDIR, separate-debug-dir,
     [look for global separate debug info in this path @<:@LIBDIR/debug@:>@],
     [${libdir}/debug])
diff --git a/gdb/doc/Makefile.in b/gdb/doc/Makefile.in
index c05e561edeb..5836d6ce024 100644
--- a/gdb/doc/Makefile.in
+++ b/gdb/doc/Makefile.in
@@ -68,6 +68,8 @@  TEXI2ROFF=texi2roff
 
 # where to find texi2dvi, ditto
 TEXI2DVI=texi2dvi
+TEXI2DVI_EXTRA_FLAGS=@TEXI2DVI_EXTRA_FLAGS@
+TEXI2DVI_CMD = $(TEXI2DVI) $(TEXI2DVI_EXTRA_FLAGS)
 
 # Package to install the docs under
 PACKAGE = @PACKAGE@
@@ -481,21 +483,26 @@  gdb-cfg.texi: ${srcdir}/${DOC_CONFIG}-cfg.texi
 # Clean these up before each run.  Avoids a catch 22 with not being
 # able to re-generate these files (to fix a corruption) because these
 # files contain a corruption.
+#
+# If TEXI2DVI_CMD includes the --build option then these files should
+# not be created in the current directory, but attempting to delete
+# them is harmless.
 GDB_TEX_TMPS = gdb.aux gdb.cp* gdb.fn* gdb.ky* gdb.log gdb.pg* gdb.toc \
 	gdb.tp* gdb.vr*
 
 # GDB MANUAL: TeX dvi file
 gdb.dvi: ${GDB_DOC_FILES}
-	$(SILENCE) rm -f $(GDB_TEX_TMPS)
-	$(ECHO_TEXI2DVI) $(TEXI2DVI) $(SILENT_Q_FLAG) $(READLINE_TEXI_INCFLAG) \
-		-I ${GDBMI_DIR} -I $(srcdir) $(srcdir)/gdb.texinfo
+	$(SILENCE) rm -fr $(GDB_TEX_TMPS) gdb.t2d/dvi/
+	$(ECHO_TEXI2DVI) $(TEXI2DVI_CMD) $(SILENT_Q_FLAG) \
+		$(READLINE_TEXI_INCFLAG) -I ${GDBMI_DIR} -I $(srcdir) \
+		$(srcdir)/gdb.texinfo
 
 gdb.ps: gdb.dvi
 	$(ECHO_DVIPS) $(DVIPS) $(SILENT_Q_FLAG) -o $@ $?
 
 gdb.pdf: ${GDB_DOC_FILES}
-	$(SILENCE) rm -f $(GDB_TEX_TMPS)
-	$(ECHO_TEXI2DVI) $(TEXI2DVI) $(SILENT_Q_FLAG) --pdf \
+	$(SILENCE) rm -fr $(GDB_TEX_TMPS) gdb.t2d/pdf/
+	$(ECHO_TEXI2DVI) $(TEXI2DVI_CMD) $(SILENT_Q_FLAG) --pdf \
 		$(READLINE_TEXI_INCFLAG) -I ${GDBMI_DIR} -I $(srcdir) \
 		$(srcdir)/gdb.texinfo
 
@@ -596,41 +603,49 @@  stabs/index.html: $(STABS_DOC_FILES)
 # Clean these up before each run.  Avoids a catch 22 with not being
 # able to re-generate these files (to fix a corruption) because these
 # files contain a corruption.
+#
+# If TEXI2DVI_CMD includes the --build option then these files should
+# not be created in the current directory, but attempting to delete
+# them is harmless.
 STABS_TEX_TMPS = stabs.aux stabs.cp* stabs.fn* stabs.ky* \
 	stabs.log stabs.pg* stabs.toc stabs.tp* stabs.vr*
 
 # STABS DOCUMENTATION: TeX dvi file
 stabs.dvi : $(STABS_DOC_FILES)
-	$(SILENCE) rm -f $(STABS_TEX_TMPS)
-	$(ECHO_TEXI2DVI) $(TEXI2DVI) $(SILENT_Q_FLAG) -I $(srcdir) \
+	$(SILENCE) rm -fr $(STABS_TEX_TMPS) stabs.t2d/dvi/
+	$(ECHO_TEXI2DVI) $(TEXI2DVI_CMD) $(SILENT_Q_FLAG) -I $(srcdir) \
 		$(srcdir)/stabs.texinfo
 
 stabs.ps: stabs.dvi
 	$(ECHO_DVIPS) $(DVIPS) $(SILENT_Q_FLAG) -o $@ $?
 
 stabs.pdf: $(STABS_DOC_FILES)
-	$(SILENCE) rm -f $(STABS_TEX_TMPS)
-	$(ECHO_TEXI2DVI) $(TEXI2DVI) $(SILENT_Q_FLAG) --pdf -I $(srcdir) \
+	$(SILENCE) rm -fr $(STABS_TEX_TMPS) stabs.t2d/pdf/
+	$(ECHO_TEXI2DVI) $(TEXI2DVI_CMD) $(SILENT_Q_FLAG) --pdf -I $(srcdir) \
 		$(srcdir)/stabs.texinfo
 
 # Clean these up before each run.  Avoids a catch 22 with not being
 # able to re-generate these files (to fix a corruption) because these
 # files contain a corruption.
+#
+# If TEXI2DVI_CMD includes the --build option then these files should
+# not be created in the current directory, but attempting to delete
+# them is harmless.
 ANNOTATE_TEX_TMPS = annotate.aux annotate.cp* annotate.fn* annotate.ky* \
 	annotate.log annotate.pg* annotate.toc annotate.tp* annotate.vr*
 
 # ANNOTATE DOCUMENTATION: TeX dvi file
 annotate.dvi : $(ANNOTATE_DOC_FILES)
-	$(SILENCE) rm -f $(ANNOTATE_TEX_TMPS)
-	$(ECHO_TEXI2DVI) $(TEXI2DVI) $(SILENT_Q_FLAG) -I $(srcdir) \
+	$(SILENCE) rm -fr $(ANNOTATE_TEX_TMPS) annotate.t2d/dvi/
+	$(ECHO_TEXI2DVI) $(TEXI2DVI_CMD) $(SILENT_Q_FLAG) -I $(srcdir) \
 		$(srcdir)/annotate.texinfo
 
 annotate.ps: annotate.dvi
 	$(ECHO_DVIPS) $(DVIPS) $(SILENT_Q_FLAG) -o $@ $?
 
 annotate.pdf: $(ANNOTATE_DOC_FILES)
-	$(SILENCE) rm -f $(ANNOTATE_TEX_TMPS)
-	$(ECHO_TEXI2DVI) $(TEXI2DVI) $(SILENT_Q_FLAG) --pdf -I $(srcdir) \
+	$(SILENCE) rm -fr $(ANNOTATE_TEX_TMPS) annotate.t2d/pdf/
+	$(ECHO_TEXI2DVI) $(TEXI2DVI_CMD) $(SILENT_Q_FLAG) --pdf -I $(srcdir) \
 		$(srcdir)/annotate.texinfo
 
 annotate.info: $(ANNOTATE_DOC_FILES)
@@ -665,6 +680,7 @@  Makefile: Makefile.in $(host_makefile_frag) ../config.status
 
 mostlyclean:
 	rm -f gdb.mm gdb.ms gdb.me links2roff
+	rm -fr annotate.t2d/ stabs.t2d/ gdb.t2d/
 	rm -f $(GDB_TEX_TMPS)
 	rm -f $(STABS_TEX_TMPS)
 	rm -f $(ANNOTATE_TEX_TMPS)