Re: [FYI] {maint,master} test defs: allow overriding of `$me'
On Monday 18 April 2011, Ralf Wildenhues wrote: Hi Stefano, * Stefano Lattarini wrote on Sun, Apr 17, 2011 at 09:36:42PM CEST: On Sunday 17 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 explains why, from within the testsuite, you'd like to be able to override it for some tests. It doesn't explain why an override from the user calling 'make check' should be possible. (IOW, I understand the this would be convenient aspect, but it seems it should be possible to construct the testsuite in a way to still not allow user overrides.) Honestly, I'd not worry about this ATM (but I share your concerns about the lack of namespace cleanliness). Presently, a determined user can anyway wreak havoc by exporting variables such as 'required', 'MISSING' and 'parallel_tests' (and there are even more in the master branch: 'original_AUTOMAKE', 'am__using_gmake', 'instspc_action'). A first step would IMHO be making these variables at least namespace-safe. Should I write a patch? Regards, Stefano
Re: [FYI] {maint,master} test defs: allow overriding of `$me'
* Stefano Lattarini wrote on Mon, Apr 18, 2011 at 10:27:04AM CEST: On Monday 18 April 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Sun, Apr 17, 2011 at 09:36:42PM CEST: http://lists.gnu.org/archive/html/automake-patches/2011-02/msg00044.html That explains why, from within the testsuite, you'd like to be able to override it for some tests. It doesn't explain why an override from the user calling 'make check' should be possible. (IOW, I understand the this would be convenient aspect, but it seems it should be possible to construct the testsuite in a way to still not allow user overrides.) Honestly, I'd not worry about this ATM (but I share your concerns about the lack of namespace cleanliness). Presently, a determined user can anyway wreak havoc by exporting variables such as 'required', 'MISSING' and 'parallel_tests' (and there are even more in the master branch: 'original_AUTOMAKE', 'am__using_gmake', 'instspc_action'). instspc_action is not dangerous, because its usage is safe, unlike original_AUTOMAKE. But 'me' is both a generic name, and dangerous (me=../../../oops). 'required' is less dangerous that way, but still a bit. A first step would IMHO be making these variables at least namespace-safe. Should I write a patch? Oh no please don't; the others are at least not so generic names, and I'd really loath the ripple that renaming $required will have. I would like to put out a stable point release in the near future and I'd prefer not so much churn before that. (Yeah, it doesn't really mesh well with me having close to no time at all right now, but it's about time we did that ...) Thanks, Ralf
New release wishlist (was: Re: [FYI] {maint, master} test defs: allow overriding of `$me')
On Monday 18 April 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Apr 18, 2011 at 10:27:04AM CEST: On Monday 18 April 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Sun, Apr 17, 2011 at 09:36:42PM CEST: http://lists.gnu.org/archive/html/automake-patches/2011-02/msg00044.html That explains why, from within the testsuite, you'd like to be able to override it for some tests. It doesn't explain why an override from the user calling 'make check' should be possible. (IOW, I understand the this would be convenient aspect, but it seems it should be possible to construct the testsuite in a way to still not allow user overrides.) Honestly, I'd not worry about this ATM (but I share your concerns about the lack of namespace cleanliness). Presently, a determined user can anyway wreak havoc by exporting variables such as 'required', 'MISSING' and 'parallel_tests' (and there are even more in the master branch: 'original_AUTOMAKE', 'am__using_gmake', 'instspc_action'). instspc_action is not dangerous, because its usage is safe, unlike original_AUTOMAKE. But 'me' is both a generic name, and dangerous (me=../../../oops). 'required' is less dangerous that way, but still a bit. A first step would IMHO be making these variables at least namespace-safe. Should I write a patch? Oh no please don't; the others are at least not so generic names, and I'd really loath the ripple that renaming $required will have. I'll just got with `$am_test_name' then (the patch for that is already written anyway, and is not invasive at all). I would like to put out a stable point release in the near future and I'd prefer not so much churn before that. Agreed. (Yeah, it doesn't really mesh well with me having close to no time at all right now, but it's about time we did that ...) JFTR, here are the things that IMHO we must do for the next release: - Apply patches for ACLOCAL_PATH support: http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00089.html - Make aclocal able to gets its extra options by tracing some special m4 macros, and deprecate the ACLOCAL_AMFLAGS hack. There is a patch avaiable for this: http://lists.gnu.org/archive/html/automake-patches/2010-10/msg00045.html - A new m4 macro that exposes Automake version and/or supported features, (this should have really been introduced time ago, and if I'm not mistaken there have been some user requests in this direction already). With this in place, we will be able to be a little more aggressive in the future w.r.t. redefition of default behaviours and with backward incompatible changes, without causing real disruption (only minor annoyances, but that's unavoidable). Currently there is no patch for this feature, sadly. - The final improvement I've primised for Yacc support, fixing automake bug#7648 and PR automake/491 at last. I've an half-baked patch series for this (my third attempt!) which basically rewrites ylwrap, and which should be fully transparent and backward-compatible. - Add new 'AM_PROG_AR' macro, triggering the 'ar-lib' script. There is a longish discussion about it at: http://lists.gnu.org/archive/html/automake-patches/2010-09/msg00121.html It would be great if Peter could provide an updated version of the patch. - We should deprecate (both in the manual and with with runtime warnings) the ansi2knr feature and the serial simple-tests driver (and remove them in automake 1.13). - Add support for non-default autotools in rebuild rules. http://lists.gnu.org/archive/html/automake-patches/2010-08/msg00182.html - Document how the Automake python support can be used easily and profitably with python virtual environments (of great usefulness in help the deployiment of mostly isolated, self-contained python apps). - Add support for user-defined recursive targets. http://lists.gnu.org/archive/html/automake/2010-08/msg00050.html http://lists.gnu.org/archive/html/automake/2010-08/msg3.html http://lists.gnu.org/archive/html/automake/2010-10/msg00011.html http://lists.gnu.org/archive/html/automake-patches/2010-10/msg00044.html - Decide whether we want an AM_ARG_ENABLE new macro, and if yes, implement it. http://lists.gnu.org/archive/html/automake/2010-09/msg00023.html - Check in at least a temporay fix for the VPATH Yacc/Lex failures with FreeBSD make: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=7884 We at least want make distcheck to pass, even when Yacc/Lex are disabled. And I'm sure I'm forgetting something ... Regards, Stefano
Re: [PATCH] test defs: don't allow `$me' to be overridden from the environment
* Stefano Lattarini wrote on Mon, Apr 18, 2011 at 12:33:25AM CEST: Subject: [PATCH] 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. This is better, but still lacking an explanation for the question I asked earlier. OK with that, and below. --- a/tests/defs.in +++ b/tests/defs.in @@ -65,10 +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 (but this should -# be done carefully, and *before* including ./defs). -if test -z $me; then +# 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). The comment change is redundant with the code change. +if test -z $am_test_name; then me=`echo $0 | sed -e 's,.*[\\/],,;s/\.test$//'` +else + me=$am_test_name fi How about reordering so it reads nicely? I.e., if test -n $am_test_name; then me=$am_test_name else ... Thanks, Ralf
Re: [PATCH] test defs: don't allow `$me' to be overridden from the environment
On Monday 18 April 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Apr 18, 2011 at 12:33:25AM CEST: Subject: [PATCH] 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. This is better, but still lacking an explanation for the question I asked earlier. Hmmm... I thought I had addressed all your questions properly, but clearly I'm wrong. Which question have I missed? OK with that, and below. --- a/tests/defs.in +++ b/tests/defs.in @@ -65,10 +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 (but this should -# be done carefully, and *before* including ./defs). -if test -z $me; then +# 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). The comment change is redundant with the code change. True, I'll revert it (the previous version of the comment is just fine). +if test -z $am_test_name; then me=`echo $0 | sed -e 's,.*[\\/],,;s/\.test$//'` +else + me=$am_test_name fi How about reordering so it reads nicely? I.e., if test -n $am_test_name; then me=$am_test_name else ... Fine with me. I'll change this too. Thanks, Stefano
Re: [PATCH] test defs: don't allow `$me' to be overridden from the environment
* Stefano Lattarini wrote on Mon, Apr 18, 2011 at 09:26:30PM CEST: On Monday 18 April 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Apr 18, 2011 at 12:33:25AM CEST: Subject: [PATCH] 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. This is better, but still lacking an explanation for the question I asked earlier. Hmmm... I thought I had addressed all your questions properly, but clearly I'm wrong. Which question have I missed? The one here: | 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. It answers why you want to make $me overridable *from within* the testsuite, but not why for the user running 'make check'. 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. Thanks, Ralf
[PATCH] java: allow both dist_JAVA and nodist_JAVA in the same Makefile.am (was: Re: bug#8434: java: cannot use JAVA with both dist_ and nodist_ prefixes)
On Saturday 09 April 2011, Ralf Wildenhues wrote: Hi Stefano, * Stefano Lattarini wrote on Wed, Apr 06, 2011 at 09:00:32PM CEST: Currently, limitations in the Automake support for java enforce the restriction that only one `_JAVA' primary can be used in a given Makefile.am. This bug is particularly annoying because *.java files listed with the `JAVA' primary are not included by default in the distribution. Ganted, one can always use EXTRA_DIST to work around this issue, but it would be nice if things Just Worked. Agreed on all accounts. Do you see a way to fix this, for arbitrary dependency structure between the java classes? Thanks for the bug report, Ralf Attached is a patch that fixes the problem. OK for a new 'java-work' branch based off of the right maint-based commits (as to make new tests like 'java-extra.test' and `java-noinst.test' available)? We can decide at a later date whether this branch has to be merged into maint or into master only. I'll push in 72 hours if there is no objection. Regards, Stefano From d76b503883525aaab6048102cbfe96bbdd9a0a85 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Mon, 18 Apr 2011 15:18:08 +0200 Subject: [PATCH] java: allow both dist_JAVA and nodist_JAVA in the same Makefile.am Fixes automake bug#8434. * automake.in (handle_java): Strip `dist_' and `nodist_' from the given prefix. Define a new internal Makefile variable `am__java_sources'. Related adjustments. * lib/am/java.am (JAVAC, JAVAROOT, CLASSPATH_ENV): Define only the first time this am file is processed. (class%DIR%.stamp): Stamp file renamed ... (class%NDIR%.stamp): ... to this, so that the `dist_' and `nodist_' prefixes are stripped from the name of the stampfile. Adjust declaration of dependencies by using the new automake-generated internal variable `$(am__java_sources)'. In the rule, use `$@' as the name of the target, rather than hard-coding it. * tests/java.test: Update and extend. * tests/java-no-duplicate.test: New test. * tests/java-mix-dist-nodist.test: Likewise. * tests/java-compile-and-install.test: Likewise. * tests/java-clean.test: Likewise. * tests/java-sources.test: Likewise. * tests/Makefile.am (TESTS): Update. --- ChangeLog | 23 ++ automake.in | 20 +++-- lib/am/java.am | 14 --- tests/Makefile.am |5 ++ tests/Makefile.in |5 ++ tests/java-clean.test | 57 ++ tests/java-compile-install.test | 86 +++ tests/java-mix-dist-nodist.test | 56 + tests/java-no-duplicate.test| 44 tests/java-sources.test | 61 +++ tests/java.test |9 +++- 11 files changed, 367 insertions(+), 13 deletions(-) create mode 100755 tests/java-clean.test create mode 100755 tests/java-compile-install.test create mode 100755 tests/java-mix-dist-nodist.test create mode 100755 tests/java-no-duplicate.test create mode 100755 tests/java-sources.test diff --git a/ChangeLog b/ChangeLog index 3ece73c..791d8e7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2011-04-18 Stefano Lattarini stefano.lattar...@gmail.com + + java: allow both dist_JAVA and nodist_JAVA in the same Makefile.am + Fixes automake bug#8434. + * automake.in (handle_java): Strip `dist_' and `nodist_' from + the given prefix. Define a new internal Makefile variable + `am__java_sources'. Related adjustments. + * lib/am/java.am (JAVAC, JAVAROOT, CLASSPATH_ENV): Define only the + first time this am file is processed. + (class%DIR%.stamp): Stamp file renamed ... + (class%NDIR%.stamp): ... to this, so that the `dist_' and `nodist_' + prefixes are stripped from the name of the stampfile. Adjust + declaration of dependencies by using the new automake-generated + internal variable `$(am__java_sources)'. In the rule, use `$@' + as the name of the target, rather than hard-coding it. + * tests/java.test: Update and extend. + * tests/java-no-duplicate.test: New test. + * tests/java-mix-dist-nodist.test: Likewise. + * tests/java-compile-and-install.test: Likewise. + * tests/java-clean.test: Likewise. + * tests/java-sources.test: Likewise. + * tests/Makefile.am (TESTS): Update. + 2011-04-06 Stefano Lattarini stefano.lattar...@gmail.com coverage: more on java support EXTRA_ and noinst_ prefixes diff --git a/automake.in b/automake.in index a8ec749..01b41a1 100755 --- a/automake.in +++ b/automake.in @@ -5107,20 +5107,32 @@ sub handle_java 'java', 'noinst', 'check'); return if ! @sourcelist; -my @prefix = am_primary_prefixes ('JAVA', 1, +my @prefixes = am_primary_prefixes ('JAVA', 1, 'java', 'noinst', 'check'); my $dir; -foreach my $curs (@prefix) +my @java_sources = (); +foreach my $prefix (@prefixes) { +
Re: [PATCH] test defs: don't allow `$me' to be overridden from the environment
On Monday 18 April 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Apr 18, 2011 at 09:26:30PM CEST: On Monday 18 April 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Apr 18, 2011 at 12:33:25AM CEST: Subject: [PATCH] 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. This is better, but still lacking an explanation for the question I asked earlier. Hmmm... I thought I had addressed all your questions properly, but clearly I'm wrong. Which question have I missed? The one here: | 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
Re: New release wishlist
On Monday 18 April 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Apr 18, 2011 at 12:07:14PM CEST: JFTR, here are the things that IMHO we must do for the next release: Right now, I'm aiming at 1.11.2, not 1.12. Ah, I didn't get that. Consider my seggestions moot for what concerns 1.11.2. The minimum for the former is *no* additional features. The important thing is no regressions, not really any new feature. I might be talked into reviewing ACLOCAL_PATH support for that, and maybe a couple of the other issues that fix bugs are safe enough (esp. we could think about deprecating ansi2knr), but other than that I'd be rather cautious. I want 1.11.2 to be pluggable for everyone that uses 1.11.1, without *any* thinking (i.e., distros should be able to just drop it in). Your list aims for 1.12. We definitely should try to have that appear within a reasonable time frame, too, if only because it's been so long in the making already. However, just only fixing existing regressions and polishing stuff and testing will take a few weekends, even without what you've listed. For 1.11.2, one thing I need to do is revert v1.11-328-ge87c030 or find another solution. For 1.12, we need to split the testsuite or implement support for multiple testsuites in one directory. Otherwise MinGW/MSYS won't work. Thanks, Ralf Agreed on all accounts. Thanks, Stefano
Re: [PATCH] test defs: don't allow `$me' to be overridden from the environment
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 stefano.lattar...@gmail.com 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 stefano.lattar...@gmail.com - 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 stefano.lattar...@gmail.com 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