[v2,0/4] Fix library testsuite compilation for build sysroot

Hi,

 This is v2 of patch series, originally posted here:

<https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00767.html>
<https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00768.html>
<https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00769.html>
<https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00770.html>

meant to address a problem with the testsuite compiler being set up across 
libatomic, libffi, libgo, libgomp with no correlation whatsoever to the 
target compiler being used in GCC compilation.  Consequently there in no 
arrangement made to set up the compilation sysroot according to the build 
sysroot specified for GCC compilation, causing a catastrophic failure 
across the testsuites affected from the inability to link executables.

 Julian reported an issue triggered in his standalone test environment by 
the original version of the libgomp part of this patch series, which is 
one of the two patches from the series that have been already applied.

 The cause has been the newly-added definition of GCC_UNDER_TEST embedded 
in the generated libgomp/testsuite/libgomp-test-support.exp configuration 
file, which is used in Julian's setup which also relies on the invocation 
of `[find_gcc]' to locate the correct compiler to use.

 This invocation is unsuitable for in-tree testing as we want to use 
exactly the same compiler invocation, as recorded in $(CC), as used to 
build the library.  So I have worked with Chung-Lin on a replacement 
version, which would work with such standalone testing while still 
satisfying the original requirement to use $(CC).

 In the end I have decided to use the documented `--tool_exec' option to 
`runtest' to contain the change within the testsuite's Makefile and its 
`check' goal, which is inherent to the build tree and as such not supposed 
to be used in standalone testing, like with `contrib/test_installed'.

 I am quite sure the problem is specific to libgomp only, as it's the only 
of the four libraries handled that had a preexisting *-test-support.exp 
test configuration file, and the remaining three only got one with the 
changes from the series.  However to keep things consistent across the 
tree I propose to use the `--tool_exec' option for all four libraries.

 This in particular means reverting the whole of the libgomp change as 
well as a part of the libgo change, both already applied, in addition to 
making further adjustments.  For libatomic and libffi this updated 
proposal merely replaces the original one.

 Verified with a cross-compiler configured for the `riscv-linux-gnu' 
target and the `x86_64-linux-gnu' host and using RISC-V/Linux QEMU in the 
user emulation mode as the target board.  Also no change in results with 
`x86_64-linux-gnu' native regression testing.

 See individual change descriptions for details.

 I'm assuming Ian will take care of the 3/4 libgo change; OK to apply the 
remaining ones to the GCC repo?

  Maciej

On Feb 13, 2020, at 3:36 PM, Maciej W. Rozycki <macro@wdc.com> wrote:
> 
> This is v2 of patch series, originally posted here:
> 
> <https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00767.html>
> <https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00768.html>
> <https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00769.html>
> <https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00770.html>
> 
> meant to address a problem with the testsuite compiler being set up across 
> libatomic, libffi, libgo, libgomp with no correlation whatsoever to the 
> target compiler being used in GCC compilation.

> In the end I have decided to use the documented `--tool_exec' option to 
> `runtest' to contain the change within the testsuite's Makefile and its 
> `check' goal, which is inherent to the build tree and as such not supposed 
> to be used in standalone testing, like with `contrib/test_installed'.


> I'm assuming Ian will take care of the 3/4 libgo change; OK to apply the 
> remaining ones to the GCC repo?

So, I really, really would like to avoid additional arguments like this.  I'd prefer that instead you push content into the built site.exp from the Makefile, or something else like this, and then use that content as you need to.  This preserves the ability to go where you need to in the tree, and do a runtest without specifying the option.

Mike, Chung-Lin --

> > In the end I have decided to use the documented `--tool_exec' option to 
> > `runtest' to contain the change within the testsuite's Makefile and its 
> > `check' goal, which is inherent to the build tree and as such not supposed 
> > to be used in standalone testing, like with `contrib/test_installed'.
> 
> 
> > I'm assuming Ian will take care of the 3/4 libgo change; OK to apply the 
> > remaining ones to the GCC repo?
> 
> So, I really, really would like to avoid additional arguments like this.  
> I'd prefer that instead you push content into the built site.exp from 
> the Makefile, or something else like this, and then use that content as 
> you need to.  This preserves the ability to go where you need to in the 
> tree, and do a runtest without specifying the option.

 Thank you, Mike, for your input.  That is what v1 did, but it seems to 
clash with some people's expectations, as discussed here:

<https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00057.html>

So it looks like we have conflicting expectations for the desired 
arrangement, and of course the current situation isn't right either.

 Or, Chung-Lin, will you be happy if we just stay away from 
libgomp/testsuite/libgomp-test-support.exp and use say 
libgomp/testsuite/libgomp-test-support-extra.exp to supply the compiler 
setting (EXTRA_DEJAGNU_SITE_CONFIG supports an arbitrary number of 
site.exp fragments)?

 Any other proposals?

  Maciej

On Feb 14, 2020, at 11:57 AM, Maciej W. Rozycki <macro@wdc.com> wrote:
> 
> Mike, Chung-Lin --
> 
>>> In the end I have decided to use the documented `--tool_exec' option to 
>>> `runtest' to contain the change within the testsuite's Makefile and its 
>>> `check' goal, which is inherent to the build tree and as such not supposed 
>>> to be used in standalone testing, like with `contrib/test_installed'.
>> 
>> 
>>> I'm assuming Ian will take care of the 3/4 libgo change; OK to apply the 
>>> remaining ones to the GCC repo?
>> 
>> So, I really, really would like to avoid additional arguments like this.  
>> I'd prefer that instead you push content into the built site.exp from 
>> the Makefile, or something else like this, and then use that content as 
>> you need to.  This preserves the ability to go where you need to in the 
>> tree, and do a runtest without specifying the option.
> 
> Thank you, Mike, for your input.  That is what v1 did, but it seems to 
> clash with some people's expectations, as discussed here:
> 
> <https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00057.html>

I didn't see any clash with expectations.  They expect in the build tree testing to work, they expect install testing to work.  I expect both, you expect both; I just don't think we differ on this.  Where I think things went wrong is there is a change to their testing environment that you didn't fully account for in your patch. You didn't intend to break it, but there is just a little more work to be done to not break it, that's all.

So, let step back.  You want to change the options to the compiler, right?

So, imagine for a second that site.exp has those options, then you can reliably grab them from that file and use them, no?  If so, then all you have to do is put the content you need into site.exp, and then once it is there, you can then merely add a little code to grab them and add them, right?

So, pick testsuite that fails, and that you broke.  Back out the last patch, and instead, in the Makefile (or whatever generate the Makefile, look for the site.exp rule, and instead pilfer the data you want create it up into an existing variable, if relevant to it, and if not, create a new one.

For some of these files we see they come from automake, so we google, then we find:

http://gnu-automake.7480.n7.nabble.com/Automake-and-dejagnu-s-site-exp-file-td4414.html

and presto, a 5 line solution to an existing problem.

Is that kinda sort similar to what you want to do?  Now that's a 12 year old answer the the problem, I didn't check to see if there is a newer answer.  It looks reasonable enough.

On Fri, 14 Feb 2020, Mike Stump wrote:

> > Thank you, Mike, for your input.  That is what v1 did, but it seems to 
> > clash with some people's expectations, as discussed here:
> > 
> > <https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00057.html>
> 
> I didn't see any clash with expectations.  They expect in the build tree 
> testing to work, they expect install testing to work.  I expect both, 
> you expect both; I just don't think we differ on this.  Where I think 
> things went wrong is there is a change to their testing environment that 
> you didn't fully account for in your patch. You didn't intend to break 
> it, but there is just a little more work to be done to not break it, 
> that's all.
> 
> So, let step back.  You want to change the options to the compiler, 
> right?
> 
> So, imagine for a second that site.exp has those options, then you can 
> reliably grab them from that file and use them, no?  If so, then all you 
> have to do is put the content you need into site.exp, and then once it 
> is there, you can then merely add a little code to grab them and add 
> them, right?
> 
> So, pick testsuite that fails, and that you broke.  Back out the last 
> patch, and instead, in the Makefile (or whatever generate the Makefile, 
> look for the site.exp rule, and instead pilfer the data you want create 
> it up into an existing variable, if relevant to it, and if not, create a 
> new one.
> 
> For some of these files we see they come from automake, so we google, 
> then we find:
> 
> http://gnu-automake.7480.n7.nabble.com/Automake-and-dejagnu-s-site-exp-file-td4414.html
> 
> and presto, a 5 line solution to an existing problem.
> 
> Is that kinda sort similar to what you want to do?  Now that's a 12 year 
> old answer the the problem, I didn't check to see if there is a newer 
> answer.  It looks reasonable enough.

 Huh?  That's exactly what v1 did, it added:

set GCC_UNDER_TEST {@CC@}

(with @CC@ substituted by `configure' of course) to `site.exp' produced by 
`make check' (as constructed by `automake'[1]).  That caused Julian and 
Chung-Lin trouble.

 So I don't really know what you are talking about or trying to suggest 
me; AFAICT your idea was already tried and failed (though Chung-Lin may 
provide further input I asked for that may help).

References:

[1] <https://www.gnu.org/software/automake/manual/html_node/DejaGnu-Tests.html>

  Maciej

On Sat, 15 Feb 2020, Maciej W. Rozycki wrote:

> > > Thank you, Mike, for your input.  That is what v1 did, but it seems to 
> > > clash with some people's expectations, as discussed here:
> > > 
> > > <https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00057.html>
> > 
> > I didn't see any clash with expectations.  They expect in the build tree 
> > testing to work, they expect install testing to work.  I expect both, 
> > you expect both; I just don't think we differ on this.  Where I think 
> > things went wrong is there is a change to their testing environment that 
> > you didn't fully account for in your patch. You didn't intend to break 
> > it, but there is just a little more work to be done to not break it, 
> > that's all.

 FAOD the clash with expectations is in that you want a fix in `site.exp' 
while Chung-Lin wants one outside `site.exp'.  The status quo does not 
clash with either of you, but it does not meet my requirements, though I 
don't care where the fix goes.  I hope this makes things clear here.

  Maciej