[slime-devel] make swank safe if loaded into an image with an older version of swank in the image
hi,
It can happen (though i admit it's a rare occurance) that you build an image with swank, update swank and then try to load swank into the existing image. hard to debug/understand errors ensue, this simple patch protects against that.
* Marco Baringer 87ir17m6mo.fsf@arsenic.bese.it : Wrote on Sat, 02 Feb 2008 21:43:27 +0100:
| It can happen (though i admit it's a rare occurance) that you build an | image with swank, update swank and then try to load swank into the | existing image. hard to debug/understand errors ensue, this simple | patch protects against that. | | Index: swank-loader.lisp [...] | +(eval-when (:compile-toplevel :load-toplevel :execute) | + (when (find-package :swank) | + (delete-package :swank) | + (delete-package :swank-io-package) | + (delete-package :swank-loader) | + (delete-package :swank-backend))) | + | (cl:defpackage :swank-loader | (:use :cl) | (:export :load-swank
This is a bad idea. In general, packages should not be automatically deleted[1], and I would prefer if package deletion was provided in a function called reload-swank-loader or something that the user can call when he knows what is happening
Secondly, this breaks a common[2] strategy to put swank fasl files in a different location: For example in my lisp's init file I may have have
(eval-when (:load-toplevel :execute :compile-toplevel) (unless (find-package :swank-loader) (make-package :swank-loader)))
(defparameter swank-loader::*fasl-directory* "/user11/madhu/build/swank/")
(load "/path/toswank-loader.lisp")
- To override the defvar in swank-loader.lisp
The logic propsed in the patch would break this usage completely by deleting the package the variable belongs to -- Madhu
[1] I hear some implementations may leak objects in deleted packages [2] By common, I mean I use this idiom and have seen others use this to customize defvars.
Madhu enometh@meer.net writes:
This is a bad idea. In general, packages should not be automatically deleted[1], and I would prefer if package deletion was provided in a function called reload-swank-loader or something that the user can call when he knows what is happening
Secondly, this breaks a common[2] strategy to put swank fasl files in a different location: For example in my lisp's init file I may have have
(eval-when (:load-toplevel :execute :compile-toplevel) (unless (find-package :swank-loader) (make-package :swank-loader)))
(defparameter swank-loader::*fasl-directory* "/user11/madhu/build/swank/")
(load "/path/toswank-loader.lisp")
- To override the defvar in swank-loader.lisp
ok, since we even suggest doing this is the documentation i will stop deleting the swank-loader package (but i'll keep deleting the other packages unless you can convince me otherwise).
The logic propsed in the patch would break this usage completely by deleting the package the variable belongs to -- Madhu
[1] I hear some implementations may leak objects in deleted packages
which implementations? they should be fixed.
* Marco Baringer 87r6ftszlo.fsf@arsenic.bese.it : Wrote on Mon, 04 Feb 2008 12:57:55 +0100:
| Madhu enometh@meer.net writes: | |> This is a bad idea. In general, packages should not be automatically |> deleted[1], and I would prefer if package deletion was provided in a |> function called reload-swank-loader or something that the user can |> call when he knows what is happening |> |> Secondly, this breaks a common[2] strategy to put swank fasl files in a |> different location: For example in my lisp's init file I may have have |> |> (eval-when (:load-toplevel :execute :compile-toplevel) |> (unless (find-package :swank-loader) |> (make-package :swank-loader))) |> |> (defparameter swank-loader::*fasl-directory* "/user11/madhu/build/swank/") |> |> (load "/path/toswank-loader.lisp") |> |> - To override the defvar in swank-loader.lisp | | ok, since we even suggest doing this is the documentation i will stop | deleting the swank-loader package (but i'll keep deleting the other | packages unless you can convince me otherwise).
It is still a bad idea for the same reason. Variables set earlier in the session in the other packages will be wiped out of you delete the package. Definitely not what I want when I want DEFVAR to behave as specified on reloading.
|> The logic propsed in the patch would break this usage completely by |> deleting the package the variable belongs to
|> |> [1] I hear some implementations may leak objects in deleted packages | | which implementations? they should be fixed.
Easily said! I think i am remembering CLL message slrnf3hva0.gou.jsnell@sbz-30.cs.Helsinki.FI and possibly some PCL related messages (which do not apply since swank doesnt do defclass.) -- Madhu
Madhu enometh@meer.net writes:
- Marco Baringer 87r6ftszlo.fsf@arsenic.bese.it :
| ok, since we even suggest doing this is the documentation i will stop | deleting the swank-loader package (but i'll keep deleting the other | packages unless you can convince me otherwise).
It is still a bad idea for the same reason. Variables set earlier in the session in the other packages will be wiped out of you delete the package. Definitely not what I want when I want DEFVAR to behave as specified on reloading.
well, the entire point of the patch is remove defvars from the swank package. if you have an older slime in an image and attempt to load a newer slime and the defvars don't get reset you'll a totally broken slime and very confusing error messages.
do you often load slime into an image which already has slime built-in?
On Feb 4, 2008 11:04 AM, Marco Baringer mb@bese.it wrote:
well, the entire point of the patch is remove defvars from the swank package. if you have an older slime in an image and attempt to load a newer slime and the defvars don't get reset you'll a totally broken slime and very confusing error messages.
What about using (do-symbols (p ...) (makunbound p)) if the loading version is different than the in-image version?
"Geo Carncross" geocar@gmail.com writes:
On Feb 4, 2008 11:04 AM, Marco Baringer mb@bese.it wrote:
well, the entire point of the patch is remove defvars from the swank package. if you have an older slime in an image and attempt to load a newer slime and the defvars don't get reset you'll a totally broken slime and very confusing error messages.
What about using (do-symbols (p ...) (makunbound p)) if the loading version is different than the in-image version?
that would have the same effect as deleting the package as far as madhu's problem goes.
* Marco Baringer [2008-02-04 17:04+0100] writes:
well, the entire point of the patch is remove defvars from the swank package. if you have an older slime in an image and attempt to load a newer slime and the defvars don't get reset you'll a totally broken slime and very confusing error messages.
Would it be a good idea to offer some restarts? Like: delete old packages use the old version (skip loading the new version) abort
Helmut.
* Marco Baringer 87bq6waeto.fsf@arsenic.bese.it : Wrote on Mon, 04 Feb 2008 17:04:03 +0100: | well, the entire point of the patch is remove defvars from the swank | package. if you have an older slime in an image and attempt to load a | newer slime and the defvars don't get reset you'll a totally broken | slime and very confusing error messages. | | do you often load slime into an image which already has slime built-in?
No, I stopped putting swank into images I build (this turned out to be a bad idea for various reasons), but frequently (at least every every month or two) update slime and reload onto an image containing slime. This has not caused problems recently. In these cases, I want to maintain the state of my session, (in defvars) not have it written over.
Another use case: -- typically I start lisp at a tty, and have a repl thread running (loop (load "/path/to/swank-loader") (swank::setup-server 4005 ...)) and attach to swank from emacs with M-x slime-connect
In this case any changes to swank files get recompiled the next time the slime connection is broken. Putting restarts in swank-loader (as Helmut suggested) would make this use case painful, and impossible in an unattended lisp.
I think the cases where a defvar changes and causes package deletion is sufficiently small to warrant explicit user intervention, where the user can say (delete-package ...) (load "/path/to-swank-loader")
This could be wrapped up and supplied as a function. So the functionality of loading is separated, and swank-loader.lisp can be used as before -- as a single entry point to load swank. -- Madhu
* Marco Baringer 87bq6waeto.fsf@arsenic.bese.it : Wrote on Mon, 04 Feb 2008 17:04:03 +0100:
| Madhu enometh@meer.net writes: | |> * Marco Baringer 87r6ftszlo.fsf@arsenic.bese.it : |> | ok, since we even suggest doing this is the documentation i will stop |> | deleting the swank-loader package (but i'll keep deleting the other |> | packages unless you can convince me otherwise). |> |> It is still a bad idea for the same reason. Variables set earlier in |> the session in the other packages will be wiped out of you delete the |> package. Definitely not what I want when I want DEFVAR to behave as |> specified on reloading. | | well, the entire point of the patch is remove defvars from the swank | package. if you have an older slime in an image and attempt to load a | newer slime and the defvars don't get reset you'll a totally broken | slime and very confusing error messages.
Which defvars are give you a problem when reloading?
I find this sledgehammer solution of deleting packages both bad design taste and bad lisp coding style.
The cost to getting back the old behaviour is now forking off a my-swank-loader.lisp without the 3 lines which delete the packages at the beginning, and using that instead. The benefits (to me and others who do not dump swank in images) is nil.
Maybe someone should commit Sam's patch of splitting up swank-loader into three files so asdf can use it, and other people with other loading requirements can also use parts of it easily?
-- Madhu
Marco Baringer mb@bese.it writes:
It can happen (though i admit it's a rare occurance) that you build an image with swank, update swank and then try to load swank into the existing image. hard to debug/understand errors ensue, this simple patch protects against that.
[...]
Index: swank-loader.lisp
[...]
+(eval-when (:compile-toplevel :load-toplevel :execute)
- (when (find-package :swank)
- (delete-package :swank)
- (delete-package :swank-io-package)
- (delete-package :swank-loader)
- (delete-package :swank-backend)))
[...]
This patch seems to break loading swank through ASDF from within SLIME. (I came across this while loading another system that depends on swank.)
Emacs just hangs and pressing C-g gets me the following message printed onto the repl buffer:
debugger invoked on a SIMPLE-TYPE-ERROR: *PACKAGE* can't be a deleted package: *PACKAGE* has been reset to #<PACKAGE "COMMON-LISP-USER">.
Reverting the patch fixes this.
Luis Oliveira luismbo@gmail.com writes:
This patch seems to break loading swank through ASDF from within SLIME. (I came across this while loading another system that depends on swank.)
Emacs just hangs and pressing C-g gets me the following message printed onto the repl buffer:
debugger invoked on a SIMPLE-TYPE-ERROR: *PACKAGE* can't be a deleted package: *PACKAGE* has been reset to #<PACKAGE "COMMON-LISP-USER">.
Reverting the patch fixes this.
does adding an (in-package :common-lisp-user) as the first thing in swank-loader.lisp fix it as well?
Marco Baringer mb@bese.it writes:
does adding an (in-package :common-lisp-user) as the first thing in swank-loader.lisp fix it as well?
No, same problem.
-
Geo Carncross -
Helmut Eller -
Luis Oliveira -
Madhu -
Marco Baringer