[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 Ring the bells that still can ring. Forget your perfect offering. There is a crack in everything. That's how the light gets in. -Leonard Cohen
* 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 Ring the bells that still can ring. Forget your perfect offering. There is a crack in everything. That's how the light gets in. -Leonard Cohen
* 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? -- -Marco Ring the bells that still can ring. Forget your perfect offering. There is a crack in everything. That's how the light gets in. -Leonard Cohen
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 Ring the bells that still can ring. Forget your perfect offering. There is a crack in everything. That's how the light gets in. -Leonard Cohen
* 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. -- Luís Oliveira http://student.dei.uc.pt/~lmoliv/
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 Ring the bells that still can ring. Forget your perfect offering. There is a crack in everything. That's how the light gets in. -Leonard Cohen
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. -- Luís Oliveira http://student.dei.uc.pt/~lmoliv/
participants (5)
-
Geo Carncross -
Helmut Eller -
Luis Oliveira -
Madhu -
Marco Baringer