On 21 Feb 2018, at 10:55, Faré wrote:
On Tue, Feb 20, 2018 at 4:44 PM, Robert Goldman rpgoldman@sift.info wrote:
I still see these "dependency not done" warnings in test-bundle on SBCL and other implementations (I haven't checked them all, but at least my big three: SBCL, Allegro, and CCL).
I can see them too.
This seems like a bug somewhere, right? And it must be an ASDF bug, probably in the definition of LOAD-OP for bundles.
I believe it's all due to this method:
(defmethod mark-operation-done :after ((o load-bundle-op) (c system)) (mark-operation-done (find-operation o 'load-op) c)))
Thanks for explaining that. That said, can you explain why we do this, instead of marking `load-bundle-op` as done? I guess because loading the bundle is intended to be equivalent to loading the system, so that if we load a bundle for a system, S, we want other systems that depend on S to know that it has been loaded.
Related to this, the first parameter to `FIND-OPERATION` is described as a "context," but there's nowhere an explanation of what a context is meant to be and in what way another operation can be a context. `find-operation` is actually confusing me a bit, because it seems to have quite different behavior for strings and symbols:
``` (defmethod find-operation ((context t) (spec operation)) spec) (defmethod find-operation ((context t) (spec symbol)) (when spec ;; NIL designates itself, i.e. absence of operation (make-operation spec))) ;; TODO: preserve the (operation-canonical-initargs context) (defmethod find-operation ((context t) (spec string)) (make-operation spec))) ;; TODO: preserve the (operation-canonical-initargs context) ```
I read this as "find an operation by making it. Ignore the context argument." So I'm not entirely sure why it exists (presumably because it's idempotent, and `MAKE-OPERATION` is not), or why it has the `context` argument. I guess that you foresaw the need for a context at some point, but never ended up using it?
Said method should probably somehow hush these specific warnings, or first recurse into all dependencies and mark them done, or be removed. You decide.
As I said above, I don't yet understand why that method exists, so I don't know what would be the implications of removing it.
Yes, we could make that dependency warning into a defined condition and muffle it in this context. I just need to understand a little better the implications of doing that.
While it's indeed a cosmetic bug to issue the warning, I believe the underlying logic is sound.
So, even after the last email, I'm inclined to hold up a release until I understand this bug and kill it.
—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org Toleration is not about respecting other people's ideas. We have every right to fight ideas we think are stupid. Toleration is about respecting other people's persons. We have every duty to respect even persons we think are stupid. — Faré