Re: the ...-unload-hook convention doesn't work

[Dave Love]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> I can not but wonder what we exactly want to achieve with `unload-feature'.
> Is there any hope of ever making it safe enough to be called in an
> actual editing session in which non-trivial work is invested and will
> _continue_ to be invested after unloading a feature?

Yes, with code that obeys the Emacs conventions in the Lisp manual.  I
frequently use it, which is why I worked on it initially.

> I do not believe
> that it currently comes even remotely close to that standard.
>
> The worst thing is that even reloading the feature often can not undo
> the damage:
>
> ELISP> (fboundp 'macroexpand)
> t
> ELISP> (require 'cl)
> cl
> ELISP> (fboundp 'macroexpand)
> t
> ELISP> (unload-feature 'cl) ;; Long output flushed
> ELISP> (require 'cl)
> cl
> ELISP> (fboundp 'macroexpand)
> nil
> ELISP> 

One of the conventions is to avoid CL at runtime.  Another one is not
to redefine Emacs functions as CL does.  The package probably could
actually make itself sanely unloadable by using the -unload-hook
feature if anyone cared to work on it.

In the event that a package has done something like this, it should
have a documented way to fiddle the feature list to control what gets
unbound.  I realized that when dealing with code that needs to
redefine several things which are currently problematic in Emacs and
would eventually have got round to suggesting the following.  (You can
currently do it knowing that `flist' is the right name until someone
changed that.)

--- loadhist.el.~1.26.~ Mon Oct  6 11:55:29 2003
+++ loadhist.el Mon Jan 12 15:15:34 2004
@@ -120,7 +120,17 @@
 (defun unload-feature (feature &optional force)
   "Unload the library that provided FEATURE, restoring all its autoloads.
 If the feature is required by any other loaded code, and prefix arg FORCE
-is nil, raise an error."
+is nil, raise an error.
+
+This function tries to undo modifications made by the package to
+hooks.  Packages may define a hook FEATURE-unload-hook that is called
+instead of the normal heuristics for doing this.  Such a hook should
+undo all the relevant global state changes that may have been made by
+loading the package or executing functions in it.  It has access to
+the package's feature list (before anything is unbound) in the
+variable `feature-list' and could remove features from it in the event
+that the package has done something normally-ill-advised, such as
+redefining an Emacs function."
   (interactive (list (read-feature "Feature: ") current-prefix-arg))
   (if (not (featurep feature))
       (error "%s is not a currently loaded feature" (symbol-name feature)))
@@ -130,8 +140,8 @@
        (if dependents
            (error "Loaded libraries %s depend on %s"
                   (prin1-to-string dependents) file))))
-  (let* ((flist (feature-symbols feature))
-         (file (car flist))
+  (let* ((feature-list (feature-symbols feature))
+         (file (car feature-list))
          (unload-hook (intern-soft (concat (symbol-name feature)
                                            "-unload-hook"))))
     ;; Try to avoid losing badly when hooks installed in critical
@@ -155,10 +165,10 @@
                       (string-match "-hooks?\\'" (symbol-name x)))
                  (and (boundp x)       ; Known abnormal hooks etc.
                       (memq x unload-feature-special-hooks)))
-            (dolist (y (cdr flist))
+            (dolist (y (cdr feature-list))
               (remove-hook x y))))))
     (if (fboundp 'elp-restore-function)        ; remove ELP stuff first
-       (dolist (elt (cdr flist))
+       (dolist (elt (cdr feature-list))
          (if (symbolp elt)
              (elp-restore-function elt))))
     (mapc
@@ -183,7 +193,7 @@
                (fmakunbound x)
                (let ((aload (get x 'autoload)))
                  (if aload (fset x (cons 'autoload aload))))))))
-     (cdr flist))
+     (cdr feature-list))
     ;; Delete the load-history element for this file.
     (let ((elt (assoc file load-history)))
       (setq load-history (delq elt load-history)))))