Subject: bug#35022: closed (Re: bug#35022: 26.1; Load order of custom-autoload, custom-load-symbol, custom-loads is backward?)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 13 Apr 2019 21:58:01 +0000

Your bug report

#35022: custom-add-load, custom-load-symbol don't support filenames with directory components

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
35022: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=35022
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#35022: 26.1; Load order of custom-autoload, custom-load-symbol, custom-loads is backward?
From: Allen Li <[email-masked]>
To: Noam Postavsky <[email-masked]>
Cc: [email-masked]
Date: Sat, 13 Apr 2019 21:57:02 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

On Thu, Apr 11, 2019 at 1:54 AM Noam Postavsky <[email-masked]> wrote:
>
> Allen Li <[email-masked]> writes:
>
> > But not for this:
> >
> > ;; Called from site-start
> > (custom-autoload 'foo-some-variable "some/path/to/foo" nil)
> > ;; Called from init.el/package-initialize
> > (custom-autoload 'foo-some-variable "foo" nil)
> >
> > I'm not sure yet what the proper approach for my problem is.
>
> Would it be too impertinent to suggest the proper aproach is this:
>
>     ;; Called from site-start
>     (add-to-list 'load-path (expand-file-name "some/path/to"))
>     (custom-autoload 'foo-some-variable "foo" nil)
>     ;; Called from init.el/package-initialize
>     (custom-autoload 'foo-some-variable "foo" nil)

The site-start is shipped by the distro which I don't have control over.

>
> Otherwise, I guess extending custom-add-load and custom-load-symbol to
> handle entries of the form (LIBRARY . RELATIVE-NAME) could make sense.

After some deliberation, I think custom-add-load's current behavior is correct,
so I'll take my issue downstream.

Thanks

Subject: 26.1; Load order of custom-autoload, custom-load-symbol, custom-loads is backward?
From: Allen Li <[email-masked]>
To: [email-masked]
Date: Wed, 27 Mar 2019 12:45:42 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

I just spent some time tracking down an unexpected behavior in custom
autoloads.

The custom-autoload function is used in autoloads/loaddef files to set
up custom variables.  custom-autoload adds a load path to the
custom-loads property of a variable.  This property is then used in
custom-load-symbol to load each of the load paths.

The problem is that custom-autoload adds load paths to the front of
custom-loads, and custom-load-symbol loads the paths in forward order.

This is unexpected because if the site distributes a package foo and the
user installs their own version of foo, the site custom-autoload will be
called first:

;; Called from site-start
(custom-autoload 'foo-some-variable "path/to/site/foo" nil)
;; Called from init.el/package-initialize
(custom-autoload 'foo-some-variable "path/to/user/foo" nil)

In this case the custom-loads property will be

("path/to/user/foo" "path/to/site/foo")

The user version will be loaded first, and the site version will be
loaded later, overriding the user version.

This behavior is unexpected; the user would expect that their version of
foo overrides the site version of foo.  Furthermore, this behavior is
somewhat non-deterministic; site overrides foo *only if* foo is loaded
via setting foo-some-variable with customize.  If foo is loaded normally
(e.g. with require), then the user version of foo will be loaded in
preference to the site verison of foo.

I have attached a patch fixing this.

Note that this is technically a breaking change; however I really doubt
any one is intentionally relying on this behavior.  I consider this a
bugfix and I think there are probably users who don't realize that they
are suffering from this bug because it's so subtle.

In GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.22.24), modified by Debian
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description: Debian GNU/Linux rodete (upgraded from: Ubuntu 14.04 LTS)
From 719573c43ecb6e37e2f7c813dfcab09c8707d59d Mon Sep 17 00:00:00 2001
From: Allen Li <[email-masked]>
Date: Wed, 27 Mar 2019 12:39:28 -0700
Subject: [PATCH] Load custom variable dependencies in FIFO order

The custom-autoload function is used in autoloads/loaddef files to set
up custom variables.  custom-autoload adds a load path to the
custom-loads property of a variable.  This property is then used in
custom-load-symbol to load each of the load paths.

The problem is that custom-autoload adds load paths to the front of
custom-loads, and custom-load-symbol loads the paths in forward order.

This is unexpected because if the site distributes a package foo and the
user installs their own version of foo, the site custom-autoload will be
called first:

;; Called from site-start
(custom-autoload 'foo-some-variable "path/to/site/foo" nil)
;; Called from init.el/package-initialize
(custom-autoload 'foo-some-variable "path/to/user/foo" nil)

In this case the custom-loads property will be

("path/to/user/foo" "path/to/site/foo")

The user version will be loaded first, and the site version will be
loaded later, overriding the user version.

This behavior is unexpected; the user would expect that their version of
foo overrides the site version of foo.  Furthermore, this behavior is
somewhat non-deterministic; site overrides foo *only if* foo is loaded
via setting foo-some-variable with customize.  If foo is loaded normally
(e.g. with require), then the user version of foo will be loaded in
preference to the site verison of foo.

* lisp/custom.el (custom-load-symbol): Load variable deps in reverse order.
---
 lisp/custom.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/custom.el b/lisp/custom.el
index f0125742d1..da407bee83 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -639,7 +639,7 @@ custom-load-symbol
       (condition-case nil
 	  (require 'cus-start)
 	(error nil))
-      (dolist (load (get symbol 'custom-loads))
+      (dolist (load (reverse (get symbol 'custom-loads)))
 	(cond ((symbolp load) (condition-case nil (require load) (error nil)))
 	      ;; This is subsumed by the test below, but it's much faster.
 	      ((assoc load load-history))
-- 
2.21.0.392.gf8f6787159e-goog