Re: [PATCH] Always organize test artifacts in a directory hierarchy

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 16-01-25 04:23 PM, Simon Marchi wrote:
> On 16-01-17 01:31 AM, Joel Brobecker wrote:
>>>> So it doesn't seem necessary.
>>>
>>> Works for me.  The patch LGTM.  Maybe wait a few more days to
>>> give others a chance to chime in though.
>>
>> LGTM too; and, for the record, I am very happy about this change.
>>
> 
> Hi Joel (and others),
> 
> I was doing some more testing, and found out that this patch breaks testing
> Ada when building in-tree.  I would merge the following patch just before
> the original one.
> 
> How does it look?
> 
> 
> From d69875543609365fdf70f09b30862834b790599a Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Mon, 25 Jan 2016 14:01:25 -0500
> Subject: [PATCH] Fix in-tree, parallel running of Ada tests
> 
> While testing the following patch,
> 
>   [PATCH] Always organize test artifacts in a directory hierarchy
>   https://sourceware.org/ml/gdb-patches/2016-01/msg00133.html
> 
> I noticed that it broke Ada testing.  This lead me to think that
> parallel testing when building in-tree didn't work previously in Ada.
> It is confirmed by this test:
> 
> $ make check TESTS="gdb.ada/fun_addr.exp" -j 2
> ...
> Running ./gdb.ada/fun_addr.exp ...
> FAIL: gdb.ada/fun_addr.exp: compilation foo.adb
> ...
> 
> This patch fixes in-tree parallel testing for Ada, and consequently
> serial and parallel testing when the aforementioned patch is applied.
> 
> The problem originates from the fact that Ada support code cd's to the
> builddir before compiling.  In itself it's not a problem, it allows to
> place intermediate auto-generated files in that directory.  The Ada
> compilation refers to the source file, which is in another directory,
> only by its base name (e.g. foo.adb).  In serial mode, that worked
> because builddir was the same as the source directory (e.g.
> gdb.ada/fun_addr/).  In an out-of-tree build, it works because the
> source directory is added as an include directory (note: this is not the
> same $srcdir as autoconf's):
> 
>   set srcdir [file dirname $source]
>   additional_flags=-I$srcdir
> 
> which becomes:
> 
>   additional_flags=-I/home/emaisin/build/binutils-gdb/gdb/testsuite/gdb.ada/fun_addr
> 
> However, when building in-tree, srcdir is relative: ./gdb.ada/fun_addr.
> When using parallel or always-in-outputs-directory mode, we are cd'ed in
> the outputs directory.  So -I$srcdir is relative to the current
> directory, which is wrong.
> 
> To fix it, I made the TCL variable srcdir (set in site.exp, from which
> everything else is derived) always absolute.  It is done by assigning
> autoconf's abs_srcdir instead of autoconf's srcdir.  This way -I$srcdir
> will always be good, regardless of where we cd'ed to.  A small apparent
> change is that when running tests, DejaGnu will say:
> 
>   Running /tmp/binutils-gdb/gdb/testsuite/gdb.ada/fun_addr.exp ...
> 
> instead of
> 
>   Running ./gdb.ada/fun_addr.exp ...
> 
> I hope it's not too much of an annoyance.  I think that it should make
> the testsuite a tiny bit more robust against other bugs of the same
> class.
> 
> Regtested in & out of tree, only with native target.
> ---
>  gdb/testsuite/Makefile.in | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
> index 7c251d3..38c3052 100644
> --- a/gdb/testsuite/Makefile.in
> +++ b/gdb/testsuite/Makefile.in
> @@ -21,6 +21,7 @@ srcdir = @srcdir@
>  prefix = @prefix@
>  exec_prefix = @exec_prefix@
>  abs_builddir = @abs_builddir@
> +abs_srcdir = @abs_srcdir@
> 
>  target_alias = @target_noncanonical@
>  program_transform_name = @program_transform_name@
> @@ -119,7 +120,7 @@ $(abs_builddir)/site.exp site.exp: ./config.status Makefile
>  	@echo "set target_alias $(target_alias)" >> ./tmp0
>  	@echo "set target_triplet ${target_canonical}" >> ./tmp0
>  	@echo "set build_triplet ${build_canonical}" >> ./tmp0
> -	@echo "set srcdir ${srcdir}" >> ./tmp0
> +	@echo "set srcdir ${abs_srcdir}" >> ./tmp0
>  	@echo "set tool gdb" >> ./tmp0
>  	@echo 'source $${srcdir}/lib/append_gdb_boards_dir.exp' >> ./tmp0
>  	@echo "## All variables above are generated by configure. Do Not Edit ##" >> ./tmp0
> 

Ping.