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

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

[Achim Gratz]

After a bootstrap build of the emacs-25 branch, there are some spurious
auto-save files lying around:

emacs/build-25> find ../lisp -name \*~ 
../lisp/calc/calc-loaddefs.el~
../lisp/loaddefs.el~
../lisp/emacs-lisp/cl-loaddefs.el~
../lisp/eshell/esh-groups.el~
../lisp/net/tramp-loaddefs.el~
../lisp/cedet/ede/loaddefs.el~
../lisp/cedet/semantic/loaddefs.el~
../lisp/cedet/srecode/loaddefs.el~
../lisp/org/org-loaddefs.el~

Moreover, these will be copied into the installation.

Please either prevent the creation of said auto-save files ,remove them
after the build or at least exclude them from getting installed.  Thank
you.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

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

[Andreas Schwab]

Achim Gratz <Stromeko@nexgo.de> writes:

> After a bootstrap build of the emacs-25 branch, there are some spurious
> auto-save files lying around:

They are not auto-save files, but backup files.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Andreas Schwab]

Achim Gratz <Stromeko@nexgo.de> writes:

> Moreover, these will be copied into the installation.

No, they aren't, see line 598 of Makefile.in:

	      rm -f $${subdir}/*~ ; \

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Achim Gratz]

Andreas Schwab writes:
> Achim Gratz writes:
>> Moreover, these will be copied into the installation.
> No, they aren't, see line 598 of Makefile.in:
>
> 	      rm -f $${subdir}/*~ ; \

That is where they are getting removed after already having been
installed.  They shouldn't get created or installed in the first place,
as there is no point of having a backup for an auto-generated file.

On another tangent, is it really necessary to invoke another rm for each
file pattern to delete?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

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

[Andreas Schwab]

Achim Gratz <Stromeko@nexgo.de> writes:

> as there is no point of having a backup for an auto-generated file.

I disagree.  Even for generated files you may want to check what has
changed when last regenerated.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Achim Gratz]

Andreas Schwab writes:
> Achim Gratz writes:
>> as there is no point of having a backup for an auto-generated file.
>
> I disagree.  Even for generated files you may want to check what has
> changed when last regenerated.

There was no previous version, this being a bootstrap build.  And if I
want to check differences between builds, I can just use different build
directories.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

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

[Phillip Lord]

On Sun, April 3, 2016 12:47 pm, Achim Gratz wrote:
> Andreas Schwab writes:
>
>> Achim Gratz writes:
>>
>>> as there is no point of having a backup for an auto-generated file.
>>
>> I disagree.  Even for generated files you may want to check what has
>> changed when last regenerated.
>
> There was no previous version, this being a bootstrap build.  And if I
> want to check differences between builds, I can just use different build
> directories.

I think this is a bug also, and one which will get worse as the number of
autoload files increases on master.

It's easy to fix -- we just need to disable backup files in the batch
autoload generator.

It seems to me that this issue is a little wider than autoload files,
though -- some files in cedet seem to show the same behaviour.

Phil

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

[Andreas Schwab]

Achim Gratz <Stromeko@nexgo.de> writes:

> There was no previous version, this being a bootstrap build.

The next build may have a new version.

> And if I want to check differences between builds, I can just use
> different build directories.

That assumes that you can reproduce the exact previous state.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Phillip Lord]

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

On Sun, April 3, 2016 12:47 pm, Achim Gratz wrote:
> Andreas Schwab writes:
>
>> Achim Gratz writes:
>>
>>> as there is no point of having a backup for an auto-generated file.
>>
>> I disagree.  Even for generated files you may want to check what has
>> changed when last regenerated.
>
> There was no previous version, this being a bootstrap build.  And if I
> want to check differences between builds, I can just use different build
> directories.

The attached patch fixes the problem in my hands (assuming we believe that
this is a bug). The solution may be a bit blunt, though, although the
function patched is documented as "use in batch during build".

Phil

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix.diff --]
[-- Type: text/x-patch; name="fix.diff", Size: 629 bytes --]

diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
index e688d6b..bc7873b 100644
--- a/lisp/emacs-lisp/autoload.el
+++ b/lisp/emacs-lisp/autoload.el
@@ -870,6 +870,9 @@ 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).
+  ;; Stop the production of backup files which make little sense
+  ;; during the build process.
+  (setq make-backup-files nil)
   (unless autoload-excludes
     (let ((default-directory (file-name-directory generated-autoload-file))
 	  file)

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

[Eli Zaretskii]

> Date: Sun, 3 Apr 2016 19:46:40 +0100
> From: "Phillip Lord" <phillip.lord@russet.org.uk>
> Cc: 23203@debbugs.gnu.org
> 
> The attached patch fixes the problem in my hands (assuming we believe that
> this is a bug). The solution may be a bit blunt, though, although the
> function patched is documented as "use in batch during build".

Indeed, I'd rather we didn't do anything that drastic.

If needed, backups can be disabled from the command line.

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

[Phillip Lord]

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

On Sun, April 3, 2016 7:58 pm, Eli Zaretskii wrote:
>> Date: Sun, 3 Apr 2016 19:46:40 +0100
>> From: "Phillip Lord" <phillip.lord@russet.org.uk>
>> Cc: 23203@debbugs.gnu.org
>>
>>
>> The attached patch fixes the problem in my hands (assuming we believe
>> that this is a bug). The solution may be a bit blunt, though, although
>> the function patched is documented as "use in batch during build".
>
> Indeed, I'd rather we didn't do anything that drastic.
>
>
> If needed, backups can be disabled from the command line.

That works too (see attached diff), although it's a bit of pain because of
duplication in the makefile.

I would ask though, when do we want backup files produced from the
batch-update-autoloads function? Changing the default behaviour of this
function seems to make the most sense to me (although perhaps not the way
that I did it).

Phil

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix2.diff --]
[-- Type: text/x-patch; name="fix2.diff", Size: 2645 bytes --]

diff --git a/lisp/Makefile.in b/lisp/Makefile.in
index f33dd01..98743e8 100644
--- a/lisp/Makefile.in
+++ b/lisp/Makefile.in
@@ -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 '(setq make-backup-files nil)' \
 	    -f batch-update-autoloads ${SUBDIRS_ALMOST}
 
 # This is required by the bootstrap-emacs target in ../src/Makefile, so
@@ -381,7 +382,8 @@ $(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)
+	   --eval '(setq make-backup-files nil)' \
+           -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
@@ -394,6 +396,7 @@ $(TRAMP_DIR)/tramp-loaddefs.el: $(TRAMP_SRC)
 	$(AM_V_GEN)$(emacs) -l autoload \
 	   --eval "(setq generate-autoload-cookie \";;;###tramp-autoload\")" \
 	   --eval "(setq generated-autoload-file (expand-file-name (unmsys--file-name \"$@\")))" \
+	   --eval '(setq make-backup-files nil)' \
 	   -f batch-update-autoloads $(TRAMP_DIR)
 
 CAL_DIR = $(lisp)/calendar
@@ -406,19 +409,22 @@ $(CAL_DIR)/cal-loaddefs.el: $(CAL_SRC)
 	$(AM_V_GEN)$(emacs) -l autoload \
 	   --eval "(setq generate-autoload-cookie \";;;###cal-autoload\")" \
 	   --eval "(setq generated-autoload-file (expand-file-name (unmsys--file-name \"$@\")))" \
+	   --eval '(setq make-backup-files nil)' \
 	   -f batch-update-autoloads $(CAL_DIR)
 
 $(CAL_DIR)/diary-loaddefs.el: $(CAL_SRC) $(CAL_DIR)/cal-loaddefs.el
 	$(AM_V_GEN)$(emacs) -l autoload \
 	   --eval "(setq generate-autoload-cookie \";;;###diary-autoload\")" \
 	   --eval "(setq generated-autoload-file (expand-file-name (unmsys--file-name \"$@\")))" \
+	   --eval '(setq make-backup-files nil)' \
 	   -f batch-update-autoloads $(CAL_DIR)
 
 $(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)
+	   --eval '(setq make-backup-files nil)' \
+           -f batch-update-autoloads $(CAL_DIR)
 
 .PHONY: bootstrap-clean distclean maintainer-clean
 

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

[Eli Zaretskii]

> Date: Sun, 3 Apr 2016 20:33:37 +0100
> From: "Phillip Lord" <phillip.lord@russet.org.uk>
> Cc: "Phillip Lord" <phillip.lord@russet.org.uk>,
>  23203@debbugs.gnu.org,
>  stromeko@nexgo.de
> 
> > If needed, backups can be disabled from the command line.
> 
> That works too (see attached diff), although it's a bit of pain because of
> duplication in the makefile.

Thanks.

> I would ask though, when do we want backup files produced from the
> batch-update-autoloads function?

Emacs always makes backups of all the files it writes, so summarily
exempting autoload files from this rule sounds wrong to me.  Who are
we to dictate to the entire world out there whether they should or
shouldn't have backups of these files?

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

[Phillip Lord]

On Sun, April 3, 2016 8:39 pm, Eli Zaretskii wrote:
>> Date: Sun, 3 Apr 2016 20:33:37 +0100
>> From: "Phillip Lord" <phillip.lord@russet.org.uk>
>> Cc: "Phillip Lord" <phillip.lord@russet.org.uk>,
>> 23203@debbugs.gnu.org,
>> stromeko@nexgo.de
>>
>>> If needed, backups can be disabled from the command line.
>>>
>>
>> That works too (see attached diff), although it's a bit of pain because
>> of duplication in the makefile.
>
> Thanks.
>
>
>> I would ask though, when do we want backup files produced from the
>> batch-update-autoloads function?
>
> Emacs always makes backups of all the files it writes, so summarily
> exempting autoload files from this rule sounds wrong to me.  Who are we to
> dictate to the entire world out there whether they should or shouldn't
> have backups of these files?

Who are we? Well, the obvious answer is core Emacs devs with commit
privileges.

Actually, my first batch did not exempt autoload files. It only did that
when called using batch-update-autoloads: i.e. in batch, and in a function
which is commented (but not documented) to be for use during the Emacs
build process. Interactive calls to "update-directory-autoloads" and the
like would be unaffected.

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.

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.

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

[Eli Zaretskii]

> 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
> 
> Actually, my first batch did not exempt autoload files. It only did that
> when called using batch-update-autoloads: i.e. in batch, and in a function
> which is commented (but not documented) to be for use during the Emacs
> build process. Interactive calls to "update-directory-autoloads" and the
> like would be unaffected.
> 
> 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.)

> 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.

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?

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

[Achim Gratz]

Eli Zaretskii writes:
> If needed, backups can be disabled from the command line.

FWIW, I think it's the way these files are created during bootstrap that
produces the spurious backups.  So I'd be more inclined to fix the build
procedure rather than anything else (whether or not that function is
advertised to users).


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

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

[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,

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

[Achim Gratz]

Phillip Lord writes:
> 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.

Make install does copy them, but then they get deleted.  For all
packaging systems I know of that's no problem since they do a DESTDIR
install and then package from there.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Samples for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra

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

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: 23203@debbugs.gnu.org,  stromeko@nexgo.de
> Date: Mon, 04 Apr 2016 23:12:57 +0100
> 
> > 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.

Thanks.  There seem to be spurious unrelated whitespace changes in the
patch.

Also, wouldn't it be more elegant to have autoload-ensure-default-file
return a cons cell reporting whether the file did exist before, so
that autoload-find-generated-file could act on that?  But if you
prefer your implementation, I won't argue.

> 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.

"make install" already removes backup files from the target tree, so
is there any problem left?

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

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: phillip.lord@russet.org.uk (Phillip Lord)
>> Cc: 23203@debbugs.gnu.org,  stromeko@nexgo.de
>> Date: Mon, 04 Apr 2016 23:12:57 +0100
>> Also, it sounds reasonable -- I've attached a patch with a variation on
>> this theme.
>
> Thanks.  There seem to be spurious unrelated whitespace changes in the
> patch.

Yeah, it was a proof-of-concept patch. It's got some debug statements in
also.

Alas, it also seems to have a bootstrap bug. Will work on it.

>> So, we need to make sure that the packaging system does not copy backup
>> files.
>
> "make install" already removes backup files from the target tree, so
> is there any problem left?

Ah, okay, that's fine; I'll work on my patch and try and get it actually
working.

Phil

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

[Phillip Lord]

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


Complete diff of my proposed change.

If anyone has time to test this it would be appreciated, as I saw some
strange (and now unrepeatable) failures with an earlier version of this.
I've not I'll install it tomorrow, as it seems functional in my hands.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Prevent-bootstrap-autoload-backup-files.patch --]
[-- Type: text/x-diff, Size: 3328 bytes --]

From 8f50973593b408978ae9cc247fe9d6423266d60c Mon Sep 17 00:00:00 2001
From: Phillip Lord <phillip.lord@russet.org.uk>
Date: Fri, 8 Apr 2016 16:22:44 +0100
Subject: [PATCH] Prevent bootstrap autoload backup files

 * lisp/emacs-lisp/autoload (autoload-find-generated-file): Suppress
   backups in newly created file.

  (autoload-ensure-default-file): Function split into two.
  (autoload-ensure-file-writeable): New function from split.

  (Bug#23203)
---
 lisp/emacs-lisp/autoload.el | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
index e688d6b..592d69d 100644
--- a/lisp/emacs-lisp/autoload.el
+++ b/lisp/emacs-lisp/autoload.el
@@ -234,9 +234,22 @@ 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))))
+      (when file-missing
+        (autoload-ensure-default-file file))
+      (with-current-buffer
+          (find-file-noselect
+           (autoload-ensure-file-writeable
+            file))
+        ;; block backups when the file has just been created, since
+        ;; the backups will just be the auto-generated headers.
+        ;; bug#23203
+        (when file-missing
+          (setq buffer-backed-up t)
+          (save-buffer))
+        (current-buffer)))))
 
 (defun autoload-generated-file ()
   (expand-file-name generated-autoload-file
@@ -357,21 +370,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,
-- 
2.8.1

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

[Phillip Lord]

phillip.lord@russet.org.uk (Phillip Lord) writes:

> Complete diff of my proposed change.
>
> If anyone has time to test this it would be appreciated, as I saw some
> strange (and now unrepeatable) failures with an earlier version of this.
> I've not I'll install it tomorrow, as it seems functional in my hands.


Installed to Emacs-25.

Phil

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

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: 23203@debbugs.gnu.org,  stromeko@nexgo.de
> Date: Tue, 12 Apr 2016 13:03:47 +0100
> 
> phillip.lord@russet.org.uk (Phillip Lord) writes:
> 
> > Complete diff of my proposed change.
> >
> > If anyone has time to test this it would be appreciated, as I saw some
> > strange (and now unrepeatable) failures with an earlier version of this.
> > I've not I'll install it tomorrow, as it seems functional in my hands.
> 
> Installed to Emacs-25.

Thanks, but why on Earth to the release branch?  This issue is nowhere
near being release-critical, and AFAICT the question of which branch
to commit it to was never even raised here.  It was obvious to me it
will go to master.

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

[Glenn Morris]

Eli Zaretskii wrote:

> Thanks, but why on Earth to the release branch?  This issue is nowhere
> near being release-critical, and AFAICT the question of which branch
> to commit it to was never even raised here.  It was obvious to me it
> will go to master.

Indeed. (It's not even a bug IMO.)

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

[Phillip Lord]

Glenn Morris <rgm@gnu.org> writes:

> Eli Zaretskii wrote:
>
>> Thanks, but why on Earth to the release branch?  This issue is nowhere
>> near being release-critical, and AFAICT the question of which branch
>> to commit it to was never even raised here.  It was obvious to me it
>> will go to master.
>
> Indeed. (It's not even a bug IMO.)

Ah, sorry. I thought bug fixes should go on 25. Is this because we after
the release candidates?

Phil

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

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: Eli Zaretskii <eliz@gnu.org>,  23203@debbugs.gnu.org,  stromeko@nexgo.de
> Date: Tue, 12 Apr 2016 17:34:43 +0100
> 
> Glenn Morris <rgm@gnu.org> writes:
> 
> > Eli Zaretskii wrote:
> >
> >> Thanks, but why on Earth to the release branch?  This issue is nowhere
> >> near being release-critical, and AFAICT the question of which branch
> >> to commit it to was never even raised here.  It was obvious to me it
> >> will go to master.
> >
> > Indeed. (It's not even a bug IMO.)
> 
> Ah, sorry. I thought bug fixes should go on 25. Is this because we after
> the release candidates?

More because has John declared the release branch open only for
serious bugs and regressions, and everything else should be discussed
first.

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

[John Wiegley]

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

>>>>> Phillip Lord <phillip.lord@russet.org.uk> writes:

> Ah, sorry. I thought bug fixes should go on 25. Is this because we after the
> release candidates?

Not all bug fixes, just those that address significant bugs and/or are clearly
not destabilizing. We're trying to get emacs-25 out the door as soon as its
capable enough, and larger-scale/unnecessary bug fixes run the risk of
regressing the stability we've achieved thus far.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

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

[Stefan Kangas]

close 23203 26.1
thanks

phillip.lord@russet.org.uk (Phillip Lord) writes:

> phillip.lord@russet.org.uk (Phillip Lord) writes:
>
>> Complete diff of my proposed change.
>>
>> If anyone has time to test this it would be appreciated, as I saw some
>> strange (and now unrepeatable) failures with an earlier version of this.
>> I've not I'll install it tomorrow, as it seems functional in my hands.
>
> Installed to Emacs-25.

The patch fixing the issue was installed (first on emacs-25 but later
reverted and moved to master).

The bug seems to have been left open by mistake, so I'm closing it now.

Best regards,
Stefan Kangas