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 08/10] Add more tests about AUTOMAKE_OPTIONS.
On Sunday 02 January 2011, Ralf Wildenhues wrote:
* 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?
Fine with me. Done.
+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.
Do you have any clarifying comment to suggest? I'd be happy to add it.
--- /dev/null
+++ b/tests/amopts-location.test
@@ -0,0 +1,80 @@
[CUT]
+
+$ACLOCAL
+AUTOMAKE_fails
What are the expected failures here?
Aren't the next greps explicit enough? If not, would this comment help
in clarifying the situation?
# Automake options 'tar-v7', 'tar-ustar' and 'tar-pax' can only be used
# as argument to AM_INIT_AUTOMAKE, and not in AUTOMAKE_OPTIONS.
In the meantime, I've added it just before the call to AUTOMAKE_fails;
let me know if it's not clear enough.
(Not sure if they should be mentioned in the test source, but
I sure am curious about what automake prints.)
Here it is:
Makefile3.am:2: error: option `tar-v7' can only be used as argument to
AM_INIT_AUTOMAKE
Makefile3.am:2: but not in AUTOMAKE_OPTIONS makefile statements
Makefile2.am:6: error: option `tar-ustar' can only be used as argument to
AM_INIT_AUTOMAKE
Makefile2.am:6: but not in AUTOMAKE_OPTIONS makefile statements
Makefile1.am:1: error: option `tar-pax' can only be used as argument to
AM_INIT_AUTOMAKE
Makefile1.am:1: but not in AUTOMAKE_OPTIONS makefile statements
Makefile.am:3: `Makefile0.am' included from here
Makefile0.am:4: `Makefile1.am' included from here
Just to be sure, this patch does not rely upon any of the previous ones,
right?
Hmm... I didn't pay attention to this fact before, but yes, I believe
the patch should not depend on earlier ones.
Anyway, I bacame aware the necessity of the new tests only when I got to
modify the automake code dealing with AUTOMAKE_OPTIONS; and that's why
I didn't add them in an earlier patch.
+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?
In truth, it's to ensure that there are no warnings referring to botched
line numbers. Technically, the grep above might be a little too strict
in this respect, but since our Makefile.ams are so simple, that shouldn't
be a problem.
JTBS, I've added a couple of new comments about this too.
How about just testing for 5 lines of output?
They are in fact eight, which shows ...
Even that is a bit fragile though, as we might have multi-line warnings.
... that you're right here. I'd personally leave the above grepping
alone. Objections?
Thanks,
Stefano
From 229aa74326b835dda56889ebbbe916485d2a0e1b Mon Sep 17 00:00:00 2001
From: Stefano Lattarini stefano.lattar...@gmail.com
Date: Sun, 2 Jan 2011 18:07:20 +0100
Subject: [PATCH] Squashed in
---
ChangeLog |5 +++--
tests/Makefile.am |2 +-
tests/Makefile.in |2 +-
tests/amopts-location.test |7 ++-
...ndirect.test = amopts-variable-expansion.test} |4 ++--
5 files changed, 13 insertions(+), 7 deletions(-)
rename tests/{amopts-indirect.test = amopts-variable-expansion.test} (92%)
diff --git a/ChangeLog b/ChangeLog
index f3b411a..af01192 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,10 +1,11 @@
-2010-12-20 Stefano Lattarini stefano.lattar...@gmail.com
+2011-01-02 Stefano Lattarini stefano.lattar...@gmail.com
+ For PR automake/547:
Add more tests about AUTOMAKE_OPTIONS.
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
on AUTOMAKE_OPTIONS support, to prevent obvious regressions.
- * tests/amopts-indirect.test: New test.
+ *
Re: [PATCH 08/10] Add more tests about AUTOMAKE_OPTIONS.
On Sunday 02 January 2011, Ralf Wildenhues wrote:
* Stefano Lattarini wrote on Sun, Jan 02, 2011 at 06:45:07PM CET:
On Sunday 02 January 2011, Ralf Wildenhues wrote:
* stefano.lattar...@gmail.com wrote on Thu, Dec 23, 2010 at 12:27:44PM
CET:
+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.
Do you have any clarifying comment to suggest? I'd be happy to add it.
How about this?
# The following should expand to `-Wnone -Wno-error foreign -Wportability'.
Yes, I like it. Comment added.
Hey, we could even grep for that in the verbose output somewhere ...
+$ACLOCAL
+AUTOMAKE_fails
What are the expected failures here?
Aren't the next greps explicit enough? If not, would this comment help
in clarifying the situation?
# Automake options 'tar-v7', 'tar-ustar' and 'tar-pax' can only be used
# as argument to AM_INIT_AUTOMAKE, and not in AUTOMAKE_OPTIONS.
The comment definitely helps, and should be enough IMHO. Thank you.
Good.
[CUT]
Just to be sure, this patch does not rely upon any of the previous ones,
right?
Hmm... I didn't pay attention to this fact before, but yes, I believe
the patch should not depend on earlier ones.
Anyway, I bacame aware the necessity of the new tests only when I got to
modify the automake code dealing with AUTOMAKE_OPTIONS; and that's why
I didn't add them in an earlier patch.
Well, if you like, you can just push this patch to master independently;
or leave it here if you prefer.
I'd rather wait than having to juggle even more with git am and
git rebase, if that's ok with you (silence gets interpreted as
a positive answer here ;-).
[BIG CUT]
All better, no objections!
Thanks!
Ralf
Regards,
Stefano
