Re: [PATCH] test defs: don't allow `$me' to be overridden from the environment

[Stefano Lattarini]

On Monday 18 April 2011, Stefano Lattarini wrote:
> On Monday 18 April 2011, Ralf Wildenhues wrote:
> > | > am_test_name is better, but doesn't explain either why it would be
> > | > needed in the first place.
> > | >
> > | Second patch of:
> > |  
> > <http://lists.gnu.org/archive/html/automake-patches/2011-02/msg00044.html>
> > | And possible similar patches in the future.
> > 
> > That still doesn't answer why a user-available override would be needed.
> >
> In truth, the variable is not meant to be user-overridable.  That's just
> an accident (i.e., I couldn't think of a better impelementation that would
> disallow this).
> 
> > It answers why you want to make $me overridable *from within* the
> > testsuite, but not why for the user running 'make check'.
> >
> In fact, I don't want to.  That would probably just cause random breakage.
> 
> > For example, tests/Makefile.am could have
> >   unset me ||:;
> > 
> > as part of AM_TESTS_ENVIRONMENT.  That wouldn't defeat users running
> > tests themselves, but it would defeat breakage induced by differences
> > in the user environment.
> >
> I like your solution more than mine; I withdraw my patch, and I'll soon
> write a new one on the lines you've suggested (that is, once support for
> AM_TESTS_ENVIRONMENT is in place)?
> 
> Thanks, and sorry for the noise,
>   Stefano
> 
Done in the attached patch, using TESTS_ENVIRONMENT for the moment.  We can
switch to AM_TESTS_ENVIRONMENT once it's in place, OK?

I'll push by tomorrow evening if there is no objection.

Regards,
  Stefano

From 3240a2adc38cae0de44419e88e55d2475d906c13 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Mon, 18 Apr 2011 22:24:56 +0200
Subject: [PATCH] tests: don't allow `$me' to be overridden from the environment

* tests/defs.in: Sanity check: abort if $me is in the environment.
* tests/self-check-me-in-env.test: New test.
* tests/Makefile.am (TESTS_ENVIRONMENT): Unset variable `me'.
(TESTS): Update.

Suggestion by Ralf Wildenhues.
---
 ChangeLog                       |   10 +++++-----
 tests/Makefile.am               |    5 +++++
 tests/Makefile.in               |    5 +++++
 tests/defs.in                   |   12 ++++++------
 tests/self-check-me-in-env.test |   28 ++++++++++++++++++++++++++++
 5 files changed, 49 insertions(+), 11 deletions(-)
 create mode 100755 tests/self-check-me-in-env.test

diff --git a/ChangeLog b/ChangeLog
index 4c0ad3e..b139a98 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,10 +1,10 @@
 2011-04-18  Stefano Lattarini  <address@hidden>
 
-       test defs: don't allow `$me' to be overridden from the environment
-       * tests/defs.in ($me): Use the namespace-safe `$am_test_name' (if
-       it's nonempty) as the default for the initialization of `$me', so
-       that a (not unlikely) environment variable `me' won't wreak havoc
-       in the testsuite.
+       tests: don't allow `$me' to be overridden from the environment
+       * tests/defs.in: Sanity check: abort if $me is in the environment.
+       * tests/self-check-me-in-env.test: New test.
+       * tests/Makefile.am (TESTS_ENVIRONMENT): Unset variable `me'.
+       (TESTS): Update.
        Suggestion by Ralf Wildenhues.
 
 2011-04-17  Stefano Lattarini  <address@hidden>
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b91a025..cc25167 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -38,7 +38,12 @@ $(parallel_tests): $(parallel_tests:-p.test=.test) 
Makefile.am
 
 MAINTAINERCLEANFILES = $(parallel_tests)
 
+# The testsuite variable `$me' should be overridable from the
+# test scripts, but not from the environment.
+TESTS_ENVIRONMENT = test x"$$me" = x || unset me;
+
 TESTS = \
+self-check-me-in-env.test \
 aclibobj.test \
 aclocal.test \
 aclocal3.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 0cc6854..9ccbdbd 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -308,7 +308,12 @@ pr401b-p.test \
 pr401c-p.test
 
 MAINTAINERCLEANFILES = $(parallel_tests)
+
+# The testsuite variable `$me' should be overridable from the
+# test scripts, but not from the environment.
+TESTS_ENVIRONMENT = test x"$$me" = x || unset me;
 TESTS = \
+self-check-me-in-env.test \
 aclibobj.test \
 aclocal.test \
 aclocal3.test \
diff --git a/tests/defs.in b/tests/defs.in
index 910c9a3..342e76d 100644
--- a/tests/defs.in
+++ b/tests/defs.in
@@ -65,13 +65,13 @@ test -f "$srcdir/defs.in" || {
 }
 
 # The name of the current test (without the `.test' suffix).
-# Test scripts can override it if they need to, through the use of
-# the namespace-safe variable `$am_test_name' (but this should be
-# done carefully, and *before* including ./defs).
-if test -z "$am_test_name"; then
+# Test scripts can override it if they need to (but this should
+# be done carefully, and *before* including ./defs).
+if test -z "$me"; then
   me=`echo "$0" | sed -e 's,.*[\\/],,;s/\.test$//'`
-else
-  me=$am_test_name
+elif env | grep '^me=' >/dev/null; then
+  echo "$0: variable \`me' is set in the environment: this is unsafe" >&2
+  exit 99
 fi
 
 # This might be used in testcases checking distribution-related features.
diff --git a/tests/self-check-me-in-env.test b/tests/self-check-me-in-env.test
new file mode 100755
index 0000000..344a861
--- /dev/null
+++ b/tests/self-check-me-in-env.test
@@ -0,0 +1,28 @@
+#! /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/>.
+
+# Sanity check for the automake testsuite.
+# Make sure that the testsuite initialization code complains when
+# $me is set in the environment.
+
+set -x
+
+exec 5>&1
+env me=foo /bin/sh -c '. ./defs' foo.test && exit 1
+env me=foo /bin/sh -c '. ./defs' foo.test 2>&1 1>&5 \
+  | grep "variable \`me' is set in the environment.*unsafe" || exit 1
+
+:
-- 
1.7.2.3