diff mbox series

[v2,1/2] libada: Remove racy duplicate gnatlib installation

Message ID alpine.LFD.2.21.1909280347010.28360@redsun52.ssa.fujisawa.hgst.com
State Superseded
Headers show
Series [v2,1/2] libada: Remove racy duplicate gnatlib installation | expand

Commit Message

Maciej W. Rozycki Sept. 28, 2019, 4:46 p.m. UTC 
For some reason, presumably historical, the `install-gnatlib' target for 
the default multilib is invoked twice, once via the `ada.install-common' 
target in `gcc/ada/gcc-interface/Make-lang.in' invoked from gcc/ and 
again via the `install-libada' target in libada/.

Apart from doing the same twice this is actually harmful in sufficiently 
parallelized `make' invocation, as the removal of old files performed 
within the `install-gnatlib' recipe in the former case actually races 
with the installation of new files done in the latter case, causing the 
recipe to fail and abort, however non-fatally, having not completed the 
installation of all the built files needed for the newly-built compiler 
to work correctly.

This can be observed with a native `x86_64-linux-gnu' bootstrap:

make[4]: Entering directory '.../gcc/ada'
rm -rf .../lib/gcc/x86_64-linux-gnu/10.0.0/adalib
rm: cannot remove '.../lib/gcc/x86_64-linux-gnu/10.0.0/adalib': Directory not
empty
make[4]: *** [gcc-interface/Makefile:512: install-gnatlib] Error 1
make[4]: Leaving directory '.../gcc/ada'
make[3]: *** [.../gcc/ada/gcc-interface/Make-lang.in:853: install-gnatlib] Error
2
make[2]: [.../gcc/ada/gcc-interface/Make-lang.in:829: ada.install-common] Error
2 (ignored)

which then causes missing files to be reported when an attempt is made 
to use the newly-installed non-functional compiler to build a 
`riscv64-linux-gnu' cross-compiler:

(cd ada/bldtools/sinfo; gnatmake -q xsinfo ; ./xsinfo sinfo.h )
error: "ada.ali" not found, "ada.ads" must be compiled
error: "s-memory.ali" not found, "s-memory.adb" must be compiled
gnatmake: *** bind failed.
/bin/sh: ./xsinfo: No such file or directory
make[2]: *** [.../gcc/ada/Make-generated.in:45: ada/sinfo.h] Error 127
make[2]: Leaving directory '.../gcc'
make[1]: *** [Makefile:4369: all-gcc] Error 2
make[1]: Leaving directory '...'
make: *** [Makefile:965: all] Error 2

Depending on timing `.../lib/gcc/x86_64-linux-gnu/10.0.0/adainclude' may
cause an installation failure instead and the resulting compiler may be 
non-functional in a different way.

Only invoke `install-gnatlib' from within gcc/ then if a legacy build 
process is being used with libada disabled and gnatlib built manually 
with `make -C gcc gnatlib'.

	gcc/
	* Makefile.in (gnat_install_lib): New variable.
	* configure.ac: Substitute it.
	* configure: Regenerate.

	gcc/ada/
	* gcc-interface/Make-lang.in (ada.install-common): Split into...
	(gnat-install-tools, gnat-install-lib): ... these.
---
On Fri, 27 Sep 2019, Arnaud Charlet wrote:

> >  Hmm, I've experimented a bit and AFAICT if `--disable-libada' is given 
> > without my proposed change applied, then the gnatlib stuff doesn't get 
> > built in the first place let alone installed anyway.
> 
> With --disable-libada you need to build gnatlib and gnattools explicitly via
> e.g.
> 
> make -C gcc gnatlib gnattools
> 
> (after having done a "make" or "make bootstrap")
> 
> and then you can use "make install"

 Ack.  This seems to be of a very limited use, e.g. it does not support 
building shared libraries or multilibs, and it breaks in my build of a 
`riscv64-linux-gnu' cross-compiler because the `make -C gcc' invocation 
does not pass the `--sysroot=' option set in SYSROOT_CFLAGS_FOR_TARGET and 
consequently the newly-built compiler cannot find system headers needed to 
build gnatlib (also there is a missing ordering dependency beetween the 
`gnatlib' and `gnattools' targets, making the simultaneous invocation as 
given break in parallel `make', with a `You must first build the GNAT 
library: make gnatlib' message).

 I see no particular reason to break what has been working however, so 
here's my proposed rewritten change.  With so many limitations and its 
mismatch with the current GCC build arrangement and practices I'd consider 
your manual build recipe a legacy one though and would rather not invest 
too much effort into supporting it.  Insted I'd recommend everyone to 
switch to building via gnattools/ and libada/ top-level directories.

 NB one can alternatively use `make -C gcc install-gnatlib' to complement 
the manual build process.

 I have verified this change by running my combined build process where a 
native `x86_64-linux-gnu' configuration is built first and then used to 
build a `riscv64-linux-gnu' cross-compiler, both with `--disable-libada' 
specified, without and with this patch applied.  I have added `make -C gcc 
gnatlib && make -C gcc gnattools' as an extra build step before `make 
install'.

 This has run up to failing to find `riscv64-linux-gnu' system headers in 
`make -C gcc gnatlib' as noted above, at which point the installation 
trees had both the same contents, including `x86_64-linux-gnu' gnatlib 
development files and static libraries as well as gnattools in particular.

 I have also verified, with the same build process and the extra `make -C 
gcc' invocations removed, that the usual configuration with libada enabled 
produces the same result as with v1 of this change.

  Maciej

Changes from v1:

- gnatlib installation now retained in gcc/ada/gcc-interface/Make-lang.in 
  and instead invoked iff `--disable-libada' has been requested at the top 
  level.
---
 gcc/Makefile.in                    |    4 ++++
 gcc/ada/gcc-interface/Make-lang.in |    5 ++++-
 gcc/configure                      |   15 +++++++++++++--
 gcc/configure.ac                   |   10 ++++++++++
 4 files changed, 31 insertions(+), 3 deletions(-)

gcc-lang-no-install-gnatlib.diff

Comments

Maciej W. Rozycki Sept. 29, 2019, 10:29 p.m. UTC | #1 
On Sat, 28 Sep 2019, Maciej W. Rozycki wrote:

> Index: gcc/gcc/ada/gcc-interface/Make-lang.in
> ===================================================================
> --- gcc.orig/gcc/ada/gcc-interface/Make-lang.in
> +++ gcc/gcc/ada/gcc-interface/Make-lang.in
> @@ -843,6 +845,7 @@ doc/gnat-style.pdf: ada/gnat-style.texi
>  #
>  # Finally, install the library
>  #
> +gnat-install-lib: gnat-install-tools

 FYI, after some consideration I've updated this prerequisite to read:

gnat-install-lib: | gnat-install-tools

I think it matches the current behaviour more accurately, though I guess 
we might as well remove the prerequisite altogether as the two recipies 
do not appear to clash with each other (but then the comment will have to 
be updated accordingly, or removed).

 I'll post the final version once committed if you agree with this update.

  Maciej
Arnaud Charlet Oct. 1, 2019, 8:24 a.m. UTC | #2 
>  I have verified this change by running my combined build process where a 
> native `x86_64-linux-gnu' configuration is built first and then used to 
> build a `riscv64-linux-gnu' cross-compiler, both with `--disable-libada' 
> specified, without and with this patch applied.  I have added `make -C gcc 
> gnatlib && make -C gcc gnattools' as an extra build step before `make 
> install'.
> 
>  This has run up to failing to find `riscv64-linux-gnu' system headers in 
> `make -C gcc gnatlib' as noted above, at which point the installation 
> trees had both the same contents, including `x86_64-linux-gnu' gnatlib 
> development files and static libraries as well as gnattools in particular.

Can you also please do a native build with --disable-libada and
make -C gcc gnatlib && make -C gcc gnattools && make install
?

Once successful, the change is OK, thanks for the extra work.

Arno
diff mbox series

Patch

Index: gcc/gcc/Makefile.in
===================================================================
--- gcc.orig/gcc/Makefile.in
+++ gcc/gcc/Makefile.in
@@ -1705,6 +1705,10 @@  $(FULL_DRIVER_NAME): ./xgcc
 # language hooks, generated by configure
 @language_hooks@
 
+# Wire in install-gnatlib invocation with `make install' for a configuration
+# with top-level libada disabled.
+gnat_install_lib = @gnat_install_lib@
+
 # per-language makefile fragments
 ifneq ($(LANG_MAKEFRAGS),)
 include $(LANG_MAKEFRAGS)
Index: gcc/gcc/ada/gcc-interface/Make-lang.in
===================================================================
--- gcc.orig/gcc/ada/gcc-interface/Make-lang.in
+++ gcc/gcc/ada/gcc-interface/Make-lang.in
@@ -822,7 +822,9 @@  doc/gnat-style.pdf: ada/gnat-style.texi
 # gnatlink, gnatls, gnatmake, gnatname, gnatprep, gnatxref, gnatfind,
 # gnatclean).
 # gnatdll is only used on Windows.
-ada.install-common:
+ada.install-common: $(gnat_install_lib) gnat-install-tools
+
+gnat-install-tools:
 	$(MKDIR) $(DESTDIR)$(bindir)
 	-if [ -f gnat1$(exeext) ] ; \
 	then \
@@ -843,6 +845,7 @@  doc/gnat-style.pdf: ada/gnat-style.texi
 #
 # Finally, install the library
 #
+gnat-install-lib: gnat-install-tools
 	-if [ -f gnat1$(exeext) ] ; \
 	then \
 	  $(MAKE) $(COMMON_FLAGS_TO_PASS) $(ADA_FLAGS_TO_PASS) install-gnatlib; \
Index: gcc/gcc/configure
===================================================================
--- gcc.orig/gcc/configure
+++ gcc/gcc/configure
@@ -814,6 +814,7 @@  SET_MAKE
 accel_dir_suffix
 real_target_noncanonical
 enable_as_accelerator
+gnat_install_lib
 REPORT_BUGS_TEXI
 REPORT_BUGS_TO
 PKGVERSION
@@ -7826,6 +7827,16 @@  else
 fi
 
 
+# If top-level libada has been disabled, then wire in install-gnatlib
+# invocation with `make install', so that one can build and install
+# the library manually with `make -C gcc all gnatlib gnattools install'.
+if test x"$enable_libada" = xno; then
+  gnat_install_lib=gnat-install-lib
+else
+  gnat_install_lib=
+fi
+
+
 if test x"$enable_as_accelerator_for" != x; then
 
 $as_echo "#define ACCEL_COMPILER 1" >>confdefs.h
@@ -18819,7 +18830,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 18822 "configure"
+#line 18833 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -18925,7 +18936,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 18928 "configure"
+#line 18939 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
Index: gcc/gcc/configure.ac
===================================================================
--- gcc.orig/gcc/configure.ac
+++ gcc/gcc/configure.ac
@@ -982,6 +982,16 @@  AC_ARG_ENABLE(languages,
 esac],
 [enable_languages=c])
 
+# If top-level libada has been disabled, then wire in install-gnatlib
+# invocation with `make install', so that one can build and install
+# the library manually with `make -C gcc all gnatlib gnattools install'.
+if test x"$enable_libada" = xno; then
+  gnat_install_lib=gnat-install-lib
+else
+  gnat_install_lib=
+fi
+AC_SUBST(gnat_install_lib)
+
 if test x"$enable_as_accelerator_for" != x; then
   AC_DEFINE(ACCEL_COMPILER, 1,
     [Define if this compiler should be built as the offload target compiler.])