GNU bug report logs - #9400
parallel-tests doesn't support `-' character in test extensions

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 9400 in the body.
You can then email your comments to 9400 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: bug-automake <at> gnu.org
Subject: parallel-tests doesn't support `-' character in test extensions
Date: Mon, 29 Aug 2011 13:59:09 +0200

[Message part 1 (text/plain, inline)]

I catched this while working some more on the Automake testsuite:

 $ cat Makefile.am
 TEST_EXTENSIONS = .t-1
 TESTS = foo.t-1
 $ cat configure.ac
 AC_INIT([foo], [1.0])
 AM_INIT_AUTOMAKE([foreign -Wall parallel-tests])
 AC_CONFIG_FILES([Makefile])
 AC_OUTPUT
 $ aclocal
 $ autoconf
 $ : > install-sh
 $ : > missing
 $ automake
 automake: bad characters in variable name `T-1_LOG_COMPILE'
 automake: T-1_LOG_COMPILER: non-POSIX variable name
 automake: AM_T-1_LOG_FLAGS: non-POSIX variable name
 automake: T-1_LOG_FLAGS: non-POSIX variable name

Do you think it would be worthwhile to lift this limitation, by
automatically transliterating `-' (and maybe other non-alphabetic
characters) to `_'?  Or should we simply give an error when something
in $(TEST_EXTENSIONS) contains non-alphabetic characters?

Regards,
  Stefano

[Message part 2 (text/html, inline)]

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: bug-automake <at> gnu.org
Cc: 9400 <at> debbugs.gnu.org
Subject: Re: bug#9400: parallel-tests doesn't support `-' character in test
	extensions
Date: Sat, 1 Oct 2011 22:12:04 +0200

[Message part 1 (text/plain, inline)]

On Monday 29 August 2011, Stefano Lattarini wrote:
> I catched this while working some more on the Automake testsuite:
> 
>  $ cat Makefile.am
>  TEST_EXTENSIONS = .t-1
>  TESTS = foo.t-1
>  $ cat configure.ac
>  AC_INIT([foo], [1.0])
>  AM_INIT_AUTOMAKE([foreign -Wall parallel-tests])
>  AC_CONFIG_FILES([Makefile])
>  AC_OUTPUT
>  $ aclocal
>  $ autoconf
>  $ : > install-sh
>  $ : > missing
>  $ automake
>  automake: bad characters in variable name `T-1_LOG_COMPILE'
>  automake: T-1_LOG_COMPILER: non-POSIX variable name
>  automake: AM_T-1_LOG_FLAGS: non-POSIX variable name
>  automake: T-1_LOG_FLAGS: non-POSIX variable name
> 
> Do you think it would be worthwhile to lift this limitation, by
> automatically transliterating `-' (and maybe other non-alphabetic
> characters) to `_'?  Or should we simply give an error when something
> in $(TEST_EXTENSIONS) contains non-alphabetic characters?
> 
> Regards,
>   Stefano
> 
For the moment, I've decided to take the easiest route, documenting the
limitation and correcting the diagnostic.  Such restriction could easily
be lifted at a later time, with no backward-compatibility issues.

Attached is the patch I'll push in a couple of days if there is no
objection.  As usual, reviews are welcome.

Regards,
  Stefano

[0001-parallel-tests-automake-error-our-on-invalid-TEST_EX.patch (text/x-patch, inline)]

From 90bea64bc5023be075b63bf7c651d0242f35a83c Mon Sep 17 00:00:00 2001
Message-Id: <90bea64bc5023be075b63bf7c651d0242f35a83c.1317499108.git.stefano.lattarini <at> gmail.com>
From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Date: Sat, 1 Oct 2011 21:31:07 +0200
Subject: [PATCH] parallel-tests: automake error our on invalid TEST_EXTENSIONS

This change fixes automake bug#9400.

* automake.in (handle_tests): Bail out if a suffix specified in
TEST_EXTENSIONS would produce an invalid `xxx_LOG_COMPILER'
variable or an invalid suffix rule.  Before this change, automake
would have issued a confusing error messages (about invalid or
non-POSIX variables being defined), and in some situations would
have even produced a broken `Makefile.in' file.
($TEST_EXTENSION_PATTERN): New helper variable.
* doc/automake.texi (Simple Tests using parallel-tests): Document
the limitations on TEST_EXTENSIONS explicitly.
* NEWS: Update.
* tests/test-extensions.test: New test.
* tests/Makefile.am (TESTS): Update.
---
 ChangeLog                  |   17 ++++++++++
 NEWS                       |    6 +++
 automake.in                |   12 ++++++-
 doc/automake.texi          |   12 +++++--
 tests/Makefile.am          |    1 +
 tests/Makefile.in          |    1 +
 tests/test-extensions.test |   73 ++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 118 insertions(+), 4 deletions(-)
 create mode 100755 tests/test-extensions.test

diff --git a/ChangeLog b/ChangeLog
index 47aee92..e52a275 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2011-10-01  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
+
+	parallel-tests: automake error our on invalid TEST_EXTENSIONS
+	This change fixes automake bug#9400.
+	* automake.in (handle_tests): Bail out if a suffix specified in
+	TEST_EXTENSIONS would produce an invalid `xxx_LOG_COMPILER'
+	variable or an invalid suffix rule.  Before this change, automake
+	would have issued a confusing error messages (about invalid or
+	non-POSIX variables being defined), and in some situations would
+	have even produced a broken `Makefile.in' file.
+	($TEST_EXTENSION_PATTERN): New helper variable.
+	* doc/automake.texi (Simple Tests using parallel-tests): Document
+	the limitations on TEST_EXTENSIONS explicitly.
+	* NEWS: Update.
+	* tests/test-extensions.test: New test.
+	* tests/Makefile.am (TESTS): Update.
+
 2011-09-22  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
 
 	tests: fix tests on aclocal search path precedences
diff --git a/NEWS b/NEWS
index b696977..fe78238 100644
--- a/NEWS
+++ b/NEWS
@@ -49,6 +49,12 @@ Bugs fixed in 1.11.0a:
     does not report spurious successes when used with concurrent FreeBSD
     make (e.g., "make check -j3").
 
+  - Automake now explicitly rejects invalid entries in TEST_EXTENSIONS when
+    the parallel-tests diver is in use, instead of issuing confusing and
+    apparently unrelated error messages (about "non-POSIX variable name"
+    or "bad characters in variable name"), or even, in some situations,
+    producing broken `Makefile.in' files.
+
   - The `silent-rules' option now also silences all compile rules if dependency
     tracking is disabled.  Also, when `silent-rules' is not used, the output from
     `make' does not contain spurious extra lines with only a backslash in them
diff --git a/automake.in b/automake.in
index 215881b..a60bc9f 100755
--- a/automake.in
+++ b/automake.in
@@ -213,6 +213,8 @@ my $DASH_D_PATTERN = "(^|\\s)-d(\\s|\$)";
 # Directories installed during 'install-exec' phase.
 my $EXEC_DIR_PATTERN =
   '^(?:bin|sbin|libexec|sysconf|localstate|lib|pkglib|.*exec.*)' . "\$";
+# Suffixes that can appear in TEST_EXTENSIONS (parallel-tests support).
+my $TEST_EXTENSION_PATTERN = '^(\.[a-zA-Z_][a-zA-Z0-9_]*|@[a-zA-Z0-9_]+@)$';
 
 # Values for AC_CANONICAL_*
 use constant AC_CANONICAL_BUILD  => 1;
@@ -4971,7 +4973,15 @@ sub handle_tests
 	    }
 	  define_variable ('TEST_EXTENSIONS', $suff, INTERNAL);
 	  # FIXME: this mishandles conditions.
-	  my @test_suffixes = (var 'TEST_EXTENSIONS')->value_as_list_recursive;
+	  my $var = rvar 'TEST_EXTENSIONS';
+	  my @test_suffixes = $var->value_as_list_recursive;
+          if ((my @invalid_test_suffixes =
+                  grep { !/$TEST_EXTENSION_PATTERN/o } @test_suffixes) > 0)
+            {
+              error $var->rdef (TRUE)->location,
+                    "invalid test extensions: @invalid_test_suffixes";
+            }
+          @test_suffixes = grep { /$TEST_EXTENSION_PATTERN/o } @test_suffixes;
 	  if ($handle_exeext)
 	    {
 	      unshift (@test_suffixes, $at_exeext)
diff --git a/doc/automake.texi b/doc/automake.texi
index a8233dd..875e191 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -8808,9 +8808,15 @@ Each log file is created when the corresponding test has completed.
 The set of log files is listed in the read-only variable
 @code{TEST_LOGS}, and defaults to @code{TESTS}, with the executable
 extension if any (@pxref{EXEEXT}), as well as any suffix listed in
-@code{TEST_EXTENSIONS} removed, and @file{.log} appended.
-@code{TEST_EXTENSIONS} defaults to @file{.test}.  Results are undefined
-if a test file name ends in several concatenated suffixes.
+@code{TEST_EXTENSIONS} removed, and @file{.log} appended.  Results
+are undefined if a test file name ends in several concatenated suffixes.
+@code{TEST_EXTENSIONS} defaults to @file{.test}; it can be overridden by
+the user, in which case any extension listed in it must be constituted
+by a dot, followed by a non-digit alphabetic character, followed by any
+number of alphabetic characters.
+@c Keep in sync with test-extensions.test.
+For example, @samp{.sh}, @samp{.T} and @samp{.t1} are valid extensions,
+while @samp{.x-y}, @samp{.6c} and @samp{.t.1} are not.
 
 @vindex _LOG_COMPILE
 @vindex _LOG_COMPILER
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1d258c9..3f9fc08 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -628,6 +628,7 @@ parallel-tests-log-override-1.test \
 parallel-tests-log-override-2.test \
 parallel-tests-log-override-recheck.test \
 parallel-tests-log-compiler-example.test \
+test-extensions.test \
 parse.test \
 percent.test \
 percent2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 7e9bc20..2e7f39f 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -906,6 +906,7 @@ parallel-tests-log-override-1.test \
 parallel-tests-log-override-2.test \
 parallel-tests-log-override-recheck.test \
 parallel-tests-log-compiler-example.test \
+test-extensions.test \
 parse.test \
 percent.test \
 percent2.test \
diff --git a/tests/test-extensions.test b/tests/test-extensions.test
new file mode 100755
index 0000000..1d5872c
--- /dev/null
+++ b/tests/test-extensions.test
@@ -0,0 +1,73 @@
+#! /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/>.
+
+# Make sure that Automake diagnose invalid entries in TEST_EXTENSIONS,
+# and do not diagnose valid (albeit more unusual) ones.
+# See automake bug#9400.
+
+parallel_tests=yes
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AC_SUBST([ext], [".e"])
+AC_OUTPUT
+END
+
+$ACLOCAL
+$AUTOCONF
+
+cat > Makefile.am << 'END'
+TESTS =
+TEST_EXTENSIONS = .sh .T .t1 ._foo .BAR .x_Y_z ._ @ext@
+END
+
+$AUTOMAKE
+
+$EGREP -i 'log|ext' Makefile.in # For debugging.
+
+for lc in sh T t1 _foo BAR x_Y_z _; do
+  uc=`echo $lc | tr '[a-z]' '[A-Z]'`
+  $FGREP "\$(${uc}_LOG_COMPILER)" Makefile.in
+  grep "^${uc}_LOG_COMPILE =" Makefile.in
+  grep "^\.${lc}\.log:" Makefile.in
+done
+grep "^@ext@\.log:" Makefile.in
+
+# The produced Makefile is not broken.
+./configure
+$MAKE all check
+
+cat > Makefile.am << 'END'
+TESTS = foo.test bar.sh
+TEST_EXTENSIONS = .test mu .x-y a-b .t.1 .sh .6c .0 .11 .@ .t33 .a=b _&_
+END
+
+AUTOMAKE_fails
+for suf in mu .x-y a-b .t.1 .6c .0 .11 '.@' '.a=b' '_&_'; do
+  suf2=`printf '%s\n' "$suf" | sed -e 's/\./\\./'`
+  $EGREP "^Makefile\.am:2:.*invalid test extension.* $suf2( |$)" stderr
+done
+
+# Verify that we accept valid suffixes, even if intermixed with
+# invalid ones.
+$EGREP '\.(sh|test|t33)' stderr && Exit 1
+
+# Verify that we don't try to handle invalid suffixes.
+$EGREP '(LOG_COMPILER|non-POSIX var|bad character)' stderr && Exit 1
+
+:
-- 
1.7.2.3

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: bug-automake <at> gnu.org
Cc: 9400 <at> debbugs.gnu.org, automake-patches <at> gnu.org
Subject: Re: bug#9400: parallel-tests doesn't support `-' character in test
	extensions
Date: Thu, 6 Oct 2011 10:56:18 +0200

tags 9400 patch
close 9400
thanks

Reference:
 <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9400#8>

On Saturday 01 October 2011, Stefano Lattarini wrote:
> For the moment, I've decided to take the easiest route, documenting the
> limitation and correcting the diagnostic.  Such restriction could easily
> be lifted at a later time, with no backward-compatibility issues.
> 
> Attached is the patch I'll push in a couple of days if there is no
> objection.  As usual, reviews are welcome.
> 
I've pushed this patch now, and I'm closing this bug report.

Regards,
  Stefano

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.