-
Notifications
You must be signed in to change notification settings - Fork 329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
swank-loader: load-swank: if asdf is available then use it #778
base: master
Are you sure you want to change the base?
Conversation
This is to prevent redefinition warnings when a system is compiled that has swank in dependencies.
I'm not sure I want to use asdf even if it's available. |
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. |
I understand that, but using ASDF will introduce more problems than it will solve. |
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. |
It should be inverted, swank.asd shouldn't load swank if it's already loaded. |
For the record and the time being, I have |
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. |
A workaround could be to rename the swank asdf system to swank-impl and add this:
Admittedly, I have not thought about the interaction with dumping binaries an such. |
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 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 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:
|
@dkochmanski That's pretty involved. Does the |
@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. |
This is to prevent redefinition warnings when a system is compiled that has swank in dependencies.