[PATCH 0/1] ada: Make the names of uninstalled cross-gnattools consistent across builds

[PATCH 0/1] ada: Make the names of uninstalled cross-gnattools consistent across builds

[Maciej W. Rozycki]

Hi,

 Having rebuilt GCC with no changes relevant to Ada I saw all the gnat 
tests fail all of a sudden.  Upon a closer inspection I have noticed that 
in the earlier build where tests passed `gnatmake' was invoked (in an 
`alpha-linux-gnu' cross-compiler build) as:

/path/to/alpha-linux/obj/gcc/gcc/gnatmake --GCC=/path/to/alpha-linux/obj/gcc/gcc/xgcc --GNATBIND=/path/to/alpha-linux/obj/gcc/gcc/gnatbind --GNATLINK=/path/to/alpha-linux/obj/gcc/gcc/gnatlink -cargs -B/path/to/alpha-linux/obj/gcc/gcc -largs --GCC=/path/to/alpha-linux/obj/gcc/gcc/xgcc -B/path/to/alpha-linux/obj/gcc/gcc  -margs --RTS=/path/to/alpha-linux/obj/gcc/alpha-linux-gnu/./libada [...]

while in the later one it was instead invoked as:

alpha-linux-gnu-gnatmake --RTS=/path/to/alpha-linux/obj/gcc/alpha-linux-gnu/./libada [...]

so rather than the uninstalled program a previously installed one from the 
install root in /path/to/alpha-linux/install/usr/bin was run with all the 
options pointing to the build tree missing, causing failures such as:

FAIL: gnat.dg/abstract1.adb (test for excess errors)
Excess errors:
alpha-linux-gnu-gcc: fatal error: cannot execute 'gnat1': posix_spawnp: No such file or directory
compilation terminated.

(why `alpha-linux-gnu-gcc' couldn't find `gnat1' via a relative path to 
its libexec dir while it finds `cc1', etc. just fine is another matter).

 I have tracked down the cause to a small difference in the two build 
scripts that I use: one runs `make pdf' after the main build and the other 
one does not.  The latter one makes the gnat testsuite work, while the 
former one causes it to fail.

 Now one might ask themselves: "Why would building PDF documentation
matter for the gnat testsuite?"  Indeed, a good question, with a 
surprising answer.  It boils down to how uninstalled cross-gnattools are 
each named.  There is a hook in gcc/ada/gcc-interface/Make-lang.in, which 
has this:

ada.all.cross:
	for tool in $(ADA_TOOLS) ; do \
	  if [ -f $$tool$(exeext) ] ; \
	  then \
	    $(MV) $$tool$(exeext) $$tool-cross$(exeext); \
	  fi; \
	done

Clearly cross-gnattools are meant to be called `gnatmake-cross', etc. in 
their uninstalled forms and this hook is supposed to take care of it.  
However in a normal build of the compiler the hook is invoked too early, 
before gnattools have been compiled and given that it's all conditional it 
silently does nothing.  Now upon a reinvocation of `make' to build PDF 
documentation gnattools will have already been built and are there in 
place, so the hook faithfully renames the executables as told:

[...]
make[4]: Nothing to be done for 'pdf'.
Making pdf in mpcheck
make[4]: Nothing to be done for 'pdf'.
make[4]: Nothing to be done for 'pdf-am'.
make[3]: Nothing to be done for 'pdf-am'.
make[2]: Entering directory '/path/to/alpha-linux/obj/gcc/gcc'
for tool in gnatbind gnatchop gnat gnatkr gnatlink gnatls gnatmake gnatname gnatprep gnatclean ; do \
  if [ -f $tool ] ; \
  then \
    mv $tool $tool-cross; \
  fi; \
done
make[2]: Leaving directory '/path/to/alpha-linux/obj/gcc/gcc'
make[1]: Entering directory '/path/to/alpha-linux/obj/gcc'
[...]

Oh well!

 And then `make install' does not care, because it has:

gnat-install-tools:
	$(MKDIR) $(DESTDIR)$(bindir)
	-if [ -f gnat1$(exeext) ] ; \
	then \
	  for tool in $(ADA_TOOLS) ; do \
	    install_name=`echo $$tool|sed '$(program_transform_name)'`$(exeext); \
	    $(RM) $(DESTDIR)$(bindir)/$$install_name; \
	    if [ -f $$tool-cross$(exeext) ] ; \
	    then \
	      $(INSTALL_PROGRAM) $$tool-cross$(exeext) $(DESTDIR)$(bindir)/$$install_name; \
	    else \
	      $(INSTALL_PROGRAM) $$tool$(exeext) $(DESTDIR)$(bindir)/$$install_name; \
	    fi ; \
	  done; \
	  $(RM) $(DESTDIR)$(bindir)/gnatdll$(exeext); \
	  $(INSTALL_PROGRAM) gnatdll$(exeext) $(DESTDIR)$(bindir)/gnatdll$(exeext); \
	fi

so it works regardless of whether the names have a `-cross' suffix or not.

 Then the test harness which looks for `gnatmake' in the build tree 
rather than `gnatmake-cross' fails to find the executable and resorts to 
generic `alpha-linux-gnu-gnatmake', having skipped all the logic to figure 
out the options pointing to the build tree.  Looking for `gnatmake' rather 
than `gnatmake-cross' in the cross-compilation case is the second bug 
here.

 This seems like a long-standing issue as there haven't been changes in 
these areas since forever and I guess nobody noticed this because they 
didn't invoke `make pdf' or a similar target that would cause the 
`ada.all.cross' `make' target to trigger again after the main build.

 This patch addresses the issue by moving the renaming of cross-gnattools 
to separate `make' targets/recipes, explicitly invoked after gnattools has 
been built in a cross-compiler configuration, while adjusting the test 
harness is accordingly.  I have verified with `alpha-linux-gnu' and 
`riscv64-linux-gnu' configurations that this works correctly.  Now 
`gnatmake' is invoked as:

/path/to/alpha-linux/obj/gcc/gcc/gnatmake-cross --GCC=/path/to/alpha-linux/obj/gcc/gcc/xgcc --GNATBIND=/path/to/alpha-linux/obj/gcc/gcc/gnatbind --GNATLINK=/path/to/alpha-linux/obj/gcc/gcc/gnatlink -cargs -B/path/to/alpha-linux/obj/gcc/gcc -largs --GCC=/path/to/alpha-linux/obj/gcc/gcc/xgcc\ -B/path/to/alpha-linux/obj/gcc/gcc\  -margs --RTS=/path/to/alpha-linux/obj/gcc/alpha-linux-gnu/./libada [...]

I have also verified `powerpc64le-linux-gnu' native configuration to see 
no change in how `gnatmake' is called and no regressions.

 This has started as a two-part patch series, but then I realised that 
decoupling the Makefile changes from test harness updates would break 
testing for someone for the intermediate repository state regardless of 
the order the two patches would come in.  While transient and unlikely to 
affect anyone right now, it could cause trouble with bisecting in the 
future.

 So I chose to merge them into one, even though this breaks the principle 
of one feature per repository change.  However I also chose to retain this 
cover letter even though for a single patch, so as not to clutter the 
submission of the change itself with this lengthy discussion bringing in 
the context.  Some of the observations made here have been necessarily 
repeated in the change description.

 Questions, comments, OK to apply?

  Maciej

[PATCH 1/1] ada: Make the names of uninstalled cross-gnattools consistent across builds

[Maciej W. Rozycki]

We suffer from an inconsistency in the names of uninstalled gnattools 
executables in cross-compiler configurations.  The cause is a recipe we 
have:

ada.all.cross:
	for tool in $(ADA_TOOLS) ; do \
	  if [ -f $$tool$(exeext) ] ; \
	  then \
	    $(MV) $$tool$(exeext) $$tool-cross$(exeext); \
	  fi; \
	done

the intent of which is to give the names of gnattools executables the 
'-cross' suffix, consistently with the compiler drivers: 'gcc-cross', 
'g++-cross', etc.

A problem with the recipe is that this 'make' target is called too early 
in the build process, before gnattools have been made.  Consequently no 
renames happen and owing to that they are conditional on the presence of 
the individual executables the recipe succeeds doing nothing.

However if a target is requested later on such as 'make pdf' that does 
not cause gnattools executables to be rebuilt, then 'ada.all.cross' does 
succeed in renaming the executables already present in the build tree.  
Then if the 'gnat' testsuite is run later on which expects non-suffixed 
'gnatmake' executable, it does not find the 'gnatmake-cross' executable 
in the build tree and may either catastrophically fail or incorrectly 
use a system-installed copy of 'gnatmake'.

Of course if a target is requested such as `make all' that does cause 
gnattools executables to be rebuilt, then both suffixed and non-suffixed 
uninstalled executables result.

Fix the problem by moving the renaming of gnattools to a separate 'make' 
recipe, pasted into a new 'gnattools-cross-mv' target and the existing 
legacy 'cross-gnattools' target.  Then invoke the new target explicitly 
from the 'gnattools-cross' recipe in gnattools/.

Update the test harness accordingly, so that suffixed gnattools are used 
in cross-compilation testsuite runs.

	gcc/
	* ada/gcc-interface/Make-lang.in (ada.all.cross): Move recipe 
	to...
	(GNATTOOLS_CROSS_MV): ... this new variable.
	(cross-gnattools): Paste it here.
	(gnattools-cross-mv): New target.

	gnattools/
	* Makefile.in (gnattools-cross): Also build 'gnattools-cross-mv' 
	in GCC_DIR.

	gcc/testsuite/
	* lib/gnat.exp (local_find_gnatmake, find_gnatclean): Use 
	'-cross' suffix where testing a cross-compiler.
---
 gcc/ada/gcc-interface/Make-lang.in |   19 ++++++++++++-------
 gcc/testsuite/lib/gnat.exp         |   22 ++++++++++++++++++----
 gnattools/Makefile.in              |    1 +
 3 files changed, 31 insertions(+), 11 deletions(-)

gcc-ada-all-cross-gnattools.diff
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
@@ -780,6 +780,7 @@ gnattools: $(GCC_PARTS) $(CONFIG_H) pref
 cross-gnattools: force
 	$(MAKE) -C ada $(ADA_TOOLS_FLAGS_TO_PASS) gnattools1-re
 	$(MAKE) -C ada $(ADA_TOOLS_FLAGS_TO_PASS) gnattools2
+	$(GNATTOOLS_CROSS_MV)
 
 canadian-gnattools: force
 	$(MAKE) -C ada $(ADA_TOOLS_FLAGS_TO_PASS) gnattools1-re
@@ -795,19 +796,23 @@ gnatlib gnatlib-sjlj gnatlib-zcx gnatlib
 	   FORCE_DEBUG_ADAFLAGS="$(FORCE_DEBUG_ADAFLAGS)" \
 	   $@
 
+gnattools-cross-mv:
+	$(GNATTOOLS_CROSS_MV)
+
+GNATTOOLS_CROSS_MV=\
+  for tool in $(ADA_TOOLS) ; do \
+    if [ -f $$tool$(exeext) ] ; \
+    then \
+      $(MV) $$tool$(exeext) $$tool-cross$(exeext); \
+    fi; \
+  done
+
 # use only for native compiler
 gnatlib_and_tools: gnatlib gnattools
 
 # Build hooks:
 
 ada.all.cross:
-	for tool in $(ADA_TOOLS) ; do \
-	  if [ -f $$tool$(exeext) ] ; \
-	  then \
-	    $(MV) $$tool$(exeext) $$tool-cross$(exeext); \
-	  fi; \
-	done
-
 ada.start.encap:
 ada.rest.encap:
 ada.man:
Index: gcc/gcc/testsuite/lib/gnat.exp
===================================================================
--- gcc.orig/gcc/testsuite/lib/gnat.exp
+++ gcc/gcc/testsuite/lib/gnat.exp
@@ -199,12 +199,19 @@ proc prune_gnat_output { text } {
 # which prevent multilib from working, so define a new one.
 
 proc local_find_gnatmake {} {
+    global target_triplet
     global tool_root_dir
+    global host_triplet
 
     if ![is_remote host] {
-        set file [lookfor_file $tool_root_dir gnatmake]
+	if { "$host_triplet" == "$target_triplet" } {
+	    set gnatmake gnatmake
+	} else {
+	    set gnatmake gnatmake-cross
+	}
+	set file [lookfor_file $tool_root_dir $gnatmake]
         if { $file == "" } {
-	    set file [lookfor_file $tool_root_dir gcc/gnatmake]
+	    set file [lookfor_file $tool_root_dir gcc/$gnatmake]
         }
         if { $file != "" } {
 	    set root [file dirname $file]
@@ -225,12 +232,19 @@ proc local_find_gnatmake {} {
 }
 
 proc find_gnatclean {} {
+    global target_triplet
     global tool_root_dir
+    global host_triplet
 
     if ![is_remote host] {
-        set file [lookfor_file $tool_root_dir gnatclean]
+	if { "$host_triplet" == "$target_triplet" } {
+	    set gnatclean gnatclean
+	} else {
+	    set gnatclean gnatclean-cross
+	}
+	set file [lookfor_file $tool_root_dir $gnatclean]
         if { $file == "" } {
-	    set file [lookfor_file $tool_root_dir gcc/gnatclean]
+	    set file [lookfor_file $tool_root_dir gcc/$gnatclean]
         }
         if { $file != "" } {
 	    set gnatclean $file;
Index: gcc/gnattools/Makefile.in
===================================================================
--- gcc.orig/gnattools/Makefile.in
+++ gcc/gnattools/Makefile.in
@@ -223,6 +223,7 @@ gnattools-cross: $(GCC_DIR)/stamp-tools
 	# gnattools2
 	$(MAKE) -C $(GCC_DIR)/ada/tools -f ../Makefile \
 	  $(TOOLS_FLAGS_TO_PASS_CROSS) common-tools
+	$(MAKE) -C $(GCC_DIR) gnattools-cross-mv
 
 # Other
 # -----