[PATCH] package.el: remove duplicated package path in load-path

[PATCH] package.el: remove duplicated package path in load-path

[Levin Du]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

Hi all,

   I find package.el will add duplicate package path to load-path.
One is (push pkg-dir load-path) in package-activate-1,
another is

   (add-to-list 'load-path (or (file-name-directory #$) (car load-path)))

in the generated PACKAGE-autoloads.el which will be also loaded in
package-activate-1.

The following patch provides one way of fixing this:
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index add73fd..c5706bf 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -468,8 +468,8 @@ Return the max version (as a string) if the package is
held at a lower version."
       (info-initialize)
       (push pkg-dir Info-directory-list))
     ;; Add to load path, add autoloads, and activate the package.
-    (push pkg-dir load-path)
-    (load (expand-file-name (format "%s-autoloads" name) pkg-dir) nil t)
+    (unless (load (expand-file-name (format "%s-autoloads" name) pkg-dir)
nil t)
+      (push pkg-dir load-path))
     (push name package-activated-list)
     ;; Don't return nil.
     t))


Hope it helps.

Regards,
Levin Du

[-- Attachment #2: Type: text/html, Size: 1600 bytes --]

Re: [PATCH] package.el: remove duplicated package path in load-path

[Stefan Monnier]

>    I find package.el will add duplicate package path to load-path.
> One is (push pkg-dir load-path) in package-activate-1,
> another is

>    (add-to-list 'load-path (or (file-name-directory #$) (car load-path)))

> in the generated PACKAGE-autoloads.el which will be also loaded in
> package-activate-1.

Indeed, in the past, <pkg>-autoloads.el did not tweak load-path itself,
which is why we do it in package-activate-1.  But I don't understand why
you have duplicate entries, since the first command will be (push
pkg-dir load-path) bu the second (in <pkg>-autoloads.el) is an
`add-to-list' which should hopefully avoid the duplication.

...oh wait, I see, the two directories are not equal (one ends in slash
while the other doesn't).
I installed the patch below to try and address this problem.


        Stefan


=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2013-08-28 16:39:51 +0000
+++ lisp/ChangeLog	2013-08-28 17:56:49 +0000
@@ -1,3 +1,8 @@
+2013-08-28  Stefan Monnier  <monnier@iro.umontreal.ca>
+
+	* emacs-lisp/package.el (package-activate-1): Don't add unnecessarily
+	to load-path.
+
 2013-08-28  Juri Linkov  <juri@jurta.org>
 
 	* isearch.el (isearch-reread-key-sequence-naturally): Use non-nil

=== modified file 'lisp/emacs-lisp/package.el'
--- lisp/emacs-lisp/package.el	2013-08-27 08:01:13 +0000
+++ lisp/emacs-lisp/package.el	2013-08-28 17:53:17 +0000
@@ -457,19 +457,26 @@
 
 (defun package-activate-1 (pkg-desc)
   (let* ((name (package-desc-name pkg-desc))
-	 (pkg-dir (package-desc-dir pkg-desc)))
+	 (pkg-dir (package-desc-dir pkg-desc))
+         (pkg-dir-dir (file-name-as-directory pkg-dir)))
     (unless pkg-dir
       (error "Internal error: unable to find directory for `%s'"
 	     (package-desc-full-name pkg-desc)))
+    ;; Add to load path, add autoloads, and activate the package.
+    (let ((old-lp load-path))
+      (load (expand-file-name (format "%s-autoloads" name) pkg-dir) nil t)
+      (when (and (eq old-lp load-path)
+                 (not (or (member pkg-dir load-path)
+                          (member pkg-dir-dir load-path))))
+        ;; Old packages don't add themselves to the `load-path', so we have to
+        ;; do it ourselves.
+        (push pkg-dir load-path)))
     ;; Add info node.
     (when (file-exists-p (expand-file-name "dir" pkg-dir))
       ;; FIXME: not the friendliest, but simple.
       (require 'info)
       (info-initialize)
       (push pkg-dir Info-directory-list))
-    ;; Add to load path, add autoloads, and activate the package.
-    (push pkg-dir load-path)
-    (load (expand-file-name (format "%s-autoloads" name) pkg-dir) nil t)
     (push name package-activated-list)
     ;; Don't return nil.
     t))