Re: [PATCH] Extend checks on remake rules.
On Thursday 16 December 2010, Ralf Wildenhues wrote:
* 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.
Thanks.
+ 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 think the same way too, but since you wrote ok with nits addressed
in the beginning of your mail, I wasn't sure whether that should not
were pointing out a nit (hence to be addressed).
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. ;-)
Thast shouldn't be really necessary if you've looked at my inline
comments.
Thanks!
Ralf
Thanks for the approval. I've now merged the patch into master,
and pushed.
Regards,
Stefano
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
Re: [PATCH] Extend checks on remake rules.
Ping on this? Original patch submission (reproposing an older patch): http://lists.gnu.org/archive/html/automake-patches/2010-12/msg00015.html Last previous message: http://lists.gnu.org/archive/html/automake-patches/2010-12/msg00018.html I will push in 72 hours (wednesday evening) if there are no further objections. Regards, Stefano
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] Extend checks on remake rules.
Also, I'd like to squash in the attached patches. Regards, Stefano From 3ba9c134769dfb8cd7f55576ad08fd74d290b5f1 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Tue, 24 Aug 2010 11:14:34 +0200 Subject: [PATCH 2/4] SquashMe1 --- ChangeLog |1 + tests/Makefile.am |1 + tests/Makefile.in |1 + tests/remake12.test | 121 +++ 4 files changed, 124 insertions(+), 0 deletions(-) create mode 100755 tests/remake12.test diff --git a/ChangeLog b/ChangeLog index 17c4d9d..f14380b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -33,6 +33,7 @@ * tests/remake10b.test: Likewise. * tests/remake10c.test: Likewise. * tests/remake11.test: Likewise. + * tests/remake12.test: Likewise. * tests/Makefile.am (TESTS): Updated. 2010-08-21 Ralf Wildenhues ralf.wildenh...@gmx.de diff --git a/tests/Makefile.am b/tests/Makefile.am index 54234c2..63e6869 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -615,6 +615,7 @@ remake10a.test \ remake10b.test \ remake10c.test \ remake11.test \ +remake12.test \ regex.test \ req.test \ reqd.test \ diff --git a/tests/Makefile.in b/tests/Makefile.in index 73188be..9a27638 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -853,6 +853,7 @@ remake10a.test \ remake10b.test \ remake10c.test \ remake11.test \ +remake12.test \ regex.test \ req.test \ reqd.test \ diff --git a/tests/remake12.test b/tests/remake12.test new file mode 100755 index 000..6614f50 --- /dev/null +++ b/tests/remake12.test @@ -0,0 +1,121 @@ +#! /bin/sh +# Copyright (C) 2010 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2, or (at your option) +# any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. + +# Test basic remake rules for Makefiles with non-default names +# and/or with multiple sources. + +required=GNUmake +. ./defs || Exit 1 + +set -e + +hash1=7532aa4708e8320c8c7098a5e3a1ec88aafb074f +hash2=99368830db9954b4d9fecd04d3996d9acea7129f +hash3=36fe4f389c0a835dfcdb0f58a8909eab43189af1 + +cat configure.in END +AC_INIT([$me], [1.0]) +AM_INIT_AUTOMAKE +AC_CONFIG_FILES([zardoz]) +AC_CONFIG_LINKS([Makefile:Makefile]) +AC_OUTPUT +END + +cat zardoz.am END +EXTRA_DIST = Makefile +#H: $hash1 +END + +cat Makefile END +include zardoz +nil: +.PHONY: nil +END + +$ACLOCAL +$AUTOCONF +$AUTOMAKE + +./configure + +$MAKE nil +grep '^#H:' zardoz.in # for debugging +$FGREP $hash1 zardoz +$FGREP $hash1 zardoz.in +$MAKE distcheck +$MAKE distclean # this shouldn't remove Makefile +ls -l +test -f Makefile + +./configure + +$sleep +cat Makefile END +my-check: + ls -l . \$(srcdir) # for debugging + test -f \$(srcdir)/quux.am + test -f \$(srcdir)/quux.in + test -f \$(srcdir)/bot.in + test -f \$(srcdir)/top.in + test ! -r \$(srcdir)/zardoz.am + test ! -r \$(srcdir)/zardoz.in + grep 'FOO' zardoz # for debugging + test x'\$(FOO)' = x'$hash3' +test: + ls -l # for debugging + test x'\$(FOO)' = x'dummy' +.PHONY: test my-check +END +sed s/^#H:.*/#H: $hash2/ zardoz.am t +cat t 'END' +check-local: my-check ## used by make distcheck below +END +mv -f t zardoz.am +cat zardoz.am # for debugging +$MAKE nil +$FGREP my-check zardoz # sanity check +$FGREP $hash1 zardoz zardoz.in Exit 1 +$FGREP $hash2 zardoz +$FGREP $hash2 zardoz.in + +./configure + +$sleep +sed 's/^\(AC_CONFIG_FILES\)(.*/\1([zardoz:top.in:quux.in:bot.in])/' \ + configure.in t +mv -f t configure.in +cat configure.in # for debugging +sed '/^#H:/d' zardoz.am quux.am +echo 'FOO = dummy' quux.am +echo 'BAR = $(BAZ)' top.in +echo BAZ = $hash3 bot.in +$MAKE test +$FGREP my-check zardoz # sanity check +$FGREP $hash3 quux.in Exit 1 +$FGREP $hash3 zardoz +$FGREP $hash1 zardoz Exit 1 +$FGREP $hash2 zardoz Exit 1 +# After the remake above, the files `zardoz.am' and `zardoz.in' +# should be no more needed +echo 'endif' zardoz.am # put in syntax error +$MAKE test +rm -f zardoz.in zardoz.am # get rid of them +$MAKE test + +echo 'FOO = $(BAR)' quux.am +$MAKE distcheck + +: -- 1.7.1 From 3be3b78774460139c52cf965eccad6258fca8f84 Mon Sep 17 00:00:00 2001 From: Stefano Lattarini stefano.lattar...@gmail.com Date: Tue, 24 Aug 2010 11:32:43 +0200 Subject: [PATCH 3/4] SquashMe2 --- ChangeLog |1 + tests/Makefile.am |1 + tests/Makefile.in |1 + tests/remake9a.test |2 +- tests/remake9b.test |2 +- tests/remake9c.test |7 ++- tests/remake9d.test | 109
