Re: [PATCH] Enable `set -e' in more tests (plus some tweakings).
* Stefano Lattarini wrote on Thu, Nov 11, 2010 at 03:52:39PM CET: Pinging this patch again, following this: http://lists.gnu.org/archive/html/automake-patches/2010-11/msg3.html Patch is OK. I didn't check whether any of the tests contained constructs problematic for set -e. I'll wait the customary 72 hours (until sunday evening) before pushing. Thanks, Ralf
testsuite results from master
Hello Stefano, I think it is time to reevaluate some of the work that has been done in the last few weeks. I'd like to ask you to postpone pushes of pending stuff that has a clock ticking, and look at the lots of new failures that a testsuite run shows on a few hosts; you can find them in some minutes on http://autobuild.josefsson.org/automake/. I might be responsible for some of the failures too, but I think it's fair to guess that your patches are responsible for at least some of these. ;-) Thanks, Ralf
Re: testsuite results from master
* Stefano Lattarini wrote on Mon, Nov 15, 2010 at 12:19:00AM CET: On Sunday 14 November 2010, Ralf Wildenhues wrote: I think it is time to reevaluate some of the work that has been done in the last few weeks. Do you mean go and fix the newly introduced regressions before doing other testsuite work, or reconsider the advisability of doing testsuite work in master and/or to have a 72-hours gracetime? If you mean the former, then I agree. Yes, I mean the former. and look at the lots of new failures Which of those failures are really new? Some of them involve test scripts and automake features unmodified by quite a long time. I might be responsible for some of the failures too, but I think it's fair to guess that your patches are responsible for at least some of these. ;-) See my other answers to this thread for more information and questions. I will try to address your questions when I have time (i.e., tonight at the earliest), but it would really help if you had access yourself. Thanks, Ralf
Re: Testsuite failures on AIX 5.3
* Stefano Lattarini wrote on Sun, Nov 14, 2010 at 10:14:54PM CET: http://autobuild.josefsson.org/automake/log-20101114190221478.txt + /bin/grep -F -v @SET_MAKE@ Makefile.in + 1 Makefile.sed + make -f Makefile.sed SHELL=/bin/sh test1 echo ' @top_srcdir@/configure.in @top_srcdir@/aclocal.m4 @srcdir@/Makefile.am \ @srcdir@/Makefile.in @top_srcdir@/configure ansi2knr.1 ansi2knr.c \ depcomp install-sh missing' | grep ' ansi2knr\.c ' @SHELL@: not found make: 1254-004 The error code from the last command is 1. Stop. + exit_status=2 IMHO this is a spurious failure due to the old hackish check: $FGREP -v @SET_MAKE@ Makefile.in Makefile.sed $MAKE -f Makefile.sed SHELL=$SHELL test1 retained from older versions of ansi.test; since this check is superseded by the later ones: $AUTOCONF ./configure $MAKE test1 my suggested fix is to just drop the hackish check. But then I'm wondering why this code did not fail in the past. Must be a regression of some kind, either in the tests, or defs.in, or in other code. FAIL: colon5.test (exit: 2) FAIL: colon6.test (exit: 2) These tests fail for the same resons of ansi.test; I suggest to drop the older hackish checks and do more proper checks with autoconf, ./configure and make. BTW, there is a pending patch of mine aimed at making the `colon*.test' tests more semantic; see: http://lists.gnu.org/archive/html/automake-patches/2010-09/msg00110.html Changing that shouldn't be done to cover up a regression though. FAIL: fn99.test (exit: 2) FAIL: fn99subdir.test (exit: 2) I'm pretty sure these tests haven't been affected by my (or yours) latest patches: the failure is pre-existent IMO. Yes, indeed. This is fairly long-standing. Also, the tests fails due to a segmentation fault in the (make-spawned) shell, so the failures are better investigated by someone having direct access to the system in question. Thanks, Ralf
Re: Testsuite failures on Tru64 OSF 5.1
Hi Peter, * Peter Rosin wrote on Mon, Nov 15, 2010 at 09:19:44AM CET: This is based off of master (but I guess it also applies to tests-init) but the backport to the msvc branch is trivial. I stole the xsi-test from libtool. Would be good if someone tested on that Alpha before pushing I suppose. Patch is ok if we also have test coverage of 'compile' in non-MSVC mode on this system (so we can be sure that the shell doesn't barf skipping over the XSI code bits). You can omit dropping the output as you like. Thanks for the quick feedback! Cheers, Ralf Subject: [PATCH] Skip MSVC oriented tests if the shell is not capable. * tests/defs: New required entry 'xsi-shell'. * tests/ar-lib.test, tests/compile3.test, tests/compile6.test: Require a XSI capable shell. --- a/tests/defs +++ b/tests/defs @@ -290,6 +290,15 @@ do echo $me: running texi2dvi -o /dev/null --version ( texi2dvi -o /dev/null --version ) || exit 77 ;; +xsi-shell) + # Try some XSI features. + echo $me: trying some XSI constructs + ( _am_dummy=a/b/c + test ${_am_dummy##*/},${_am_dummy%/*},${_am_dummy#??}${_am_dummy%$_am_dummy}, \ + = c,a/b,b/c, \ + eval 'test $(( 1 + 1 )) -eq 2 \ + test ${#_am_dummy} -eq 5' ) /dev/null 21 || exit 77 + ;; *) # Generic case: the tool must support --version. echo $me: running $tool --version
Re: Testsuite failures on Tru64 OSF 5.1
* Stefano Lattarini wrote on Sun, Nov 14, 2010 at 11:00:16PM CET: http://autobuild.josefsson.org/automake/log-201011141902490189000.txt FAIL: backcompat5.test (exit: 1) [CUT] find ../foo-1.0 -print ## useful for debugging find: missing conjunction *** Exit 1 Stop. Seems like a limitation (bug) of the find utility; what I'd do is replacing the above find command simply with: find ../foo-1.0 WDYT? I think the comment is taken as part of the command. There shouldn't be any comments in recipe commands. FAIL: instspc-*-build.test (exit: 1) FAIL: instspc-*-install.test (exit: 1) Re-running the tests with `/bin/ksh -x' might really help here. No idea what might have ben gone wrong otherwise. And yes, this is clearly a bug introduced by my recent instspc.test refactoring patch. Not sure. instspc.test was skipped on most of my test systems because I did not have gcc in $PATH (deliberately, but for other reasons). How come the verbose output didn't land in test-suite.log though? Rest comes later. Cheers, Ralf
Re: Testsuite failures on AIX 5.3
* Stefano Lattarini wrote on Tue, Nov 16, 2010 at 08:04:49PM CET: Ralf, could you try the attached patches when you have time, and see if they fix the issues with ansi.test, colon5.test and colon6.test? They do. Please apply. Thanks! Ralf From f140b6bce15a1705ec8258e8914e7b605ef15f6f Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Tue, 16 Nov 2010 19:18:56 +0100 Subject: [PATCH] Fix regression in ansi.test (failure on AIX 5.3). * tests/ansi.test: Remove redundant hackish check done using a hand-postprocessed Makefile.in. This checs worked by setting the `$(SHELL)' macro on the command line of make, but this is not supported by the AIX 5.3 make implementation. This bug has been lurking for a long time, and was activated by commit v1.11-125-gc1f6cdb Enable `errexit' shell flag in various tests. Report by Ralf Wildenhues. [...]
Re: [SIMPLE PATCH] {master} Be more consistent in copyright notices.
* Stefano Lattarini wrote on Wed, Nov 17, 2010 at 12:22:57AM CET: While modifying the test script `suffix13.test' recently, I noticed that it uses an obsolete form of copyright notice: # This file is part of GNU Automake. # # GNU Automake is free software; you can redistribute it and/or modify [...] Unfortunately, there is nothing simple about copyright notices; see e.g. http://lists.gnu.org/archive/html/bug-standards/2010-11/msg5.html for related issues. Also, one plan of the ever-delayed upgrade to GPLv3+ plus exception was to let some of the helper scripts stand on their own, so that might influence this text as well. I will ask about this issue. Thanks for bringing it to my attention, Ralf
Re: [PATCHES] Fix spurious failures of some tests `silent*.test' with SunStudio Fortran
* Stefano Lattarini wrote on Wed, Nov 17, 2010 at 07:25:32PM CET: On Wednesday 17 November 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Wed, Nov 17, 2010 at 03:25:02PM CET: F77bar.o NOTICE: Invoking /opt/SunStudio/sunstudio12.1/prod/bin/f90 -f77 -ftrap=%none -g -c -o bar.o bar.f Now, these failure are clearly spurious, and easy worked around, so I'd like to apply the attached patches to temporary bugfixing branches, and merge them to maint. Well, the question that comes to mind is, is there a flag with which we could silence the compiler? Yes: `-silent'. But testing to see if it's supported would probably be more complex than just filtering out the offending messages... hmm... unless we hack the configure.in to do the testing for us, which shouldn't be hard to do. Do you want me to try that route and see how it works? I haven't made up my mind yet. At the moment I'm trying to find out whether it is suitable or desirable for the silent-rules option to trigger configure checks that set additional flags like -silent if verbose mode is not enabled. If we go this way, we shouldn't just do it for this one compiler. I know at least two more: another Fortran compiler, and MSVC, where this issue came up before already. A survey about other compilers would be good, before implementing something along these lines. The user might just not want to read that NOTICE. If not, then I'd squash the two patches together (is there a point in keeping them separate?), Well, they fix bugs introduced in two different commits, so they should be applied to different bug-fixing branches IMHO. However you like. I wouldn't call this a bug in the commit though, rather a bug in the compiler which we work around. and relax the sed pattern to kill all lines matching '^NOTICE:'. I can amend both the patches to do so. Yes, I think for the moment that is the best route. Even that might eventually not be enough with localization, I don't think that's an issue, since the SunStudio `f77' is a simple shell script wrapping the real compiler. No i18n/l10n involved. So? The next version might change. Let's not narrow our minds for one particular compiler (version) only. That is not maintainable. We only do such things in Libtool because for shared library semantics, there is often no other way. but at least then you aren't relying on specific compiler flags that might not be valid with a different compiler version, or different flags in $FCFLAGS or $FFLAGS. Cheers, Ralf
Re: [PATCHES] Fix spurious Lex/Yacc-related failures in tests `silent*.test' w.r.t. Solaris XPG4 make.
* Stefano Lattarini wrote on Wed, Nov 17, 2010 at 09:10:08PM CET: I'm seeing this failure with Solaris XPG4 make: $ MAKE=/usr/xpg4/bin/make sh silentyacc.test silentyacc: running bison --version bison (GNU Bison) 2.4.1 Written by Robert Corbett and Richard Stallman. [...] A similar failure happens with heirlom make on Debian GNU/Linux. Also, the tests `silentlex.test' and `silent5.test' experience similar spurious failures with those two make implementations. Now, these failures (which are due to the fact that those versions of make do not chain implicit rules) are spurious in regard to the being-tested silent-rules functionality, and easily worked around by adding proper explicit dependency declarations to the generated Makefile.am. So I'd like to apply the attached patches to temporary bugfixing branches, and merge them to maint. Objections? Yes. Do you expect our users to have to add the same rules (which expose internal details, namely the object file names) to their Makefile.am files, just so things are silent? I don't think so. So no, I don't think covering up those test failures is the right way to go. Even more, the added rules even change semantics of the build independently of silent-rules or not; *that* is something we need to fix in Automake proper. I think there might even be a pending patch from you for this. Remember, we don't have to (and I don't want to) fix all test failures just so that we don't see any FAILs in the logs. If things are broken, then it is good to see that in the logs. Aside, again I don't see why this couldn't have been a single patch. Thanks, Ralf Subject: [PATCH] Fix spurious failure in silent5.test w.r.t. Solaris XPG4 make. * tests/silent5.test (Makefile.am): Declare some dependencies explicitly, to cater to make implementations that don't otherwise chain implicit rules. This prevents (at least) Solaris XPG4 make from incorrectly using its own builtin .l - .o and .y - .o rules. Subject: [PATCH] Fix spurious failure in silentlex.test w.r.t. Solaris XPG4 make. * tests/silentlex.test (Makefile.am): Explicitly declare some dependencies to cater to make implementations that don't otherwise chain implicit rules. This prevents (at least) Solaris XPG4 make from incorrectly using its own builtin .l - .o rules.
Re: [PATCH] New test on repeated automake options.
Hi Stefano, * Stefano Lattarini wrote on Fri, Nov 19, 2010 at 12:26:18PM CET: * tests/repeated-options.test: New test, check that automake does not complain on repeated options, nor generate broken or incorrect makefiles. * tests/Makefile.am (TESTS): Updated. OK. Thanks, Ralf
Sync config-ml.in from GCC.
I pushed this to maint in Ian's name, to sync the script from GCC. Cheers, Ralf 2010-11-19 Ian Lance Taylor i...@... Sync config-ml.in from GCC. * config-ml.in: Add Go support: treat GOC and GOCFLAGS like other compiler/flag environment variables. diff --git a/lib/config-ml.in b/lib/config-ml.in index f2497ad..2dd0d51 100644 --- a/lib/config-ml.in +++ b/lib/config-ml.in @@ -2,7 +2,7 @@ # wanting multilib support. # # Copyright (C) 1995, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, -# 2005, 2006, 2007 Free Software Foundation, Inc. +# 2005, 2006, 2007, 2008, 2010 Free Software Foundation, Inc. # # This file is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -517,6 +517,7 @@ multi-do: prefix=$(prefix) \ exec_prefix=$(exec_prefix) \ GCJFLAGS=$(GCJFLAGS) $${flags} \ + GOCFLAGS=$(GOCFLAGS) $${flags} \ CXXFLAGS=$(CXXFLAGS) $${flags} \ LIBCFLAGS=$(LIBCFLAGS) $${flags} \ LIBCXXFLAGS=$(LIBCXXFLAGS) $${flags} \ @@ -750,7 +751,7 @@ if [ -n ${multidirs} ] [ -z ${ml_norecursion} ]; then break fi done -ml_config_env='CC=${CC_}$flags CXX=${CXX_}$flags F77=${F77_}$flags GCJ=${GCJ_}$flags GFORTRAN=${GFORTRAN_}$flags' +ml_config_env='CC=${CC_}$flags CXX=${CXX_}$flags F77=${F77_}$flags GCJ=${GCJ_}$flags GFORTRAN=${GFORTRAN_}$flags GOC=${GOC_}$flags' if [ ${with_target_subdir} = . ]; then CC_=$CC' ' @@ -758,6 +759,7 @@ if [ -n ${multidirs} ] [ -z ${ml_norecursion} ]; then F77_=$F77' ' GCJ_=$GCJ' ' GFORTRAN_=$GFORTRAN' ' + GOC_=$GOC' ' else # Create a regular expression that matches any string as long # as ML_POPDIR. @@ -822,6 +824,18 @@ if [ -n ${multidirs} ] [ -z ${ml_norecursion} ]; then esac done + GOC_= + for arg in ${GOC}; do + case $arg in + -[BIL]${ML_POPDIR}/*) + GOC_=${GOC_}`echo X${arg} | sed -n s/X\\(-[BIL]${popdir_rx}\\).*/\\1/p`/${ml_dir}`echo X${arg} | sed -n s/X-[BIL]${popdir_rx}\\(.*\\)/\\1/p`' ' ;; + ${ML_POPDIR}/*) + GOC_=${GOC_}`echo X${arg} | sed -n s/X\\(${popdir_rx}\\).*/\\1/p`/${ml_dir}`echo X${arg} | sed -n s/X${popdir_rx}\\(.*\\)/\\1/p`' ' ;; + *) + GOC_=${GOC_}${arg} ;; + esac + done + if test x${LD_LIBRARY_PATH+set} = xset; then LD_LIBRARY_PATH_= for arg in `echo $LD_LIBRARY_PATH | tr ':' ' '`; do
Re: [PATCH] {master} release-stats: account for generated `instspc-*.test' tests.
* Stefano Lattarini wrote on Fri, Nov 19, 2010 at 09:18:33PM CET: I've realized that my patch on instspc.test split refactoring has broken the release-stats target, since now there are other generated tests besides the `*-p.test' tests. The attached patch fix this glitch in a quick dirty way; a better fix would probably involve a refactoring of the whole release-stats target, but that can be done later IMHO. So, OK to apply the attached patch to a temporary branch based off of commit `v1.11-395-ge118126' Overhauled and modularized tests in `instspc.test'., and merge to master? This will break again when we add the next set of generated tests. How about determining them by grepping the test source for 'GENERATED AUTOMATICALLY' in the test source? OK with that change, and after testing suitably. Thanks, Ralf From 00dbb4fe7d27510718a60d721c932d4f8fb6b2dc Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Fri, 19 Nov 2010 21:16:03 +0100 Subject: [PATCH] release-stats: account for generated `instspc-*.test' tests. * Makefile.am (release-stats): Also account for generated tests `instspc-*.test'.
Re: [RFC] Docs: document silent make rules in a new chapter
* Stefano Lattarini wrote on Thu, Nov 18, 2010 at 09:22:48PM CET: On Thursday 18 November 2010, Nick Bowler wrote: On 2010-11-18 20:31 +0100, Stefano Lattarini wrote: +...@vindex @code{AM_V_GEN} +...@c FIXME: wouldn't $(AM_V_SILENT) be clearer? Should we deprecate +...@c $(AM_V_at)? It should be kept for backward-compatibility, of +...@c course. AM_V_GEN is a long enough name as it is; AM_V_SILENT would be even worse in this regard. AM_V_at is very useful for targets which have multiple commands. It's not that interesting to see GEN foo.bar five times in a row. There's probably a misunderstanding here; I was suggesting to rename `AM_V_at' to `AM_V_SILENT', for clarity; and deprecate *only* the old name `AM_V_at'. Does my proposal make sense now? It makes sense, but it's a long name. It's a close call I'd say but I wouldn't want to deprecate AM_V_at, simply because it is shorter. Other renaming suggestions have been made before, see e.g. this thread: http://lists.gnu.org/archive/html/bug-automake/2010-04/msg1.html But I'm quite hesitant to do any renames at all unless there is a clear advantage. Automake has had a slightly bad reputation in the past for not being backward compatible, and I wouldn't want that to return. (And I don't like overly verbose makefiles with lots of duplication either.) Cheers, Ralf
libtool --help: honor $AUTOCONF, $AUTOMAKE
I came across an interesting bug: the perl coverage output for a full run of the Automake test suite was showing a single invocation of the installed automake program, rather than the uninstalled $builddir/tests/automake-1.11a which should have been run. Some tracking revealed that this happened in automake/tests/subobj9.test which looks innocent enough. Turns out that it comes from './libtool --help' which tries to be helpful in printing some 'automake --version' output. This is flawed for a couple of reasons: the automake command at the time './libtool --help' is run may be unrelated to the automake used for bootstrapping the package; also, the user may have wanted to use an override by setting $AUTOMAKE. Now, the issue could be fixed with the two patches below, to Libtool and Automake, respectively. Question is whether we could do better than that: In Libtool, getopt.m4sh is used for the libtoolize and libtool scripts. For libtoolize, it seems appropriate to do as below. For libtool, the best answer would be the Autoconf and Automake versions used to bootstrap the package that created this particular libtool script. Comments before I apply? Thanks, Ralf Avoid running installed automake from 'libtool --help'. * tests/subobj9.test: Export AUTOCONF and AUTOMAKE. Together with fixed Libtool, this fixes check-coverage to not invoke installed automake. diff --git a/tests/subobj9.test b/tests/subobj9.test index 2045d58..83f3a31 100755 --- a/tests/subobj9.test +++ b/tests/subobj9.test @@ -64,6 +64,9 @@ $AUTOMAKE -a # Skip this test on configure errors (e.g., broken C++ compilers). ./configure || Exit 77 +# Ensure './libtool --help' will use the right tool versions. +export AUTOCONF AUTOMAKE + # Opportunistically check that --tag=CXX is used when supported. if ./libtool --help | grep tag=TAG; then $MAKE print stdout || { cat stdout; Exit 1; } Honor $AUTOCONF, $AUTOMAKE in --help output. * libltdl/config/getopt.m4sh (func_help): Use $AUTOCONF and $AUTOMAKE if set, for --version outout. diff --git a/libltdl/config/getopt.m4sh b/libltdl/config/getopt.m4sh index 2196743..e9363bc 100644 --- a/libltdl/config/getopt.m4sh +++ b/libltdl/config/getopt.m4sh @@ -592,8 +592,8 @@ func_help () s*\$LTCFLAGS*'$LTCFLAGS'* s*\$LD*'$LD'* s/\$with_gnu_ld/'$with_gnu_ld'/ - s/\$automake_version/'`(automake --version) 2/dev/null |$SED 1q`'/ - s/\$autoconf_version/'`(autoconf --version) 2/dev/null |$SED 1q`'/ + s/\$automake_version/'`(${AUTOMAKE-automake} --version) 2/dev/null |$SED 1q`'/ + s/\$autoconf_version/'`(${AUTOCONF-autoconf} --version) 2/dev/null |$SED 1q`'/ p d }
Re: [PATCH] {maint} Improve and extend tests on de-ansification support.
* Stefano Lattarini wrote on Tue, Nov 16, 2010 at 12:15:12AM CET: On Monday 15 November 2010, Ralf Wildenhues wrote: Well then we should adjust maintainer-check to not complain. Either way, maintainer-check results should not deteriorate. I'm not keen on meddling with the current maintainer-check rules, which are already quite hackish and not very easy to extend IMHO. ;-) So I'd like to seize this opportunity to push again my patch on a re-implementation of maintainer-check (in perl), which offers easy whitelisting of false positives, and more flexibility in pre-processing the input lines (which can be useful in case of more complex checks): http://lists.gnu.org/archive/html/automake-patches/2010-07/msg00084.html Gnulib has a few more facilities in this area, and several GNU projects use them already. It would be good if we worked toward common facilities here, rather than diverging more from it. Besides, I kinda dislike using perl where sed would suffice. Yes, I'm probably being selfish and stubborn on this one ... Another nit at cited patch of yours is that makefile rules run in parallel, the perl script tests don't. This helps speed, but also allows me to choose between stopping on first error or not (make -k). Thanks, Ralf
Re: [PATCH 4/5] Tests defs: avoid some useless subshells.
* Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:26:05PM CET: * tests/defs: In the loop on $required tools: avoid subshells where not neded. OK except for the last hunk: --- a/tests/defs +++ b/tests/defs @@ -297,12 +297,12 @@ do *) # Generic case: the tool must support --version. echo $me: running $tool --version - ( $tool --version ) || exit 77 + $tool --version || exit 77 It is not likely but possible that $tool is a special builtin, in which case the shell is allowed to exit after an error. Please leave the subshell here. ;; esac done Thanks, Ralf
Re: [PATCH 5/5] Tests required tools: also try `-v' option for GNU compilers.
* Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:27:43PM CET: * tests/defs.in: In the loop on $required tools, for gcc and g++, also run gcc -v (resp. g++ -v), to get more information, and for consistency with gcj. Did this help you for anything in any way? Patch is OK. Thanks, Ralf
Re: [PATCH 0/5] More patches for the tests-init branch
* Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:18:15PM CET: Tests defs: don't let useless variables leak in test scripts. Tests defs: new subroutine `skip' for test skipping. Tests defs: some cleanup and minor fixes. No ticking clock for this patches at the moment; the clock will be started only after the current testsuite regressions have been solved. Patches (1) through (3) will have to wait until somebody has figured out why the *+#$)! /bin/sh on Tru64 exits upon set +e case `(set -o) 2/dev/null` in *posix*) set -o posix;; esac if 'set -e' has been called at some earlier point (leading to the instspc*test and probably other spurious test failures). Might be some internal shell data corruption, not sure. Cheers, Ralf
Re: [PATCH 4/5] Tests defs: avoid some useless subshells.
* Stefano Lattarini wrote on Sat, Nov 20, 2010 at 02:14:57PM CET: On Saturday 20 November 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:26:05PM CET: # Generic case: the tool must support --version. echo $me: running $tool --version - ( $tool --version ) || exit 77 + $tool --version || exit 77 It is not likely but possible that $tool is a special builtin, in which case the shell is allowed to exit after an error. Please leave the subshell here. Good catch; I'll add a comment to explain why the subshell is needed in this case. Good idea. BTW, it's ok if I add also your name to the ChangeLog entry? Sure, if you like, but I didn't really contribute all that much to this patch. :-P Cheers, Ralf
Re: [SIMPLE PATCH] {master} Remove long-deprecated options --Werror and --Wno-error.
* Stefano Lattarini wrote on Fri, Nov 26, 2010 at 04:11:54PM CET: It seems the automake options `--Werror' and `--Wno-error' (kept for backward-compatibility only and, according to the git/CVS history, deprecated since commit c037f202602bf50f4e82a7f63d4624c3256df7f7, git-describe Release-1-6-1b-35-gc037f20, dated 2002-07-06) are not used anymore: http://www.google.com/codesearch?hl=enlr=q=%22--Werror%22+automakesbtn=Search http://www.google.com/codesearch?hl=ensa=Nq=%22--Werror%22++lang:automakect=rrcs_r=lang:automake The attached patch (for master) removes those options. OK to apply? OK, thanks. Cheers, Ralf From 437f05023998787bbec052065766626973098fc5 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Fri, 26 Nov 2010 16:04:06 +0100 Subject: [PATCH] Remove long-deprecated options --Werror and --Wno-error. These options has been deprecated at least since commit Release-1-6-1b-35-gc037f20, dated 2002-07-06. * automake.in (parse_arguments): Do not recognize anymore options `--Werror' and `--Wno-error' as synonyms of respectively `-Werror' and `-Wno-error'. * tests/werror.test: Update: use `-Werror' instead of `--Werror'.
Re: [FYI] {maint} Fix many typos in ChangeLog
Hi Stefano, * Stefano Lattarini wrote on Thu, Nov 25, 2010 at 12:25:47PM CET: I applied the attached patch to maint as abvious, merged into master and branch-1.11, and pushed. Thanks, also for the other patch. From 2601fd8f5688a2e9cb4c723c1c42007d3c019fca Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Thu, 25 Nov 2010 12:16:45 +0100 Subject: [PATCH] Fix many typos in ChangeLog @@ -111,7 +111,7 @@ Fix regression in ansi.test (failure on AIX 5.3). * tests/ansi.test: Remove redundant hackish check done using a - hand-postprocessed Makefile.in. This checs worked by setting + hand-postprocessed Makefile.in. This checks worked by setting That should be These checks, otherwise the sentence is still wrong. the `$(SHELL)' macro on the command line of make, but this is not supported by the AIX 5.3 make implementation. This bug has been lurking for a long time, and was activated by Cheers, Ralf
Re: [FYI] {maint} Tests: fix spurious failures in silent*.test for $CC != gcc
* Stefano Lattarini wrote on Thu, Nov 25, 2010 at 11:00:41PM CET: I pushed the attached patch to maint, and merged into master and branch-1.11. Thank you, that looks good. I haven't had a chance to test this, though, with Savannah still down (see http://identi.ca/group/fsfstatus). Cheers, Ralf See real-world motivations for this patch at: http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00222.html http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00223.html -*-*-*- Fix spurious silent*.test failures for $CC != gcc In some tests on automake-produced silent rules, we forced the use of gcc depmode to improve testsuite coverage; but this has unsurprisingly led to spurious failures when some non-GNU C compilers were used. So we are now careful to require GCC in tests that force gcc depmode. From reports by Ralf Wildenhues. * silent5.test: Test removed, its content split into ... * silent-many-generic.test, silent-many-gcc.test: ... these new sister tests, the latter of which forces gcc depmode and lists gcc in $required. * silentlex.test: Test removed, its content split into ... * silent-lex-generic.test, silent-lex-gcc.test: ... these new sister tests, the latter of which forces gcc depmode and lists gcc in $required. * silentyacc.test: Test removed, its content split into ... * silent-yacc-generic.test, silent-yacc-gcc.test: ... these new sister tests, the latter of which forces gcc depmode and lists gcc in $required. * tests/Makefile.am (TESTS): Updated.
Re: [PATCH] {master} Improve and extend tests on `:=' variable assignments.
* Stefano Lattarini wrote on Thu, Nov 25, 2010 at 02:37:28PM CET: The attached patch is based off of maint, and intended for master. OK to apply? With nits addressed. Thanks, Ralf Improve and extend tests on `:=' variable assignments. * tests/colneq.test: Avoid useless use of wildcards and extra variable assignments in Makefile.am. Use command-line automake options instead of editing AUTOMAKE_OPTIONS in Makefile.am. Make grepping of the generated Makefile.in slightly stricter. Add a trailing `:' command. * tests/colneq.test: Avoid useless use of EXTRA_DIST special variable in Makefile.am. Do not create dummy files which are not needed anymore. Also run autoconf, ./configure and make. Add a trailing `:' command. * tests/colneq3.test: New test, similar to colneq.test, but running ./configure and make. * tests/Makefile.am (TESTS): Update. --- a/tests/colneq.test +++ b/tests/colneq.test @@ -21,14 +22,14 @@ set -e cat Makefile.am 'END' -ICONS := $(wildcard *.xbm) Please leave the wildcard line in. Removing it makes assumptions about the parser. -data_DATA = $(ICONS) +FOOBAR := zardoz END $ACLOCAL AUTOMAKE_fails grep ':=.*not portable' stderr -echo 'AUTOMAKE_OPTIONS = -Wno-portability' Makefile.am -$AUTOMAKE -grep 'ICONS :=' Makefile.in +$AUTOMAKE -Wno-portability +grep '^FOOBAR *:= *zardoz *$' Makefile.in + +: --- a/tests/colneq2.test +++ b/tests/colneq2.test @@ -20,14 +20,23 @@ set -e +cat configure.in 'END' +AC_OUTPUT +END + cat Makefile.am 'END' t = a b c -EXTRA_DIST = $(t:=.test) Please leave the EXTRA_DIST line in. It is something different if automake skips a normal variable containing this, and a variable that is special to automake. +FOO = $(t:=.test) +.PHONY: test +test: + test x'$(FOO)' = x'a.test b.test c.test' END -: a.test -: b.test -: c.test - $ACLOCAL +$AUTOCONF $AUTOMAKE + +./configure +$MAKE test + +: --- /dev/null +++ b/tests/colneq3.test @@ -0,0 +1,46 @@ +# Test that := definitions work as expected at make time. + +required=GNUmake +. ./defs || Exit 1 + +set -e + +cat configure.in 'END' +AC_OUTPUT +END + +cat Makefile.am 'END' +BAR := $(FOO) +BAZ = $(FOO) +FOO := foo Uh, oh. Thin ice. This is OK, but we gotta remember that it won't work reliably if some of these variables are actually automake-set before they are overridden. automake generally orders all of its variable settings before all of the user ones (so the user ones are preferred). When I override, e.g., libdir here, however, it doesn't get reordered to the user part. I wonder whether that is a bug in automake. +.PHONY: test +test: + test x'$(FOO)' = x'foo' + test x'$(BAZ)' = x'foo' + test x'$(BAR)' = x +END + +$ACLOCAL +$AUTOCONF +$AUTOMAKE -Wno-portability Trailing white space. + +./configure +$MAKE test + +:
Re: [PATCH] {master} Improve and extend tests on `:=' variable assignments.
* Stefano Lattarini wrote on Mon, Nov 29, 2010 at 09:09:59PM CET: On Monday 29 November 2010, Ralf Wildenhues wrote: I approve the patch but ask you to keep that coverage in, now you update the patch with an unrelated new change whose applicability depends on completely different factors (namely deciding whether some behavior is desirable or not), Why should its applicatibility depend on such a decision? The testcase just serves to expose the current behaviour explicitly, without telling if it's desirable or not. IMVHO an entry in the bug tracker is more applicable for issues that may be valid or invalid. A commit adding a test that is later removed because it's invalid is not so easily searched for a couple of years later. Closed PRs on the other hand are straightforward to search. Cheers, Ralf
savannah git repo updated again
Hello Stefano, all, I pushed the commits that were missing from the rewound git repo upstream again, please check that the tree matches yours, then it should be good for pushing to again. Thanks, Ralf
Re: [PATCHES] {master} Optimize tests `instspc-*.test' for speed.
* Stefano Lattarini wrote on Thu, Dec 02, 2010 at 06:17:49PM CET: Since Savannah is up again now, I'm pinging this patch. Thanks. I'll wait the customary 72 hours (until sunday evening) before pushing, if there are no objections. I think this patch deserves a proper review. Sorry for the delay, I'll try to make time soonish. Thanks, Ralf
Re: [PATCH] Extend checks on remake rules.
* Stefano Lattarini wrote on Sun, Dec 05, 2010 at 06:06:00PM CET: I'll wait the customary 72 hours (until wednesday evening) before pushing. This patch should have proper testing on several systems, and seems ok when tested and with nitse below addressed. If you still don't have access, please ping the person involved again, and me so that I can test the patch eventually. A couple of issues in this patch I'm seeing just from glancing at it is that it touches this sed limitation (quoting autoconf.info): If a sed script is specified on the command line and ends in an `a', `c', or `i' command, the last line of inserted text should be followed by a newline. Otherwise some `sed' implementations (e.g., OpenBSD 3.9) do not append a newline to the inserted text. In remake1a, your sanity check requires that 'make -n' works to really not remake anything. If that were the main purpose of the test, that would be fine, but as a sanity check I'd rather not use -n here. What is remake11.test for? Some of the tests look like they are testing the same things really as other new or existing tests. Can you explain them (ideally, the comment at the start of the test should be unique for each test so we can infer from that that each of the tests is actually required)? But maybe I'm just overlooking something? I wish you'd get yourself convinced to get rid of some of the 'for debugging' type comments, these things seem a bit obvious to need a comment. OTOH, I can see that some of them really help getting over the wait, isn't that a typo sort of feeling. Hmm, maybe leave them in, skipping over them isn't that hard. The updated rebased patch is attached. In it I have: - removed the overly hackish and complex tests remake2{a,b,c}.test, introduced by the previous version of my patch; I'll hopefully be able to find a saner way to improve coverage in that area (it can always be done in a follow-up patch without problems); - removed a uselessly-added make distcheck from `remake4.test'; - removed all usages of in-line shell commments in make rules (since it turned out they're unportable to at least Tru64/OSF 5.1 make); this affected only test `remake12.test'; - removed usages of `##' special automake comments which weren't on lines of their own (this usage is not part of the automake API) - added or extended comments for trickier tests; - thrown in some micellanous minor improvements and simplifications. Good! Thanks, Ralf
Re: [PATCH] {master} Extend and improve tests on DejaGnu support.
Hi Stefano, * Stefano Lattarini wrote on Fri, Dec 10, 2010 at 06:06:53PM CET: On Thursday 09 December 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Sun, Nov 14, 2010 at 02:29:33PM CET: + $MAKE distcheck Can't you either omit this distcheck, or the vpath = : case above, without decreasing coverage? The vpath = : case also checks the reaction to failing tests in a VPATH build, so I'd rather not remove it. As for the distcheck, I think it has a real, albeit limited, usefulness, since it shows that the package is self-contained and its distribution works as expected. So I'd prefer to keep it, but I can also remove it if you insist. Hmm... but thinking about it, running make distcheck only once would IMHO be better in terms of performance, and wouldn't hinder coverage. So what about the following squash-in? Looks better. OK to go with it! Thanks! Ralf
Re: [PATCH] {master} More uses of AS_HELP_STRING in automake macros.
Hi Stefano, * Stefano Lattarini wrote on Sun, Dec 12, 2010 at 02:47:35PM CET: * m4/depend.m4 (AM_DEP_TRACK): Use `AS_HELP_STRING' to format the help message added to the generated configure. * m4/dmalloc.m4 (AM_WITH_DMALLOC): Likewise. * m4/lispdir.m4 (AM_PATH_LISPDIR): Likewise. * m4/maintainer.m4 (AM_MAINTAINER_MODE): Likewise. * m4/multi.m4 (AM_ENABLE_MULTILIB): Likewise. * tests/help-depend.test: Updated. * tests/help-depend2.test: Likewise. * tests/help-maintainer.test: Likewise. Also, bumped all serial numbers of the modified m4 files. Did you check that the configure --help output was still right? Patch is ok then. Thanks, Ralf
Re: [PATCH] {tests-init} Tests defs: de-uglify code for requiring an XSI-capable shell.
* Stefano Lattarini wrote on Sun, Dec 12, 2010 at 10:06:58PM CET: This is just a cosmetic patch. I will push in 72 hours (Wednesday evening) if nobody objects. Please don't. It's a feature that the code is the same as the one tested in Libtool _LT_CHECK_SHELL_FEATURES: bug-tested and all. I don't see this fixing a bug, so it can only regress. Thanks, Ralf Tests defs: de-uglify code for requiring an XSI-capable shell. * tests/defs (xsi-shell): Use `foo', not `_am_dummy', as test variable. Use multiple eval where appropriate. Indent using only spaces, not tabs.
Fix testsuite failure of check12.test without DejaGNU.
Merged to master. Cheers, Ralf Fix testsuite failure of check12.test without DejaGNU. * tests/check12.test: Require runtest. diff --git a/tests/check12.test b/tests/check12.test index caa8e0f..34c8409 100755 --- a/tests/check12.test +++ b/tests/check12.test @@ -17,6 +17,7 @@ # Test to make sure dejagnu tests, automake-style tests, and check-local # target can coexist. +required=runtest . ./defs || Exit 1 set -e
Re: [PATCH 0/2] Tests initialization: put default definition of AC_CONFIG_AUX_DIR in the pre-populated configure.in.
* Stefano Lattarini wrote on Mon, Dec 13, 2010 at 09:42:08PM CET: On Monday 13 December 2010, Ralf Wildenhues wrote: I'm undecided on this one. On the one hand, the safety increase is a plus, but on the other hand, the tests become less readable, if only because it is less obvious what is going on. If you don't remember, you have to start reading the generated configure.in file after rerunning the test with keep_testdirs=:. Hmm. There is precedent with the 'parallel_tests' settings, Speaking against my own interest (i.e. seeing the patch applied), I must say that the 'parallel_tests' setting was more compelled, since it was needed for having generated tests that are just a thin layer around their sister tests. but even that looks unobvious to me, just like those actions triggered by a 'required' setting other than is this test run or not. Maybe we could solve this problem with a new `write_configure' subroutine in tests/defs, so that we could end up having e.g.: That's better because of this: I like this idea because, as a general rule, I think that explicit is better than implicit. Also, the `write_configure' subroutine might be easily extended in the future, to provide additional goodies. WDYT? but it still has the same problems with respect to the test being obvious and copy-pasteable. Again: what is the risk here that this change is trying to avoid (which presumably was the reason for it in the first place)? Thanks, Ralf
Re: [PATCH] Improve tests on generated portions of configure help screen. (was: Re: [PATCH] {master} More uses of AS_HELP_STRING in automake macros.)
Hi Stefano, * Stefano Lattarini wrote on Tue, Dec 14, 2010 at 10:41:17AM CET: OK, I've split the patch in two parts: the first part (for maint) improves the tests on automake-generated portions of configure help screens, the second part (for master) introduces more uses of `AS_HELP_STRING' in the automake macros, and should not require any updating or tightening of the tests *once the first part is applied and merged*. Attached is the first patch (for maint). Ok to apply? Both of these patches are OK, thanks. Cheers, Ralf From 3a5f8cc69e6687d2f074522cf9d69be4eea6d8f8 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Sun, 12 Dec 2010 14:17:41 +0100 Subject: [PATCH] Improve tests on generated portions of configure help screen. * tests/help-depend.test: Grepping of configure help screen relaxed to cater for possible line wrapping, and tightened in other respects. * tests/help-depend2.test: Likewise. * tests/help-dmalloc.test: Likewise. * tests/help-lispdir.test: Likewise. * tests/help-maintainer.test: Likewise. * tests/help-multilib.test: Likewise. * tests/help-silent.test: Likewise. * tests/help-upc.test: Likewise. * tests/help-init.test: Grepping of configure help screen tightened.
Re: Fix testsuite failure of check12.test without DejaGNU.
* Stefano Lattarini wrote on Mon, Dec 13, 2010 at 08:37:35PM CET: On Monday 13 December 2010, Ralf Wildenhues wrote: Fix testsuite failure of check12.test without DejaGNU. * tests/check12.test: Require runtest. JFTR, notice that I tend to have available and installed as many programs required by the Automake testsuite as possible even in my testing sandboxes, to ensure an higer coverage. The obvious downside of this habit is that I might forget to add some tools to $required sometimes, causing this kind of spurious failures. Luckily, such mistakes are very easily fixed, so I find them a more than acceptable trade-off. I hope you agree with me on this. Sure, that's fine. Thanks, Ralf
Re: [PATCH 2/2] Extended tests on AC_CONFIG_AUX_DIR.
* Stefano Lattarini wrote on Wed, Dec 15, 2010 at 09:49:55PM CET: On Tuesday 14 December 2010, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Sat, Dec 11, 2010 at 03:00:34PM CET: Can you update this patch to not require the previous 1/2? Yes, the updated patch is attached. I will push it in 72 hours, but I'd like to have it reviewed anew if possible. -# The ././ prefix confuses Automake into thinking it is doing a -# subdir build. Yes, this is hacky. (This comment should be retained, along with the usage below.) Why? Isn't such a relying on automake internals a Bad Thig? Sure it is. That said, I've restored the original auxdir.test (with minor improvements, see the attached patch) to retain such an usage. What was the previous `auxdir.test' resulting from the application of my original patch has been moved in a new test `auxdir8.test' OK. --- a/tests/auxdir2.test +++ b/tests/auxdir2.test @@ -25,4 +25,6 @@ set -e : Makefile.am $ACLOCAL -$AUTOMAKE +$AUTOMAKE -a This changes the code paths exercised (i.e., potentially removes coverage); Not really IMHO; It may easily, after we modify automake.in a bit. At least it's not far-fetched to assume that. Granted, it requires knowing the code a bit to see or assume that. the test is expected to fail anyway, and the `-a' just help to ensure that it fails for the right reason (i.e. use of computed AC_CONFIG_AUX_DIR, not missing auxiliary file). That said ... please use either $AUTOMAKE -a $AUTOMAKE ... I've done this (the former). Good. +if mkdir aux; then What happens with this command on w32? I checked: MinGW mkdir fails DJGPP mkdir fails Cygwin mkdir passes It seems that none of the systems actually cause harmful problems. Good, because they shouldn't; Sure, but that's one of the things where I'm wary. We've had configure tests in Libtool which froze a system, or required root access to fix, on some of the older unices. I simply wasn't sure the above wasn't one of them. this second, conditional check... + AUTOMAKE_fails + grep '^configure\.in:2:.*aux.*W32' stderr ... serves just to ensure that Automake fails with a correct error (i.e. about portability) even on platforms where the use of `aux' is valid, and even when an `aux' directory actually exist. Sure. +nil=__no_such_program +unset NONESUCH || : # just to be sure just to be sure is a fairly meaningless comment. It actually made me wonder whether you meant the || : or the unset part with it. I meant the unset part. I'm not sure what you want to be sure of with the unset here. That no `NONESUCH' variable is in the environment. I've gone with this now: # Make sure that we don't have a NONESUCH variable set # in the environment. unset NONESUCH || : Where I'd just remove the comment, because it's redundant: the code explains what it does. ;-) +out=out0 $MAKE test I'd write env out=out0 ... here (and below), but only to make it clearer what is happening. No big deal either way, so use whatever you prefer. (It makes a difference when the command is a shell special one.) Well, not using `env' means one less fork ;-) And I'm pretty confident that `make' is not going to become a shell builtin ;-) OK. Subject: [PATCH] Extended tests on AC_CONFIG_AUX_DIR. * tests/auxdir.test: Enable `errexit' shell flag. Prefer `$me' over hard-coded test name. Use proper m4 quoting. Add trailing `:' command. * tests/auxdir2.test: Likewise. Try to call automake also with the `-a' option, so that it will not fail for spurious reasons. * tests/auxdir3.test: Add an explanatory comment and a trailing `:' command. * tests/auxdir4.test: Prefer `$me' over hard-coded test name. Make grepping of automake stderr slightly stricter. Also, now this just checks for unportable auxdir names (and it has been Generally, prefer active voice rather than passive in log entries: Check for unportable foo... (this is documented in GCS). FWIW I'd write s/auxdir/auxiliary directory/ extended in this respect). Checks for non-existent auxdirs has also here (...ies) been moved out to ... * tests/auxdir5.test: .. this new test. * tests/auxdir6.test: New test. * tests/auxdir7.test: Likewise. * tests/auxdir8.test: Likewise. * tests/auxdir9.test: Likewise. * tests/Makefile.am (TESTS): Updated. --- a/tests/auxdir2.test +++ b/tests/auxdir2.test +# Both these two invocations are meant. Comment writing is hard! :-) The above still doesn't explain *why* both are done, so while it does greater than zero information, it could still be better. What about this? # Exercise both code paths concerning auxiliary files. +$AUTOMAKE -a $AUTOMAKE Patch is ok then. Thanks, Ralf
Re: [PATCH] Extend checks on remake rules.
* Stefano Lattarini wrote on Thu, Dec 16, 2010 at 01:27:10AM CET: On Wednesday 15 December 2010, Ralf Wildenhues wrote: [ magic strings ] Why all the variation in these? That makes the tests harder to read. I'd rather not change the use of magic strings on the other remake tests, unless you insist. The test remak3a.test is IMHO simple enough not to cause confusion, and changing the tests remake8{a,b}.test would be quite cumbersome. I don't insist. + test x'$(am_fingerprint)' = x'DummyValue' Why do you quote DummyValue here? Partly for symmetry with the left side, and partly (especially I'd say) to make the leading `x' stick out better as not a part of the expected value. The leading x should not be needed on either side. True, should not -- but I got in the habit of always using it, even where technically not needed, rather then risking to forgot it one time when it's really required. (several instances below) Should I remove them? (sorry if I ask, but the should above does not make it clear if you're asking to remove them or just noting that they are not really required). Ah, the fun of English negation and passive voice. I'm not totally firm in this area either, but I think that should not be needed has the meaning of is not needed, at least I think so rather than don't do this or I'll poke you with a stick. I hope somebody corrects me on this if my interpretation is wrong. Attached is also the amended patch. I will push in 72 hours if there are no more objections (and if all my testing on Linux and Solaris succeeds). I don't object, but TBH, I didn't look at the patch again. ;-) Thanks! Ralf
Avoid false positive in sc_tests_plain_make maintainer-check.
I'm seeing this in master now: ../automake/tests/remake11.test:makefiles_am_list=`find . -name Makefile.am | LC_ALL=C sort` ../automake/tests/remake11.test:makefiles_list=`echo $makefiles_am_list | sed 's/\.am$//'` Do not run make in the above tests. Use $MAKE instead. Makefile:1090: recipe for target `sc_tests_plain_make' failed make: *** [sc_tests_plain_make] Error 1 Merging the patch below in the tests-remake-extend branch to avoid it. Thanks, Ralf Avoid false positive in sc_tests_plain_make maintainer-check. * Makefile.am (sc_tests_plain_make): Ensure to only match full `make' words. Avoid false positive with remake11.test. diff --git a/Makefile.am b/Makefile.am index ae096cb..15aa3b3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -342,7 +342,7 @@ sc_tests_make_without_am_makeflags: ## Tests should never call make directly. sc_tests_plain_make: - @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*make'; then \ + @if grep -v '^#' $(srcdir)/tests/*.test | $(EGREP) ':[ ]*make( |$$)'; then \ echo 'Do not run make in the above tests. Use $$MAKE instead.' 12; \ exit 1; \ fi diff --git a/Makefile.in b/Makefile.in index 04d7067..1c1bf81 100644 --- a/Makefile.in +++ b/Makefile.in @@ -1087,7 +1087,7 @@ sc_tests_make_without_am_makeflags: fi sc_tests_plain_make: - @if grep -v '^#' $(srcdir)/tests/*.test | grep ':[ ]*make'; then \ + @if grep -v '^#' $(srcdir)/tests/*.test | $(EGREP) ':[ ]*make( |$$)'; then \ echo 'Do not run make in the above tests. Use $$MAKE instead.' 12; \ exit 1; \ fi
Re: [PATCH] Improvements and extend tests on canonicalization.
* Stefano Lattarini wrote on Sat, Dec 11, 2010 at 06:41:46PM CET: Ping on this? Reference: http://lists.gnu.org/archive/html/automake-patches/2010-09/msg00106.html The updated patch is attached. I will push it in 72 hours (by tuesday evening) unless there are objections. --- /dev/null +++ b/tests/canon7.test @@ -0,0 +1,93 @@ +# Stress test on canonicalization. [...] +cat Makefile.am 'END' +noinst_PROGRAMS = dummy_static dummy_dynamic ,foo-bar +noinst_LIBRARIES = libb.az+baz.a +noinst_LTLIBRARIES = lib~zardoz,,.la + +dummy_static_SOURCES = dummy.c lib.h +dummy_dynamic_SOURCES = $(dummy_static_SOURCES) + +dummy_static_LDADD = $(noinst_LIBRARIES) +dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) + +_foo_bar_SOURCES = libs.c +libb_az_baz_a_SOURCES = libs.c +lib_zardoz___la_SOURCES = libd.c + +check-local: + ls -l + ./,foo-bar + ./dummy_static + ./dummy_dynamic + ./,foo-bar | grep 'Hello, FooBar!' + ./dummy_static | grep 'Hello from Static!' + ./dummy_dynamic | grep 'Hello from Dynamic!' +END + +cat foobar.c 'END' +#include stdio.h +int main(void) +{ + printf(Hello, FooBar!\n); + return 0; +} +END + +cat dummy.c 'END' +#include stdio.h +#include lib.h +int main(void) +{ + printf(Hello from %s!\n, dummy_func()); + return 0; +} +END + +echo 'char *dummy_func(void);' lib.h +echo 'char *dummy_func(void) { return Dynamic; }' libd.c +echo 'char *dummy_func(void) { return Static; }' libs.c I'm not sure how you tested this, but ',foo-bar' fails to link for me on GNU/Linux (and supposedly everywhere else, lacking a 'main' symbol). Also, the code passes pointers to read-only memory (literal strings) in non-const pointers (literal strings may not be modifiable). I'm pushing the fix below. Note that you're lucky using convenience archives here. With shared libraries, the const data would need special handling in order to work on w32. (See the Libtool documentation, git version, for details.) Thanks, Ralf Fix canon7.test failure. * tests/canon7.test (_foo_bar_SOURCES): Add foobar.c. (lib.h, libd.c, libs.c): Use const for constant strings. diff --git a/tests/canon7.test b/tests/canon7.test index 9b3d8d0..3f25d6f 100755 --- a/tests/canon7.test +++ b/tests/canon7.test @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES) dummy_static_LDADD = $(noinst_LIBRARIES) dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) -_foo_bar_SOURCES = libs.c +_foo_bar_SOURCES = libs.c foobar.c libb_az_baz_a_SOURCES = libs.c lib_zardoz___la_SOURCES = libd.c @@ -76,9 +76,9 @@ int main(void) } END -echo 'char *dummy_func(void);' lib.h -echo 'char *dummy_func(void) { return Dynamic; }' libd.c -echo 'char *dummy_func(void) { return Static; }' libs.c +echo 'const char *dummy_func(void);' lib.h +echo 'const char *dummy_func(void) { return Dynamic; }' libd.c +echo 'const char *dummy_func(void) { return Static; }' libs.c libtoolize $ACLOCAL
Re: [PATCH] Improvements and extend tests on canonicalization.
* Stefano Lattarini wrote on Thu, Dec 16, 2010 at 08:16:25PM CET: On Thursday 16 December 2010, Ralf Wildenhues wrote: --- a/tests/canon7.test +++ b/tests/canon7.test @@ -43,7 +43,7 @@ dummy_dynamic_SOURCES = $(dummy_static_SOURCES) dummy_static_LDADD = $(noinst_LIBRARIES) dummy_dynamic_LDADD = $(noinst_LTLIBRARIES) -_foo_bar_SOURCES = libs.c +_foo_bar_SOURCES = libs.c foobar.c _foo_bar_SOURCES = foobar.c should suffice here. Feel free to fix that. Thanks! Ralf
Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.
* Stefano Lattarini wrote on Mon, Dec 13, 2010 at 07:54:05PM CET: OK to apply to a temporary branch off of maint, and merge to master? The patch is ok with nits addressed. BTW, notice that I'm planning to further extend the Lex/Yacc tests and make them more semantic, but that should be better done in a follow-up patch IMVHO. If there is any way I can get you to not do any more of such conversions: Please don't work even more on *these* tests. They are Good Enough[tm]. The incremental gain from more change is not worth the additional work from you nor review from me. Actually, lex and yacc support has *several* *real* issues, with maybe more than a dozen reported bugs in the last few years, many of them unfixed. See the list archives. It would be really nice if somebody who knows their ways around bison/yacc and flex/lex well (or is willing to learn) could look into some of the issues. I should warn that it's not easy though to come up with coherent/consistent and portable improvements. Subject: [PATCH] Extend, fix and improve tests on Lex and Yacc support. * tests/lexcpp.test: New test script, on support for Lex + C++. * tests/lexvpath.test: New test script, test build and rebuild rules for lexers in VPATH setup. * tests/yacc-basic.test: New test script, run simple semantic checks on basic Yacc support (similarly to what lex3.test does for Lex support). * tests/lex.test: Don't create useless dummy source file joe.l. Remove extra blank lines. (Makefile.am): Remove useless definition of $(LDADD). * tests/lex4.test: Add trailing `:' command. Do not create dummy useless lex source file. * tests/lex2.test: Likewise. Call automake with the `-a' option, so that it doesn't fail for the absence of `ylwrap' script. Make grepping of automake stderr stricter. * tests/yacc7.test: Add trailing `:' command. Enable `errexit' shell flag earlier (just after having sourced ./defs). * tests/yacc4.test: Likewise. Also ... (configure.in): Use pre-populated skeleton set up by ./defs, instead of writing one from scratch. Other minor cosmetic changes. * tests/yacc5.test: Likewise. * tests/yaccvpath.test: Likewise. Also ... ($distdir): New variable. Use it throughout. * tests/lex5.test: Likewise. * tests/lex3.test: Likewise. Check the distdir, rather than grepping the distribution tarball. Extend the test on the created binary, and be sure to avoid hangs. Add some comments. * tests/yacc.test: Use stricter grepping. Add trailing `:'. * tests/yacc6.test: Likewise. * tests/yacc3.test: Likewise. Prefer `cp -f' over plain `cp'. Do not create unused file `Makefile.sed'. Remove useless rules from Makefile.am. Other minor cosmetic changes. * tests/yacc2.test: Make grepping of generated `Makefile.in' and of automake error messages stricter. Do not redirect output of grep to /dev/null. Prefer `cp -f' over plain `cp'. Move call to aclocal earlier. Reduce the number of empty blank lines. Fix a typo in comments. * tests/yacc8.test: Fixed bugs that reduced the completeness of the tests. Added trailing `:' command. (configure.in): Use pre-populated skeleton set up by ./defs, instead of writing one from scratch. * tests/yaccpp.test: Test also extensions `.y++', `.ypp', and `.yxx', rather than only `.yy'. * tests/defs.in ($required): Better recognition of requirement flex. * tests/Makefile.am (TESTS): Update. --- a/tests/defs.in +++ b/tests/defs.in @@ -113,6 +113,13 @@ do echo $me: running etags --version -o /dev/null ( etags --version -o /dev/null ) || exit 77 ;; +flex) + # Since flex is required, we pick LEX for ./configure. + LEX=flex + export LEX + echo $me: running flex --version + ( flex --version ) || exit 77 + ;; I don't understand why the new 'required=flex' entry should be needed in defs.in. Any tests that fail without the defs.in change? +++ b/tests/lex.test @@ -26,22 +26,16 @@ END cat Makefile.am 'END' bin_PROGRAMS = zot zot_SOURCES = joe.l -LDADD = @LEXLIB@ Please don't remove that. It is documented to be required. END --- a/tests/lex3.test +++ b/tests/lex3.test +# Basic semantic checks on Lex support. # Test associated with PR 19. # From Matthew D. Langston. cat Makefile.am 'END' @@ -46,38 +44,44 @@ END cat foo.l 'END' %% -END return EOF; +GOOD return EOF; . %% int main () { - while (yylex () != EOF) -; - - return 0; + if (yylex () == EOF) +return 0; + else +return 1; That's not semantic either. One wouldn't write a lexer like that. } END +# Program should build and run. $MAKE -echo 'This is the END' | ./foo -$MAKE distcheck +echo GOOD | ./foo +echo BAD | ./foo Exit 1 --- a/tests/lex5.test +++ b/tests/lex5.test @@ -33,10 +29,9 @@ AC_OUTPUT END cat Makefile.am 'END' -AUTOMAKE_OPTIONS = subdir-objects -LDADD = @LEXLIB@ - -bin_PROGRAMS= foo/foo
Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.
* Stefano Lattarini wrote on Thu, Dec 16, 2010 at 10:10:29PM CET: On Thursday 16 December 2010, Ralf Wildenhues wrote: BTW, notice that I'm planning to further extend the Lex/Yacc tests and make them more semantic, but that should be better done in a follow-up patch IMVHO. If there is any way I can get you to not do any more of such conversions: Please don't work even more on *these* tests. They are Good Enough[tm]. IMHO they're not. See just below. The incremental gain from more change is not worth the additional work from you nor review from me. Actually, lex and yacc support has *several* *real* issues, with maybe more than a dozen reported bugs in the last few years, many of them unfixed. See the list archives. Yes, but I woldn't dare trying to modify the Lex/Yacc related code with the limited understanding I have of it, without having a *much* stronger testsuite than it's currently available. The *current* tests are good enough for the current code. How many bugs have we found due to the testsuite frenzy in the last few months with its 200+ patches? I'd guess less than 5. That's an awful signal to noise ratio. How much have you gotten to know the Automake code base (outside of tests/) as a result of that? I don't know, I cannot guess, but normally one needs to work on code to get to know it really. Why not enhance the testsuite as you go along fixing bugs or otherwise improving code outside the testsuite? That ensures that we progress. Also, while working on the code, it is often clearer which semantics you are possibly changing; then, we can expose them in the testsuite as we stumble over them. It would be really nice if somebody who knows their ways around bison/yacc and flex/lex well I'm not that somebody, unfortunately. Don't give yourself up before starting. (or is willing to learn) Well, maybe I might *try* to be this somebody... once I feel backed up by a strong-enough testsuite. I don't believe this argumentation, for reasons stated above. Of course good testsuite coverage doesn't ever fully make up for knowing portability issues, semantics of tools, and searching history and archives regularly. That just comes with it. --- a/tests/defs.in +++ b/tests/defs.in @@ -113,6 +113,13 @@ do echo $me: running etags --version -o /dev/null ( etags --version -o /dev/null ) || exit 77 ;; +flex) + # Since flex is required, we pick LEX for ./configure. + LEX=flex + export LEX + echo $me: running flex --version + ( flex --version ) || exit 77 + ;; I don't understand why the new 'required=flex' entry should be needed in defs.in. Any tests that fail without the defs.in change? Not for me, but since we do something similar for bison, I was just trying to be consistent. Should I remove this hunk? Yes, please. We don't need unneeded code. cat foo.l 'END' %% -END return EOF; +GOOD return EOF; . %% int main () { - while (yylex () != EOF) -; - - return 0; + if (yylex () == EOF) +return 0; + else +return 1; That's not semantic either. One wouldn't write a lexer like that. Yes, but this will have IMVHO a lower risk of hanging due to possible errors/blunders in other parts of the testcase. Hanging which, BTW, would happen ... } END +# Program should build and run. $MAKE -echo 'This is the END' | ./foo -$MAKE distcheck +echo GOOD | ./foo +echo BAD | ./foo Exit 1 ... here. I think we should keep the lexer my stupid but safer way. Maybe; but *now*, that is a good time for adding a comment in main above, because that is one bit of unobvious code. (Unobvious is the reason why the code does not look nor work like a normal lexer.) OK with that fixed. # Don't redefine several times the same variable. -cp Makefile.am Makefile.src +cp -f Makefile.am Makefile.src Why should you need this change? Generally, I don't see why you ever need 'cp -f'. I dimly remember that I used to think there were cp implementations which might prompt if stdout/stderr is a tty and the dest file exists, unless the `-f' option is used.. That is true, but when you run a test, there is no tty involved. What am I missing? But I might be very mistaken here, and since you tell me the `-f' is useless ... No. It is useless *here*. Thanks, Ralf
Re: [PATCH] Test `$(var:suf=rpl)' expansion in special automake variables.
Hello Stefano, * Stefano Lattarini wrote on Thu, Dec 09, 2010 at 01:46:48PM CET: Test `$(var:suf=rpl)' expansion in special automake variables. * tests/posixsubst-data.test: New test. [...] * tests/posixsubst-scripts.test: Likewise. --- /dev/null +++ b/tests/posixsubst-scripts.test @@ -0,0 +1,89 @@ +# Test that POSIX variable expansion `$(var:str=rpl)' works when used +# with the SCRIPTS primary. +cat configure.in 'END' +AC_OUTPUT +END + +cat Makefile.am 'END' +t1 = foo1 foo2 +t2 = bar1x bar2 +t3 = quu-baz + +bar1 bar2 quux.pl: + : $@ +CLEANFILES = bar1 bar2 quux.pl + +# Try also with `:=', to ensure the parser is not unduly confused +# into thinking that it's an unportable assignement operator. This comment is bogus, right? There is no such thing as ':=' within $(var:A=B), and the code below does not use := to assign variables. What do you mean with this text? Also try an empty match suffix.? (Repetitious as it is, it the same text is in all the other tests of this patch as well.) +dist_sbin_SCRIPTS = $(t1:=.sh) +libexec_SCRIPTS = $(t2:x=) +nodist_bin_SCRIPTS = $(t3:-baz=x.pl) + +check-local: test1 test2 +.PHONY: test1 test2 +test1: + ls -l + test -f bar1 + test -f bar2 + test -f quux.pl +test2: distdir + ls -l $(distdir) + ## the scripts foo1.sh and foo2.sh should be distributed + test -f $(distdir)/foo1.sh + test -f $(distdir)/foo2.sh + ## the scripts bar1, bar2 and quux.pl shouldn't be distributed + test ! -r $(distdir)/bar1 + test ! -r $(distdir)/bar2 + test ! -r $(distdir)/quux.pl + +installcheck-local: + ls -l $(prefix)/libexec $(prefix)/bin $(prefix)/sbin + test -f $(prefix)/sbin/foo1.sh + test -x $(prefix)/sbin/foo1.sh + test -f $(prefix)/sbin/foo2.sh + test -x $(prefix)/sbin/foo2.sh + test -f $(prefix)/libexec/bar1 + test -x $(prefix)/libexec/bar1 + test -f $(prefix)/libexec/bar2 + test -x $(prefix)/libexec/bar2 + test -f $(prefix)/bin/quux.pl + test -x $(prefix)/bin/quux.pl These 'test -x' commands fail on MinGW/MSYS because the file system does not actually have execute permission information. Instead, it is emulated by looking at the file, and returning 0 if the file starts with, e.g., a COFF header or with '#!'. (The same heuristic is used when executing a script as well.) You can thus fix this by either creating actual scripts in above rules and the file creation below, or by removing the tests here. I'm not sure which you prefer. Thanks, Ralf +END + +: foo1.sh +: foo2.sh + +$ACLOCAL +$AUTOCONF +$AUTOMAKE + +cwd=`pwd` || Exit 1 +./configure --prefix=$cwd/_inst +$MAKE +$MAKE test1 test2 +$MAKE install +$MAKE installcheck +$MAKE distcheck
Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.
* Stefano Lattarini wrote on Fri, Dec 17, 2010 at 03:33:32PM CET: On Friday 17 December 2010, Ralf Wildenhues wrote: The *current* tests are good enough for the current code. Yes, but not enough if we plan to modify that code You can add tests as you go modifying the code. Doing that is very helpful and instructive actually, because while you focus on the code, it is easier to have a very clear picture of what invariants need to be tested. (well, it depends on which part of the code we are referring, obviously; some parts are excellently tested, some decently, some scarcely or nigh to nothing). This is especially true in case heavy refactorings are to be done (not that I plan/want to do them ATM, but you never know). Oh, heavy refactoring is a completely different beast. For that, it would be better to have a clear design goal first, then add specific test coverage for affected bits and work on the implementation. Here, some more back and forth may be needed. I don't see the past testsuite work to be tailored towards that at all though. I also don't see how you never know works. I don't program by accident ... How many bugs have we found due to the testsuite frenzy in the last few months with its 200+ patches? I'd guess less than 5. Maybe even less. But to be honest, that was not the point of my testsuite patches; what I was really aiming for was to make it difficult to introduce in the future *new* bugs in the already-working paths of the code. Honestly: how much do you think you have made it less difficult? Honestly: which of the bugs that the new tests will catch do you think you would have made inadvertently without having the tests? That's an awful signal to noise ratio. How much have you gotten to know the Automake code base (outside of tests/) as a result of that? Not much (apart from the parts I had to understand to provide my few non-tetsuite related patches). But I've come to know quite well many automake APIs, interfaces, limitations tricks, and many portability problems of various shells, utilities and make implementations. This knowledge is IMHO a necessary (not sufficient!) prerequisite for any half-serious automake hacking. Good! That's great. It would be really nice if somebody who knows their ways around bison/yacc and flex/lex well I'm not that somebody, unfortunately. Don't give yourself up before starting. Well, that wasn't a giving up, just the stating of a fact: that my *current* Lex/Yacc fu is pratically non-existent. Not that this fact can't be changed with some trying and sweating ... Hehe. # Don't redefine several times the same variable. -cp Makefile.am Makefile.src +cp -f Makefile.am Makefile.src Why should you need this change? Generally, I don't see why you ever need 'cp -f'. I dimly remember that I used to think there were cp implementations which might prompt if stdout/stderr is a tty and the dest file exists, unless the `-f' option is used.. That is true, but when you run a test, there is no tty involved. That part (about the tty) is not true. Sorry about that. Even when I run it from the shell prompt? cp should only ever prompt when given -i. That may happen through a shell alias. But aliases are not normally enabled in scripts. If and when we find a cp that does prompt from a script, then we can think of going back to this; but only after amending autoconf.texi (which, by the way, also documents that some old cp versions do not accept -f at all). Cheers, Ralf
Re: [PATCH] Test `$(var:suf=rpl)' expansion in special automake variables.
* Stefano Lattarini wrote on Sat, Dec 18, 2010 at 10:35:43AM CET: On Saturday 18 December 2010, Ralf Wildenhues wrote: +# Try also with `:=', to ensure the parser is not unduly confused +# into thinking that it's an unportable assignement operator. This comment is bogus, right? There is no such thing as ':=' within $(var:A=B), but there is in $(var:=x) (below this is $(t1:=.sh)). and the code below does not use := to assign variables. Indeed, and so we want to enssure the automake parser does not (errenously) think we are trying to do so. What do you mean with this text? Also try an empty match suffix.? Yes; maybe a comment like this would be better? # Also try an empty match suffix, to ensure that the `:=' in there is # not confused by the parser with an unportable assignement operator. Yes, thanks. These 'test -x' commands fail on MinGW/MSYS because the file system does not actually have execute permission information. Ouch. Instead, it is emulated by looking at the file, and returning 0 if the file starts with, e.g., a COFF header or with '#!'. (The same heuristic is used when executing a script as well.) You can thus fix this by either creating actual scripts in above rules and the file creation below, or by removing the tests here. I'm not sure which you prefer. I'd prefer the former, to keep the coverage a little bigger on non-MinGW systems. Is that ok? Yes, that's what I meant. Thanks, Ralf
Re: [FYI] distlinksbrk.test: Work around botched make -k.
* Stefano Lattarini wrote on Tue, Dec 21, 2010 at 03:49:49PM CET: I've applied the patch below to a temporary bug-fixing branch, merged it to maint, merged maint to master, and pushed. The testcase still works with GNU make, FreeBSD make and Solaris make, but I couldn't test if it really fixes the problem with HP-UX make and IRIX make. Thanks. This makes the test pass on HP/UX 11.23 and IRIX 6.5. Cheers, Ralf distlinksbrk.test: Work around botched make -k. * tests/distlinksbrk.test: Run make multiple times and grep its output each time for a single error message, rather than running make -k one single time and grepping its output for all the expected error messages. This should work around make implementations with limited (broken?) `-k' support; for more information, see these subthreads on the automake-patches list: - 2010-11-15, Testsuite failures on HP-UX 11.23, http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00162.html - 2010-11-15, Testsuite failures on IRIX 6.5, http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00166.html
Happy New Year!
... and apologies for my longer than predicted time off the net. All the best to you and GNU! Pushed to maint as below. Cheers, Ralf Bump copyright years. * aclocal.in (write_aclocal, version): Bump copyright years. * automake.in (gen_copyright, version): Likewise. * doc/automake.texi: Likewise. diff --git a/aclocal.in b/aclocal.in index b5c2c41..2210fe3 100644 --- a/aclocal.in +++ b/aclocal.in @@ -8,7 +8,8 @@ eval 'case $# in 0) exec @PERL@ -S $0;; *) exec @PERL@ -S $0 $@;; esac' # aclocal - create aclocal.m4 by scanning configure.ac # Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, -# 2005, 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc. +# 2005, 2006, 2007, 2008, 2009, 2010, 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 @@ -818,7 +819,8 @@ $output; $output = # generated automatically by aclocal $VERSION -*- Autoconf -*- # Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, -# 2005, 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc. +# 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, +# Inc. # This file is free software; the Free Software Foundation # gives unlimited permission to copy and/or distribute it, # with or without modifications, as long as this notice is preserved. @@ -910,7 +912,7 @@ sub version() { print EOF; aclocal (GNU $PACKAGE) $VERSION -Copyright (C) 2010 Free Software Foundation, Inc. +Copyright (C) 2011 Free Software Foundation, Inc. License GPLv2+: GNU GPL version 2 or later http://gnu.org/licenses/gpl-2.0.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. diff --git a/automake.in b/automake.in index bf45fa7..fa110a5 100755 --- a/automake.in +++ b/automake.in @@ -7,7 +7,7 @@ eval 'case $# in 0) exec @PERL@ -S $0;; *) exec @PERL@ -S $0 $@;; esac' # automake - create Makefile.in from Makefile.am # Copyright (C) 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, -# 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010 Free Software +# 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software # Foundation, Inc. # This program is free software; you can redistribute it and/or modify @@ -258,7 +258,7 @@ my %standard_prefix = # Copyright on generated Makefile.ins. my $gen_copyright = \ # Copyright (C) 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, -# 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010 Free Software +# 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software # Foundation, Inc. # This Makefile.in is free software; the Free Software Foundation # gives unlimited permission to copy and/or distribute it, @@ -8404,7 +8404,7 @@ sub version () { print EOF; automake (GNU $PACKAGE) $VERSION -Copyright (C) 2010 Free Software Foundation, Inc. +Copyright (C) 2011 Free Software Foundation, Inc. License GPLv2+: GNU GPL version 2 or later http://gnu.org/licenses/gpl-2.0.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. diff --git a/doc/automake.texi b/doc/automake.texi index ab5f861..1f4b9ec 100644 --- a/doc/automake.texi +++ b/doc/automake.texi @@ -24,8 +24,8 @@ Makefiles from template files. Copyright @copyright{} 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, -2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010 Free Software Foundation, -Inc. +2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software +Foundation, Inc. @quotation Permission is granted to copy, distribute and/or modify this document
Re: [PATCH v3] Work around a bug in Solaris make's file-inclusion mechanism.
Hi Stefano, this has since been applied to maint: * Stefano Lattarini wrote on Fri, Dec 17, 2010 at 08:46:26PM CET: Subject: [PATCH] Work around a bug in file-inclusion mechanism of Solaris make. * automake.in (handle_single_transform): In the name of the dependency file: collapse multiple slash characters into a single one. * tests/subobj11a.test: New test. * tests/subobj11b.test: Likewise. * tests/subobj11c.test: Likewise. * tests/depcomp8a.test: Likewise. * tests/depcomp8b.test: Likewise. * tests/Makefile.am (TESTS): Updated. * NEWS: Updated. I'm pushing this patch to fix a maintainer-check regression introduced by this patch. Cheers, Ralf Fix maintainer-check regression. * tests/subobj11a.test: Pass DISTCHECK_CONFIGURE_FLAGS in the environment. diff --git a/tests/subobj11a.test b/tests/subobj11a.test index c39e67c..3bf3012 100755 --- a/tests/subobj11a.test +++ b/tests/subobj11a.test @@ -76,7 +76,7 @@ $FGREP z...@rd@z quux $MAKE -$MAKE DISTCHECK_CONFIGURE_FLAGS='--enable-dependency-tracking' distcheck -$MAKE DISTCHECK_CONFIGURE_FLAGS='--disable-dependency-tracking' distcheck +DISTCHECK_CONFIGURE_FLAGS='--enable-dependency-tracking' $MAKE distcheck +DISTCHECK_CONFIGURE_FLAGS='--disable-dependency-tracking' $MAKE distcheck :
Re: [FYI] {master} Add some tests on 'cygnus' mode
Hi Stefano, * Stefano Lattarini wrote on Wed, Dec 22, 2010 at 11:55:04PM CET: Attached is what I pushed (to *master* only). From 7e7a8a6497cbc2473a3e25dd1067f2d44346325e Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Tue, 21 Dec 2010 22:59:19 +0100 Subject: [PATCH] Add some tests on 'cygnus' mode. * tests/clean2.test: Extend. * tests/cygnus-check-without-all.test: New test. * tests/cygnus-dependency-tracking.test: Likewise. * tests/cygnus-distclean.test: Likewise. * tests/cygnus-imply-foreign.test: Likewise. * tests/cygnus-no-dist.test: Likewise. * tests/cygnus-no-installinfo.test: Likewise. * tests/cygnus-requires-maintainer-mode.test: Likewise. * tests/Makefile.am (TESTS): Update. Thank you. I'm pushing this followup patch to fix spurious failures of the test on fast systems. I'll be opening a PR for the underlying issue in a minute. Cheers, Ralf Fix caching bug in cygnus mode test. * tests/cygnus-requires-maintainer-mode.test: Remove autom4te.cache directory before rerunning aclocal, to remove trace caches. diff --git a/tests/cygnus-requires-maintainer-mode.test b/tests/cygnus-requires-maintainer-mode.test index 4f893f2..f6db92d 100755 --- a/tests/cygnus-requires-maintainer-mode.test +++ b/tests/cygnus-requires-maintainer-mode.test @@ -39,6 +39,7 @@ cat sub/Makefile.am 'END' AUTOMAKE_OPTIONS = cygnus END +rm -rf autom4te.cache $ACLOCAL AUTOMAKE_fails grep '^configure\.in:.*AM_MAINTAINER_MODE.*required.*cygnus' stderr @@ -47,6 +48,7 @@ cat configure.in 'END' AM_MAINTAINER_MODE END +rm -rf autom4te.cache $ACLOCAL $AUTOMAKE --cygnus
Re: [PATCH] docs: how to work around checks on invalid primary/directory couples
* Stefano Lattarini wrote on Sun, Jan 02, 2011 at 12:23:48PM CET: On Sunday 02 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Sat, Dec 25, 2010 at 10:57:57AM CET: Ok for maint? OK with nits addressed. Do we have testsuite coverage for this? Not yet, but once this patch is applied, I might easily extend the new tests in pending patch Warnings about primary/prefix mismatch fixed and extended to cover these new examples, too. Cool. Attached is what I squashed in, and the amended patch. Thanks. I'll wait for an ACK before pushing. OK, except that I overlooked a couple of nits in my first review. :-) Feel free to address them as you prefer and push. --- a/doc/automake.texi +++ b/doc/automake.texi @@ -2008,6 +2009,33 @@ xmldir = $(datadir)/xml xml_DATA = file.xml @end example +This feature can also be used to work around the sanity checks Automake work around sounds like there is a bug, but in this case there is no bug. How about avoid or override; you used the latter already in the previous hunk, so I guess the former might be nicer, avoiding repetition. +performs against suspicious directory/primary couples (in the unlikely s/against/to flag/ or to diagnose +case these checks are undesirable, and you really know what you're doing). +For example, Automake would error out on this input: + +...@example +# Forbidden directory combinations, automake will error out on this. +pkglib_PROGRAMS = foo +doc_LIBRARIES = libquux.a +...@end example + +...@noindent +but it will succeeds with this: succeed +...@example +# Work around forbidden directory combinations. Do not use this +# without a very good reason! +my_execbindir = $(pkglibdir) +my_doclibdir = $(docdir) +my_execbin_PROGRAMS = foo +my_doclib_LIBRARIES = libquux.a +...@end example + +The @samp{exec} substring of the @samp{my_execbindir} variable is not +there by accident: it lets the files be installed at the right time +(@pxref{The Two Parts of Install}). I'd write s/is not there by accident: it// mostly because it reads better (more like a book, less like a newspaper headline). * Stefano Lattarini wrote on Sun, Jan 02, 2011 at 01:08:13PM CET: On Sunday 02 January 2011, Ralf Wildenhues wrote: +pkglib_PROGRAMS = foo +doc_LIBRARIES = libquux.a Another nit: can we find less obviously bogus combinations that users might actually want to use? [...] I haven't tought about this yet. But IMHO it's no big deal if the usages in our examples are bogus; in fact, the bogusness helps to show how powerful and potentially dangerous the idiom is. Anyway, you're obviously free to amend to example to make them less bogus, if you still think this would be useful. I have no strong opinion on the matter. Agreed, this can be improved later. Thanks, Ralf
Re: [PATCH 01/10] Add new tests on strictness and warnings precedence and overriding.
Hi Stefano, I'm not sure if I'll get through the whole series today, but I gotta start somewhere, so here we go. Feel free to push the patch series (as far as OKed) on a new branch based off of maint, if that is helpful for you. Thanks. * stefano.lattar...@gmail.com wrote on Thu, Dec 23, 2010 at 12:27:37PM CET: --- /dev/null +++ b/tests/strictness-overriding.test How about 'strictness-override.test'? (I'll explain in another mail why I'm keen on not too long names.) @@ -0,0 +1,116 @@ +#! /bin/sh +# Copyright (C) 2010 Free Software Foundation, Inc. 2011. Sorry! +# The strictness specified in Makefile.am:AUTOMAKE_OPTIONS should +# override that specified in configure.in:AM_INIT_AUTOMAKE, and both +# should override the strictness specified on the command line. +# NOTE: this current semantic might not be the best one (even if it has +# been in place for quite a long time); see also Automake bug #7673. I don't think it is grammatically correct to use 'semantic' as a noun in singular form, similar to how 'information' works; 'semantic' is only ever used as adjective or adverb AFAIK. Here, the current semantics would be fine. +# Update this test if the semantic is changed. Likewise: the current semantics are +. ./defs || Exit 1 + +set -e + +# We want complete control over automake options. +AUTOMAKE=$original_AUTOMAKE + +cat Makefile.am 'END' +AUTOMAKE_OPTIONS = +END + +set_strictness() Space before open parenthesis; several instances. +{ + set +x Curious: why turn off tracing here? + sed $2 $2-t -e s|^\\(AUTOMAKE_OPTIONS\\) *=.*|\\1 = $1| \ +-e s|^\\(AM_INIT_AUTOMAKE\\).*|\\1([$1])| + mv -f $2-t $2 + set -x + cat $2 To avoid caching please 'rm -rf autom4te.cache' here. +} + +ok() +{ + $AUTOMAKE -Werror $* +} + +ko() +{ + AUTOMAKE_fails $* + grep 'required file.*README' stderr +} + +$ACLOCAL + +# Leave out only one of the required files, to avoid too much +# repetitions in the error messages. repetition +touch INSTALL NEWS AUTHORS ChangeLog COPYING Most of the above comments apply to several tests in this patch. Thank you, Ralf
Re: [PATCH 02/10] New test on silent-rules mode and portability warnings.
* stefano.lattar...@gmail.com wrote on Thu, Dec 23, 2010 at 12:27:38PM CET: * tests/silent-rules-nowarn.test: New test. * tests/Makefile.am (TESTS): Update. How about silent-nowarn.test for consistency with the other names? OK with or without that change. Thanks, Ralf
Re: [PATCH 03/10] Warnings win over strictness on command line.
* stefano.lattar...@gmail.com wrote on Thu, Dec 23, 2010 at 12:27:39PM CET: 2010-12-20 Stefano Lattarini stefano.lattar...@gmail.com + Warnings win over strictness on command line. Please add a line like For PR automake/547: on a line by itself here, as done in some (older) ChangeLog entries, and put automake-gn...@sources.redhat.com in Cc:, that way your mail gets added to the Gnats trail. Thanks. You can omit the other mention of 547 below then. (I'm not actually sure about the exact regex used to track Gnats additions, but it did include something like PR $PACKAGE/$number.) The same applies to patches 04, 07, 08, and 09 of this series. + This change ensures that, on the command line at least, explicitly Ensure that on the command line, explicitly ... (it is clear that you are speaking about this change). + defined warnings always take precedence over implicit strictness + implied warnings. Related to Automake bug#7669 a.k.a. PR/547. + * automake.in (parse_arguments): Parse warnings only after the + strictness level has been processed. + * tests/gnuwarn.test: Update, plus miscellaneous improvements. + * tests/warnings-win-over-strictness.test: New test. + * tests/Makefile.am (TESTS): Update. --- a/automake.in +++ b/automake.in @@ -8444,26 +8444,26 @@ EOF # Parse command line. sub parse_arguments () { - # Start off as gnu. - set_strictness ('gnu'); + my $strictness = 'gnu'; This local variable shadows the global $strictness. Please use $strict or so instead, to avoid ambiguities for the reader. + my $cygnus = 0; + my $ignore_deps = 0; + my @warnings = (); - my $cli_where = new Automake::Location; my %cli_options = ( 'libdir=s' = \$libdir, - 'gnu' = sub { set_strictness ('gnu'); }, - 'gnits' = sub { set_strictness ('gnits'); }, - 'cygnus'= sub { set_global_option ('cygnus', $cli_where); }, - 'foreign' = sub { set_strictness ('foreign'); }, - 'include-deps' = sub { unset_global_option ('no-dependencies'); }, - 'i|ignore-deps' = sub { set_global_option ('no-dependencies', - $cli_where); }, + 'gnu' = sub { $strictness = 'gnu'; }, + 'gnits' = sub { $strictness = 'gnits'; }, + 'foreign' = sub { $strictness = 'foreign'; }, + 'cygnus'= \$cygnus, + 'include-deps' = sub { $ignore_deps = 0; }, + 'i|ignore-deps' = sub { $ignore_deps = 1; }, 'no-force' = sub { $force_generation = 0; }, 'f|force-missing' = \$force_missing, 'a|add-missing' = \$add_missing, 'c|copy'= \$copy_missing, 'v|verbose' = sub { setup_channel 'verb', silent = 0; }, - 'W|warnings=s' = \parse_warnings, + 'W|warnings=s' = \...@warnings, ); use Getopt::Long; Getopt::Long::config (bundling, pass_through); @@ -8492,6 +8492,15 @@ sub parse_arguments () Getopt::Long::GetOptions %cli_options, 'version' = sub {}, 'help' = sub {} or exit 1; + set_strictness ($strictness); + my $cli_where = new Automake::Location; + set_global_option ('cygnus', $cli_where) if $cygnus; + set_global_option ('no-dependencies', $cli_where) if $ignore_deps; I wonder whether there was a rule to put postponed Perl-style 'if $cond' on a line of its own, indented by two more spaces, except for very short statements preceding the 'if'. The current code doesn't do this throughout, but it might improve readability. Does the new code still preserve the cygnus implies foreign strictness semantics (I'm assuming yes, since you added tests around that area)? + for my $warning (@warnings) + { +parse_warnings('-W', $warning); + } Please use GNU-style brace indentation and spacing: for my $warning (@warnings) { parse_warnings ('-W', $warning); } I know it's a bit awkward for Perl code, but the existing code uses it throughout and I prefer consistency. return unless @ARGV; if ($ARGV[0] =~ /^-./) --- a/tests/gnuwarn.test +++ b/tests/gnuwarn.test @@ -20,9 +20,11 @@ set -e +# We need (almost) complete control over automake options. +AUTOMAKE=$original_AUTOMAKE -Werror + cat configure.in 'END' AC_PROG_CC -AC_OUTPUT Let's keep this line, even if not absolutely needed. END # Needed by --gnu. @@ -40,12 +42,14 @@ END $ACLOCAL # Don't warn in foreign mode -$AUTOMAKE -Wnone --add-missing --foreign +$AUTOMAKE --add-missing --foreign # Warn in gnu mode -AUTOMAKE_fails -Wnone --add-missing --gnu -grep 'Makefile.am:1:.*CFLAGS' stderr -grep 'Makefile.am:2:.*LDFLAGS' stderr +AUTOMAKE_fails --add-missing --gnu +grep '^Makefile\.am:1:.*CFLAGS' stderr +grep '^Makefile\.am:2:.*LDFLAGS' stderr # No reason to warn about CXXFLAGS since it's not used. grep CXXFLAGS stderr Exit
Re: [PATCH 12/10] More checks on warnings/strictness in precedence (metawarnings).
* Stefano Lattarini wrote on Fri, Dec 24, 2010 at 09:21:56PM CET: More checks on warnings/strictness in precedence (metawarnings). metawarnings is not a proper word, how about just eliding the part in parentheses? Yes, I know I'm probably too picky on language; if it starts bothering you, I'll offer more beer when we should meet. ;-) * tests/metawarnings-win-over-strictness.test: New test, similar to `warnings-win-over-strictness.test', but with the explicit warning levels being meta-warnings (like `-Wall' and `-Wnone'). How about s/the explicit ...(\(.*\))/catch-all warning flags \1/ and just naming the test warning-groups-win-over-strictness.test? * tests/Makefile.am (TESTS): Update. --- /dev/null +++ b/tests/metawarnings-win-over-strictness.test +# Check that, on the command line, explicitly-defined warning levels take +# precedence over implicit strictness-implied warnings, even when these +# explicit warning levels are meta-warnings (as `-Wall' and `-Wnone'). see above +# Since we are at it, throw in also *.am and *.m4 file inclusions. also throw in +cat configure.in END +AC_INIT([$me], [1.0]) +m4_include([am-init-automake.m4]) +AC_PROG_CC +AC_CONFIG_FILES([Makefile]) +# Other similar tests do not use AC_OUTPUT, so we use it here, +# for completness and for better coverage. completeness +AC_OUTPUT +END + +cat Makefile.am END +include automake-options.am +FOO := bar +END + +: automake-options.am +echo 'AM_INIT_AUTOMAKE' am-init-automake.m4 double space I'm not sure it works to put AM_INIT_AUTOMAKE in an included macro. We definitely have places in autotools somewhere that grep for it, but I don't know if any of those are relevant here. +$ACLOCAL --force +AUTOMAKE_fails -Werror -Wall --foreign +grep '^Makefile\.am:.*:=.*not portable' stderr $sleep or remove autom4te.cache here. +: automake-options.am +echo 'AM_INIT_AUTOMAKE([-Werror -Wnone gnu])' am-init-automake.m4 +$ACLOCAL --force +$AUTOMAKE Likewise here. You are not using command-line options here. Is that intended? If not, then it seems like the test description does not fully match the action. +echo 'AUTOMAKE_OPTIONS = -Werror -Wnone gnits' automake-options.am +echo 'AM_INIT_AUTOMAKE' am-init-automake.m4 +$ACLOCAL --force +$AUTOMAKE trailing white space. OK with that addressed. Thanks, Ralf
Re: [PATCH 08/10] Add more tests about AUTOMAKE_OPTIONS.
* stefano.lattar...@gmail.com wrote on Thu, Dec 23, 2010 at 12:27:44PM CET: In view of soon-to-follow refactorings (still in the pursuit of a fix for Automake bug#7669 a.k.a. PR/547), we add some more tests How about s/we // on AUTOMAKE_OPTIONS support, to prevent obvious regressions. * tests/amopts-indirect.test: New test. * tests/amopts-location.test: Likewise. * tests/Makefile.am (TESTS): Update. --- /dev/null +++ b/tests/amopts-indirect.test +# Check that AUTOMAKE_OPTIONS support indirections. s/indirections/variable expansion/ and adjust test name? +cat Makefile.am 'END' +AUTOMAKE_OPTIONS = $(foo) foreign +AUTOMAKE_OPTIONS += ${bar} +foo = $(foo1) +foo1 = ${foo2} +foo2 = -Wnone +foo2 += $(foo3) +foo3 = -Wno-error +bar = -Wportability This seems a wee bit convoluted (i.e., hard to parse by human upon first reading), but oh well, that's more of a testsuite QoI nit. +## This will give a warning with `-Wportability' +zardoz := +## This would give a warning with `-Woverride'. +install: +END + +$ACLOCAL +AUTOMAKE_run 0 +grep '^Makefile\.am:.*:=.*not portable' stderr +grep README stderr Exit 1 +$EGREP '(install|override)' stderr Exit 1 --- /dev/null +++ b/tests/amopts-location.test @@ -0,0 +1,80 @@ +# Check that errors about AUTOMAKE_OPTIONS refers to correct +# locations. + +. ./defs || Exit 1 + +set -e + +cat Makefile.am 'END' +# comment \ +# continued +include Makefile0.am +END + +cat Makefile0.am 'END' +#1 +#2 +#3 +include Makefile1.am +END + +cat Makefile1.am 'END' +AUTOMAKE_OPTIONS = tar-pax +# comment +END + +cat Makefile2.am 'END' +## automake comment +bar: + : +line = \ +continued +AUTOMAKE_OPTIONS = tar-ustar +END + +cat Makefile3.am 'END' +quux = a +AUTOMAKE_OPTIONS = +quux += b +AUTOMAKE_OPTIONS += tar-v7 +zardoz = 1 +END + +cat configure.in 'END' +AC_CONFIG_FILES([Makefile2 Makefile3]) +END + +$ACLOCAL +AUTOMAKE_fails What are the expected failures here? (Not sure if they should be mentioned in the test source, but I sure am curious about what automake prints.) Just to be sure, this patch does not rely upon any of the previous ones, right? +grep '^Makefile1\.am:1:.*tar-pax' stderr +grep '^Makefile2\.am:6:.*tar-ustar' stderr +grep '^Makefile3\.am:2:.*tar-v7' stderr +grep '^Makefile\.am:3:.*Makefile0\.am.*included from here' stderr +grep '^Makefile0\.am:4:.*Makefile1\.am.*included from here' stderr +cat stderr \ + | grep -v '^Makefile\.am:3:' \ + | grep -v '^Makefile0\.am:4:' \ + | grep -v '^Makefile1\.am:1:' \ + | grep -v '^Makefile2\.am:6:' \ + | grep -v '^Makefile3\.am:2:' \ + | grep . Exit 1 What is this for, to ensure there are not more warnings? How about just testing for 5 lines of output? Even that is a bit fragile though, as we might have multi-line warnings. Thanks, Ralf
Re: [PATCH 12/10] More checks on warnings/strictness in precedence (metawarnings).
* Stefano Lattarini wrote on Sun, Jan 02, 2011 at 07:49:16PM CET: On Sunday 02 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Fri, Dec 24, 2010 at 09:21:56PM CET: I'm not sure it works to put AM_INIT_AUTOMAKE in an included macro. Apparently it does, at least when only automake, aclocal and autoconf are involved. Good then. I don't think so. The test works also with autoconf 2.62, BTW. +$ACLOCAL --force +AUTOMAKE_fails -Werror -Wall --foreign +grep '^Makefile\.am:.*:=.*not portable' stderr $sleep or remove autom4te.cache here. Oops, I tought that $ACLOCAL --force was enough, but now I see from the output of aclocal --help that this is not the case :-( Well yes, that is the gist of the bug report I still have to make. ;-) Sorry for the noise. No worries. OK with that addressed. Below is what I squashed in. OK, thanks, Ralf
Re: [PATCH] {master} Tests: normalize use of the 'errexit' shell flag.
Hi Stefano, * Stefano Lattarini wrote on Mon, Jan 03, 2011 at 03:04:05PM CET: * tests/maken3.test (check_targets): Remove redundant call to 'set -e'. * tests/maken4.test: Likewise. * tests/ansi5.test: Call 'set -e' just after './defs' has been sourced. * tests/ansi6.test: Likewise. * tests/ansi7.test: Likewise. * tests/cond16.test: Likewise. * tests/cond17.test: Likewise. * tests/cond18.test: Likewise. * tests/cond19.test: Likewise. * tests/cond20.test: Likewise. * tests/cond21.test: Likewise. * tests/instdat2.test: Likewise. * tests/instdir-texi.test: Likewise. * tests/parallel-tests3.test: Likewise. * tests/remake1a.test: Likewise. * tests/ccnoco.test: Likewise, and add trailing `:' command. * tests/comment4.test: Likewise. * tests/gcj4.test: Likewise. * tests/nodist2.test: Likewise. * tests/nodist3.test: Enable 'errexit' shell flag (this should have been done in commit v1.11-248-g317e17b, but the relevant hunk has been forgotten somehow). * tests/output.test: Likewise. * tests/gnits2.test: Likewise, and display captured stderr to script's stderr, not to script's stdout. * tests/gnits3.test: Likewise. Also, prefer 'cat' over 'echo' to append to Makefile.am, and really check that the exit status of make installcheck indicates failure. [...] --- a/tests/maken3.test +++ b/tests/maken3.test @@ -122,7 +122,6 @@ $AUTOCONF check_targets () { - set -e for target in \ all install install-strip uninstall clean distclean check \ info html dvi pdf ps \ --- a/tests/maken4.test +++ b/tests/maken4.test @@ -124,7 +124,6 @@ $AUTOCONF check_targets () { - set -e for target in \ all install install-strip uninstall clean distclean check \ info html dvi pdf ps \ These two were added by me in fear of shells resetting errexit upon function entry (like some shells do with -x aka xtrace). Luckily I couldn't find any shells which do this upon testing (by looking for 'e' in the output of) sh -ec 'f () { echo $-; }; f' but generally, here's a nice (and more complete than autoconf.texi) list of issues with set -e: http://www.in-ulm.de/~mascheck/various/set-e/ Please, before removing such seemingly superfluous code, consider first whether it may have been added for a reason. If you do this research, then I don't have to, and can review more patches. Patch is OK. Thanks, Ralf
Re: [PATCH] {maint} Improve, extend and tweak tests on Texinfo support.
* Stefano Lattarini wrote on Mon, Jan 03, 2011 at 02:38:48PM CET: Subject: [PATCH] Improve, extend and tweak tests on Texinfo support. * tests/instdir-texi.test: Add a call to `ls -l' after that to `make', for debugging. When looking for required tools, do not redirect the output of $tool --help to /dev/null, and do not uselessly run it in a subshell. * tests/txinfo.test: Rewritten to run autoconf, ./configure and make. All checks moved into Makefile.am. * tests/txinfo8.test: Likewise, and modernize the generated configure.in. * tests/txinfo2.test: Moved checks into Makefile.am, and other minor improvements. * tests/txinfo5.test: Enable `errexit' shell flag, and related changes. Add trailing `:' command. * tests/txinfo6.test: Likewise, and make grepping of generated Makefile.in stricter. * tests/txinfo7.test: Enable `errexit' shell flag, and related changes. Add trailing `:' command. Do not add unnecessary stuff to Makefile.am. * tests/txinfo9.test: Verify that more targets which are expected to be generated only once really are. Make grepping less strict, to avoid exposing too much internal details. More minor changes. * tests/txinfo16.test: Add trailing `:'. Prefer cat over echo for appending to configure.in. Updated/fixed heading comments. * tests/txinfo23.test: Likewise, and extended a little by making it check that no info file is created in the $(srcdir). * tests/txinfo24.test: Likewise. * tests/txinfo25.test: Likewise. * tests/txinfo18.test: Add trailing `:'. Prefer cat over echo for appending to configure.in. Also, check that index files are cleaned also by make clean, not only by make distclean. * tests/txinfo22.test: Prefer `$me' over hard-coded test name, and added trailing `:' command. This testcase also used to check that automake ignores in-line comments when using variables, but preserves them in the output; these checks (added in commit Release-1-7f-4-g9177ef8) do not really pertain to this test, so they have been moved ... * tests/comments-in-var-definition.test: ... into this new test. * tests/txinfo4.test: Escape literal dots in grep regexps. Add trailing `:' command. * tests/txinfo29.test: Likewise. Relax grepping of generated Makefile.in w.r.t. whitespaces. Prefer `cat' over `echo' to append to configure.in. * tests/txinfo3.test: Likewise. * tests/vtexi.test: Improve grepping of Makefile.in (sometimes make it stricter, sometimes laxer). Move `set -e' setting just after the inclusion of ./defs. De-uglify a sed command. Other minor cosmetic improvements. * tests/vtexi2.test: Make grepping of Makefile.in stricter. Add trailing `:' command. * tests/vtexi3.test: New test on version.texi support. * tests/vtexi4.test: Likewise. * tests/Makefile.am (TESTS): Updated. OK with nits addressed. --- /dev/null +++ b/tests/comments-in-var-definition.test How about s/definition/defn/? That is still unique, easily understood, and a lot shorter. @@ -0,0 +1,47 @@ +# Make sure Automake ignores in-line comments when using variables, +# but preserve them in the output. preserves + +. ./defs || Exit 1 + +set -e + +# Use a slash in the comment, because automake takes the dirname +# of TEXINFO_TEX to compute $(am__TEXINFO_TEX_DIR). +cat Makefile.am 'END' +TEXINFO_TEX = tex/texinfo.tex# some comment w/ a slash +info_TEXINFOS = main.texi +END + +cat main.texi 'END' +\input texinfo +...@setfilename main.info +END + +mkdir tex +: tex/texinfo.tex + +$ACLOCAL +$AUTOMAKE + +grep TEX Makefile.in # for debugging +grep '^TEXINFO_TEX *= *tex/texinfo\.tex *# some comment w/ a slash *$' Makefile.in +grep '^am__TEXINFO_TEX_DIR *=.*[/ ]tex *$' Makefile.in +$EGREP 'am__TEXINFO_TEX_DIR.*=.*(comment|#)' Makefile.in Exit 1 These two lines access internal details that could change. Acceptable if it must be that way but better if we can do without. --- /dev/null +++ b/tests/vtexi3.test +# Check that vers*.texi files are automatically created and distributed +# if @included into a texi source. Also check that they correctly contain +# the @values definitions they are advertised to. +# See also the related test `vtexi4.test', which does similar checks, but +# for version.texi only, and requires makeinfo, tex and texi2dvi. +day='([1-9]|1[0-9]|2[0-9]|3[01])' +month='(January|February|March|April|May|June|July|August|September|October|November|December)' +year='20[0-9][0-9]' # hopefully automake will be obsolete in 80 years ;-) I do not agree with the tone of the comment, and as punishment will require that the code also works later than the mentioned date. ;- Also, your comment writing style seems to be degrading away from writing whole sentences (including leading capitalization and final period) again here and below. +date=$day $month $year + +do_check () +{ + # basename of the vers*.texi file + vfile=$1 + # $(srcdir) of the current build + srcdir=$2 + # vers*.texi must be
Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.
* Stefano Lattarini wrote on Wed, Jan 05, 2011 at 08:22:13PM CET: On Wednesday 05 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Tue, Jan 04, 2011 at 06:38:04PM CET: + # will take precedence over warning settings defined implicitly by the + # strictness. Well, this works in the current code base, but only by accident: namely, only because process_option_list is only ever called once, and with all options at once. Hmm... no, it's potentially called many times in `handle_options()'. But the later [PATCH 7/9] takes care of this. Ah, ok. If some code later calls it like process_option_list (first-set-of-options); process_option_list (second-set-of-options); then things will go wrong again. I suspect that it will mean that AM_INIT_AUTOMAKE([foreign -Wno-portability]) AUTOMAKE_OPTIONS = gnu won't do what we want. Hmm. What exactly is it that we want to happen in this case? Should gnu override -Wno-portability if specified in a (to-be) higher order place? I assumed without saying that yes, this was to be the intended behaviour. And I still think it should be. Sorry for not having been explicit about that before. I see two ways out: warnings are only switched after all options are processed. This is not good IMO, as it breaks usages like the the one in your example above. Makes sense. Thanks for explaining patiently, I think I now understand better. I hope to finish review (and approval) of this patch series this weekend. Cheers, Ralf
Re: [PATCH] {master} yacc: make clean removes .c and .h files from non-distributed .y
Hi Stefano, * Stefano Lattarini wrote on Fri, Jan 07, 2011 at 04:02:59PM CET: The patch is based off of maint. OK to apply to a temporary branch and merge to master? Yes. Thank you! And thing you developed it off of maint, it might be a candidate for later merging into branch-1.11. Cheers, Ralf Subject: [PATCH] yacc: make clean removes .c and .h files from non-distributed .y Previously, while automake did *not* distribute C source and header files derived from non-distributed Yacc sources, it still caused them to be removed only by make maintainer-clean only, and not by simply make clean or make distclean. This caused make distcheck to fail, unless the developer put those generated .c and .h files in CLEANFILES or in DISTCLEANFILES by hand. This change fixes this issue, by making non-distributed `.c' and `.h' files generated by non-distributed Yacc sources cleaned by make clean. * tests/automake.in (lang_yacc_target_hook): Make C source and header files derived from non-distributed Yacc files cleaned by make clean, not only by make maintainer-clean. * tests/yacc-clean.test: New test. * tests/Makefile.am (TESTS): Update. * NEWS: Update.
Re: bug#7773: (lack of) config.h description in manual
As part of addressing Automake bug#7766 and bug#7773, I'm pushing the following in Karl's name, to the maint branch. Cheers, Ralf 2011-01-08 Karl Berry k...@freefriends.org Eric Blake ebl...@redhat.com docs: reference defining directories in amhello node. * doc/automake.texi (amhello Explained): Point to Autoconf manual for how to convert directory values into macros. (Optional): Fix grammar nit. diff --git a/doc/automake.texi b/doc/automake.texi index 43ad581..c63dbf3 100644 --- a/doc/automake.texi +++ b/doc/automake.texi @@ -1736,6 +1736,11 @@ amhello Explained The only important effect of this second line is therefore to install @file{README} during @code{make install}. +One thing not covered in this example is accessing the installation +directory values (@pxref{Standard Directory Variables}) from your +program code, that is, converting them into defined macros. For this, +...@pxref{defining Directories,,, autoconf, The Autoconf Manual}. + @node Generalities @chapter General ideas @@ -2905,7 +2910,7 @@ Optional of Automake required the use of @code{AM_CONFIG_HEADER} (@pxref{Macros}); this is no longer the case. -As for @code{AC_CONFIG_FILES} (@pxref{Requirements}), parts of the +As with @code{AC_CONFIG_FILES} (@pxref{Requirements}), parts of the specification using shell variables will be ignored as far as cleaning, distributing, and rebuilding is concerned.
Re: [PATCH] yacc: extend and improve tests
Hi Stefano, * Stefano Lattarini wrote on Sat, Jan 08, 2011 at 02:33:34PM CET: Here are some testsuite enhancements that might help the on-going work on Yacc support. OK for a proper temporary branch off of 'yacc-clean' (after having merged into it the older temporary branch 'tests-lexyacc-extend' a.k.a. commit v1.11-249-gd4dcf50), to be merged in master afterwards? Sure. You decide where and when to merge this best. OK with nits below addressed. Thanks! Ralf Subject: [PATCH] yacc: extend and improve tests * tests/yacc-basic.test: Also check that the intermediate C file is mentioned in the generated Makefile.in, and that it is created by the first make invocation. * tests/yacc.test: Test removed, it has become obsolete now. Is that true? AFAICS the other tests all require 'bison' to be present, while this one does not. * tests/yacc3.test: Test removed, superseded by ... * tests/yacc-d-basic.test: ... this new test. * tests/yacc2.test: Add reference to that new test in the heading comments. * tests/yacc-d-vpath.test: New test. * tests/yaccvpath.test: Updated heading comments. Do not require gcc anymore, as any working C compiler should be enough. Remove redundant comments. * tests/yacc-nodist.test: New test. * tests/yacc-dist-nobuild.test: New test. * tests/Makefile.am (TESTS): Update. --- a/tests/yacc-basic.test +++ b/tests/yacc-basic.test @@ -55,12 +59,18 @@ $AUTOMAKE -a ./configure $MAKE +# The `parse.c' must be created and not removed (i.e., not treated The `parse.c' file +# like an intermediate file in GNU make sense). in the +test -f parse.c echo a | ./foo echo b | ./foo Exit 1 # The generated file `parse.c' must be shipped. +$MAKE echo-distcom +$MAKE -s echo-distcom | grep '[ /]parse.c ' $MAKE distdir +ls -l $distdir test -f $distdir/parse.c # Sanity check on distribution. --- /dev/null +++ b/tests/yacc-d-basic.test @@ -0,0 +1,157 @@ +#! /bin/sh +# Copyright (C) 2011 Free Software Foundation, Inc. If this supersedes yacc3.test, then does it not also derive from it? I think in that case it is prudent to add the copyright years from that file here as well. +# Tests on basic Yacc support for when we have -d in YFLAGS, AM_YFLAGS +# and maude_YFLAGS. s/and/or/ ? +. ./defs || Exit 1 Doesn't this require bison? +set -e + +tab='' +distdir=$me-1.0 + +cat configure.in 'END' +AC_PROG_CC +AC_PROG_YACC +AC_CONFIG_FILES([foo/Makefile bar/Makefile baz/Makefile]) +AC_OUTPUT +END + +cat Makefile.am 'END' +SUBDIRS = foo bar baz +END + +mkdir foo bar baz + +cat foo/Makefile.am 'END' +bin_PROGRAMS = zardoz +zardoz_SOURCES = parse.y main.c +.PHONY: echo-distcom +echo-distcom: + @echo ' ' $(DIST_COMMON) ' ' +END +cp foo/Makefile.am bar/Makefile.am +cp foo/Makefile.am baz/Makefile.am + +cat foo/parse.y 'END' +%{ +#include parse.h +int yylex () { return 0; } +void yyerror (char *s) {} +%} +%% +x : 'x' {}; +%% +END +cp foo/parse.y bar/parse.y + +cat foo/main.c 'END' +#include parse.h +int main () +{ + return yyparse (); +} +END +cp foo/main.c bar/main.c + +# Even the generated header file is renamed when target-specific YFLAGS +# are used. This might not be the best semantic, but it has been in place semantics +# for quite a long time, so just go along with it for now. Just out of curiosity: can you explain why this may not be good semantics? +sed 's/parse\.h/zardoz-parse.h/' foo/parse.y baz/parse.y +sed 's/parse\.h/zardoz-parse.h/' foo/main.c baz/main.c + +$ACLOCAL +$AUTOCONF + +$AUTOMAKE -a +$FGREP parse.h foo/Makefile.in bar/Makefile.in baz/Makefile.in Exit 1 + +cat foo/Makefile.am END +BUILT_SOURCES = parse.h +YFLAGS=\ +-d +END +$AUTOMAKE -Wno-gnu foo/Makefile + +sed 's/EOL$//' bar/Makefile.am END +AM_YFLAGS${tab}= -d EOL +BUILT_SOURCES = parse.h +END +$AUTOMAKE bar/Makefile + +cat baz/Makefile.am END +BUILT_SOURCES = zardoz-parse.h +zardoz_YFLAGS =-d${tab} +END +$AUTOMAKE baz/Makefile + +./configure + +$MAKE + +test -f foo/parse.c +test -f foo/parse.h +test -f bar/parse.c +test -f bar/parse.h +test -f baz/zardoz-parse.c +test -f baz/zardoz-parse.h + +# The generated C and header files must be shipped. +for dir in foo bar; do + cd $dir + $MAKE echo-distcom + $MAKE -s echo-distcom | grep '[ /]parse.c ' + $MAKE -s echo-distcom | grep '[ /]parse.h ' + cd .. +done +cd baz +$MAKE echo-distcom +$MAKE -s echo-distcom | grep '[ /]zardoz-parse.c ' +$MAKE -s echo-distcom | grep '[ /]zardoz-parse.h ' Did you try with some non-GNU make (e.g., heirloom or BSD make)? +cd .. + +$MAKE distdir +ls -l $distdir +test -f $distdir/foo/parse.c +test -f $distdir/foo/parse.h +test -f $distdir/bar/parse.c +test -f $distdir/bar/parse.h +test -f $distdir/baz/zardoz-parse.c +test -f $distdir/baz/zardoz-parse.h + +# Sanity check on distribution. s/on // +$MAKE distcheck + +# While we are at it, make
Re: [PATCHES] yacc: support variable expansions in *YFLAGS definition
[ dropping the PR Cc: ] * Stefano Lattarini wrote on Fri, Jan 07, 2011 at 11:50:51PM CET: On Friday 07 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Fri, Jan 07, 2011 at 03:36:43PM CET: Currently, automake is not smart enough to resolve variable expansions in AM_YFLAGS (or foo_YFLAGS) when scanning them for the `-d' flag. The attached two-patch series should fix the bug (the first patch is a testsuite enhancement, the second one really fixes the bug). OK to apply to the temporary branch 'yacc-clean' (recently merged into master with commit v1.11-575-ga297a16) and merge to master again? The first one is OK with nits addressed. I'll reply to the second separately (as my mailer somehow only quoted the first patch). I note that some of the tests ({cmdline,force}-override) in these patches require either yacc or bison to be present, but don't have 'required' lines. If you want to make a distinction (feel free to decide), then you could add a 'yacc' entry in tests/defs.in that doesn't check --version (as non-GNU yaccs won't understand that); preapproved. Thanks! Ralf Subject: [PATCH 1/2] tests: more on *YFLAGS support * tests/yflags-var-expand.test: New test, still xfailing. It exposes automake bug#7800 -- automake fails to honor `-d' in AM_YFLAGS when variable expansions are involved. * tests/yflags-d-false-positive.test: New test, checking that automake do not spuriously see `-d' in *YFLAGS when that isn't really there. * tests/yflags-force-override.test: New test, checking that automake can cope with definition of the YFLAGS variable in Makefile.am (even if that is an extremely bad practice, as that variable is user-reserved). * tests/yflags-cmdline-override.test: New test, checking that automake can cope with user-redefinition of YFLAGS at configure time and/or at make time. * tests/yflags-conditional.test: New test, checks that automake warns on conditionally-defined *YFLAGS variables. * tests/Makefile.am (TESTS, XFAIL_TESTS): Update. --- /dev/null +++ b/tests/yflags-cmdline-override.test @@ -0,0 +1,88 @@ +#! /bin/sh +# Copyright (C) 2011 Free Software Foundation, Inc. +# Check that automake can cope with user-redefinition of $(YFLAGS) +# at configure time and/or at make time. + +. ./defs || Exit 1 + +set -e + +unset YFLAGS || : + +cat configure.in 'END' +AC_PROG_CC +AC_PROG_YACC +AC_OUTPUT +END + +cat Makefile.am 'END' +bin_PROGRAMS = foo +foo_SOURCES = foo.y +# A minor automake wart: automake doesn't generate the code to clean s/the // +# `*.output' files generated by yacc (it's not even clear if that would +# be useful in general, so it's probably better to be conservative). +CLEANFILES = foo.output +# Another automake wart: `-d' flag won't be given at automake time, so +# automake won't be able to generate the code to clean `foo.h' :-( +MAINTAINERCLEANFILES = foo.h +END + +cat foo.y 'END' +%{ +int yylex () { return 0; } +void yyerror (char *s) { return; } +int main () { return 0; } +%} +%% +foobar : 'f' 'o' 'o' 'b' 'a' 'r' {}; +END + +$ACLOCAL +$AUTOMAKE -a +$AUTOCONF + +./configure YFLAGS='-d -v' +$MAKE +ls -l +test -f foo.c +test -f foo.h +test -f foo.output + +$MAKE maintainer-clean +ls -l + +./configure YFLAGS='-v' +$MAKE +ls -l +test -f foo.c +test ! -r foo.h +test -f foo.output + +$MAKE maintainer-clean +ls -l + +./configure YFLAGS='-v' +YFLAGS=-d $MAKE -e +ls -l +test -f foo.c +test -f foo.h +test ! -r foo.output + +$MAKE maintainer-clean +ls -l + +: --- /dev/null +++ b/tests/yflags-conditional.test @@ -0,0 +1,46 @@ +# Check that automake complains about conditionally-defined *_YFLAGS. + +. ./defs || Exit 1 + +set -e + +cat configure.in 'END' +AC_PROG_CC +AC_PROG_YACC +AM_CONDITIONAL([COND], [:]) +END + +cat Makefile.am 'END' +bin_PROGRAMS = foo bar +foo_SOURCES = foo.y +bar_SOURCES = bar.y +if COND +AM_YFLAGS = $(YFLAGS) +bar_YFLAGS = -v +endif COND +END + +: ylwrap + +$ACLOCAL +AUTOMAKE_fails +grep Makefile\.am:5:.*AM_YFLAGS.* defined conditionally stderr +grep Makefile\.am:6:.*bar_YFLAGS.* defined conditionally stderr --- /dev/null +++ b/tests/yflags-d-false-positives.test +# Check for false positives in automake recognition of `-d' in YFLAGS. + +. ./defs || Exit 1 + +set -e + +cat configure.in 'END' +AC_PROG_CC +AC_PROG_YACC +END + +$ACLOCAL + +cat Makefile.am 'END' +bin_PROGRAMS = zardoz foobar +zardoz_SOURCES = zardoz.y +foobar_SOURCES = foobar.y +AM_YFLAGS = -xd --d - d --output=d +foobar_YFLAGS = - d $(foovar)-d -dd Is that '- d' a typo? +END + +$AUTOMAKE -a +$EGREP '(foobar|zardoz)\.h.*:' Makefile.in Exit 1 +$EGREP '(foobar|zardoz)\.h' Makefile.in | $FGREP -v '$(YLWRAP) ' Exit 1 --- /dev/null +++ b/tests/yflags-force-override.test +# Check that automake can cope with definition of the $(YFLAGS) variable a definition +# in Makefile.am (even if that is an extremely
Re: [PATCHES] yacc: support variable expansions in *YFLAGS definition (was: Re: bug#7800: automake fails to honor `-d' in AM_YFLAGS when variable expansions are involved)
* Stefano Lattarini wrote on Fri, Jan 07, 2011 at 11:50:51PM CET: Subject: [PATCH 2/2] yacc: support variable expansions in *YFLAGS definition. This commit fixes automake bug#7800. OK with nits addressed. Thanks! Ralf * automake.in (lang_yacc_target_hook): Use 'value_as_list_recursive' instead of 'variable_value' to get the value of *YFLAGS variables. Related changes. ($DASH_D_PATTERN): Removed as obsolete. Just Removed. is sufficient. There is nothing obsolete about this, this is not public API, and it is merely unneeded now. * tests/Makefile.am (XFAIL_TESTS): Remove yflags-var-expand.test. * tests/yacc-clean.test: Remove a now-useless workaround. Strictly speaking, useless is not the correct description here. It is simply not needed any more. I'd just write Remove workaround. but I am aware that I'm far off into picky picky land here already ... * NEWS: Update. --- a/NEWS +++ b/NEWS @@ -56,6 +56,12 @@ Bugs fixed in 1.11.0a: - The code for automatic dependency tracking works around a Solaris make bug triggered by sources containing repeated slashes when the `subdir-objects' option was used. + + - Automake is now smart enough to detect the presence of the `-d' flag Let's not brag: s/is now smart enough to detect/now detects/ +in the various `*YFLAGS' variables even when their definitions involve +indirections through other variables, such as in: + foo_opts = -d + AM_YFLAGS = $(foo_opts) New in 1.11: --- a/automake.in +++ b/automake.in @@ -208,8 +208,6 @@ my $INCLUDE_PATTERN = ('^include\s+' . '|(\$\(srcdir\)/' . $PATH_PATTERN . ')' . '|([^/\$]' . $PATH_PATTERN . '))\s*(#.*)?' . \$); -# Match `-d' as a command-line argument in a string. -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.*)' . \$; @@ -6063,11 +6061,14 @@ sub lang_yacc_target_hook { my ($self, $aggregate, $output, $input, %transform) = @_; -my $flag = $aggregate . _YFLAGS; -my $flagvar = var $flag; -my $YFLAGSvar = var 'YFLAGS'; -if (($flagvar $flagvar-variable_value =~ /$DASH_D_PATTERN/o) - || ($YFLAGSvar $YFLAGSvar-variable_value =~ /$DASH_D_PATTERN/o)) +my $flagvar = var ($aggregate . _YFLAGS); +my $YFLAGSvar = var ('YFLAGS'); +# We cannot work reliably with conditionally-defined YFLAGS. +$flagvar-check_defined_unconditionally if $flagvar; +$YFLAGSvar-check_defined_unconditionally if $YFLAGSvar; +my @flags = $flagvar ? $flagvar-value_as_list_recursive : (); +my @YFLAGS = $YFLAGSvar ? $YFLAGSvar-value_as_list_recursive : (); +if (grep (/^-d$/, @flags) || grep (/^-d$/, @YFLAGS)) { (my $output_base = $output) =~ s/$KNOWN_EXTENSIONS_PATTERN$//; my $header = $output_base . '.h';
Re: bug#7766: (lack of) config.h description in manual
Hi Karl, * Karl Berry wrote on Sat, Jan 01, 2011 at 01:00:20AM CET: Finally, the amhello explained node is pretty long. It seems to break up naturally into three pieces, for each of the files being explained. Perhaps it would be good to make them each separate (sub)nodes. Hmm, I see a natural split into two, not three nodes. Below, I replaced the existing one rather than using another lower hierarchy level. Does this seem acceptable to you? Thanks, Ralf docs: split 'amhello Explained' node. * doc/automake.texi (amhello Explained): Split node ... (amhello/configure.ac, amhello/Makefile.am): ... into these two. (Top, Hello World): Adjust, and add @anchor for stable URL links. Suggestion by Karl Berry in automake bug#7766. diff --git a/doc/automake.texi b/doc/automake.texi index c63dbf3..9e29951 100644 --- a/doc/automake.texi +++ b/doc/automake.texi @@ -157,7 +157,8 @@ Top A Small Hello World * Creating amhello::Create @file{amhello-1.0.tar.gz} from scratch -* amhello Explained:: @file{configure.ac} and @file{Makefile.am} explained +* amhello/configure.ac::Configure setup explained +* amhello/Makefile.am:: Makefile setup explained General ideas @@ -1392,9 +1393,11 @@ Hello World meaning of the @file{configure.ac} and @file{Makefile.am} files read by the Autotools. +...@anchor{amhello Explained} @menu * Creating amhello::Create @file{amhello-1.0.tar.gz} from scratch -* amhello Explained:: @file{configure.ac} and @file{Makefile.am} explained +* amhello/configure.ac::Configure setup explained +* amhello/Makefile.am:: Makefile setup explained @end menu @node Creating amhello @@ -1555,8 +1558,10 @@ Creating amhello direct you to the right manual when seeking answers. -...@node amhello Explained -...@subsection @file{amhello-1.0} Explained +...@node amhello/configure.ac +...@subsection Configure setup explained + +...@cindex @file{configure.ac}, Hello World Let us begin with the contents of @file{configure.ac}. @@ -1664,6 +1669,10 @@ amhello Explained @command{autoscan} to Create @file{configure.ac}, autoconf, The Autoconf Manual}). + +...@node amhello/Makefile.am +...@subsection Makefile setup explained + @cindex @file{Makefile.am}, Hello World We now turn to @file{src/Makefile.am}. This file contains
Re: [PATCH] yacc: extend and improve tests
* Stefano Lattarini wrote on Sat, Jan 08, 2011 at 08:33:28PM CET: On Saturday 08 January 2011, Ralf Wildenhues wrote: --- /dev/null +++ b/tests/yacc-d-basic.test @@ -0,0 +1,157 @@ +#! /bin/sh +# Copyright (C) 2011 Free Software Foundation, Inc. If this supersedes yacc3.test, then does it not also derive from it? Actually no; I've basically written it from scratch (with some help from `yacc-basic.test'). Ignore my comment then. +# Even the generated header file is renamed when target-specific YFLAGS +# are used. This might not be the best semantic, but it has been in place semantics I went for behaviour instead. Not to get into a British vs. US English bikeshed argumentation, but I usually use the latter, just because it's more prevalent in the code already, and I mildly prefer consistency. So if you don't mind, use behavior, but otherwise I'll try not to make this a topic any further. +# for quite a long time, so just go along with it for now. Just out of curiosity: can you explain why this may not be good semantics? Because the details of outfile renamings when per-object flags are involved should be (as much as possible) an internal detail of automake. But in this case this detail must be exposed, since the output header file might be #included in other files (such as in the examples in this same testcase). Not a big deal, but noting this wart in a comment couldn't hurt either IMHO. Sure. I really was interested only. +$MAKE -s echo-distcom | grep '[ /]zardoz-parse.c ' +$MAKE -s echo-distcom | grep '[ /]zardoz-parse.h ' Did you try with some non-GNU make (e.g., heirloom or BSD make)? Yes; NetBSD make (debian port) and Heirloom/Solaris make works correctly, while FreeBSD make fails the distcheck with: ... ERROR: files left in build directory after distclean: ./foo/parse.h ./foo/parse.c ./bar/parse.h ./bar/parse.c ./baz/zardoz-parse.h ./baz/zardoz-parse.c *** Error code 1 There are failures in other tests with FreeBSD (and Solaris) make, but I'd rather leave them in place as they expose real issues (no point in sweeping the dirt under the rug to have a 100% testsuite pass in any case, right?). ACK. +# Sanity check on distribution. s/on // Really? Sounds weird to me (but I made the edit anyway). Oops. Hmm. I guess that depends on whether you read 'check' as noun or verb. If the latter, then probably s/on/the/ would've been better though. I will push soonish (tonight or tomorrow). Thanks! Ralf
Re: [Automake-commit] [SCM] GNU Automake branch, maint, updated. v1.11-268-gbb528a7
[ from -commit ] * Stefano Lattarini wrote on Sun, Jan 09, 2011 at 03:30:43PM CET: cosmetics: remove trailing whitespaces * doc/automake.texi: Remove trailing whitespaces. [...] --- a/doc/automake.texi +++ b/doc/automake.texi @@ -157,8 +157,8 @@ Use Cases for the GNU Build System A Small Hello World * Creating amhello::Create @file{amhello-1.0.tar.gz} from scratch -* amhello's configure.ac Setup Explained:: -* amhello's Makefile.am Setup Explained:: +* amhello's configure.ac Setup Explained:: +* amhello's Makefile.am Setup Explained:: General ideas @@ -1396,8 +1396,8 @@ by the Autotools. @anchor{amhello Explained} @menu * Creating amhello::Create @file{amhello-1.0.tar.gz} from scratch -* amhello's configure.ac Setup Explained:: -* amhello's Makefile.am Setup Explained:: +* amhello's configure.ac Setup Explained:: +* amhello's Makefile.am Setup Explained:: @end menu These are unfortunately going to return when I next regenerate menus with emacs ^C ^U ^A. Hmpf. Cheers, Ralf
Re: [PATCH] {maint} Improve, extend and tweak tests on Texinfo support.
Hi Stefano, sorry for the delay. * Stefano Lattarini wrote on Tue, Jan 04, 2011 at 03:41:45PM CET: On Monday 03 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Jan 03, 2011 at 02:38:48PM CET: Subject: [PATCH] Improve, extend and tweak tests on Texinfo support. [...] OK with nits addressed. I have a couple of questions below; I'll wait to push until they've been addressed. --- /dev/null +++ b/tests/comments-in-var-definition.test How about s/definition/defn/? That is still unique, easily understood, and a lot shorter. Fine with me (even if I still don't understand this bias against longer test names ;-) The longer the names, and the more the tests, the earlier we will exceed the command line length limit in our 'check' rules (important to fix on all systems it happens) and our 'distdir' rule (important at least for the maintainer's machine). So, support for more than one parallel-tests testsuite per Makefile.am is needed soonish. Besides, while I agree that the 8+3 names are often lacking descriptiveness, I also don't like typing too much. For example, I'm not sure why we named the 'posixsubst*.test' files that way; there is little specifically posixy about these substitution rules. +grep '^am__TEXINFO_TEX_DIR *=.*[/ ]tex *$' Makefile.in +$EGREP 'am__TEXINFO_TEX_DIR.*=.*(comment|#)' Makefile.in Exit 1 These two lines access internal details that could change. Acceptable if it must be that way but better if we can do without. I added those lines to avoid reducing coverage in the code I moved from `txinfo22.test' -- which indeed had a check: test -d $(am__TEXINFO_TEX_DIR) in its Makefile.am. Oh, ok, I didn't realize we were already using internal details before. Hhmm... but maybe it would be simpler safer to just add back that check (and new similar ones) in the Makefile.am of `comments-in-var-defn.test'? I think so; here is what I'll squash in if there are no objections: Fine as well, but I don't see how it makes much of a difference. am__TEXINFO_TEX_DIR is still an internal detail. --- a/tests/comments-in-var-defn.test +++ b/tests/comments-in-var-defn.test @@ -21,11 +21,20 @@ set -e +cat configure.in 'END' +AC_OUTPUT +END + # Use a slash in the comment, because automake takes the dirname # of TEXINFO_TEX to compute $(am__TEXINFO_TEX_DIR). cat Makefile.am 'END' TEXINFO_TEX = tex/texinfo.tex# some comment w/ a slash info_TEXINFOS = main.texi +.PHONY: test +test: + test tex/texinfo.tex = $(TEXINFO_TEX) + test -d '$(am__TEXINFO_TEX_DIR)' + case '$(am__TEXINFO_TEX_DIR)' in tex|./tex) :;; *) exit 1;; esac END cat main.texi 'END' @@ -41,7 +50,9 @@ $AUTOMAKE grep TEX Makefile.in # for debugging grep '^TEXINFO_TEX *= *tex/texinfo\.tex *# some comment w/ a slash *$' Makefile.in -grep '^am__TEXINFO_TEX_DIR *=.*[/ ]tex *$' Makefile.in -$EGREP 'am__TEXINFO_TEX_DIR.*=.*(comment|#)' Makefile.in Exit 1 + +$AUTOCONF +./configure +$MAKE test --- /dev/null +++ b/tests/vtexi3.test +day='([1-9]|1[0-9]|2[0-9]|3[01])' +month='(January|February|March|April|May|June|July|August|September|October|November|December)' +year='20[0-9][0-9]' # hopefully automake will be obsolete in 80 years ;-) I do not agree with the tone of the comment, and as punishment will require that the code also works later than the mentioned date. ;- Eh eh :-) (because that's a joke, right?) Not totally. Why do you hope that it will be obsolete? ;-) Also, your comment writing style seems to be degrading away from writing whole sentences (including leading capitalization and final period) again here and below. Yes, I tend to do so for short comments, especially in tests. If you would prefer to set a policy mandating that comments are always to be capitalized correctly and to consist of whole sentences, please just do so (ideally stating that in HACKING and tests/README ;-) and I'll follow the new policy as consistently as I can (while I like writing casual-style comments sometimes, I have no strong feeling on the matter). Me neither, at least not strongly enough to do something about it. I merely noted it though. For the moment, I've amended the comments in this test for proper capitalization, punctuation and grammar (see the attached squash-in). Thanks. +: ../foobar.info +: ../quux.info +: ../zardoz.info These commands are not guaranteed to portably update the time stamp of the files in question on old systems. :-O Hmm, the autoconf.texi blurb on `touch' states that this is no longer a practical issue, but IIRC the policy was still enforced in GCC sources, making me wonder whether there still are broken systems out there ... Anyway, you can easily avoid the issue by echo stamp ... I'd prefer to use `touch' if that's ok with you, since it makes the purpose of the commands even clearer
Re: [PATCH] {maint} Improve, extend and tweak tests on Texinfo support.
* Stefano Lattarini wrote on Tue, Jan 11, 2011 at 02:14:26AM CET: On Monday 10 January 2011, Ralf Wildenhues wrote: The longer the names, and the more the tests, the earlier we will exceed the command line length limit in our 'check' rules (important to fix on all systems it happens) and our 'distdir' rule (important at least for the maintainer's machine). Ouch, I never tought about these issues :-( So, support for more than one parallel-tests testsuite per Makefile.am is needed soonish. Or better (if possible) finding out a way to transparently avoid commandline-lenght issues when calling $(MAKE) recursively. There was a previous attempt of yours at this IIRC, but it didn't work out. Maybe it's time to give it a second shot? I don't think there is any way to avoid the limit with portable make alone. When gnu-make infrastructure is in place, we can think about a replacement rule for that, but we should provide multiple test suites anyway, that's also nice for subsetting in general. Besides, while I agree that the 8+3 names are often lacking descriptiveness, I also don't like typing too much. But how often do you type the name of the testcases after all? (I mean, without the help of tab completion of course ;-). Oh, this is obviously not a big deal, but I actually try NetBSD csh sometimes which doesn't seem to provide it; this is mostly to ensure that the $SHELL setting from the environment doesn't leak into our code. For example, I'm not sure why we named the 'posixsubst*.test' files that way; there is little specifically posixy about these substitution rules. Well, they are the only POSIX-mandated textual substitutions for make macros, so I thought the test names were appropriated -- or am I missing something? What's wrong with s/^posix// though? Lots of other things are Posix-mandated too, but we don't make a big deal out of that either? Cheers, Ralf
Re: [PATCH] {master} tests: more consistent checks about invalid options
Hi Stefano, * Stefano Lattarini wrote on Tue, Jan 11, 2011 at 01:48:41PM CET: Another (minor) testsuite patch. Discouraged, but OK with nit addressed. Thanks, Ralf * tests/aclocal.test: Grepping of automake stderr for messages reporting invalid options made stricter. * tests/no-outdir-option.test: Likewise. Also, create a dummy `Makefile.am', to ensure that the automake failures are really caused only by unrecognized options. * tests/automake.test: Added trailing `:' command. Removed redundant checks on `--help' and `--version' option (already performed in the test `help*.test'). --- a/tests/no-outdir-option.test +++ b/tests/no-outdir-option.test @@ -20,10 +20,12 @@ set -e +: Makefile.am + AUTOMAKE_fails -Wno-error --output-dir=foo -$EGREP '(invalid|unrecognized) option.*--output-dir' stderr +grep 'unrecognized option.*--output-dir' stderr This strikes me as wrong. Why would you have written the test like this in the first place if there wasn't a good reason for this? (And just the time required to think about this, track down the reason for the original choice, etc., wastes more than would have been gained by this patch IMVHO.) Maybe because Getopt::Long::GetOptions could throw a (differently spelled) error itself? AUTOMAKE_fails -Wno-error -o foo -$EGREP '(invalid|unrecognized) option.*-o' stderr +grep 'unrecognized option.*-o' stderr
Re: documentation about '-d' in AM_YFLAGS is suboptimal
* Stefano Lattarini wrote on Wed, Jan 12, 2011 at 01:36:08PM CET: On Wednesday 12 January 2011, Юрий Пухальский wrote: Package isn't enclosed because otherwise the message doesn't get through. Finally had time to make some tests, and now i've got it:) When i set AM_YFLAGS to -dt it doesn't distribute a header, probably because it's searching for -d to be alone. When i set AM_YFLAGS = -d -t, it works. ... I agree the documentation might be clearer about this. Ralf, what about the attached patch (for 'yacc-work' and 'maint')? Tested with make pdf ps dvi html MAKEINFOFLAGS=--no-split. The output in all forms looked fine. OK with nits addressed. Thanks! Ralf From cf43febea57a25173e665e2c610b1ccafdd26b01 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Wed, 12 Jan 2011 13:23:59 +0100 Subject: [PATCH] docs: clustered '-d' not recognized in YFLAGS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes automake bug#7828. * doc/automake.texi (Yacc and Lex): Tell that automake recognizes s/Tell/Document/ (tell is rarely a good choice in written text. I don't know why.) '-d' in AM_YFLAGS only if it's not clusterd with other options. clustered From a report by . My email client renders this as a bunch of question marks only. Please ensure that both the git log entry and the ChangeLog entry are properly encoded in UTF-8. --- a/doc/automake.texi +++ b/doc/automake.texi @@ -6078,7 +6078,12 @@ intended for the @file{Makefile.am} author. @code{AM_YFLAGS} is usually used to pass the @option{-d} option to @command{yacc}. Automake knows what this means and will automatically adjust its rules to update and distribute the header file built by -...@samp{yacc -d}. What Automake cannot guess, though, is where this +...@samp{yacc -...@footnote{please note that @command{automake} recognizes +...@option{-d} in @code{AM_YFLAGS} only if it is not clustered with other +options; for example, it won't be recognized if @code{AM_YFLAGS} is +...@samp{-dt}, but it will be if @code{AM_YFLAGS} is @samp{-d -t} or +...@samp{-t -d}.}. Generally, @footnote's should be used sparingly, as they disrupt the flow of reading and don't render well in the info output. I guess it's acceptable here though. Please use @option for denoting the options here. +What Automake cannot guess, though, is where this header will be used: it is up to you to ensure the header gets built before it is first used. Typically this is necessary in order for dependency tracking to work when the header is included by another
Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.
Here comes my (belated, as always) re-review; I am not repeating nits already dealt with before. * Stefano Lattarini wrote on Wed, Jan 05, 2011 at 08:22:13PM CET: On Wednesday 05 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Tue, Jan 04, 2011 at 06:38:04PM CET: This is derived from [PATCH 07/10] of the older series. It requires a review. Warnings win over strictness in AM_INIT_AUTOMAKE. This change ensures that, for what concerns the options specified in AM_INIT_AUTOMAKE, explicitly-defined warnings always take precedence over implicit strictness-implied warnings. Related to Automake bug#7669 a.k.a. PR/547. * lib/Automake/Options.pm (_process_option_list): Parse explicit warnings only after the strictness level has been set. * tests/warnings-win-over-strictness.test: Extend. --- a/lib/Automake/Options.pm +++ b/lib/Automake/Options.pm @@ -313,11 +314,7 @@ sub _process_option_list (\%$@) } elsif (/^(?:--warnings=|-W)(.*)$/) { - foreach my $cat (split (',', $1)) - { - msg 'unsupported', $where, unknown warning category `$cat' - if switch_warning $cat; - } + push @warnings, split (',', $1); } else { @@ -326,6 +323,14 @@ sub _process_option_list (\%$@) return 1; } } + # We process warnings here, so that any explicitly-given warning setting + # will take precedence over warning settings defined implicitly by the + # strictness. Well, this works in the current code base, but only by accident: namely, only because process_option_list is only ever called once, and with all options at once. Hmm... no, it's potentially called many times in `handle_options()'. But the later [PATCH 7/9] takes care of this. But that's exactly what I mean: you need patch 7/9 precisely because you have changed the requirements that you push onto callers of process_*options_list. Let me show you what I mean: with your patch series, you would also need something like the following: To this POD text in Options.pm: | =item Cprocess_option_list ($where, @options) | | =item Cprocess_global_option_list ($where, @options) | | Process Automake's option lists. C@options should be a list of | words, as they occur in CAUTOMAKE_OPTIONS or CAM_INIT_AUTOMAKE. you would need to add the following: These functions should be called at most once for each set of options having the same precedence; i.e., do not call it twice for two options from CAM_INIT_AUTOMAKE. It is important for maintainability that the POD documentation is correct. And actually, now that we see the requirement printed, it is clear that this is not a good move: the API has suddenly become less easy to use, because it is easier for the callers of process_*option_list to use it wrongly. http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html One better API would be one that either - checks that options of the same precedence are not split up when passed, or - computes the set of active warnings only strictly after all options (of a certain precedence) have been processed. And upon rereading the code, it is also quite clear where the other reported precedence bugs come from: the global_options list is abused for two sets of options: those from AM_INIT_AUTOMAKE, which should have lower precedence than AUTOMAKE_OPTIONS, and those from the command line, which should have higher precedence than AUTOMAKE_OPTIONS. The early conflation made correct precedence semantics impossible. The right way here would be to have three stores of options. And now that also makes it clear why the automake code printed some of the options as arguments in the rebuild rule for Makefile.in: because precedence was botched, that was needed in order to avoid these options to get lost. Or so at least I believe. Hmm, set_strictness has some of the same problems, except one complication is that some warnings may apply already at global level, before any Makefile.am file is even opened. I'm guessing three sets are needed here too. So, now with that said, I'm not sure whether I should approve this patch. What do you think? If some code later calls it like process_option_list (first-set-of-options); process_option_list (second-set-of-options); then things will go wrong again. I suspect that it will mean that AM_INIT_AUTOMAKE([foreign -Wno-portability]) AUTOMAKE_OPTIONS = gnu won't do what we want. Hmm. What exactly is it that we want to happen in this case? Should gnu override -Wno-portability if specified in a (to-be) higher order place? I assumed without saying that yes, this was to be the intended behaviour. And I still think it should be. Sorry for not having been explicit about that before. I agree that it should be, but this, too, should be documented (in autoconf.texi and maybe also NEWS) and tested, when it works. I see two ways
Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.
* Stefano Lattarini wrote on Wed, Jan 12, 2011 at 09:11:41PM CET: On Wednesday 12 January 2011, Ralf Wildenhues wrote: So, now with that said, I'm not sure whether I should approve this patch. What do you think? I think that you should, provided that I add the sanity check you suggested above. OK. Thanks, Ralf
Avoid testsuite failures due to Autoconf Fortran change
My recent Autoconf change provoked some testsuite failures in Automake, see http://hydra.nixos.org/build/856303/log/raw. I'm pushing the fix below to maint and merging to branch-1.11 and master. Cheers, Ralf Avoid testsuite failures due to Autoconf Fortran change. Autoconf v2.68-21-g727ce95 causes AC_F77_LIBRARY_LDFLAGS to require computing the canonical host name. Ensure config.guess and config.sub files are present for respective checks. * tests/compile_f_c_cxx.test: Add stub files. * tests/flibs.test: Likewise. * tests/fort4.test: Use $AUTOMAKE -a for installing files. diff --git a/tests/compile_f_c_cxx.test b/tests/compile_f_c_cxx.test index a995527..d1836fb 100755 --- a/tests/compile_f_c_cxx.test +++ b/tests/compile_f_c_cxx.test @@ -39,6 +39,8 @@ END : foo.f : bar.c : baz.cc +: config.guess +: config.sub $ACLOCAL $AUTOMAKE diff --git a/tests/flibs.test b/tests/flibs.test index b9fe541..d35c4c6 100755 --- a/tests/flibs.test +++ b/tests/flibs.test @@ -48,6 +48,8 @@ zardoz_LDADD = @FLIBS@ END : zardoz.f +: config.guess +: config.sub $ACLOCAL $AUTOMAKE diff --git a/tests/fort4.test b/tests/fort4.test index b928a61..84df399 100755 --- a/tests/fort4.test +++ b/tests/fort4.test @@ -67,7 +67,7 @@ LDADD = $(FCLIBS) END $ACLOCAL -$AUTOMAKE +$AUTOMAKE -a # The Fortran 77 linker should be preferred: grep '.\$(FCLINK)' Makefile.in Exit 1
Re: [PATCH] {maint} Improve, extend and tweak tests on Texinfo support.
Hi Stefano, * Stefano Lattarini wrote on Mon, Jan 03, 2011 at 02:38:48PM CET: From f236e0eb21df62ff4e5c01ecfaefe75d2e3d3d9b Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Fri, 24 Dec 2010 02:56:35 +0100 Subject: [PATCH] Improve, extend and tweak tests on Texinfo support. * tests/instdir-texi.test: Add a call to `ls -l' after that to `make', for debugging. When looking for required tools, do not redirect the output of $tool --help to /dev/null, and do not uselessly run it in a subshell. * tests/txinfo.test: Rewritten to run autoconf, ./configure and make. All checks moved into Makefile.am. * tests/txinfo8.test: Likewise, and modernize the generated configure.in. [...] This patch has caused a couple of regressions, txinfo.test and txinfo8.test now fail when makeinfo is not found. See here: http://hydra.nixos.org/build/856303/log/raw How about not requiring makeinfo in these tests? Thanks, Ralf
Re: [PATCH 6/9] Change signature of 'Automake::Options::_process_option_list()'.
* Stefano Lattarini wrote on Tue, Jan 04, 2011 at 06:48:06PM CET: Change signature of 'Automake::Options::_process_option_list()'. This only modifies internal details in the automake implementation, bearing no externally visible effect, but preparing the way for the final fix of Automake bug#7669 a.k.a. PR/547. * lib/Automake/Options.pm (_process_option_list): Now accepts as s/Now accepts/Accept/ arguments a list of hash references with keys 'option' and 'where', where 'option' is an option as might occur in AUTOMAKE_OPTIONS or M_INIT_AUTOMAKE, and 'where' is the location where that occurred. s/^/A/ s/that/it/ (process_option_list, process_global_option_list): Update. * automake.in (handle_options, scan_autoconf_traces): Update. --- a/lib/Automake/Options.pm +++ b/lib/Automake/Options.pm @@ -222,30 +222,35 @@ sub unset_global_option ($) } -=item Cprocess_option_list ($where, @options) +=item Cprocess_option_list (@options) The placeholder variable names here should match those actually used in the perl code inside the function, i.e., @list, to make it unambiguous wrt. _process_option_list. -=item Cprocess_global_option_list ($where, @options) +=item Cprocess_global_option_list (@options) Likewise. -Process Automake's option lists. C@options should be a list of -words, as they occur in CAUTOMAKE_OPTIONS or CAM_INIT_AUTOMAKE. +Process Automake's option lists. C@options should be a list of hash +references with keys Coption and Cwhere, where Coption is an option +as might occur in CAUTOMAKE_OPTIONS or CAM_INIT_AUTOMAKE, and s/might/they/ +Cwhere is the location where that option occurred. Return 1 on error, 0 otherwise. =cut # $BOOL -# _process_option_list (\%OPTIONS, $WHERE, @OPTIONS) -# -- -# Process a list of options. Return 1 on error, 0 otherwise. -# \%OPTIONS is the hash to fill with options data, $WHERE is -# the location where @OPTIONS occurred. -sub _process_option_list (\%$@) +# _process_option_list (\%OPTIONS, @OPTIONS) The placeholder should be the upper-cased version of the variable name used, i.e., @LIST. +# -- +# Process a list of options. \%OPTIONS is the hash to fill with +# options data. +# options as get passed to public subroutines process_option_list() and +# process_global_option_list(). These sentences are botched. Please fix. Also, the documentation above requires the changes discussed in the review of 4/9. +sub _process_option_list (\%@) { - my ($options, $where, @list) = @_; + my ($options, @list) = @_; my @warnings = (); - foreach (@list) + foreach my $h (@list) { + my $_ = $h-{option}; + my $where = $h-{where}; Quoting of literal strings inside {}? $options-{$_} = $where; if ($_ eq 'gnits' || $_ eq 'gnu' || $_ eq 'foreign') { @@ -314,7 +319,8 @@ sub _process_option_list (\%$@) } elsif (/^(?:--warnings=|-W)(.*)$/) { - push @warnings, split (',', $1); + my @w = map { { cat = $_, loc = $where} } split (',', $1); + push @warnings, @w; } else { @@ -326,24 +332,22 @@ sub _process_option_list (\%$@) # We process warnings here, so that any explicitly-given warning setting # will take precedence over warning settings defined implicitly by the # strictness. - foreach my $cat (@warnings) + foreach my $w (@warnings) { - msg 'unsupported', $where, unknown warning category `$cat' - if switch_warning $cat; + msg 'unsupported', $w-{loc}, unknown warning category `$w-{cat}' + if switch_warning $w-{cat}; see above } return 0; } -sub process_option_list ($@) +sub process_option_list (@) { - my ($where, @list) = @_; - return _process_option_list (%_options, $where, @list); + return _process_option_list (%_options, @_); } -sub process_global_option_list ($@) +sub process_global_option_list (@) { - my ($where, @list) = @_; - return _process_option_list (%_global_options, $where, @list); + return _process_option_list (%_global_options, @_); } =item Cset_strictness ($name) OK with that fixed. Thanks, Ralf
Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.
* Stefano Lattarini wrote on Thu, Jan 13, 2011 at 10:27:48PM CET: On Thursday 13 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Wed, Jan 12, 2011 at 09:11:41PM CET: On Wednesday 12 January 2011, Ralf Wildenhues wrote: So, now with that said, I'm not sure whether I should approve this patch. What do you think? I think that you should, provided that I add the sanity check you suggested above. OK. Hmm... Wait, this sanity check is OK only starting from [PATCH 7/9] onward, as until then the 'handle_options()' subroutine still calls 'process_option_list()' multiple times (that's why the bug is still present for AUTOMAKE_OPTIONS at this point). So, OK to add the sanity check in [PATCH 7/9] (or in a new follow-up patch to be placed between patches 7 and 8 in this series)? Well yes, but why not show the check diff if you have it already? Thanks, Ralf
Re: [PATCH 4/9] Warnings win over strictness in AM_INIT_AUTOMAKE.
* Stefano Lattarini wrote on Fri, Jan 14, 2011 at 12:11:27AM CET: [Ralf Wildenhues] If some code later calls it like process_option_list (first-set-of-options); process_option_list (second-set-of-options); then things will go wrong again. I suspect that it will mean that AM_INIT_AUTOMAKE([foreign -Wno-portability]) AUTOMAKE_OPTIONS = gnu won't do what we want. Hmm. What exactly is it that we want to happen in this case? Should gnu override -Wno-portability if specified in a (to-be) higher order place? [Stefano Lattarini] I assumed without saying that yes, this was to be the intended behaviour. And I still think it should be. Sorry for not having been explicit about that before. [Ralf Wildenhues] I agree that it should be, but this, too, should be documented (in autoconf.texi and maybe also NEWS) and tested, when it works. What about the attached patch? It also adds a test for another situation I hadn't thought about previously. OK to apply the patch in a new commit between [PATCH 2/9] and [PATCH 3/9]? Well yes, but why omit the documentation bits that I asked for? (efficient communication, and all that) Thanks, Ralf Subject: [PATCH] More tests on warnings and strictness. * tests/warnings-strictness-interactions.test: New test. * tests/warnings-unknown.test: Likewise. * tests/Makefile.am (TESTS): Update.
Re: bug#7833: automake uses two different values for DejaGNU srcdir
* 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. 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. 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.0 +0100 +++ am/dejagnu.am.1 2011-01-14 20:27:38.0 +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.0 +0100 +++ am/dejagnu.am.2 2011-01-14 20:20:22.0 +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
autobuild logs
I've put up a set of logs at http://autobuild.josefsson.org/automake/ again. Quite a few new failures, so I think some maintenance work is warranted. Cheers, Ralf
testsuite issues on MinGW/MSYS
I'm seeing a couple of weird issues on MinGW/MSYS. First of all, several spurious failures of this kind: ./defs: line 33: /bin/sed: Resource temporarily unavailable The line in question is: me=`echo $argv0 | sed -e 's,.*[\\/],,;s/\.test$//'` This is on a loaded system, but I don't remember having seen this before at all. So I wonder what actually causes this behavior. There is also a stack dump from some sed invocation, so maybe my installation is corrupted. Anyway, the above causes all kinds of ugly misbehavior, starting with all tests being run in the same directory '.dir', and several tests failing because 'make dist' tries to 'mkdir -1', as $(PACKAGE) is empty. I'm merging the following as a stop-gap measure, to at least make the behavior obvious. And I'm going to review Stefano's pending tests-init patches next, as they address a number of related issues around this area. Cheers, Ralf tests: avoid spurious failures due to fork failure in test setup * tests/defs: Ensure $me is always nonempty, to avoid spurious failures on MinGW/MSYS in case the preceding sed command could not be spawned. diff --git a/tests/defs b/tests/defs index 64ed985..0f84d0a 100644 --- a/tests/defs +++ b/tests/defs @@ -1,7 +1,8 @@ # -*- shell-script -*- # # Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, -# 2005, 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc. +# 2005, 2006, 2007, 2008, 2009, 2010, 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 @@ -31,6 +32,7 @@ test -f ./defs-static || { # The name of the current test (without the `.test' suffix.) me=`echo $argv0 | sed -e 's,.*[\\/],,;s/\.test$//'` +test -n $me || exit 99 ## ##
Re: [PATCH 1/5] Tests defs: don't let useless variables leak in test scripts.
Hi Stefano, and apologies for the long delay, there were three patches in this series that haven't been reviewed yet: * Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:21:16PM CET: * tests/defs ($priv_check_temp, $overwrite_status, $ro_dir_temp, $create_status, $r2h, $libtool_found, $gettext_found, $aclocaldir, $extra_includes): Unset once they've served their purpose. This is OK; all of the variables are set in all code paths. Thanks, Ralf
Re: [PATCH 2/5] Tests defs: new subroutine `skip' for test skipping.
Hi Stefano, * Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:23:21PM CET: Tests defs: new subroutine `skip' for test skipping. I don't like having this patch alone. If we switch to functions, then we should so wholesale, and at once, and fully. Also, there is precedent in {coreutils,gnulib}/tests/init.sh which has more features than this implementation and I think we should follow (and/or improve upon if that is not sufficient): # Print warnings (e.g., about skipped and failed tests) to this file number. # Override by defining to say, 9, in init.cfg, and putting say, # export ...ENVVAR_SETTINGS...; exec 92; $(SHELL) in the definition # of TESTS_ENVIRONMENT in your tests/Makefile.am file. # This is useful when using automake's parallel tests mode, to print # the reason for skip/failure to console, rather than to the .log files. : ${stderr_fileno_=2} warn_() { echo $@ 1$stderr_fileno_; } fail_() { warn_ $ME_: failed test: $@; Exit 1; } skip_() { warn_ $ME_: skipped test: $@; Exit 77; } framework_failure_() { warn_ $ME_: set-up failure: $@; Exit 99; } Generally, when we change something, it is a good idea to avoid NIH. (That doesn't mean we have to change to remove existing NIH, but avoid new. The point being that stuff that is new is untested and broken.) That said, I found minor issues in the proposed patch which I'm listing so you take measures to avoid them when redoing. This kind of patch should really be written with a script, not manually. * tests/defs.in (skip): New subroutine, causing the running test script to be SKIP'd, possibly with a meaningful message. Use it throughout. This file is ... * tests/REAMDE: Updated. * tests/color.test: Use the new `skip' subroutine. * tests/color2.test: Likewise. * tests/compile2.test: Likewise. * tests/defs.in: Likewise. ... listed twice. * tests/dejagnu7.test: Likewise. * tests/distlinks.test: Likewise. * tests/distlinksbrk.test: Likewise. * tests/fn99.test: Likewise. * tests/fn99subdir.test: Likewise. * tests/forcemiss2.test: Likewise. * tests/fort5.test: Likewise. * tests/gettext3.test: Likewise. * tests/install2.test: Likewise. * tests/instfail-info.test: Likewise. * tests/instfail-java.test: Likewise. * tests/instfail-libtool.test: Likewise. * tests/instfail.test: Likewise. * tests/instmany-mans.test: Likewise. * tests/instmany-python.test: Likewise. * tests/instmany.test: Likewise. * tests/instsh3.test: Likewise. * tests/makej2.test: Likewise. * tests/mkinst3.test: Likewise. * tests/parallel-tests3.test: Likewise. * tests/subobj9.test: Likewise. * tests/symlink2.test: Likewise. * tests/tar.test: Likewise. * tests/tar2.test: Likewise. * tests/txinfo26.test: Likewise. * tests/vala2.test: Likewise. * tests/vala3.test: Likewise. * tests/vala5.test: Likewise. * tests/instdir-texi.test: Likewise, plus some other minor improvements. Please refrain from doing other changes that don't belong in the same patch. * tests/txinfo21.test: Likewise. [...] --- a/tests/instsh3.test +++ b/tests/instsh3.test @@ -18,10 +18,11 @@ required=non-root . ./defs || Exit 1 + set -e unrelated change # Solaris /usr/ucb/touch does not accept -t. -touch -t $old_timestamp foo || Exit 77 +touch -t $old_timestamp foo || touch utility doesn't accept '-t' option missing skip ./install-sh -d d1
Avoid VPATH-related distcheck failures with non-GNU make
OK, so all the added '$MAKE distcheck' for safety at the end of the tests caused roughly a thousand new test failures across the board, it seems, drowning other signals. Most of these failures apparently have two reasons: 1) Cleanup failed to remove some directory because of stale NFS handles, 2) The native make is not up to the task of VPATH builds. Now, distcheck is not a GCS-mandated target, so it need not work everywhere. Also, we already document in the GNU-global INSTALL file that GNU make is preferred when doing VPATH builds (although Automake currently still does try to work around all issues except those from Solaris 2.6 and IRIX make for the standard targets). So, how about changing *those* $MAKE distcheck invocations which don't have added value over checking that the distribution is complete to call ensure_distcheck_, with this in defs.in: ensure_distcheck_ () { if $MAKE --version -v | grep GNU; then $MAKE distcheck else : fi } (not turning the test result into a SKIP since this is not a major point of the test). Remains only the task to identify for which tests this is true. Addressing (1) right may turn out to be more tricky: if there are files which are held open by some process, then that would probably be a bug in the test. Otherwise, I'm suspecting bugs in the NFS implementation. Maybe some of the clean rules are lacking '-' prefix (to ignore return status), but that should only be added for those rules where we really cannot miss an important failure (e.g., bug in the rule) that we must address. Thanks, Ralf
Re: [PATCH] tests: fix comment typo
Hi Jim, * Jim Meyering wrote on Sat, Jan 15, 2011 at 10:49:07AM CET: It looks like I never sent this patch: Thanks. I added a ChangeLog entry, bumped copyright years, and pushed to maint. Cheers, Ralf From 2a898e3a7bc80dac3506f8ef325a95ef375971ef Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Thu, 14 Oct 2010 16:39:22 +0200 Subject: [PATCH] tests: fix comment typo * tests/substref.test: Fix grammar in a comment.
Fix remake rule-induced test failures with HP-UX make.
Quoting parts of: http://autobuild.josefsson.org/automake/log-201101150749395803000.txt | FAIL: colon6.test (exit: 1) | === | | /tmp/am/build-hppa2.0w-hp-hpux11.23/tests:/tmp/local/hppa2.0w-hp-hpux11.23/bin:/tmp/bin:/opt/aCC/bin:/opt/ansic/bin:/bin:/usr/bin:/sbin:/usr/sbin:/usr/bin/X11:/usr/ccs/bin:/usr/contrib/bin/X11:/usr/vue/bin:/opt/imake/bin | + set -e | + pwd | /tmp/am/build-hppa2.0w-hp-hpux11.23/tests/colon6.dir | + cat | + 1 configure.in 0 | AC_INIT([colon6], [1.0]) | AM_INIT_AUTOMAKE | # With this, version.good should depend from version.gin, while | # Makefile should not depend from either of them. | AC_CONFIG_FILES([demo/Makefile demo/version.good:demo/version.gin]) | AC_OUTPUT | + mkdir demo | + : | + 1 demo/version.gin | + : | + 1 demo/Makefile.am | + aclocal-1.11a -Werror | + automake-1.11a --foreign -Werror -Wall | + /bin/grep -E Makefile:.*(demo|version) demo/Makefile.in | + grep version\.good:.*version\.gin demo/Makefile.in | version.good: $(top_builddir)/config.status $(srcdir)/version.gin | + autoconf | + : | + srcdir=.. | + mkdir build | + cd build | + ../configure | checking for a BSD-compatible install... ../install-sh -c | checking whether build environment is sane... yes | checking for a thread-safe mkdir -p... ../install-sh -c -d | checking for gawk... no | checking for mawk... no | checking for nawk... no | checking for awk... awk | checking whether make sets $(MAKE)... yes | checking that generated files are newer than configure... done | configure: creating ./config.status | config.status: creating demo/Makefile | config.status: creating demo/version.good | + test -f demo/version.good | + test ! -s demo/version.good | + cd demo | + sleep 2 | + echo Rebuilt (srcdir=..) | + 1 ../../demo/version.gin | + make version.good | No suffix list. | cd .. /bin/sh ./config.status demo/version.good | config.status: creating demo/version.good | + /bin/grep -F Rebuilt (srcdir=..) version.good | Rebuilt (srcdir=..) | + rm -f version.good | + make Makefile | No suffix list. | Make: Don't know how to make am--refresh. Stop. | *** Error exit code 1 | | Stop. | + exit_status=1 | + set +e | + cd /tmp/am/build-hppa2.0w-hp-hpux11.23/tests | + test 0 != 0 | + echo colon6: exit 1 | colon6: exit 1 | + exit 1 [...] | /tmp/am/build-hppa2.0w-hp-hpux11.23/tests:/tmp/local/hppa2.0w-hp-hpux11.23/bin:/tmp/bin:/opt/aCC/bin:/opt/ansic/bin:/bin:/usr/bin:/sbin:/usr/sbin:/usr/bin/X11:/usr/ccs/bin:/usr/contrib/bin/X11:/usr/vue/bin:/opt/imake/bin | + set -e | + pwd | /tmp/am/build-hppa2.0w-hp-hpux11.23/tests/remake1a.dir | + fingerprint==/FiNgErPrInT/= | + cat | + 1 configure.in 0 | AC_INIT([remake1a], [1.0]) | AM_INIT_AUTOMAKE | AC_CONFIG_FILES([sub/Makefile]) | AC_OUTPUT | + mkdir sub | + : | + 1 sub/Makefile.am | + aclocal-1.11a -Werror | + autoconf | + automake-1.11a --foreign -Werror -Wall | + ./configure | checking for a BSD-compatible install... ./install-sh -c | checking whether build environment is sane... yes | checking for a thread-safe mkdir -p... ./install-sh -c -d | checking for gawk... no | checking for mawk... no | checking for nawk... no | checking for awk... awk | checking whether make sets $(MAKE)... yes | checking that generated files are newer than configure... done | configure: creating ./config.status | config.status: creating sub/Makefile | + sleep 2 | + echo # =/FiNgErPrInT/= | + 1 sub/Makefile.am | + cd sub | + make Makefile | No suffix list. | cd .. /bin/sh /tmp/am/build-hppa2.0w-hp-hpux11.23/tests/remake1a.dir/missing --run automake-1.11a --foreign sub/Makefile | Make: Don't know how to make am--refresh. Stop. | *** Error exit code 1 I'm not quite sure why HP-UX make seems to trigger rebuild rules for prerequisites that have the exact time stamps as targets (in this case, config.status and subdir Makefile, but I also tried with a reduced makefile), but it is not Posix conforming. Working around it properly would probably require sleeping inside config.status (an obvious no-go), and in practice it should not do more harm than updating some rules more than necessary. How about the patch below to not let this cause spurious testsuite failures? If you prefer, I can split the typo fixes in a separate patch. Thanks, Ralf Fix remake rule-induced test failures with HP-UX make. * tests/colon6.test: Update timestamp of subdir Makefile, to ensure we do not spuriously invoke the nonexistent toplevel am--refresh rule. Fix comment typos. * tests/remake1a.test: Require GNU make. diff --git a/tests/colon6.test b/tests/colon6.test index bf5cf2d..984f13e 100755 --- a/tests/colon6.test +++ b/tests/colon6.test @@ -22,8 +22,8 @@ cat configure.in END AC_INIT([$me], [1.0]) AM_INIT_AUTOMAKE -# With this, version.good should depend from version.gin, while -# Makefile should not depend from either of them. +# With this, version.good should depend on version.gin, while +# Makefile should not depend on either of them.
Fix libobj2.test failure with non-GNU make: define $(AR).
automake currently defines $(AR) only if it sees a _LIBRARIES primary. That may be an issue or not (and the wrong value for cross setups that it defines is definitely an issue), but not important for this test IMVHO, so how about this patch to avoid a spurious test failure as in http://autobuild.josefsson.org/automake/log-201101150749395803000.txt test -f ./subdir/fsusage.c test -f libobj2-1.0/subdir/fsusage.c tv ./subdir/libtu.a Make: Cannot load tv. Stop. *** Error exit code 1 (The important point for fixing the other $(AR) issues is that the definition must not clash with one from AC_PROG_LIBTOOL or from users.) Thanks, Ralf Fix libobj2.test failure with non-GNU make: define $(AR). * tests/libobj2.test: Ensure $(AR) is suitably defined. diff --git a/tests/libobj2.test b/tests/libobj2.test index 3b3ec98..66ca0d0 100755 --- a/tests/libobj2.test +++ b/tests/libobj2.test @@ -24,6 +24,7 @@ cat configure.in 'END' AC_CONFIG_FILES([subdir/Makefile]) AC_PROG_CC AC_PROG_RANLIB +AC_CHECK_TOOLS([AR], [ar]) AC_LIBOBJ([fsusage]) AC_OUTPUT END
Re: tests: always update generated tests silently
* Stefano Lattarini wrote on Sat, Jan 15, 2011 at 12:26:24PM CET: As I wrote (or copied? I can't remember) in my pending documentation patch on silent rules support: @cindex default verbosity for silent-rules Note that silent rules are @emph{disabled} by default; the user must enable them explicitly at either @command{configure} run time or at @command{make} run time. We think that this is a good policy, since it provides the casual user with enough information to prepare a good bug report in case anything breaks. Ah yes. I haven't forgotten that patch. I can try to finish my review of it this weekend, but it will be more of a rewrite than a review, I'm afraid. Also, I like the automake silent-rules support because I can decide at at any step what the verbosity is to be. Just silencing some rules unconditionally would IMHO be a step backward from the current behaviour. Sure, in general I very much agree with you. It's just that in this particular case, I think the printed commands add a lot to noise, but add very little in way of information, unlike with compile commands, where often the particular command is interesting. With the generated test rules, that is not the case: the rules are completely static, no variation among systems, compilers, shells, anything. And of course I don't aim to change anything more general about silent rules. Alternatively, we could prefix them with @$(AM_V_GEN) @$(AM_V_at) ... then in silent-rules mode the GEN line would still be output. Yes please (if you really must silence the rules unconditionally, of course). Or should we go the next step and use AM_SILENT_RULES([yes])? Again from my pending documentation patch: Still, notwithstanding the rationales above, a developer who wants to make silent rules enabled by default in his own package can do so by adding a @samp{yes} argument to the @code{AM_SILENT_RULES} call in @file{configure.ac}. We advise against this approach, though. This advice should be changed if the automake's own build system starts using AM_SILENT_RULES([yes]). Why do you think that? The Automake package is fairly special in that its own build rules are almost completely trivial, unlike most packages which actually use a compiler of some sort. But I think it's a good advice as is ... Sure. Anyway, going with AM_SILENT_RULES([yes]), while not good per se, would probably be better than starting to silencing rules unconditionally. OK. Elsewhere, you write: A possible mediation would be to enable silent by default only in builds done from a cloned git repository. I've done something similar already (caveat: for toy projects only). WDYT? No, I don't really see why that should have much to do with it. Thanks, Ralf
Re: testsuite issues on MinGW/MSYS
* Stefano Lattarini wrote on Sat, Jan 15, 2011 at 12:07:45PM CET: On Saturday 15 January 2011, Ralf Wildenhues wrote: I'm seeing a couple of weird issues on MinGW/MSYS. First of all, several spurious failures of this kind: ./defs: line 33: /bin/sed: Resource temporarily unavailable @@ -31,6 +32,7 @@ test -f ./defs-static || { # The name of the current test (without the `.test' suffix.) me=`echo $argv0 | sed -e 's,.*[\\/],,;s/\.test$//'` +test -n $me || exit 99 A agree with your change, but I'd definitely add a comment (extracted from your explanations above) and an error message. Personally, I'd go for something like this: me=`echo $argv0 | sed -e 's,.*[\\/],,;s/\.test$//'` \ test -n $me \ || { echo $argv0: failed to define \$me 2; exit 99; } Like this? Thanks, Ralf 2011-01-15 Ralf Wildenhues ralf.wildenh...@gmx.de Stefano Lattarini stefano.lattar...@gmail.com tests: explain MSYS setup failure issue, improve test. * tests/defs: Add comment and failure message, improve fail logic. diff --git a/tests/defs b/tests/defs index 0f84d0a..b357df6 100644 --- a/tests/defs +++ b/tests/defs @@ -31,8 +31,10 @@ test -f ./defs-static || { . ./defs-static || exit 99 # The name of the current test (without the `.test' suffix.) -me=`echo $argv0 | sed -e 's,.*[\\/],,;s/\.test$//'` -test -n $me || exit 99 +# Guard against failure to spawn sed (seen on MSYS), or empty $argv0. +me=`echo $argv0 | sed -e 's,.*[\\/],,;s/\.test$//'` \ + test -n $me \ + || { echo $argv0: failed to define \$me 2; exit 99; } ## ##
Re: [PATCH] Update docs w.r.t. warning and strictness options.
* Stefano Lattarini wrote on Sat, Jan 15, 2011 at 12:41:16PM CET: And here finally is the promised documentation patch, which should conclude the patch series (this time for good, I hope!). Thanks, and sorry for the delay, No problem. Let's see if we can get through it quickly. I think you should not let the remainder of the patch series have to wait for this, though. Subject: [PATCH] Update docs w.r.t. warning and strictness options. * doc/automake.texi (Strictness): Document that some warnings are turned off by default in `foreign' strictness. (Options): Divide into new sections Options generalities and s/: /: / List of Automake options. Fix typo (colon instead of full s/ / / stop). Document option precedence (AUTOMAKE_OPTIONS wins over AM_INIT_AUTOMAKE which wins over command line). Also document interactions between options specifying strictness and those specifying warnings. --- a/doc/automake.texi +++ b/doc/automake.texi @@ -320,6 +320,11 @@ Support for test suites * DejaGnu Tests:: Interfacing with the external testing framework * Install Tests:: Running tests on installed packages +Changing Automake's Behavior + +* Options generalities::Automake options, strictness, and their precedence info output should not exceed 80 characters (probably not 79, haven't checked), this line is too long. How about this instead? * Options generalities::Semantics of Automake options Please also rerun emacs ^C ^U ^V before committing, thanks. +* List of Automake options::A comprehensive list of Automake options + Miscellaneous Rules * Tags::Interfacing to cscope, etags and mkid @@ -1899,7 +1904,9 @@ The valid strictness levels are: Automake will check for only those things that are absolutely required for proper operations. For instance, whereas GNU standards dictate the existence of a @file{NEWS} file, it will not be required in -this mode. The name comes from the fact that Automake is intended to be +this mode. This strictness will also turn off some warnings by default +(among them, the @samp{portability} warnings). s/the @samp{\(.*\)}/\1/ +The name comes from the fact that Automake is intended to be used for GNU programs; these relaxed rules are not the standard mode of operation. @@ -9023,8 +9030,16 @@ will now be rerun each time the version number is bumped, when only @node Options @chapter Changing Automake's Behavior +@menu +* Options generalities::Automake options, strictness, and their precedence +* List of Automake options::A comprehensive list of Automake options +@end menu + +@node Options generalities +@section Automake options, strictness, and their precedence Karl recommends section names to be identical to node names in general. How about just naming this Option generalities? Various features of Automake can be controlled by options. Except where -noted otherwise, options can be specified in one of several ways: Most +noted otherwise, options can be specified in one of several ways. Most options can be applied on a per-@file{Makefile} basis when listed in a special @file{Makefile} variable named @code{AUTOMAKE_OPTIONS}. Some of these options only make sense when specified in the toplevel @@ -9034,7 +9049,44 @@ of these options only make sense when specified in the toplevel require changes to the @command{configure} script can only be specified there. These are annotated below. -Currently understood options are: +As a general rule, options specified in @code{AUTOMAKE_OPTIONS} take +precedence over those specified in @code{AM_INIT_AUTOMAKE} OK until here. , which in +turn take precedence over those specified on the command line@footnote{ +We're painfully aware that this last precedence sounds wrong and is +against all the established conventions, but it's due to historical +reasons, and presently cannot be easily changed. It might be fixed +in a future Automake version though, so try not to rely on it.}. No. We already agreed to fixing this, so we should not document the broken behavior. We should fix it instead. Can we just omit this for the moment? Maybe with a @comment FIXME: document interaction with command-line args when fixed. if you prefer that. +Also, some care must be taken about the interactions among strictness +level and warning categories. As a general rule, strictness-implied +warnings are overridden by those specified by explicit options. For +example, even if @samp{portability} warnings are disabled by default +in @option{foreign} strictness, an usage like this: s/an usage like// (but see below) +@example +AUTOMAKE_OPTIONS = -Wportability foreign +@end example +@noindent +will end up being enabling those portability warnings. s/being // Empty lines before @example and after @end example, and before @noindent (the last two should be
Re: Fix remake rule-induced test failures with HP-UX make.
[ adding autoconf-patches ] * Ralf Wildenhues wrote on Sat, Jan 15, 2011 at 11:31:55AM CET: Quoting parts of: http://autobuild.josefsson.org/automake/log-201101150749395803000.txt I'm not quite sure why HP-UX make seems to trigger rebuild rules for prerequisites that have the exact time stamps as targets (in this case, config.status and subdir Makefile, but I also tried with a reduced makefile), but it is not Posix conforming. Working around it properly would probably require sleeping inside config.status (an obvious no-go), and in practice it should not do more harm than updating some rules more than necessary. Found it documented in HP-UX/IA 11.31 make(1): If no target names are specified on the command line, make updates the first target in the (first) makefile that is not an inference rule. A target is updated in two cases: firstly, if it depends on files that are newer than the target; secondly, if it depends on files that have same modification time as that of the target. Missing files are deemed to be out-of-date. [...] Anybody have a support contract to report this Posix violation upstream? OK to commit? Thanks, Ralf docs: HP-UX make remakes targets with same time stamps as prereqs * doc/autoconf.texi (Timestamps and Make): Document HP-UX 11.31 make issue with targets having the same time stamps as their prerequisites. Signed-off-by: Ralf Wildenhues ralf.wildenh...@gmx.de diff --git a/doc/autoconf.texi b/doc/autoconf.texi index 9f5494e..81664e4 100644 --- a/doc/autoconf.texi +++ b/doc/autoconf.texi @@ -20183,7 +20183,9 @@ Timestamps and Make date dest-stamp @end example - +Apart from timestamp resolution, there are also differences in handling +equal timestamps. HP-UX @command{make} updates targets if it has the +same time stamp as one of its prerequisites, in violation of Posix rules. @c Portable C and C++ Programming
Re: Fix remake rule-induced test failures with HP-UX make.
* Stefano Lattarini wrote on Sat, Jan 15, 2011 at 01:51:54PM CET: On Saturday 15 January 2011, Ralf Wildenhues wrote: Quoting parts of: http://autobuild.josefsson.org/automake/log-201101150749395803000.txt [...] How about the patch below to not let this cause spurious testsuite failures? Seems (almost) good. Just a couple of minor nits below ... Oh, and please remember ot apply the patch to the maint-based temporary branch 'tests-colon-extend'. FYI, last commit there should be `a46eda3752054eccd208f630611fefe0308eafc1'. I amended tests-{colon,remake}-extend with the respective fixes, ... If you prefer, I can split the typo fixes in a separate patch. Well, I usually prefer having cosmetic/typofixing changes separated from bugfixes -- but not to the point of requiring you to rewrite this patch ;-) So, do whatever you deem more appropriate. ... and split out the typo patch. @@ -60,27 +60,31 @@ for vpath in : false; do cd demo - # version.good should depend from version.gin $sleep + # Avoid relying on non-existent toplevel am--refresh rule, + # presumably due to spurious timestamp issues. + touch Makefile The comment seems too terse to me; what about this (stolen from your explanation above)? # Some make implementations (e.g., HP-UX make) seems to trigger rebuild # rules for prerequisites that have the exact time stamps as targets (in # this case, config.status and subdir Makefile). # Since our setup here lacks a top-level Makefile, and thus the toplevel # am--refresh rule, the rebuild rules are doomed to fail. # We thus touch the Makefile to avoid triggering them. Thanks. While again I prefer documenting such issues in autoconf.texi, some more verbosity might be prudent here; I went with this: + # HP-UX make considers targets with exact time stamps as one of their + # prerequisites out of date. Ensure Makefile is newer than config.status to + # avoid triggering the am--refresh rule in the (here-nonexistent) toplevel + # Makefile. + touch Makefile --- a/tests/remake1a.test +++ b/tests/remake1a.test @@ -17,6 +17,7 @@ # Make sure remaking rules in subdir are correctly generated. # See also sister grepping test `remake.test'. +required=GNUmake . ./defs || Exit 1 fingerprint='=/FiNgErPrInT/=' [ OK, I'm going to be really nitpicking here ... ] If you require GNU make in this test, IMHO you can as well change the line: $MAKE Makefile below with simply: $MAKE On the other hand, the test is perfectly correct also as it stands, so you decide if the suggested change is worth the additional noise. I prefer the longer line, as it is clearer what the desired action is, and also it wouldn't remake unneeded other stuff 'all' might depend on. Thanks for the quick review, pushed to master, Ralf
Re: [PATCH] Update docs w.r.t. warning and strictness options.
* Stefano Lattarini wrote on Sat, Jan 15, 2011 at 02:23:56PM CET: On Saturday 15 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Sat, Jan 15, 2011 at 12:41:16PM CET: And here finally is the promised documentation patch, which should conclude the patch series (this time for good, I hope!). Thanks, and sorry for the delay, No problem. Let's see if we can get through it quickly. I think you should not let the remainder of the patch series have to wait for this, though. OK, if we don't get through this patch today, I'll push the previous patches to a public branch (after having fixed the dates in the ChangeLog entries). BTW, is the name 'warns-win-over-strictness' acceptable for that branch? Sure. I have an intended use case where it might not be ideal that warnings win over other options, by the way. I hope to be able to finish the gnu-make writeup tomorrow (where it happens). --- a/doc/automake.texi +++ b/doc/automake.texi * Options generalities::Semantics of Automake options OK. Please also rerun emacs ^C ^U ^V before committing, thanks. You mean ^C ^U ^A, right? Yes, sorry. @@ -9023,8 +9030,16 @@ will now be rerun each time the version number is bumped, when only +@node Options generalities +@section Automake options, strictness, and their precedence Karl recommends section names to be identical to node names in general. How about just naming this Option generalities? I chose the name above only for consistency with the menu entries. If you tell me this consistency is not needed, I heartily agree with your proposed renaming, which makes things clearer IMHO. OK. , which in +turn take precedence over those specified on the command line@footnote{ +We're painfully aware that this last precedence sounds wrong and is +against all the established conventions, but it's due to historical +reasons, and presently cannot be easily changed. It might be fixed +in a future Automake version though, so try not to rely on it.}. No. We already agreed to fixing this, so we should not document the broken behavior. We should fix it instead. Wait, IMVHO this fix cannot just be in the next automake release without a clear deprecation of the older behaviour first. The backward-incompatibility would be too great and sharp otherwise. The right thing to do (again IMVHO) is implement the fix in a proper master-based branch, and merge it back into master only after automake 1.12 has been released. WDYT? Hmm. I would prefer to delay this decision until we have to cross that bridge; i.e.: - before we release 1.11.2, we should think about deprecation again, - when we have a patch to change precedence, we can try to evaluate how disruptive it is, and then decide whether it can go in 1.12 or 1.13. But anyway I don't want behavior that we want to change be documented and thus set in stone in the manual now, if it previously hasn't been documented. So, can we please decouple these things from the patch series we are discussing here? Can we just omit this for the moment? Maybe with a @comment FIXME: document interaction with command-line args when fixed. if you prefer that. Before discussing about this any further, I'll wait for a reply to my argumentation above. Empty lines before @example and after @end example, and before @noindent (the last two should be collapsed to just one). Oh, sorry. I just copied and pasted from some pre-existing code, which lacks such spacing. Indeed. I'll fix them in maint presently. +@node List of Automake options +@section A comprehensive list of Automake options This @section should be renamed as List of Automake options, right? Yep. Thanks! Ralf
docs: ensure example are separated with empty lines in the input (was: [PATCH] Update docs w.r.t. warning and strictness options.)
* Ralf Wildenhues wrote on Sat, Jan 15, 2011 at 02:49:33PM CET: * Stefano Lattarini wrote on Sat, Jan 15, 2011 at 02:23:56PM CET: On Saturday 15 January 2011, Ralf Wildenhues wrote: Empty lines before @example and after @end example, and before @noindent (the last two should be collapsed to just one). Oh, sorry. I just copied and pasted from some pre-existing code, which lacks such spacing. Indeed. I'll fix them in maint presently. Pushing as follows. Cheers, Ralf docs: ensure example are separated with empty lines in the input * doc/automake.texi (Extending aclocal, Emacs Lisp, Rebuilding) (API Versioning, Renamed Objects, Multiple Outputs): Add empty lines before `@example' and after `@end example' lines, so info output is rendered correctly, and a following @noindent honored. Report by Stefano Lattarini. diff --git a/doc/automake.texi b/doc/automake.texi index 80a5ce2..73c0e51 100644 --- a/doc/automake.texi +++ b/doc/automake.texi @@ -3456,6 +3456,7 @@ Extending aclocal current implementation, however it requires a stricter style from the macro authors. Hopefully it is easy to revise the existing macros. For instance, + @example # bad style AC_PREREQ(2.57) @@ -3465,8 +3466,10 @@ Extending aclocal AX_BAR ]) @end example + @noindent should be rewritten as + @example AC_DEFUN([AX_FOOBAR], [AC_PREREQ([2.57])dnl @@ -7455,18 +7458,20 @@ Emacs Lisp There are two ways to avoid byte-compiling. Historically, we have recommended the following construct. + @example lisp_LISP = file1.el file2.el ELCFILES = @end example + @noindent @code{ELCFILES} is an internal Automake variable that normally lists all @file{.elc} files that must be byte-compiled. Automake defines @code{ELCFILES} automatically from @code{lisp_LISP}. Emptying this variable explicitly prevents byte-compilation. -Since Automake 1.8, we now recommend using @code{lisp_DATA} instead. As -in +Since Automake 1.8, we now recommend using @code{lisp_DATA} instead: + @example lisp_DATA = file1.el file2.el @end example @@ -8925,9 +8930,11 @@ Rebuilding from @file{configure.ac}. For instance, the following statement will cause @file{configure} to be rerun each time @file{version.sh} is changed. + @example AC_SUBST([CONFIG_STATUS_DEPENDENCIES], ['$(top_srcdir)/version.sh']) @end example + @noindent Note the @samp{$(top_srcdir)/} in the file name. Since this variable is to be used in all @file{Makefile}s, its value must be sensible at @@ -8953,12 +8960,14 @@ Rebuilding today. They are mainly used when the version of a package is updated automatically by a script (e.g., in daily builds). Here is what some old-style @file{configure.ac}s may look like: + @example AC_INIT . $srcdir/version.sh AM_INIT_AUTOMAKE([name], $VERSION_NUMBER) @dots{} @end example + @noindent Here, @file{version.sh} is a shell fragment that sets @code{VERSION_NUMBER}. The problem with this example is that @@ -8969,12 +8978,14 @@ Rebuilding straightforward, because shell variables are not allowed in @code{AC_INIT}'s arguments. We recommend that @file{version.sh} be replaced by an M4 file that is included by @file{configure.ac}: + @example m4_include([version.m4]) AC_INIT([name], VERSION_NUMBER) AM_INIT_AUTOMAKE @dots{} @end example + @noindent Here @file{version.m4} could contain something like @samp{m4_define([VERSION_NUMBER], [1.2])}. The advantage of this @@ -10300,12 +10311,14 @@ API Versioning @example AM_INIT_AUTOMAKE([1.6.1])dnl Require Automake 1.6.1 or better. @end example + @noindent or, in a particular @file{Makefile.am}: @example AUTOMAKE_OPTIONS = 1.6.1 # Require Automake 1.6.1 or better. @end example + @noindent Automake will print an error message if its version is older than the requested version. @@ -11192,6 +11205,7 @@ Renamed Objects false_SOURCES = generic.c false_CPPFLAGS = -DEXIT_CODE=1 @end example + @noindent Obviously the two programs are built from the same source, but it would be bad if they shared the same object, because @file{generic.o} @@ -11361,6 +11375,7 @@ Multiple Outputs data.h: data.foo data.c foo data.foo @end example + @noindent therefore a parallel @command{make} will have to serialize the builds of @file{data.c} and @file{data.h}, and will detect that the second is
Re: [PATCH 2/5] Tests defs: new subroutine `skip' for test skipping.
[ Jim, I have a question below ] * Stefano Lattarini wrote on Sat, Jan 15, 2011 at 03:16:51PM CET: On Saturday 15 January 2011, Ralf Wildenhues wrote: * Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:23:21PM CET: Tests defs: new subroutine `skip' for test skipping. I don't like having this patch alone. If we switch to functions, then we should so wholesale, and at once, and fully. OK, but the main point of this patch was *not* to introduce more functions in the testing framework, only to ensure that SKIP'd tests offer a more informative message about the reasons of the SKIP [...] Granted, but why stop with SKIP and disallow more informative FAILs? At least we can define the respective functions and use them in new tests. I introduced the skip function only to reduce code duplication, since using: skip this test shouldn't be run on a win32-like system is clearer and less error-prone than having to resort to: echo this test shouldn't be run on a win32-like system 2 Exit 77 Sure. But let's reap other benefits as well while we're at it. Also, there is precedent in {coreutils,gnulib}/tests/init.sh which has more features than this implementation and I think we should follow (and/or improve upon if that is not sufficient): [...] Generally, when we change something, it is a good idea to avoid NIH. (That doesn't mean we have to change to remove existing NIH, but avoid new. The point being that stuff that is new is untested and broken.) Sorry, but I don't fully understand what you're requesting here. Apologies for being a bit dense. You're stating that we should start using init.sh in Automake? If yes, I (mostly) agree (while the init.sh is not yet fully adequate for our purposes, it shouldn't be diffucult to make it more flexible; and the advantage given by code reuse would be obvious). The main problem here is that I lack a copyright assignment for Gnulib, so I can't help in making init.sh flexible enough for our needs :-( On the other hand, if you're stating that we should try to be as init.sh-compatible as possible (in view of a possible future passage to init.sh), I agree even more, and I could amend the patch to use `skip_' instead of `skip'. Or again, are you stating that our `skip' subroutine should be implemented through (basically) a copy paste from init.sh? Well, I'm not sure if using gnulib's init.sh outright is a good goal right away. We've had so much testsuite churn now, and many tests fail at the moment, that I'm tempted to call for a couple of weeks of no testsuite changes except fixing FAILures. Back to my original point though: even if we don't use init.sh right away, it is a good idea to make new code of ours similar to gnulib's, with the goal of being more mergeable at some point in the future. And while we do that, we can already fold back bugfixes to gnulib where appropriate. As a first step, we can copy code from gnulib. Jim, gnulib/tests/init.sh is GPLv3+ right now. We would like to use code added/modified in commit v0.0-3988-g0ae8d63. Do you mind if we copy that into Automake's testsuite setup, currently GPLv2+? AFAICS you're the sole author of that and prior changes around these lines. Oh, Stefano, can you please start the assignment process for gnulib? If that is not possible, then I'll have to do these changes sometime. This would be fine with me too (but might require a couple of preliminary patches, such as reaming `$me' to `$ME_' in the testsuite, etc.). Certainly not; a ME_=$me avoids that really useless churn; alternatively a s/ME_/me/g for the parts taken from init.sh. Doing one of these is prudent for any merging of pending branches anyway. A global s/me/ME_/g can be done later when we actually turn to use init.sh (and all branches using $me have long been dead). That said, I found minor issues in the proposed patch which I'm listing so you take measures to avoid them when redoing. This kind of patch should really be written with a script, not manually. Ah, but how can a script devise the correct, non-preexisting messages for the various possible SKIPs? Well, skip_ should cope with not being passed any message: it shouldn't output anything in that case. That's a change we could submit to gnulib. (If gnulib wants to require a message, that can be done with maintainer checks.) All in all, this calls for a three-step conversion (can be done in one commit, but avoids the sort of bug I found in the prior patch submission): - s/Exit 77/skip_/g - check/fix any occurrences in here-documents manually: //,/^E[NO][DF]/{ /skip_/p; } - edit some or all skip_ occurrences to add a short explanation (less than (79 - strlen (: skipped test: ) - length of longest test name) characters). Cheers, Ralf
Re: Avoid VPATH-related distcheck failures with non-GNU make
Hello Stefano, * Stefano Lattarini wrote on Sat, Jan 15, 2011 at 04:50:31PM CET: On Saturday 15 January 2011, Ralf Wildenhues wrote: ensure_distcheck_ () { if $MAKE --version -v | grep GNU; then $MAKE distcheck else : fi } Hmm... To me, this sounds an awful lot like sweeping the dirt under the rug. OK, so here's a more open alternative: let's try to fix the failures. Are you going to work on this, or at least provide significant help? The current amount of failures from this hunting for regressions in Automake proper harder than it should be, because there are many failures which could be testsuite regressions, or regressions for features we don't advertise, or long-standing issues for which it thus is not imperative that we fix them quickly. I reject the notion of disallowing VPATH builds with GNU make only. The reason is simple: I make good use of this feature every time I test the Automake package with some non-GNU make (I build several platforms off of one tree shared via NFS). It would make my testing significantly harder if GNU make needed to be added to the mix. Just diagnosing non-GNU make at configure time in a VPATH setup is slightly problematic because many people still do ../configure gmake (note the missing MAKE=gmake on the configure command line). Or, as I also happen to do, go back to a build tree several months old and do 'git pull cd build make' in it, forgetting that MAKE=gmake was set at configure time (portably warning about this at make time is surprisingly hard to do right, when you can't assume GNU make). What about a more radical but IMHO more honest approach, to be implemented in four steps: [1] Clearly document in the manual that VPATH builds are expected to work with GNU make only. See above. For lots of packages, portable make works just fine even in VPATH mode. I'm fine with mentioning that GNU make or non-VPATH should be tried if there are problems with another make in a VPATH build. [2] Update AM_INIT_AUTOMAKE to make it complain(*) if it detects that $MAKE is not GNU make but a VPATH build is being attempted. The user should be allowed to override this check, obviously. See above. (*) With a warning or an error? Not sure yet, but it should be warning in the next Automake version, not to suddenly disrupt backward compatibility. Generally, I want to keep even git master of Automake always in a close-to-releasable state. Even if all current failures are only issues in the testsuite, or long-standing bugs, this is too much to just ignore it. Cheers, Ralf
Re: tests: avoid new failures due to nonexistent makeinfo.
* Stefano Lattarini wrote on Sun, Jan 16, 2011 at 12:05:33PM CET: I have serious doubts about this patch ... On Sunday 16 January 2011, Ralf Wildenhues wrote: http://hydra.nixos.org/build/858836/log/raw and other logs show new testsuite failures of txinfo.test and txinfo8.test: I'm pushing the fix below to tests-texinfo and merging to maint. tests: avoid new failures due to nonexistent makeinfo. * tests/txinfo.test, tests/txinfo8.test: Create stub info files so 'missing --run makeinfo' won't fail even if makeinfo is not present. Wait, wait, how can that happen? Currently, due to a patch I've applied Thursday (based on a report of yours, and committed as v1.11-264-g21fec67), 'txinfo.tex' *requires* makeinfo, while 'txinfo8.tex' already creates textutils.info (and ensures it's newer than textutils.texi, so that make won't try to rebuild it with missing --run makeinfo or similar). Ouch. I actually had a vague memory of something like this, but then I couldn't find the patch that fixed this, and saw the log from yesterday http://hydra.nixos.org/build/858836/ I somehow figured the bug wasn't fixed after all. The underlying reason is that we haven't merged maint to branch-1.11 and master since. Apologies. I am going to revert my patch and do the merges. Thanks for spotting this, Ralf
Fix parallel-tests.test failure with HP-UX make
Now that we know HP-UX make updates files with the same time stamp as their prerequisites, it is obvious how to fix the long-standing parallel-tests.test failure. Pushing to maint as below. Cheers, Ralf Fix parallel-tests.test failure with HP-UX make. * tests/parallel-tests.test: Sleep inside inner tests, so logs are newer than logs of tests they depend on, for HP-UX make. diff --git a/tests/parallel-tests.test b/tests/parallel-tests.test index 29111c2..399b23b 100755 --- a/tests/parallel-tests.test +++ b/tests/parallel-tests.test @@ -41,14 +41,19 @@ foo.log: bar.log bar.log: baz.log END +# foo.test and bar.test sleep to ensure their logs are always strictly newer +# than the logs of their prerequisites, for HP-UX make. The quoting pleases +# maintainer-check. cat foo.test 'END' #! /bin/sh echo this is $0 +sleep '1' exit 0 END cat bar.test 'END' #! /bin/sh echo this is $0 +sleep '1' exit 99 END cat baz.test 'END'
tests: avoid failing command substitution in errexit mode.
Quoting http://autobuild.josefsson.org/automake/log-20110115074814053.txt | FAIL: vartypos.test (exit: 1) | = | | /tmp/am/build-alphaev67-dec-osf5.1/tests:/tmp/local/alphaev67-dec-osf5.1/bin:/tmp/bin:/bin:/usr/bin:/sbin:/usr/sbin:/usr/opt/svr4/bin:/etc:/usr/etc:/usr/dt/bin:/usr/bin/X11:/usr/ucb | + set -e | + pwd | /tmp/am/build-alphaev67-dec-osf5.1/tests/vartypos.dir | + cat | + cat | + aclocal-1.11a -Werror | + AUTOMAKE_fails | + AUTOMAKE_run 1 | expected_exitcode=1 | + shift | exitcode=0 | + automake-1.11a --foreign -Werror -Wall | exitcode=1 | + cat stderr | automake: warnings are treated as errors | Makefile.am:2: warning: variable `nodist_foo_SOURCES' is defined but no program or | Makefile.am:2: library has `foo' as canonical name (possible typo) | Makefile.am:1: warning: variable `foo_SOURCES' is defined but no program or [...] | Makefile.am:13: warning: variable `libfoo_a_DEPENDENCIES' is defined but no program or | Makefile.am:13: library has `libfoo_a' as canonical name (possible typo) | + cat stdout | + test 1 = 1 | + grep -v .libfoo_a. | + grep as canonical stderr | + grep -v .foo. | | FAIL: vartypo2.test (exit: 1) | = | | /tmp/am/build-alphaev67-dec-osf5.1/tests:/tmp/local/alphaev67-dec-osf5.1/bin:/tmp/bin:/bin:/usr/bin:/sbin:/usr/sbin:/usr/opt/svr4/bin:/etc:/usr/etc:/usr/dt/bin:/usr/bin/X11:/usr/ucb | vartypo2: running libtoolize --version | libtoolize (GNU libtool 1.3075 2009-02-01) 2.2.7a | Written by Gary V. Vaughan g...@gnu.org, 2003 | | Copyright (C) 2009 Free Software Foundation, Inc. | This is free software; see the source for copying conditions. There is NO | warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | + set -e | + pwd | /tmp/am/build-alphaev67-dec-osf5.1/tests/vartypo2.dir | + cat | + cat | + libtoolize | libtoolize: putting auxiliary files in `.'. | libtoolize: linking file `./ltmain.sh' | libtoolize: You should add the contents of the following files to `aclocal.m4': [...] | + aclocal-1.11a -Werror -Wno-syntax -I /tmp/am/build-alphaev67-dec-osf5.1/../automake/m4 -I /tmp/local/alphaev67-dec-osf5.1/share/aclocal | + AUTOMAKE_fails --add-missing | + AUTOMAKE_run 1 --add-missing | expected_exitcode=1 | + shift | exitcode=0 | + automake-1.11a --foreign -Werror -Wall --add-missing | exitcode=1 | + cat stderr | configure.in:4: installing `./config.guess' | configure.in:4: installing `./config.sub' | automake: warnings are treated as errors | Makefile.am:3: warning: variable `EXTRA_libfoo_la_SOURCES' is defined but no program or | Makefile.am:3: library has `libfoo_la' as canonical name (possible typo) [...] | Makefile.am:5: library has `libfoo_la' as canonical name (possible typo) | + cat stdout | + test 1 = 1 | + grep -v .libfoo_la. | + grep as canonical stderr Ah, Tru64 and IRIX sh seem to exit upon a failed command substution when 'set -e' is enabled. I'm not even sure whether that's conforming to some version of Posix or not. This issue is similar to this from autoconf.texi: When `set -e' is in effect, a failed command substitution in Solaris `/bin/sh' cannot be ignored, even with `||'. $ /bin/sh -c 'set -e; foo=`false` || echo foo; echo bar' $ bash -c 'set -e; foo=`false` || echo foo; echo bar' foo bar but apparently not the same, as these two tests pass in the Solaris logs. Fortunately, the issue is easily avoided here. I'll be pushing the patch below to master shortly. Cheers, Ralf tests: avoid failing command substitution in errexit mode. * tests/vartypo2.test, tests/vartypos.test: Rewrite to not use a command substitution with a nonzero exit status, that causes IRIX and Tru64/OSF sh to fail the whole test. diff --git a/tests/vartypo2.test b/tests/vartypo2.test index a212c2b..f811bbc 100755 --- a/tests/vartypo2.test +++ b/tests/vartypo2.test @@ -53,7 +53,7 @@ AUTOMAKE_fails --add-missing # Makefile.am:5: library has `libfoo_la' as canonical name (possible typo) -test -z `grep 'as canonical' stderr | grep -v ' .libfoo_la. '` +grep 'as canonical' stderr | grep -v ' .libfoo_la. ' Exit 1 test `grep 'variable.*is defined but' stderr | wc -l` = 6 # If we add a global -Wnone, all warnings should disappear. diff --git a/tests/vartypos.test b/tests/vartypos.test index fe4f8e6..03747ed 100755 --- a/tests/vartypos.test +++ b/tests/vartypos.test @@ -71,7 +71,8 @@ AUTOMAKE_fails # Makefile.am:13: warning: variable `libfoo_a_DEPENDENCIES' is defined but no program or # Makefile.am:13: library has `libfoo_a' as canonical name (possible typo) -test -z `grep 'as canonical' stderr | grep -v ' .foo. ' | grep -v ' .libfoo_a. '` +grep 'as canonical' stderr | grep -v ' .foo. ' | grep -v ' .libfoo_a. ' \ + Exit 1 test `grep 'variable.*is defined but' stderr | wc -l` = 13 # If we add a global -Wnone, all warnings should disappear.
Re: tests-init branch
* Stefano Lattarini wrote on Sun, Jan 16, 2011 at 04:11:04PM CET: I've just merged the tests-init branch into master, and pushed. Thanks. At this point, that branch has IMHO pretty much exhausted its usefulness, and could be removed. Future non-obvious testsuite work could be done in more short lived, purpose oriented branches. Okay with me, however you like. Cheers, Ralf