Re: test-bundle
On 21 Feb 2018, at 10:55, Faré wrote: On Tue, Feb 20, 2018 at 4:44 PM, Robert Goldman <rpgold...@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• 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é
Re: test-bundle
> (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. > It's in addition, not instead of, and considering how asdf 3 works as opposed to 1 & 2, it's probably not effective unless you recurse for all dependencies. > 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 once made sense, back when I was trying to support the crazy and actually buggy ASDF 1 behavior (notably used by asdf-ecl) of carrying extra build information in operation objects. This was definitely deprecated last year in ASDF 3.2 or 3.3, although 3.3 also opened the way to doing it in a non-buggy way in the future if you want. I don't recommend that: it will be super complex to get right for little short-term benefit; but you may have to it or something just as hard, anyway, if you want to properly support cross-compilation, for instance. What you do with find-operation depends on your long-term plans. You could get rid of the context and use make-operation everywhere instead — that's what the current implementation does, anyway (it used to try to preserve the operation initargs, even though component-operation-times used to drop them). > 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? > The context used to be (from ASDF 1, before refactoring into find-operation) another operation from which you extracted the initargs. In the future, it might be reborn, after some kind of canonicalization, for e.g. optimization settings, cross-compilation, etc.. Or that context could be moved to a different object — but that would mean a new API incompatible with the current one for ASDF extensions. Cross-compilation will require something somewhat backward-incompatible, anyway. > 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. > I believe that before ASDF 3, this method was intended hat you could indeed load a bundle and the system would therefore assume that the system is loaded and not recurse into loading the individual files of the bundle. This is definitely not working anymore (if it ever did) — you'd have to recursively mark all objects as loaded with a suitable timestamp. Also, considering that ECL abandoned the idea of a *load-system-operation* (which that was supporting), the point of it is moot. I recommend either recursing or removing the method over hushing the warning. Note that, in the context of upgrade, removing the method requires conditionally defining an empty method (or using the MOP to remove it) in a suitable when-upgrading form. > 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. > My current recommendation would be to just remove the method: wrap it in when-upgrading with proper version spec, and empty out its body. > So, even after the last email, I'm inclined to hold up a release until I > understand this bug and kill it. > Good luck. A lot of ASDF is historically-motivated cruft. Just like CL itself. —♯ƒ • François-René ÐVB Rideau •Reflection• http://fare.tunes.org In a centralized society social status is a zero-sum game along a single hierarchy. That's why politics can never help the weak, only swap roles.
Re: test-bundle
On Tue, Feb 20, 2018 at 4:44 PM, Robert Goldman <rpgold...@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))) Said method should probably somehow hush these specific warnings, or first recurse into all dependencies and mark them done, or be removed. You decide. 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• 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é
test-bundle
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). ``` Running test-bundle.script with sbcl-1.4.3-macosx-x64 ; loading #P"/Users/rpg/lisp/asdf/test/test-bundle.script" ; Registering system test-asdf ;; loading #P"/Users/rpg/lisp/asdf/test/test-asdf.asd" TEST-BUNDLE ASDF-TEST::*BUNDLE-1* => #P"/Users/rpg/lisp/asdf/build/fasls/sbcl-1.4.3-macosx-x64/asdf/test/test-asdf/bundle-1--system.fasl" ASDF-TEST::*BUNDLE-2* => #P"/Users/rpg/lisp/asdf/build/fasls/sbcl-1.4.3-macosx-x64/asdf/test/test-asdf/bundle-2--system.fasl" ;; loading #P"/Users/rpg/lisp/asdf/build/fasls/sbcl-1.4.3-macosx-x64/asdf/test/test-asdf/bundle-1--system.fasl" WARNING: Computing just-done stamp for action (LOAD-OP "test-asdf/bundle-1"), but dependency (LOAD-OP "test-asdf/bundle-1" "file1") wasn't done yet! WARNING: Computing just-done stamp for action (LOAD-OP "test-asdf/bundle-1"), but dependency (LOAD-OP "test-asdf/bundle-1" "file3") wasn't done yet! WARNING: Computing just-done stamp for action (LOAD-OP "test-asdf/bundle-1"), but dependency (COMPILE-OP "test-asdf/bundle-1") wasn't done yet! ;; loading #P"/Users/rpg/lisp/asdf/build/fasls/sbcl-1.4.3-macosx-x64/asdf/test/file1.fasl" ;; loading #P"/Users/rpg/lisp/asdf/build/fasls/sbcl-1.4.3-macosx-x64/asdf/test/file3.fasl" ;; loading #P"/Users/rpg/lisp/asdf/build/fasls/sbcl-1.4.3-macosx-x64/asdf/test/test-asdf/bundle-2--system.fasl" WARNING: Computing just-done stamp for action (LOAD-OP "test-asdf/bundle-2"), but dependency (LOAD-OP "test-asdf/bundle-2" "file2") wasn't done yet! WARNING: Computing just-done stamp for action (LOAD-OP "test-asdf/bundle-2"), but dependency (COMPILE-OP "test-asdf/bundle-2") wasn't done yet! ``` This seems like a bug somewhere, right? And it must be an ASDF bug, probably in the definition of LOAD-OP for bundles. So, even after the last email, I'm inclined to hold up a release until I understand this bug and kill it.
