[asdf-devel] The issue at hand

[Faré]

On Fri, Jan 24, 2014 at 12:21 AM, Daniel Herring <dherring at tentpost.com> wrote:
>> For those cases, I have pushed a change that checks to make sure that
>> all operations that are instantiated are subclasses of either
>> DOWNWARD-OPERATION, UPWARD-OPERATION, SIDEWAY-OPERATION,
>> SELFWARD-OPERATION, or a new NON-PROPAGATING-OPERATION. If this run-time
>> test fails, an error will be signaled and the programmer will have to
>> fix the offending OPERATION subclass's definition.
>
> Backwards compatibility is a really important goal for low-level
> infrastructure.  It provides the stability needed for others to trust
> building on a framework.  While the proposed changes do affect a minority of
> the userbase, they do "punish" past efforts to use the advanced API
> features...
>
> Would it be possible for your test to inject the old default behavior into
> such classes?
>
The issue at stake is that the old ASDF2 behavior was a bug.
A deep, essential bug, that required massive refactoring and cleanup.

The downward and sideway propagation of operations was baked
into ASDF1's TRAVERSE algorithm, which went essentially unchanged in ASDF2,
though its implementation was gradually broken down
from one giant spaghetti function to plenty of small ones,
allowing rpgoldman and I to make sense of it and fix many bugs.

However, this propagation was a bug.
Not only is this propagation an expensive nuisance for many operations,
necessitating various workarounds (like perform methods that do nothing),
some essential operations need to propagate UPWARD, not downward:
PREPARE-OP, PREPARE-SOURCE-OP, etc.
Because ASDF1 couldn't express this propagation,
these operations were missing,
and the required dependencies were expressed implicitly
through the order of the TRAVERSE algorithm, which didn't quite work
in incremental builds across systems, though this bug was largely hidden
by the even more glaring bug that ASDF, after earlier DEFSYSTEMs,
completely failed to propagate timestamps from one action to the next.

ASDF3 thus involved two complete rewrites of TRAVERSE,
which had already been completely revamped since ASDF1
(though faithfully preserving its semantics).
Now, to fix this design bug in ASDF2, from ASDF1,
from MK-DEFSYSTEM, from DEFSYSTEM,
it was just not possible to be 100% compatible.
Being 100% compatible meant keeping the bug. Defeats the purpose.
Whoever wrote extensions that relied on the bug would suffer,
so that everyone else could be rid of the bug.
That's not the first time this happens,
though this time, it's the breakage will be subtle,
and hard to debug for who isn't asking me for help.
So I fixed everything in Quicklisp and at work.
But I can't fix what I don't have access to.

Now comes Robert, who has been badly bitten,
for the breakage was subtle enough that it took him a lot of time
before he figured out I was the culprit. Oops. My bad.
Then he wonders how to warn other people who like him
haven't yet upgraded to ASDF3, and have proprietary code not on Quicklisp
that includes ASDF extensions.

While I sympathize, I conjecture that the set is empty,
yet that his fix will cause breakage of its own.
I'm actually surprised that Anton didn't find more breakage
when Robert introduced an error at instantiation time
for operation classes that don't inherit from
NON-PROPAGATING-OPERATION or at least one of the propagating classes.
I strongly suspect that no one in quicklisp uses these ASDF extensions,
and so that the patch does break software,
just not the building of this software.
But I'm a bit too lazy to try right now.

Robert or Anton: could you grep the archives for my previous audit,
and try *using* some of the operations defined in Quicklisp?
This definitely needs be done before the release.

> I'm not thinking of any clean CLOS or MOP tricks, but a direct
> implementation might not be too hard.
>
> For example, add an API-VERSION field to the OPERATION class.  By default,
> it will be uninitialized, signifying :asdf2 behavior.  New subclasses could
> explicitly set it to :asdf3.  The mixins or a variant of the subclass test
> might also set it to :asdf3.  Then the dependency traversal code could
> maintain a check for this field along with the new mixins.
>
> If we are unable to find a transparent solution, then a secondary goal is to
> find a solution that allows new libraries the option to support both the old
> and new ASDF semantics.  Since ASDF can upgrade in-place, I believe this
> rules out reader conditionals.  Does ASDF have a hook API that notifies
> libraries of the possible need to re-initialize their ASDF extensions when
> it reloads?
>
Backward compatibility is key.
If we don't care about backward compatibility,
then there's no problem anyway.
And if we do, then your solution has the issue that
since existing client code doesn't use this versioning technique,
we have to either assume it all working (in which case there is
nothing to do and Robert can revert his patches),
or all non-working (in which case you break all the code that does work,
which makes the patch a regression rather than an advancement.

Regards,

—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
Don't worry about results. You can't wish them into existence. Focus on the
process. You have decision power on it. And it will take care of results.