[patch] Fix parallel gdb/ make check//% site.exp

[patch] Fix parallel gdb/ make check//% site.exp

[Jan Kratochvil]

Hi,

sometimes `make check//unix/-m32' just did not produce any results.

Caught it is due to site.exp existing in gdb/testsuite/ thus satisfying VPATH
make dependency on it but runtest does not execute anything without site.exp
file in its current directory.


Regards,
Jan


gdb/testsuite/
2009-11-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* Makefile.in (check-single, $(TEST_TARGETS), check-gdb.base%): Change
	`site.exp' dependency to `./site.exp'.

--- gdb/testsuite/Makefile.in	19 Oct 2009 09:51:43 -0000	1.28
+++ gdb/testsuite/Makefile.in	9 Nov 2009 12:12:52 -0000
@@ -147,7 +147,7 @@ DO_RUNTEST = \
 	  export TCL_LIBRARY ; fi ; \
 	$(RUNTEST)
 
-check-single: all site.exp
+check-single: all ./site.exp
 	$(DO_RUNTEST) $(RUNTESTFLAGS)
 
 # A list of all directories named "gdb.*" which also hold a .exp file.
@@ -170,7 +170,7 @@ check-parallel:
 	$(SHELL) $(srcdir)/dg-extract-results.sh -L \
 	  $(addsuffix /gdb.log,$(TEST_DIRS)) > gdb.log
 
-$(filter-out check-gdb.base%,$(TEST_TARGETS)): check-gdb.%: all site.exp
+$(filter-out check-gdb.base%,$(TEST_TARGETS)): check-gdb.%: all ./site.exp
 	@if test ! -d gdb.$*; then mkdir gdb.$*; fi
 	$(DO_RUNTEST) --directory=gdb.$* --outdir=gdb.$* $(RUNTESTFLAGS)
 
@@ -179,7 +179,7 @@ BASE1_FILES = $(patsubst $(srcdir)/%,%,$
 BASE2_FILES = $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/gdb.base/[n-z]*.exp))
 
 # Handle each half of gdb.base.
-check-gdb.base%: all site.exp
+check-gdb.base%: all ./site.exp
 	@if test ! -d gdb.base$*; then mkdir gdb.base$*; fi
 	$(DO_RUNTEST) $(BASE$*_FILES) --outdir gdb.base$* $(RUNTESTFLAGS)
 

Re: [patch] Fix parallel gdb/ make check//% site.exp

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> -check-single: all site.exp
Jan> +check-single: all ./site.exp

I think make will still search vpath for "./site.exp".
So, won't that cause the same problem?
What am I missing?

Tom

Re: [patch] Fix parallel gdb/ make check//% site.exp

[Doug Evans]

On Mon, Nov 9, 2009 at 4:15 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> sometimes `make check//unix/-m32' just did not produce any results.
>
> Caught it is due to site.exp existing in gdb/testsuite/ thus satisfying VPATH
> make dependency on it but runtest does not execute anything without site.exp
> file in its current directory.
>
>
> Regards,
> Jan
>
>
> gdb/testsuite/
> 2009-11-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>        * Makefile.in (check-single, $(TEST_TARGETS), check-gdb.base%): Change
>        `site.exp' dependency to `./site.exp'.
>
> --- gdb/testsuite/Makefile.in   19 Oct 2009 09:51:43 -0000      1.28
> +++ gdb/testsuite/Makefile.in   9 Nov 2009 12:12:52 -0000
> @@ -147,7 +147,7 @@ DO_RUNTEST = \
>          export TCL_LIBRARY ; fi ; \
>        $(RUNTEST)
>
> -check-single: all site.exp
> +check-single: all ./site.exp
>        $(DO_RUNTEST) $(RUNTESTFLAGS)
>
>  # A list of all directories named "gdb.*" which also hold a .exp file.
> @@ -170,7 +170,7 @@ check-parallel:
>        $(SHELL) $(srcdir)/dg-extract-results.sh -L \
>          $(addsuffix /gdb.log,$(TEST_DIRS)) > gdb.log
>
> -$(filter-out check-gdb.base%,$(TEST_TARGETS)): check-gdb.%: all site.exp
> +$(filter-out check-gdb.base%,$(TEST_TARGETS)): check-gdb.%: all ./site.exp
>        @if test ! -d gdb.$*; then mkdir gdb.$*; fi
>        $(DO_RUNTEST) --directory=gdb.$* --outdir=gdb.$* $(RUNTESTFLAGS)
>
> @@ -179,7 +179,7 @@ BASE1_FILES = $(patsubst $(srcdir)/%,%,$
>  BASE2_FILES = $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/gdb.base/[n-z]*.exp))
>
>  # Handle each half of gdb.base.
> -check-gdb.base%: all site.exp
> +check-gdb.base%: all ./site.exp
>        @if test ! -d gdb.base$*; then mkdir gdb.base$*; fi
>        $(DO_RUNTEST) $(BASE$*_FILES) --outdir gdb.base$* $(RUNTESTFLAGS)
>

I'll defer to Tom on whether this is a fix,
but if the patch does go in, I'd like to see comments added to the
code explaining why things are the way they are.
site vs ./site.exp seems fairly subtle.

Re: [patch] Fix parallel gdb/ make check//% site.exp

[Jan Kratochvil]

On Mon, 09 Nov 2009 16:37:42 +0100, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> -check-single: all site.exp
> Jan> +check-single: all ./site.exp
> 
> I think make will still search vpath for "./site.exp".
> So, won't that cause the same problem?

A nice catch, verified now VPATH does work even for relative paths.  The only
safe way is to use absolute path there.
	make-3.81/ vpath_search(): 
	  /* If there are no VPATH entries or FILENAME starts at the root,
	     there is nothing we can do.  */


> What am I missing?

It was a testing mistake as when I edited ./Makefile I changed its timestamp
which did falsely "fix" it by:
	  Considering target file `site.exp'.
	    Pruning file `config.status'.
	    Pruning file `Makefile'.
	   Finished prerequisites of target file `site.exp'.
	   Prerequisite `config.status' is older than target `site.exp'.
	   Prerequisite `Makefile' is newer than target `site.exp'.
	  Must remake target `site.exp'.
	  Ignoring VPATH name `/home/jkratoch/redhat/gdb-clean/gdb/testsuite/site.exp'.


On Mon, 09 Nov 2009 17:38:04 +0100, Doug Evans wrote:
> but if the patch does go in, I'd like to see comments added to the
> code explaining why things are the way they are.
> site vs ./site.exp seems fairly subtle.

Followed.


Thanks,
Jan


gdb/testsuite/
2009-11-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* Makefile.in (abs_builddir): New.
	(site.exp): New target `$(abs_builddir)/site.exp'.  New comment.
	(check-single, $(TEST_TARGETS), check-gdb.base%): Change `site.exp' to
	`$(abs_builddir)/site.exp'.

--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -22,6 +22,7 @@ VPATH = @srcdir@
 srcdir = @srcdir@
 prefix = @prefix@
 exec_prefix = @exec_prefix@
+abs_builddir = @abs_builddir@
 
 target_alias = @target_noncanonical@
 program_transform_name = @program_transform_name@
@@ -102,7 +103,11 @@ install:
 
 uninstall: force
 
-site.exp: ./config.status Makefile
+# Use absolute `site.exp' path everywhere to suppress VPATH lookups for it.
+# $(RUNTEST) is looking up `site.exp' only in the current directory.
+# Bare `site.exp' is present as a target here if user requests it explicitly.
+
+$(abs_builddir)/site.exp site.exp: ./config.status Makefile
 	@echo "Making a new config file..."
 	-@rm -f ./tmp?
 	@touch site.exp
@@ -147,7 +151,7 @@ DO_RUNTEST = \
 	  export TCL_LIBRARY ; fi ; \
 	$(RUNTEST)
 
-check-single: all site.exp
+check-single: all $(abs_builddir)/site.exp
 	$(DO_RUNTEST) $(RUNTESTFLAGS)
 
 # A list of all directories named "gdb.*" which also hold a .exp file.
@@ -170,7 +174,7 @@ check-parallel:
 	$(SHELL) $(srcdir)/dg-extract-results.sh -L \
 	  $(addsuffix /gdb.log,$(TEST_DIRS)) > gdb.log
 
-$(filter-out check-gdb.base%,$(TEST_TARGETS)): check-gdb.%: all site.exp
+$(filter-out check-gdb.base%,$(TEST_TARGETS)): check-gdb.%: all $(abs_builddir)/site.exp
 	@if test ! -d gdb.$*; then mkdir gdb.$*; fi
 	$(DO_RUNTEST) --directory=gdb.$* --outdir=gdb.$* $(RUNTESTFLAGS)
 
@@ -179,7 +183,7 @@ BASE1_FILES = $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/gdb.base/[a-m]*.exp)
 BASE2_FILES = $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/gdb.base/[n-z]*.exp))
 
 # Handle each half of gdb.base.
-check-gdb.base%: all site.exp
+check-gdb.base%: all $(abs_builddir)/site.exp
 	@if test ! -d gdb.base$*; then mkdir gdb.base$*; fi
 	$(DO_RUNTEST) $(BASE$*_FILES) --outdir gdb.base$* $(RUNTESTFLAGS)
 

Re: [patch] Fix parallel gdb/ make check//% site.exp

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> 2009-11-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
Jan> 	* Makefile.in (abs_builddir): New.
Jan> 	(site.exp): New target `$(abs_builddir)/site.exp'.  New comment.
Jan> 	(check-single, $(TEST_TARGETS), check-gdb.base%): Change `site.exp' to
Jan> 	`$(abs_builddir)/site.exp'.

Ok.

Tom

Re: [patch] Fix parallel gdb/ make check//% site.exp

[Doug Evans]

On Mon, Nov 9, 2009 at 9:19 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
>
> gdb/testsuite/
> 2009-11-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>        * Makefile.in (abs_builddir): New.
>        (site.exp): New target `$(abs_builddir)/site.exp'.  New comment.
>        (check-single, $(TEST_TARGETS), check-gdb.base%): Change `site.exp' to
>        `$(abs_builddir)/site.exp'.
>
> --- a/gdb/testsuite/Makefile.in
> +++ b/gdb/testsuite/Makefile.in
> @@ -22,6 +22,7 @@ VPATH = @srcdir@
>  srcdir = @srcdir@
>  prefix = @prefix@
>  exec_prefix = @exec_prefix@
> +abs_builddir = @abs_builddir@
>
>  target_alias = @target_noncanonical@
>  program_transform_name = @program_transform_name@
> @@ -102,7 +103,11 @@ install:
>
>  uninstall: force
>
> -site.exp: ./config.status Makefile
> +# Use absolute `site.exp' path everywhere to suppress VPATH lookups for it.
> +# $(RUNTEST) is looking up `site.exp' only in the current directory.
> +# Bare `site.exp' is present as a target here if user requests it explicitly.
> +
> +$(abs_builddir)/site.exp site.exp: ./config.status Makefile
>        @echo "Making a new config file..."
>        -@rm -f ./tmp?
>        @touch site.exp
> @@ -147,7 +151,7 @@ DO_RUNTEST = \
>          export TCL_LIBRARY ; fi ; \
>        $(RUNTEST)
>
> -check-single: all site.exp
> +check-single: all $(abs_builddir)/site.exp
>        $(DO_RUNTEST) $(RUNTESTFLAGS)
>
>  # A list of all directories named "gdb.*" which also hold a .exp file.
> @@ -170,7 +174,7 @@ check-parallel:
>        $(SHELL) $(srcdir)/dg-extract-results.sh -L \
>          $(addsuffix /gdb.log,$(TEST_DIRS)) > gdb.log
>
> -$(filter-out check-gdb.base%,$(TEST_TARGETS)): check-gdb.%: all site.exp
> +$(filter-out check-gdb.base%,$(TEST_TARGETS)): check-gdb.%: all $(abs_builddir)/site.exp
>        @if test ! -d gdb.$*; then mkdir gdb.$*; fi
>        $(DO_RUNTEST) --directory=gdb.$* --outdir=gdb.$* $(RUNTESTFLAGS)
>
> @@ -179,7 +183,7 @@ BASE1_FILES = $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/gdb.base/[a-m]*.exp)
>  BASE2_FILES = $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/gdb.base/[n-z]*.exp))
>
>  # Handle each half of gdb.base.
> -check-gdb.base%: all site.exp
> +check-gdb.base%: all $(abs_builddir)/site.exp
>        @if test ! -d gdb.base$*; then mkdir gdb.base$*; fi
>        $(DO_RUNTEST) $(BASE$*_FILES) --outdir gdb.base$* $(RUNTESTFLAGS)
>
>

Thanks.

I didn't know about abs_builddir.  At Cygnus we had a general rule of
keeping trees configured with relative paths movable, and I always
forget whether we care about that anymore.
OOC, does that impinge on this or do we no longer care about such things?

Re: [patch] Fix parallel gdb/ make check//% site.exp

[Jan Kratochvil]

On Mon, 09 Nov 2009 18:33:23 +0100, Doug Evans wrote:
> At Cygnus we had a general rule of keeping trees configured with relative
> paths movable, and I always forget whether we care about that anymore.
> OOC, does that impinge on this or do we no longer care about such things?

I would find it also great but in my experience it (already) does not work.

At least VPATH + srcdir are already absolute in the `gdb/testsuite.unix.*'
subdirs.


Regards,
Jan

Re: [patch] Fix parallel gdb/ make check//% site.exp

[Daniel Jacobowitz]

On Mon, Nov 09, 2009 at 06:50:29PM +0100, Jan Kratochvil wrote:
> On Mon, 09 Nov 2009 18:33:23 +0100, Doug Evans wrote:
> > At Cygnus we had a general rule of keeping trees configured with relative
> > paths movable, and I always forget whether we care about that anymore.
> > OOC, does that impinge on this or do we no longer care about such things?
> 
> I would find it also great but in my experience it (already) does not work.

I agree with Jan; I've never had this work reliably and it's too
fragile for me to trust.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fix parallel gdb/ make check//% site.exp

[Jan Kratochvil]

On Mon, 09 Nov 2009 18:32:06 +0100, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> 2009-11-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> Jan> 	* Makefile.in (abs_builddir): New.
> Jan> 	(site.exp): New target `$(abs_builddir)/site.exp'.  New comment.
> Jan> 	(check-single, $(TEST_TARGETS), check-gdb.base%): Change `site.exp' to
> Jan> 	`$(abs_builddir)/site.exp'.
> 
> Ok.

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2009-11/msg00053.html


Thanks,
Jan