swank-loader: load-swank: if asdf is available then use it

[dkochmanski]

This is to prevent redefinition warnings when a system is compiled that has swank in dependencies.

[stassats]

I'm not sure I want to use asdf even if it's available.

[dkochmanski]

This addresses the issue where loading a system "swank" with asdf (i.e as another system dependency), loads the same swank on top of already existing one. That lead to the recent issue with the package invariant and now signals numerous "redefinition" warnings.

I can't think of another idea short of nuking swank.asd and telling people not to depend on swank, or leaving these redefinition warnings as is.

[stassats]

I understand that, but using ASDF will introduce more problems than it will solve.

[dkochmanski]

I think that unless there is a tangible regression, we should merge it. That said this issue does not impact me much so I'm not going to press further. If there are change request to this pull request I'll of course comply.

[stassats]

It should be inverted, swank.asd shouldn't load swank if it's already loaded.

[melisgl]

For the record and the time being, I have :(depends-on (:feature (:not :swank) "swank")) in my ASDF system, but that doesn't solve the issue for other packages depending on Swank.

[melisgl]

With the proposed change, ASDF may load different version of Swank, and that may break things for people who have an old Swank in Quicklisp, but want to use a newer Slime.

[melisgl]

A workaround could be to rename the swank asdf system to swank-impl and add this:

(asdf:defsystem "swank"
  :depends-on ((:feature (:not :swank) "swank-impl")))

Admittedly, I have not thought about the interaction with dumping binaries an such.

[dkochmanski]

Assuming that the system "x" depends on the system "swank", it stands to reason that when "x" is build the system "swank" from the same registry should be taken.

On the other hand, it also does make sense that if the programmer configures their system to use a particular swank with slime, then that's what they want to use.

The worst case scenario is that both "swank" versions are incompatible. "x" depends on "swank-1" and slime depends on "swank-2". Another scenario is that "swank-1" and "swank-2" are one and the same, but asdf operations (like compilation) still require swank to be processed by asdf (the recent commit that caused redefinitions was motivated by the fact that loaded swank made the system swank ignored, so the resulting binary did not have swank the library bundled).

Having that in mind, perhaps a good solution would be to have swank loaded and started in a different package when it is loaded by slime than the swank that is loaded as a system into a running image. For sake of backward compatibility the "visible" system would be loaded in the package swank and the system used by slime would be loaded in the package slime-swank.

That said I don't feel very motivated with proposing such solution in a form of code if I'm going to meet a wall in a form of "don't wanna".

So to reiterate requirements:

1. Slime wants to loaded the swank bundled with it (unless it is slime-connect, everything goes then)
2. Swank wants to be loaded as its own system (i.e from quicklisp) when it is a dependency
3. Libraries that depend on swank expect swank to be actually compiled by ASDF (the fact that it worked with SBCL even if it was not processed comes from the fact that SBCL dumps images in opposition to building clean-slate executables form dependencies)

[melisgl]

@dkochmanski That's pretty involved. Does the :(depends-on (:feature (:not :swank) "swank")) workaround, especially in its packaged up form #778 (comment), not work?

[dkochmanski]

@melisgl that won't work, because swank won't be compiled as part of the resulting executable if the swank is loaded when we try to build it.

Coincidentally that works for implementations that dump images, but not for implementations that build executables from scratch.

That particular issue prompted making swank a "normal" system in the first place.