Subject: Re: bug#7833: automake uses two different values for DejaGNU srcdir

From: Stefano Lattarini <[email-masked]>
Date: Sun, 16 Jan 2011 15:09:15 +0100

Hello Ralf, and sorry for the late reply.

On Friday 14 January 2011, Ralf Wildenhues wrote:
> * Ian Lance Taylor wrote on Thu, Jan 13, 2011 at 06:29:39PM CET:
> > Ralf Wildenhues writes:
> > 
> > > Am I correct in assuming that it is hopeless to assume GCC will work
> > > when either values are relative?
> > 
> > Actually, as far as I know, GCC is likely to work with a relative
> > srcdir.  The problem arises with DejaGNU .exp files which themselves
> > change directories.  As far as I know, the GCC DejaGNU files do not do
> > that.
> 
> Thanks for the feedback.
> 
> Through testing I found out again that DejaGNU does not cope with white
> space in absolute directory names anyway, regardless of srcdir being
> relative or absolute.
>
While trying to cook up some test cases, I experienced this too, but I
thought it was due to my lack of experience and knowledge of DejaGnu.
Thanks for saving me some other useless efforts!

> That obviates my first point.  Remains only the
> fairly minor src+build-tree relocatability point.
> 
> I have these two alternative patches which I would both be happy with.
>
FWIW, I had preperad a patch also, which is basically your second patch
with a testcase added (kudos to Ian Taylor for his suggestion of how to
expose the bug).  It's attached for reference.

BTW, even if you decide to go with your first patch at last, it should
be possible to easily modify the testcase to have it do checks opposite
to the ones it currently does, and thus reuse it.

> I should note that the "Life is easiest" comment dates back to the very
> first addition of dejagnu support in Automake, from way before recorded
> history where we could learn from a related bug.
> 
> Tried both in a multilib target directory in the GCC tree, both seem to
> work.  Any preferences, from a DejaGNU standpoint?
>
> Thanks,
> Ralf
> 
>    * lib/am/dejagnu.am (check-DEJAGNU): Set srcdir to be relative,
>    consistent with the --srcdir argument and the srcdir setting in
>    site.exp.
>    Report by Ian Lance Taylor in automake bug#7833.
> 
> --- am/dejagnu.am.orig	2011-01-14 20:27:32.000000000 +0100
> +++ am/dejagnu.am.1	2011-01-14 20:27:38.000000000 +0100
> @@ -1,5 +1,5 @@
>  ## automake - create Makefile.in from Makefile.am
> -## Copyright (C) 1994, 1995, 1996, 1997, 2000, 2001, 2003, 2006
> +## Copyright (C) 1994, 1995, 1996, 1997, 2000, 2001, 2003, 2006, 2011
>  ## Free Software Foundation, Inc.
>  
>  ## This program is free software; you can redistribute it and/or modify
> @@ -43,8 +43,7 @@
>  
>  .PHONY: check-DEJAGNU
>  check-DEJAGNU: site.exp
> -## Life is easiest with an absolute srcdir, so do that.
> -	srcdir=`$(am__cd) $(srcdir) && pwd`; export srcdir; \
> +	srcdir='$(srcdir)'; export srcdir; \
>  	EXPECT=$(EXPECT); export EXPECT; \
>  ## Allow this to work when expect and DejaGnu are in tree.
>  ## Only required when --cygnus in force.
> 
> 
> 
> 
>    * lib/am/dejagnu.am (RUNTESTDEFAULTFLAGS): Quote argument
>    to --srcdir.
>    (site.exp): Set srcdir to "$(abs_srcdir)".
>    Report by Ian Lance Taylor in automake bug#7833.
> 
> --- am/dejagnu.am.orig	2011-01-14 20:27:32.000000000 +0100
> +++ am/dejagnu.am.2	2011-01-14 20:20:22.000000000 +0100
> @@ -1,5 +1,5 @@
>  ## automake - create Makefile.in from Makefile.am
> -## Copyright (C) 1994, 1995, 1996, 1997, 2000, 2001, 2003, 2006
> +## Copyright (C) 1994, 1995, 1996, 1997, 2000, 2001, 2003, 2006, 2011
>  ## Free Software Foundation, Inc.
>  
>  ## This program is free software; you can redistribute it and/or modify
> @@ -19,7 +19,7 @@
>  DEJATOOL = $(PACKAGE)
>  
>  ## Default flags to pass to dejagnu.  The user can override this.
> -RUNTESTDEFAULTFLAGS = --tool $$tool --srcdir $$srcdir
> +RUNTESTDEFAULTFLAGS = --tool $$tool --srcdir "$$srcdir"
>  
>  ## In Cygnus mode, these are found in the build tree.
>  ## Otherwise they are looked for in $PATH.
> @@ -77,7 +77,7 @@
>  	@echo '## these variables are automatically generated by make ##' >site.tmp
>  	@echo '# Do not edit here.  If you wish to override these values' >>site.tmp
>  	@echo '# edit the last section' >>site.tmp
> -	@echo 'set srcdir $(srcdir)' >>site.tmp
> +	@echo 'set srcdir "$(abs_srcdir)"' >>site.tmp
>  	@echo "set objdir `pwd`" >>site.tmp
>  ## Quote the *_alias variables because they might be empty.
>  ?BUILD?	@echo 'set build_alias "$(build_alias)"' >>site.tmp
> 
> 

Regards,
  Stefano
From 19e0ff24229cdb8560d555300e39977ac12bfffa Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <[email-masked]>
Date: Thu, 13 Jan 2011 20:33:26 +0100
Subject: [PATCH] dejagnu: ensure 'srcdir' is defined as an absolute directory

This change fixes automake bug#7833.

* lib/am/dejagnu.am (check-DEJAGNU): Prefer using $(abs_srcdir)
over getting the absolute path of $(srcdir) dynamically.
(site.exp): Use $(abs_srcdir) instead of $(srcdir) when defining
TCL variable 'srcdir'.
* tests/dejagnu-absolute-dirs.test: New test.
* tests/Makefile.am (TESTS): Update.

Report and suggestions by Ian Lance Taylor.
---
 ChangeLog                        |   12 +++++
 lib/am/dejagnu.am                |    4 +-
 tests/Makefile.am                |    1 +
 tests/Makefile.in                |    1 +
 tests/dejagnu-absolute-dirs.test |   92 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 108 insertions(+), 2 deletions(-)
 create mode 100755 tests/dejagnu-absolute-dirs.test

diff --git a/ChangeLog b/ChangeLog
index be35937..6f00b10 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2011-01-15  Stefano Lattarini  <[email-masked]>
+
+	dejagnu: ensure 'srcdir' is defined as an absolute directory
+	This change fixes automake bug#7833.
+	* lib/am/dejagnu.am (check-DEJAGNU): Prefer using $(abs_srcdir)
+	over getting the absolute path of $(srcdir) dynamically.
+	(site.exp): Use $(abs_srcdir) instead of $(srcdir) when defining
+	TCL variable 'srcdir'.
+	* tests/dejagnu-absolute-dirs.test: New test.
+	* tests/Makefile.am (TESTS): Update.
+	Report and suggestions by Ian Lance Taylor.
+
 2011-01-15  Ralf Wildenhues  <[email-masked]>
 
 	docs: ensure example are separated with empty lines in the input
diff --git a/lib/am/dejagnu.am b/lib/am/dejagnu.am
index 08de45c..4e9a3f3 100644
--- a/lib/am/dejagnu.am
+++ b/lib/am/dejagnu.am
@@ -44,7 +44,7 @@ endif ! %?CYGNUS%
 .PHONY: check-DEJAGNU
 check-DEJAGNU: site.exp
 ## Life is easiest with an absolute srcdir, so do that.
-	srcdir=`$(am__cd) $(srcdir) && pwd`; export srcdir; \
+	srcdir='$(abs_srcdir)'; export srcdir; \
 	EXPECT=$(EXPECT); export EXPECT; \
 ## Allow this to work when expect and DejaGnu are in tree.
 ## Only required when --cygnus in force.
@@ -77,7 +77,7 @@ site.exp: Makefile
 	@echo '## these variables are automatically generated by make ##' >site.tmp
 	@echo '# Do not edit here.  If you wish to override these values' >>site.tmp
 	@echo '# edit the last section' >>site.tmp
-	@echo 'set srcdir $(srcdir)' >>site.tmp
+	@echo 'set srcdir "$(abs_srcdir)"' >>site.tmp
 	@echo "set objdir `pwd`" >>site.tmp
 ## Quote the *_alias variables because they might be empty.
 ?BUILD?	@echo 'set build_alias "$(build_alias)"' >>site.tmp
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 890e69b..8145a13 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -263,6 +263,7 @@ dejagnu4.test \
 dejagnu5.test \
 dejagnu6.test \
 dejagnu7.test \
+dejagnu-absolute-dirs.test \
 depacl2.test \
 depcomp.test \
 depcomp2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 7600c14..53cfbb8 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -530,6 +530,7 @@ dejagnu4.test \
 dejagnu5.test \
 dejagnu6.test \
 dejagnu7.test \
+dejagnu-absolute-dirs.test \
 depacl2.test \
 depcomp.test \
 depcomp2.test \
diff --git a/tests/dejagnu-absolute-dirs.test b/tests/dejagnu-absolute-dirs.test
new file mode 100755
index 0000000..895e201
--- /dev/null
+++ b/tests/dejagnu-absolute-dirs.test
@@ -0,0 +1,92 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that DejaGnu testsuites have 'srcdir' defined to an absolute path
+# (both as TCL variable and as environment variable), and 'objdir' defined
+# (as a TCL variable) to an absolute path.
+
+required=runtest
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+AUTOMAKE_OPTIONS = dejagnu
+DEJATOOL = tcl env
+EXTRA_DIST = env.test/env.exp tcl.test/tcl.exp lib/tcl.exp
+END
+
+mkdir env.test tcl.test lib
+
+# DejaGnu can change $srcdir behind our backs, so we have to
+# save its original value.
+cat > lib/tcl.exp << 'END'
+send_user "tcl_lib_srcdir: $srcdir\n"
+set orig_srcdir $srcdir
+END
+
+cat > env.test/env.exp << 'END'
+set env_srcdir $env(srcdir)
+send_user "env_srcdir: $env_srcdir\n"
+if { [ regexp "^/" $env_srcdir ] } {
+    pass "test_env_src"
+} else {
+    fail "test_env_src"
+}
+END
+
+cat > tcl.test/tcl.exp << 'END'
+send_user "tcl_srcdir: $srcdir\n"
+if { [ regexp "^/" $srcdir ] } {
+    pass "test_tcl_src"
+} else {
+    fail "test_tcl_src"
+}
+send_user "tcl_orig_srcdir: $orig_srcdir\n"
+if { [ regexp "^/" $orig_srcdir ] } {
+    pass "test_tcl_orig_src"
+} else {
+    fail "test_tcl_orig_src"
+}
+send_user "tcl_objdir: $objdir\n"
+if { [ regexp "^/" $objdir ] } {
+    pass "test_tcl_objdir"
+} else {
+    fail "test_tcl_objdir"
+}
+END
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE --add-missing
+
+./configure --srcdir=.
+
+$MAKE check
+
+# Sanity check: all tests have run.
+test -f env.log
+test -f env.sum
+test -f tcl.log
+test -f tcl.sum
+
+$MAKE distcheck
+
+:
-- 
1.7.2.3