bug#38694: 26.1; update-*-autoloads fails with local generated-autoload-file

[Lars Ingebrigtsen]

Braun Gábor <braungb88@gmail.com> writes:

> Let the current directory have only the following two files:

[...]

> ;; Local Variables:
> ;; generated-autoload-file: "foo-autoloads.el"
> ;; End:

> M-x update-file-autoloads RET
> test-bar.el RET
> test-autoloads.el RET
>
> It produces an error message, which I copied from buffer *Messages*:
>
> autoload-generate-file-autoloads: DIRECTORY/test-bar.el:0:0: error: 
> wrong-type-argument: (stringp nil)

I can confirm this bug.

[...]

> IMHO, the root of the problem is that update-file-autoloads and
> update-directory-autoloads bind generated-autoload-file buffer locally
> (as it is a file local variable in test-foo.el) but intend to bind the
> default value.  Note that the docstrings of both functions
> mention the case of generated-autoload-file being buffer-local, so
> they ought to support this case.
>
> As the documented behaviour of these functions do not
> depend on the current buffer, I propose to simply switch to a
> temporary buffer so that the local settings don't interfere.
> This is a preemptive measure of other similar latent problems.
>
> Admittedly, creating a temporary buffer just to bind variables'
> default value seems to be wasteful, and I am very interested in
> learning a better, more idiomatic approach.

I don't think there is, and that's perhaps unfortunate.  What we have
here is a pretty unusual situation, though.

> +  ;; The temporary buffer is only to bind the default value of e.g.,
> +  ;; `generated-autoload-file'.  The current buffer is irrelevant.
> +  (with-temp-buffer
>    (let* ((generated-autoload-file (or outfile generated-autoload-file))

But I don't think this is the right way to fix this?  We want to heed
the buffer-local value of generated-autoload-file, and we want to bind
the global value of generated-autoload-file, I think?  Otherwise, what
would be the point of

> ;; Local Variables:
> ;; generated-autoload-file: "foo-autoloads.el"
> ;; End:

I'm not very familiar with this code, though, so I may be misreading it.

So the patch would be something like

+  (let ((autoload-file (or outfile generated-autoload-file)))
+    (with-temp-buffer
+      (let* ((generated-autoload-file autoload-file)

Or more complete (with indentation changes) below.

Does that makes sense?

diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
index 2eef451200..ad39f5642b 100644
--- a/lisp/emacs-lisp/autoload.el
+++ b/lisp/emacs-lisp/autoload.el
@@ -924,17 +924,20 @@ update-file-autoloads
   (interactive (list (read-file-name "Update autoloads for file: ")
                     current-prefix-arg
                     (read-file-name "Write autoload definitions to file: ")))
-  (let* ((generated-autoload-file (or outfile generated-autoload-file))
-        (autoload-modified-buffers nil)
-        ;; We need this only if the output file handles more than one input.
-        ;; See https://debbugs.gnu.org/22213#38 and subsequent.
-        (autoload-timestamps t)
-         (no-autoloads (autoload-generate-file-autoloads file)))
-    (if autoload-modified-buffers
-        (if save-after (autoload-save-buffers))
-      (if (called-interactively-p 'interactive)
-          (message "Autoload section for %s is up to date." file)))
-    (if no-autoloads file)))
+  (let ((autoload-file (or outfile generated-autoload-file)))
+    (with-temp-buffer
+      (let* ((generated-autoload-file autoload-file)
+            (autoload-modified-buffers nil)
+            ;; We need this only if the output file handles more than
+            ;; one input.  See https://debbugs.gnu.org/22213#38 and
+            ;; subsequent.
+            (autoload-timestamps t)
+             (no-autoloads (autoload-generate-file-autoloads file)))
+        (if autoload-modified-buffers
+            (if save-after (autoload-save-buffers))
+          (if (called-interactively-p 'interactive)
+              (message "Autoload section for %s is up to date." file)))
+        (if no-autoloads file)))))
 
 (defun autoload-find-destination (file load-name)
   "Find the destination point of the current buffer's autoloads.


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no