Re: 25-clean-targets.patch

[Tom Tromey]

>>>>> "Akim" == Akim Demaille <address@hidden> writes:

Akim> so naturally the question is: do we really want to keep this
Akim> zillions of pseudo targets?  The current automake, indeed, does
Akim> this instead:

Akim> ----------
Akim> clean-am: clean-this-am
Akim> clean-this-am:
Akim>   do this

Akim> clean-am: clean-that-am
Akim> clean-that-am:
Akim>   do that
Akim> ----------

Akim> which will result in

Akim> ----------
Akim> clean-am: clean-this-am clean-that-am

Akim> clean-this-am:
Akim>   do this

Akim> clean-that-am:
Akim>   do that
Akim> ----------


This transformation seems safe enough to me.  I don't have a problem
with it.

What you don't want to do is inline the actual rules.  This is
tempting (and it is what I thought you were talking about at first).
However, it breaks parallel builds and also `make -k'.

Akim> -    my $category;
Akim> -    foreach $category (sort keys %dependencies)
Akim> +    foreach (sort target_cmp keys %dependencies)

I've been moving away from using $_ in any but the most trivial loops.
I thought $_ wasn't necessarily preserved across function calls.

Akim> +        &pretty_print_rule ("$_:", "\t",
Akim> +                     sort @{$dependencies{$_}});
Akim> + $output_rules .= $actions{$_};

e.g. if &pretty_print_rule trashes $_ then you're in trouble.

In other places, like &flatten, using $_ is ok with me.

Akim> +           unless (defined $contents{$targets})
Akim> +             {

Please use `if (! ...)' here.  I find `unless' confusing when used
like this.


Otherwise this patch is ok.


Now though I'm worried.  I had been hoping to release 1.5 someday.
But these patches are fairly large, and, while they are good cleanups,
they actually have the effect of destabilizing parts of automake
which, while ugly, were not big contributors to bugginess.

This would be easier to quantify if I had an actual list of "must fix"
problems for 1.5, which I don't :-(.

Tom