unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Lars Ingebrigtsen <larsi@gnus.org>
To: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Cc: 39823@debbugs.gnu.org, rms@gnu.org, Noam Postavsky <npostavs@gmail.com>
Subject: bug#39823: 26.3; update-directory-autoloads regression from Emacs 26 to Emacs 27
Date: Thu, 01 Oct 2020 19:20:01 +0200	[thread overview]
Message-ID: <87h7rdq2wu.fsf@gnus.org> (raw)
In-Reply-To: <87a6x747bq.fsf@gnus.org> (Lars Ingebrigtsen's message of "Wed, 30 Sep 2020 17:23:21 +0200")

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I think it would make more sense to introduce a new function, say,
> `make-directory-autoloads' (because it doesn't so much update the
> autoloads as create an autoload file, I think?) with a sane function
> signature, and then deprecate update-directory-autoloads (which is only
> used a handful of place in the Emacs tree).

Man, that was a lot more invasive than I thought, because the
dynamically bound variable was used all over the place.

But with the following patch

(make-directory-autoloads "~/src/emacs/trunk/lisp/calc" "/tmp/autofoo.el")

works.  Emacs builds after applying the patch, and starts, and no tests
fail.

Does this look OK to everybody?

diff --git a/doc/lispref/loading.texi b/doc/lispref/loading.texi
index 6833af9c26..aa6ef307b1 100644
--- a/doc/lispref/loading.texi
+++ b/doc/lispref/loading.texi
@@ -577,7 +577,7 @@ Autoload
 define function @var{function-name}"}.
 
 @findex update-file-autoloads
-@findex update-directory-autoloads
+@findex make-directory-autoloads
 @cindex magic autoload comment
 @cindex autoload cookie
 @anchor{autoload cookie}
@@ -590,7 +590,7 @@ Autoload
 file generated by @code{update-file-autoloads} can be changed from the
 above defaults, see below.)
 Building Emacs loads @file{loaddefs.el} and thus calls @code{autoload}.
-@kbd{M-x update-directory-autoloads} is even more powerful; it updates
+@kbd{M-x make-directory-autoloads} is even more powerful; it updates
 autoloads for all files in the current directory.
 
   The same magic comment can copy any kind of form into
diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
index 0bcd7b7dcd..fe6a72c0a1 100644
--- a/lisp/emacs-lisp/autoload.el
+++ b/lisp/emacs-lisp/autoload.el
@@ -254,13 +254,12 @@ make-autoload
 ;; the doc-string in FORM.
 ;; Those properties are now set in lisp-mode.el.
 
-(defun autoload-find-generated-file ()
+(defun autoload-find-generated-file (file)
   "Visit the autoload file for the current buffer, and return its buffer."
   (let ((enable-local-variables :safe)
         (enable-local-eval nil)
         (find-file-hook nil)
-        (delay-mode-hooks t)
-        (file (autoload-generated-file)))
+        (delay-mode-hooks t))
     ;; We used to use `raw-text' to read this file, but this causes
     ;; problems when the file contains non-ASCII characters.
     (with-current-buffer (find-file-noselect
@@ -268,11 +267,13 @@ autoload-find-generated-file
       (if (zerop (buffer-size)) (insert (autoload-rubric file nil t)))
       (current-buffer))))
 
-(defun autoload-generated-file ()
+(defun autoload-generated-file (outfile)
   "Return `generated-autoload-file' as an absolute name.
 If local to the current buffer, expand using the default directory;
 otherwise, using `source-directory'/lisp."
-  (expand-file-name generated-autoload-file
+  (expand-file-name (if (local-variable-p 'generated-autoload-file)
+                        generated-autoload-file
+                      outfile)
                     ;; File-local settings of generated-autoload-file should
                     ;; be interpreted relative to the file's location,
                     ;; of course.
@@ -454,13 +455,12 @@ autoload-find-file
 (defvar no-update-autoloads nil
   "File local variable to prevent scanning this file for autoload cookies.")
 
-(defun autoload-file-load-name (file)
+(defun autoload-file-load-name (file outfile)
   "Compute the name that will be used to load FILE."
   ;; OUTFILE should be the name of the global loaddefs.el file, which
   ;; is expected to be at the root directory of the files we're
   ;; scanning for autoloads and will be in the `load-path'.
-  (let* ((outfile (default-value 'generated-autoload-file))
-         (name (file-relative-name file (file-name-directory outfile)))
+  (let* ((name (file-relative-name file (file-name-directory outfile)))
          (names '())
          (dir (file-name-directory outfile)))
     ;; If `name' has directory components, only keep the
@@ -608,7 +608,7 @@ autoload--make-defs-autoload
         `(register-definition-prefixes ,file ',(sort (delq nil strings)
 						     'string<))))))
 
-(defun autoload--setup-output (otherbuf outbuf absfile load-name)
+(defun autoload--setup-output (otherbuf outbuf absfile load-name output-file)
   (let ((outbuf
          (or (if otherbuf
                  ;; A file-local setting of
@@ -616,7 +616,7 @@ autoload--setup-output
                  ;; should ignore OUTBUF.
                  nil
                outbuf)
-             (autoload-find-destination absfile load-name)
+             (autoload-find-destination absfile load-name output-file)
              ;; The file has autoload cookies, but they're
              ;; already up-to-date. If OUTFILE is nil, the
              ;; entries are in the expected OUTBUF,
@@ -717,7 +717,7 @@ autoload-generate-file-autoloads
                   (setq load-name
                         (if (stringp generated-autoload-load-name)
                             generated-autoload-load-name
-                          (autoload-file-load-name absfile)))
+                          (autoload-file-load-name absfile outfile)))
                   ;; FIXME? Comparing file-names for equality with just equal
                   ;; is fragile, eg if one has an automounter prefix and one
                   ;; does not, but both refer to the same physical file.
@@ -725,8 +725,10 @@ autoload-generate-file-autoloads
                              (not
                               (if (memq system-type '(ms-dos windows-nt))
                                   (equal (downcase outfile)
-                                         (downcase (autoload-generated-file)))
-                                (equal outfile (autoload-generated-file)))))
+                                         (downcase (autoload-generated-file
+                                                    outfile)))
+                                (equal outfile (autoload-generated-file
+                                                outfile)))))
                     (setq otherbuf t))
                   (save-excursion
                     (save-restriction
@@ -740,7 +742,8 @@ autoload-generate-file-autoloads
                                                  (file-name-sans-extension
                                                   (file-name-nondirectory file))))
                                (setq output-start (autoload--setup-output
-                                                   otherbuf outbuf absfile load-name))
+                                                   otherbuf outbuf absfile
+                                                   load-name outfile))
                                (let ((standard-output (marker-buffer output-start))
                                      (print-quoted t))
                                  (princ `(push (purecopy
@@ -758,7 +761,8 @@ autoload-generate-file-autoloads
                             ;; If not done yet, figure out where to insert this text.
                             (unless output-start
                               (setq output-start (autoload--setup-output
-                                                  otherbuf outbuf absfile load-name)))
+                                                  otherbuf outbuf absfile
+                                                  load-name outfile)))
                             (autoload--print-cookie-text output-start load-name file))
                            ((= (following-char) ?\;)
                             ;; Don't read the comment.
@@ -789,7 +793,7 @@ autoload-generate-file-autoloads
                ((not otherbuf)
                 (unless output-start
                   (setq output-start (autoload--setup-output
-                                      nil outbuf absfile load-name)))
+                                      nil outbuf absfile load-name outfile)))
                 (let ((autoload-print-form-outbuf
                        (marker-buffer output-start)))
                   (autoload-print-form form)))
@@ -801,9 +805,8 @@ autoload-generate-file-autoloads
                         ;; then passing otherbuf=nil is enough, but if
                         ;; outbuf is nil, that won't cut it, so we
                         ;; locally bind generated-autoload-file.
-                        (let ((generated-autoload-file
-                               (default-value 'generated-autoload-file)))
-                          (autoload--setup-output nil outbuf absfile load-name)))
+                        (autoload--setup-output nil outbuf absfile load-name
+                                                outfile))
                        (autoload-print-form-outbuf
                         (marker-buffer other-output-start)))
                   (autoload-print-form form)
@@ -937,7 +940,7 @@ update-file-autoloads
           (message "Autoload section for %s is up to date." file)))
     (if no-autoloads file)))
 
-(defun autoload-find-destination (file load-name)
+(defun autoload-find-destination (file load-name output-file)
   "Find the destination point of the current buffer's autoloads.
 FILE is the file name of the current buffer.
 LOAD-NAME is the name as it appears in the output.
@@ -947,12 +950,12 @@ autoload-find-destination
   (catch 'up-to-date
     (let* ((buf (current-buffer))
            (existing-buffer (if buffer-file-name buf))
-           (output-file (autoload-generated-file))
+           (output-file (autoload-generated-file output-file))
            (output-time (if (file-exists-p output-file)
                             (file-attribute-modification-time
 			     (file-attributes output-file))))
            (found nil))
-      (with-current-buffer (autoload-find-generated-file)
+      (with-current-buffer (autoload-find-generated-file output-file)
         ;; This is to make generated-autoload-file have Unix EOLs, so
         ;; that it is portable to all platforms.
         (or (eq 0 (coding-system-eol-type buffer-file-coding-system))
@@ -1038,7 +1041,20 @@ update-directory-autoloads
 use the existing value of `generated-autoload-file'.  If any Lisp
 file binds `generated-autoload-file' as a file-local variable,
 write its autoloads into the specified file instead."
-  (interactive "DUpdate autoloads from directory: ")
+  (declare (obsolete make-directory-autoloads "28.1"))
+  (make-directory-autoloads dirs generated-autoload-file))
+
+;;;###autoload
+(defun make-directory-autoloads (dir output-file)
+  "Update autoload definitions for Lisp files in the directories DIRS.
+DIR can be either a single directory or a list of
+directories.  (The latter usage is discouraged.)
+
+The autoloads will be written to OUTPUT-FILE.
+
+The function does NOT recursively descend into subdirectories of the
+directory or directories specified."
+  (interactive "DUpdate autoloads from directory: \nfWrite to file: ")
   (let* ((files-re (let ((tmp nil))
 		     (dolist (suf (get-load-suffixes))
                        ;; We don't use module-file-suffix below because
@@ -1049,10 +1065,10 @@ update-directory-autoloads
                          (push suf tmp)))
                      (concat "\\`[^=.].*" (regexp-opt tmp t) "\\'")))
 	 (files (apply #'nconc
-		       (mapcar (lambda (dir)
-				 (directory-files (expand-file-name dir)
-						  t files-re))
-			       dirs)))
+		       (mapcar (lambda (d)
+				 (directory-files (expand-file-name d)
+                                                  t files-re))
+			       (if (consp dir) dir (list dir)))))
          (done ())                      ;Files processed; to remove duplicates.
          (changed nil)                  ;Non-nil if some change occurred.
 	 (last-time)
@@ -1060,16 +1076,12 @@ update-directory-autoloads
          ;; files because of file-local autoload-generated-file settings.
 	 (no-autoloads nil)
          (autoload-modified-buffers nil)
-	 (generated-autoload-file
-	  (if (called-interactively-p 'interactive)
-	      (read-file-name "Write autoload definitions to file: ")
-	    generated-autoload-file))
 	 (output-time
-	  (if (file-exists-p generated-autoload-file)
-	      (file-attribute-modification-time
-	       (file-attributes generated-autoload-file)))))
+	  (and (file-exists-p output-file)
+	       (file-attribute-modification-time
+                (file-attributes output-file)))))
 
-    (with-current-buffer (autoload-find-generated-file)
+    (with-current-buffer (autoload-find-generated-file output-file)
       (save-excursion
 	;; Canonicalize file names and remove the autoload file itself.
 	(setq files (delete (file-relative-name buffer-file-name)
@@ -1126,8 +1138,7 @@ update-directory-autoloads
             (progress (make-progress-reporter
                        (byte-compile-info
                         (concat "Scraping files for "
-                                (file-relative-name
-                                 generated-autoload-file)))
+                                (file-relative-name output-file)))
                        0 (length files) nil 10))
             (file-count 0)
             file-time)
@@ -1190,6 +1201,8 @@ batch-update-autoloads
   ;; Exclude those files that are preloaded on ALL platforms.
   ;; These are the ones in loadup.el where "(load" is at the start
   ;; of the line (crude, but it works).
+  (message "FOOOO")
+  (setq debug-on-error t)
   (unless autoload-excludes
     (let ((default-directory (file-name-directory generated-autoload-file))
 	  file)
@@ -1205,7 +1218,7 @@ batch-update-autoloads
   (let ((args command-line-args-left))
     (batch-update-autoloads--summary args)
     (setq command-line-args-left nil)
-    (apply #'update-directory-autoloads args)))
+    (make-directory-autoloads args generated-autoload-file)))
 
 (provide 'autoload)
 
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index a173fc060a..e77077fc52 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1013,7 +1013,6 @@ package-autoload-ensure-default-file
     (write-region (autoload-rubric file "package" nil) nil file nil 'silent))
   file)
 
-(defvar generated-autoload-file)
 (defvar autoload-timestamps)
 (defvar version-control)
 
@@ -1021,14 +1020,14 @@ package-generate-autoloads
   "Generate autoloads in PKG-DIR for package named NAME."
   (let* ((auto-name (format "%s-autoloads.el" name))
          ;;(ignore-name (concat name "-pkg.el"))
-         (generated-autoload-file (expand-file-name auto-name pkg-dir))
+         (output-file (expand-file-name auto-name pkg-dir))
          ;; We don't need 'em, and this makes the output reproducible.
          (autoload-timestamps nil)
          (backup-inhibited t)
          (version-control 'never))
-    (package-autoload-ensure-default-file generated-autoload-file)
-    (update-directory-autoloads pkg-dir)
-    (let ((buf (find-buffer-visiting generated-autoload-file)))
+    (package-autoload-ensure-default-file output-file)
+    (make-directory-autoloads pkg-dir output-file)
+    (let ((buf (find-buffer-visiting output-file)))
       (when buf (kill-buffer buf)))
     auto-name))
 
diff --git a/test/lisp/vc/vc-bzr-tests.el b/test/lisp/vc/vc-bzr-tests.el
index b68a694512..408d6e8e23 100644
--- a/test/lisp/vc/vc-bzr-tests.el
+++ b/test/lisp/vc/vc-bzr-tests.el
@@ -131,7 +131,6 @@ vc-bzr-test-faulty-bzr-autoloads
                  (make-directory bzrdir)
                  (expand-file-name "foo.el" bzrdir)))
          (default-directory (file-name-as-directory bzrdir))
-         (generated-autoload-file (expand-file-name "loaddefs.el" bzrdir))
          (process-environment (cons (format "HOME=%s" homedir)
                                     process-environment)))
     (unwind-protect
@@ -148,7 +147,9 @@ vc-bzr-test-faulty-bzr-autoloads
           ;; causes bzr status to fail.  This simulates a broken bzr
           ;; installation.
           (delete-file ".bzr/checkout/dirstate")
-          (should (progn (update-directory-autoloads default-directory)
+          (should (progn (make-directory-autoloads
+                          default-directory
+                          (expand-file-name "loaddefs.el" bzrdir))
                          t)))
       (delete-directory homedir t))))
 


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





  reply	other threads:[~2020-10-01 17:20 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-28 15:11 bug#39823: 26.3; update-directory-autoloads regression from Emacs 26 to Emacs 27 Maxim Cournoyer
2020-03-13  0:43 ` Noam Postavsky
2020-03-13  8:30   ` Eli Zaretskii
2020-03-14  2:06     ` Maxim Cournoyer
2020-03-13 23:01   ` Michael Heerdegen
2020-03-14  2:03   ` Maxim Cournoyer
2020-03-17 11:48     ` Noam Postavsky
2020-03-14  3:34   ` Richard Stallman
2020-03-17  2:16     ` Maxim Cournoyer
2020-03-19  1:46       ` Richard Stallman
2020-04-08  3:55         ` Maxim Cournoyer
2020-04-08  6:31           ` Eli Zaretskii
2020-04-08 16:41             ` Maxim Cournoyer
2020-04-08 19:09               ` Eli Zaretskii
2020-04-19  3:13                 ` Maxim Cournoyer
2020-04-19 12:19                   ` Noam Postavsky
2020-06-22  4:10                     ` Maxim Cournoyer
2020-06-22 15:07                       ` Noam Postavsky
2020-06-23 20:42                         ` Maxim Cournoyer
2020-09-30 15:23                       ` Lars Ingebrigtsen
2020-10-01 17:20                         ` Lars Ingebrigtsen [this message]
2020-10-01 17:54                           ` Eli Zaretskii
2020-10-01 18:06                             ` Lars Ingebrigtsen
2020-10-02  2:36                               ` Lars Ingebrigtsen
2020-10-02 17:54                                 ` Maxim Cournoyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h7rdq2wu.fsf@gnus.org \
    --to=larsi@gnus.org \
    --cc=39823@debbugs.gnu.org \
    --cc=maxim.cournoyer@gmail.com \
    --cc=npostavs@gmail.com \
    --cc=rms@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).