bug#23203: 25.0.91; some loaddefs files have auto-save remnants after building (and install doesn't ignore them)

[phillip.lord@russet.org.uk (Phillip Lord)]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 3 Apr 2016 21:11:34 +0100
>> From: "Phillip Lord" <phillip.lord@russet.org.uk>
>> Cc: "Phillip Lord" <phillip.lord@russet.org.uk>,
>>  23203@debbugs.gnu.org,
>>  stromeko@nexgo.de
>> I think that the comment is wrong, as I use this function outside of the
>> Emacs build process (and I am sure others do also), so I think we should
>> remove that. Alternatively, if it is right, we should move it do the
>> docstring.
>
> Fixing the doc string is always a good thing, but it's orthogonal to
> the issue at hand.  (I do agree that the comment is wrong.)

Yes, it's orthogonal -- will fix that later.


>> But, I think having a "no backup files" default is still sensible for a
>> function used in batch; if it is always used this way in the Emacs build,
>> then it it telling us something. So, I would change it, and update the
>> docstring. We could also split the function, so that there is a version
>> which does not suppress backup files.
>
> I don't think I agree that batch mode should behave differently wrt
> backups.
>
> How about this alternative: only disable backing up the initial
> (effectively empty) contents of the autoloads file?  AFAICT, the
> backup files are created during a bootstrap only because we first
> write the initial "rubric" into it, using write-region, and only after
> that visit it.  This defeats the normal mechanism of backing up just
> once per session, and leaves a backup file whose contents are not
> interesting.

That sounds plausible.

>
> So an alternative would be to modify autoload-ensure-default-file so
> that it returns some indication about the fact it created the file,
> and then change its caller to set buffer-backed-up after it visits the
> file, thus preventing the backup _only_ when the file is first
> created.
>
> This should at least solve Achim's problem, but without affecting
> anything else.
>
> WDYT?

Also, it sounds reasonable -- I've attached a patch with a variation on
this theme.

My concerns is that we will still produce backup files which may end up
in a dist build. Consider these commands:

make (from bootstrap)
rm lisp/loaddefs.el
make

Removing loaddefs will result in generation of all the associated
loaddef files, all of which will now be backups.

So, we need to make sure that the packaging system does not copy backup
files.





[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix3.diff --]
[-- Type: text/x-diff, Size: 4733 bytes --]

diff --git a/lisp/Makefile.in b/lisp/Makefile.in
index f33dd01..463252a 100644
--- a/lisp/Makefile.in
+++ b/lisp/Makefile.in
@@ -57,7 +57,7 @@ EMACS = ../src/emacs${EXEEXT}
 
 # Command line flags for Emacs.
 
-EMACSOPT = -batch --no-site-file --no-site-lisp
+EMACSOPT = -batch --no-site-file --no-site-lisp --debug-init
 
 # Extra flags to pass to the byte compiler
 BYTE_COMPILE_EXTRA_FLAGS =
@@ -183,6 +183,7 @@ $(lisp)/loaddefs.el: $(LOADDEFS)
 	    --eval '(setq autoload-ensure-writable t)' \
 	    --eval '(setq autoload-builtin-package-versions t)' \
 	    --eval '(setq generated-autoload-file (expand-file-name (unmsys--file-name "$@")))' \
+	    --eval '(toggle-debug-on-error)' \
 	    -f batch-update-autoloads ${SUBDIRS_ALMOST}
 
 # This is required by the bootstrap-emacs target in ../src/Makefile, so
@@ -381,7 +382,7 @@ $(MH_E_DIR)/mh-loaddefs.el: $(MH_E_SRC)
 	$(AM_V_GEN)$(emacs) -l autoload \
 	   --eval "(setq generate-autoload-cookie \";;;###mh-autoload\")" \
 	   --eval "(setq generated-autoload-file (expand-file-name (unmsys--file-name \"$@\")))" \
-	   -f batch-update-autoloads $(MH_E_DIR)
+           -f batch-update-autoloads $(MH_E_DIR)
 
 # Update TRAMP internal autoloads. Maybe we could move tramp*.el into
 # an own subdirectory. OTOH, it does not hurt to keep them in
@@ -418,7 +419,7 @@ $(CAL_DIR)/hol-loaddefs.el: $(CAL_SRC) $(CAL_DIR)/diary-loaddefs.el
 	$(AM_V_GEN)$(emacs) -l autoload \
 	   --eval "(setq generate-autoload-cookie \";;;###holiday-autoload\")" \
 	   --eval "(setq generated-autoload-file (expand-file-name (unmsys--file-name \"$@\")))" \
-	   -f batch-update-autoloads $(CAL_DIR)
+           -f batch-update-autoloads $(CAL_DIR)
 
 .PHONY: bootstrap-clean distclean maintainer-clean
 
diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
index e688d6b..98bb0d7 100644
--- a/lisp/emacs-lisp/autoload.el
+++ b/lisp/emacs-lisp/autoload.el
@@ -226,7 +226,6 @@ make-autoload
 ;; A doc-string-elt property of ELT says that (nth ELT FORM) is
 ;; the doc-string in FORM.
 ;; Those properties are now set in lisp-mode.el.
-
 (defun autoload-find-generated-file ()
   "Visit the autoload file for the current buffer, and return its buffer.
 If a buffer is visiting the desired autoload file, return it."
@@ -234,9 +233,20 @@ autoload-find-generated-file
 	(enable-local-eval nil))
     ;; We used to use `raw-text' to read this file, but this causes
     ;; problems when the file contains non-ASCII characters.
-    (let ((delay-mode-hooks t))
-      (find-file-noselect
-       (autoload-ensure-default-file (autoload-generated-file))))))
+    (let* ((delay-mode-hooks t)
+           (file (autoload-generated-file))
+           (file-missing (not (file-exists-p file))))
+      (message "File missing %s %s" file file-missing)
+      (when file-missing
+        (autoload-ensure-default-file file))
+      (with-current-buffer
+          (find-file-noselect
+           (autoload-ensure-file-writeable
+            file))
+        (when file-missing
+          (setq buffer-backed-up t)
+          (save-buffer))
+        (current-buffer)))))
 
 (defun autoload-generated-file ()
   (expand-file-name generated-autoload-file
@@ -357,21 +367,22 @@ autoload-ensure-writable
 ;;;###autoload
 (put 'autoload-ensure-writable 'risky-local-variable t)
 
+(defun autoload-ensure-file-writeable (file)
+  ;; Probably pointless, but replaces the old AUTOGEN_VCS in lisp/Makefile,
+  ;; which was designed to handle CVSREAD=1 and equivalent.
+  (and autoload-ensure-writable
+       (let ((modes (file-modes file)))
+         (if (zerop (logand modes #o0200))
+             ;; Ignore any errors here, and let subsequent attempts
+             ;; to write the file raise any real error.
+             (ignore-errors (set-file-modes file (logior modes #o0200))))))
+  file)
+
 (defun autoload-ensure-default-file (file)
   "Make sure that the autoload file FILE exists, creating it if needed.
 If the file already exists and `autoload-ensure-writable' is non-nil,
 make it writable."
-  (if (file-exists-p file)
-      ;; Probably pointless, but replaces the old AUTOGEN_VCS in lisp/Makefile,
-      ;; which was designed to handle CVSREAD=1 and equivalent.
-      (and autoload-ensure-writable
-	   (let ((modes (file-modes file)))
-	     (if (zerop (logand modes #o0200))
-		 ;; Ignore any errors here, and let subsequent attempts
-		 ;; to write the file raise any real error.
-		 (ignore-errors (set-file-modes file (logior modes #o0200))))))
-    (write-region (autoload-rubric file) nil file))
-  file)
+  (write-region (autoload-rubric file) nil file))
 
 (defun autoload-insert-section-header (outbuf autoloads load-name file time)
   "Insert the section-header line,