On Wed, Jun 13, 2001 at 11:44:52AM +0300, Kalle Olavi Niemitalo wrote:
> This bug is also causing the @TRUE@ conditions reported by
> Richard Boulton in "Bug with conditionals. [PATCH]".
You're right, this does seem to be the main cause of the problem I
reported.
I tried fixing the problem (by passing by reference instead).
Unfortunately, this doesn't solve the problem; rather, it reveals that
variable_conditions_reduce() is very bug ridden indeed.
There seem to be two further problems:
i) variable_conditions_reduce() currently checks conditions for redundancy
against only those conditions which it has already checked, rather than
against all the conditions it hasn't yet discarded. Thus, for example:
"FOO", "FOO BAR" will not reduce
(since first "FOO" is checked against the empty conditional and then
"FOO BAR" is checked against "FOO"), but
"FOO BAR", "FOO" will reduce to "FOO BAR"
(since "FOO BAR" is checked against the empty conditional and then
"FOO" is checked against "FOO BAR").
ii) The check for tautology of a condition performed by
variable_conditions_reduce() checks whether all conditions in @CONDS are
true for every condition in @WHENS. What is wanted is a check whether
each condition in @CONDS is true for any of the conditions in @WHENS.
The check is also the wrong way round: it keeps a condition if it is
tautologous, rather than discarding it.
I've put together another test case for this problem, cond12.test, which
calls variable_conditions_reduce() to check the exact results for a set of
inputs. I've also put together a patch to fix the problem, which causes
cond11.test and cond12.test to pass (and all other tests to pass), and
removes a FIXME from the code.
The patch involves a straightforward fix for the bug reported by Kalle
Olavi Niemitalo and for problem (i) in this email.
(ii) is fixed by redefining the behaviour of conditionals_true_when() such
that it returns TRUE if all the @CONDS are true for a condition in @WHENS.
The behaviour when WHENS is empty is also altered to be more logical (this
is where the FIXME is fixed).
This should not affect any other code since conditionals_true_when() is
only called with an array of size != 1 in @WHENS by
variable_conditions_reduce().
The test suite all passes for me, including the two new tests, after this
patch is applied. This patch supercedes the patch I submitted earlier.
Hoping for a reply this time. ;-)
--
Richard
Index: ChangeLog
===================================================================
RCS file: /cvs/automake/automake/ChangeLog,v
retrieving revision 1.1419
diff -u -r1.1419 ChangeLog
--- ChangeLog 2001/06/12 14:37:44 1.1419
+++ ChangeLog 2001/06/14 19:01:32
@@ -1,3 +1,21 @@
+2001-06-06 Richard Boulton <[EMAIL PROTECTED]>
+
+ * (conditionals_true_when): Pass parameters by reference, avoiding
+ bug which put all parameters in @CONDS instead of @WHENS. Report
+ by Kalle Olavi Niemitalo.
+ Return true if for each entry A in @CONDS there is an entry B in @WHENS
+ such that A is true when B. Previously returned true if for each
+ A in @CONDS, A is true for all B in @WHENS.
+ Remove FIXME; now treats an empty @WHENS as an empty conditional.
+ * (variable_conditions_reduce): Check whether each condition is
+ implied by any of the other conditions (other those already
+ discarded), rather than checking only against those already
+ considered (and kept). Also, fix sense of check: was keeping
+ tautologous terms instead of discarding them.
+ * tests/Makefile.am (TESTS): Added cond11.test and cond12.test.
+ * tests/cond11.test: New file.
+ * tests/cond12.test: New file.
+
2001-06-12 Tom Tromey <[EMAIL PROTECTED]>
* automake.texi (ANSI): Minor clarification.
Index: automake.in
===================================================================
RCS file: /cvs/automake/automake/automake.in,v
retrieving revision 1.1137
diff -u -r1.1137 automake.in
--- automake.in 2001/06/09 00:34:28 1.1137
+++ automake.in 2001/06/14 19:01:46
@@ -5315,25 +5315,34 @@
# $BOOLEAN
-# &conditionals_true_when (@CONDS, @WHENS)
+# &conditionals_true_when (\@CONDS, \@WHENS)
# ----------------------------------------
-# Same as above, but true if all the @CONDS are true when *ALL*
-# the @WHENS are sufficient.
+# Same as above, but true if all the @CONDS are true for at least one of the
+# conditional strings in @WHENS.
#
-# If there are no @CONDS, then return true, of course. *BUT*, even if there
-# are @CONDS but @WHENS is empty, return true. This is counter intuitive,
-# and against all the rules of logic, but is needed by the current code.
-# FIXME: Do something saner when the logic of conditionals is understood.
-sub conditionals_true_when (@@)
+# If there are no @CONDS, then return true.
+# If there are no @WHENS, then behave as if @WHENS contained a single empty
+# condition.
+sub conditionals_true_when ($$)
{
- my (@conds, @whens) = @_;
+ my ($condsref, $whensref) = @_;
- foreach my $cond (@conds)
+ foreach my $cond (@$condsref)
{
- foreach my $when (@whens)
- {
+ if (@$whensref == 0) {
+ return 0
+ unless conditional_true_when($cond, "");
+ } else {
+ my $allfalse = 1;
+ foreach my $when (@$whensref)
+ {
+ if (conditional_true_when ($cond, $when)) {
+ $allfalse = 0;
+ last;
+ }
+ }
return 0
- unless conditional_true_when ($cond, $when);
+ if $allfalse;
}
}
@@ -5845,7 +5854,7 @@
# If this condition cannot be true when the parent conditions
# are true, then skip it.
next
- if ! conditionals_true_when ((@parent_conds), ($vcond));
+ if ! conditionals_true_when ([@parent_conds], [$vcond]);
push (@this_conds, $vcond);
@@ -5918,7 +5927,7 @@
next if ! $ok;
next
- if ! conditionals_true_when ((@parent_conds), ($perm));
+ if ! conditionals_true_when ([@parent_conds], [$perm]);
# This permutation was not already handled, and is valid
# for the parents.
@@ -5933,23 +5942,28 @@
# Filter a list of conditionals so that only the exclusive ones are
# retained. For example, if both `COND1_TRUE COND2_TRUE' and
# `COND1_TRUE' are in the list, discard the latter.
+# If the list is empty, return TRUE
sub variable_conditions_reduce
{
my (@conds) = @_;
my @ret = ();
- foreach my $cond (@conds)
+ my $cond;
+ while(@conds > 0)
{
+ $cond = shift(@conds);
+
# FALSE is absorbent.
if ($cond eq 'FALSE')
{
return ('FALSE');
}
- elsif (conditionals_true_when (($cond), (@ret)))
+ elsif (!conditionals_true_when ([$cond], [@ret, @conds]))
{
push (@ret, $cond);
}
}
+ return "TRUE" if @ret == 0;
return @ret;
}
Index: tests/Makefile.am
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.am,v
retrieving revision 1.307
diff -u -r1.307 Makefile.am
--- Makefile.am 2001/06/09 00:34:29 1.307
+++ Makefile.am 2001/06/14 19:01:47
@@ -60,6 +60,8 @@
cond8.test \
cond9.test \
cond10.test \
+cond11.test \
+cond12.test \
condincl.test \
condincl2.test \
condlib.test \
Index: tests/Makefile.in
===================================================================
RCS file: /cvs/automake/automake/tests/Makefile.in,v
retrieving revision 1.406
diff -u -r1.406 Makefile.in
--- Makefile.in 2001/06/09 00:34:30 1.406
+++ Makefile.in 2001/06/14 19:01:48
@@ -126,6 +126,8 @@
cond8.test \
cond9.test \
cond10.test \
+cond11.test \
+cond12.test \
condincl.test \
condincl2.test \
condlib.test \
Index: tests/cond11.test
===================================================================
RCS file: cond11.test
diff -N cond11.test
--- /dev/null Tue May 5 13:32:27 1998
+++ cond11.test Thu Jun 14 12:01:48 2001
@@ -0,0 +1,41 @@
+#! /bin/sh
+
+# Test for bug in conditionals. From Richard Boulton.
+# This checks that, if LDADD is set from a conditional variable
+# and an AC_SUBST, the _DEPENDENCIES variable is set correctly.
+
+. $srcdir/defs || exit 1
+
+cat > configure.in << 'END'
+AC_INIT(Makefile.am)
+AM_INIT_AUTOMAKE(foo,0.0)
+AC_PROG_CC
+AM_CONDITIONAL(USE_A,[test x = x])
+AC_OUTPUT(Makefile)
+AC_SUBST(SUBSTVAR)
+END
+
+cat > Makefile.am << 'END'
+
+if USE_A
+foolibs=faz.la
+else
+foolibs=
+endif
+
+noinst_PROGRAMS = foo
+foo_SOURCES = foo.c
+LDADD = $(SUBSTVAR) $(foolibs)
+END
+
+: > config.guess
+: > config.sub
+: > compile
+
+$ACLOCAL || exit 1
+$AUTOMAKE || exit 1
+
+#Should be two dependency setting lines
+count=`grep 'foo_DEPENDENCIES =' Makefile.in | wc -l|sed 's/ //g'`
+test "x$count" == "x2" &&
+ grep '^.USE_A_TRUE.foo_DEPENDENCIES =' Makefile.in
Index: tests/cond12.test
===================================================================
RCS file: cond12.test
diff -N cond12.test
--- /dev/null Tue May 5 13:32:27 1998
+++ cond12.test Thu Jun 14 12:01:48 2001
@@ -0,0 +1,80 @@
+#! /bin/sh
+
+# Test behaviour of variable_conditions_reduce()
+# This checks the result of variable_conditions_reduce() for a wide variety
+# of cases.
+
+. $srcdir/defs || exit 1
+
+head -n 1 $srcdir/../automake >>automake_tmp
+cat << 'END' >> automake_tmp
+my $failed = 0;
+sub check_reduce($$) {
+ my ($inref, $outref) = @_;
+ my @result = sort &Automake::variable_conditions_reduce(@$inref);
+ my $correct = 1;
+ $correct = 0 if (join(",", @result) ne join(",", @$outref));
+
+ if (! $correct) {
+ print '"'.join(",", @$inref) . '" => "' .
+ join(",", @result) . '" expected "' .
+ join(",", @$outref) . '"' . "\n";
+ $failed = 1;
+ }
+}
+
+# If no conditions are given, TRUE should be returned
+check_reduce([""], ["TRUE"]);
+# A single condition should be passed through unchanged
+check_reduce(["FOO"], ["FOO"]);
+check_reduce(["FALSE"], ["FALSE"]);
+check_reduce(["TRUE"], ["TRUE"]);
+
+# TRUE and false should be discarded and overwhelm the result, respectively
+check_reduce(["FOO", "TRUE"], ["FOO"]);
+check_reduce(["FOO", "FALSE"], ["FALSE"]);
+
+# Repetitions should be removed
+check_reduce(["FOO", "FOO"], ["FOO"]);
+check_reduce(["TRUE", "FOO", "FOO"], ["FOO"]);
+check_reduce(["FOO", "TRUE", "FOO"], ["FOO"]);
+check_reduce(["FOO", "FOO", "TRUE"], ["FOO"]);
+
+# Two different conditions should be preserved, but TRUEs should be removed
+check_reduce(["FOO", "BAR"], ["BAR,FOO"]);
+check_reduce(["TRUE", "FOO", "BAR"], ["BAR,FOO"]);
+check_reduce(["FOO", "TRUE", "BAR"], ["BAR,FOO"]);
+check_reduce(["FOO", "BAR", "TRUE"], ["BAR,FOO"]);
+
+# A condition implied by another condition should be removed.
+check_reduce(["FOO BAR", "BAR"], ["FOO BAR"]);
+check_reduce(["BAR", "FOO BAR"], ["FOO BAR"]);
+check_reduce(["TRUE", "FOO BAR", "BAR"], ["FOO BAR"]);
+check_reduce(["FOO BAR", "TRUE", "BAR"], ["FOO BAR"]);
+check_reduce(["FOO BAR", "BAR", "TRUE"], ["FOO BAR"]);
+
+check_reduce(["BAR FOO", "BAR"], ["BAR FOO"]);
+check_reduce(["BAR", "BAR FOO"], ["BAR FOO"]);
+check_reduce(["TRUE", "BAR FOO", "BAR"], ["BAR FOO"]);
+check_reduce(["BAR FOO", "TRUE", "BAR"], ["BAR FOO"]);
+check_reduce(["BAR FOO", "BAR", "TRUE"], ["BAR FOO"]);
+
+# Check that reduction happens even when there are two conditionals to remove.
+check_reduce(["FOO", "FOO BAR", "BAR"], ["FOO BAR"]);
+check_reduce(["FOO", "FOO BAR", "BAZ", "FOO BAZ"], ["FOO BAR", "FOO BAZ"]);
+check_reduce(["FOO", "FOO BAR", "BAZ", "FOO BAZ", "FOO BAZ BAR"], ["FOO BAZ BAR"]);
+
+# Duplicated condionals should be removed
+check_reduce(["FOO", "BAR", "BAR"], ["BAR,FOO"]);
+
+# Equivalent conditionals in different forms should be reduced: which one is
+# left is unfortunately order dependent.
+check_reduce(["BAR FOO", "FOO BAR"], ["FOO BAR"]);
+check_reduce(["FOO BAR", "BAR FOO"], ["BAR FOO"]);
+
+exit $failed;
+END
+cat $srcdir/../automake >>automake_tmp
+chmod +x automake_tmp
+
+./automake_tmp
