[asdf-install-devel] Patches - unit test improvements, clarifying refactorings

[Dan Muller]

Dan Muller wrote:
> Gary King wrote:
>
>>I've pulled all of the patches from before "Tue Dec 25 15:26:37
>>Eastern Standard Time" except for this one:
>>
>>> Dan Muller<pikdj2002 at sneakemail.com>**20071225224604] {
>>> hunk ./asdf-install/defpackage.lisp 6
>>> -  #+asdf
>>> -  (:import-from #:asdf #:*defined-systems*)
>>> hunk ./asdf-install/installer.lisp 437
>>> -         #+asdf
>>> -         (*defined-systems* (if propagate $
>>> -                              (make-hash-table :test 'equal)
>>> -                              *defined-systems*))
>>> }
>>
>>Unless I'm confused, this is necessary for the propagate command to
>>work (hmmm, I wonder if there is a test for that?).
>
> You may be right. I checked that *DEFINED-SYSTEMS* wasn't referenced
> elsewhere in ASDF-INSTALL, but that was silly, because of course that's
> not the point of binding it, is it? It's for its side-effect on ASDF.
> But is this really necessary, given the :before method that's defined
> for asdf::find-system elsewhere? (I think that's the name, I don't
> have the source handy here at work.)
>
> If I did get this wrong, it might have affected my other patches, e.g.
> the choice of results that I test for in the unit test patch.
>

I looked at the ASDF internals again, and I believe that defining the
:AROUND (not :BEFORE) method for ASDF:FIND-COMPONENT (not FIND-SYSTEM)
is adequate, and the binding of ASDF::*DEFINED-SYSTEMS* is
redundant. Unless I'm misunderstanding the purpose of PROPAGATE.

IMO, the binding of *DEFINED-SYSTEMS* is more elegant, but the fact
that it's an internal symbol, whereas FIND-COMPONENT is exported,
makes the latter the preferred technique.

A unit test for PROPAGATE would certainly be welcome. :)

-- 
Dan Muller