Hi Stefano, * Stefano Lattarini wrote on Sun, Feb 06, 2011 at 06:32:56PM CET: > Hello Ralf, and sorry for the delay.
No worries. I'm waay more behind. > On Thursday 03 February 2011, Ralf Wildenhues wrote: > > * Stefano Lattarini wrote on Wed, Feb 02, 2011 at 12:10:15AM CET: > > > A testsuite-enhancement patch stemmed from my brief foray into > > > Automake's python support. This patch is in small part cosmetic, > > > but IMHO offers real improvements and valuable additions, and > > > also fixes a couple of glitches in python.m4. > > > > OK for master (branched off of maint, if you prefer) with nits > > addressed. > > > > Before pushing, please test on a system without a python interpreter > > installed (you can rename you pythons temporarily). > > > Nah, IMHO is better to do something like: > > $ cd ~/src/automake/tests > $ mkdir xbin && cd xbin > $ for f in /bin/* /usr/bin/* /usr/local/bin/*; do > > case $f in *python*);; *) ln -s $f .;; esac > > done > $ cd .. > $ PATH=`pwd`/xbin make check TESTS='...' Neat; but also expensive. > BTW, following your advice, I have uncovered a bug in python-dist.test. > Fixed in the attached squash-in. Cool. > > > Subject: [PATCH] python: extend and improve tests, fix minor glitches > > > > > > * tests/python-virtualenv.test: New test, checking that python > > > support offered by automake works well with virtualenvs. > > > > What is virtualenvs? > > > "virtual python environments" created by the 'virtualenv' program. I > fixed the ChangeLog entry to be more precise (see attached squash-in). Thanks. > > And a no-contents version in instdir.test, analogously to the rest. > > > Hmm... I see that this test lacks entries for the PROGRAMS, LIBRARIES > and LTLIBRARIES primaries. Is this deliberate? I think so; to avoid requirements. > Should that be fixed in a follow-up patch? Well, the overlap of instdir.test with the instdir-*.test is pretty large anyway, so it's not too important to have this full. > > > --- a/tests/python2.test > > > +++ b/tests/python2.test > > > > > @@ -20,6 +20,8 @@ > > > > > > set -e > > > > > > +$ACLOCAL > > > + > > > > > > echo 1. pythondir not defined > > > > > > @@ -28,8 +30,8 @@ PYTHON = x > > > python_PYTHON = foo.py > > > END > > > > > > -$ACLOCAL > > > AUTOMAKE_fails -a > > > +grep 'pythondir.*undefined' stderr > > > > These changes will make the test less resilient against changes to > > python.m4. I like the increased coverage but the increased brittleness > > of the test is somewhat of a downer. Hmm. > > > Would you maybe like a more lax regexp, e.g.: > $EGREP 'pythondir.*(un|not )defined' stderr > or even: > $EGREP 'pythondir.*(un|not )defined|define.*pythondir' stderr > > But IMHO this would be overkill, as changing the test in case the error > messages are modifed should be a no-brainer. For the moment, I've not > touched this hunk (nor the similar ones below). OK. It's a close call anyway, and not a big problem either way. > > > --- a/tests/python5.test > > > +++ b/tests/python5.test > > > > > @@ -24,16 +24,32 @@ set -e > > > > > > cat >>configure.in <<EOF > > > # Hopefully the Python team will never release such a version. > > > -AM_PATH_PYTHON(9999.9) > > > +AM_PATH_PYTHON([9999.9]) > > > > Nice that you do it here, but up in python.m4 you should then, too. > > > Definitely. But in a follow-up patch IMHO (and since we are at it, we > should fix underquoting in all the other *.m4 automake files). I will > submit this patch in the coming week. OK. > > > --- a/tests/python6.test > > > +++ b/tests/python6.test > > > +# Simulate no Python. > > > +./configure PYTHON=: > > > +test x"`cat py`" = x":" > > > > A colon doesn't need quoting. > > > True, but it doesn't hurt either, and IMVHO improves consistency. > Bikeshedding anyway, True, but makes reading slightly harder IMVHO. > so I'll simply remove it. Thanks. > > The cat py output doesn't either, > > so this can just be > > test x`cat py` = x: > > > Hmmm... but why make the test less reliable in face of possible "weird" > failures (in case, let's say, PYTHON gets erroneously redfined to ': ', > note the trailing space)? > > For the moment, I've kept the quotes around `cat py`. OK with me. > Attached is what I've squashed into the previous version of the patch. > > I will push in 72 hours if there are no further objections. OK but I have a question below. > --- a/tests/instdir-ltlib.test > +++ b/tests/instdir-ltlib.test > @@ -14,7 +14,8 @@ > # You should have received a copy of the GNU General Public License > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > -# If $(libdir) is the empty string, then nothing should be installed there. > +# If $(libdir) or $(pyexecdir) is the empty string, then nothing should > +# be installed there. > # This test exercises the libtool code paths. > > required=libtoolize > @@ -26,6 +27,7 @@ cat >>configure.in <<'END' > AC_PROG_CC > AM_PROG_CC_C_O > AC_PROG_LIBTOOL > +AM_PATH_PYTHON > AC_OUTPUT > END > > --- a/tests/instdir-prog.test > +++ b/tests/instdir-prog.test > @@ -14,7 +14,8 @@ > # You should have received a copy of the GNU General Public License > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > -# If $(bindir) is the empty string, then nothing should be installed there. > +# If $(bindir), $(libdir) or $(pyexecdir) is the empty string, then > +# nothing should be installed there. > # This test exercises the prog and libs code paths. > > . ./defs || Exit 1 > @@ -25,6 +26,7 @@ cat >>configure.in <<'END' > AC_PROG_CC > AM_PROG_CC_C_O > AC_PROG_RANLIB > +AM_PATH_PYTHON > AC_OUTPUT > END > > @@ -36,6 +38,8 @@ bin_PROGRAMS = p > nobase_bin_PROGRAMS = np sub/np > lib_LIBRARIES = libfoo.a > nobase_lib_LIBRARIES = libnfoo.a sub/libnfoo.a > +pyexec_PROGRAMS = py > +nobase_pyexec_PROGRAMS = npy sub/npy > END > > cat >p.c <<'END' Do these two tests require python now? In that case maybe the merging wasn't such a good idea after all; several of the machines I test on do not have python in the default PATH. Thanks, Ralf