bug#30215: Visiting files from zip archives should not modify directory time

bug#30215: Visiting files from zip archives should not modify directory time

[Juri Linkov]

Visiting a file from zip archive changes the modification time of
its directory.  It's possible to prevent directory time modification
by setting ‘buffer-file-truename’ later after the extractor prepares
the file in its buffer:

diff --git a/lisp/arc-mode.el b/lisp/arc-mode.el
index adb3669..d6add45 100644
--- a/lisp/arc-mode.el
+++ b/lisp/arc-mode.el
@@ -1045,8 +1045,6 @@ archive-extract
         (setq just-created t)
         (with-current-buffer buffer
           (setq buffer-file-name arcfilename)
-          (setq buffer-file-truename
-                (abbreviate-file-name buffer-file-name))
           ;; Set the default-directory to the dir of the superior buffer.
           (setq default-directory arcdir)
           (make-local-variable 'archive-superior-buffer)
@@ -1077,6 +1075,9 @@ archive-extract
 	      (progn
 		(set-buffer-modified-p nil)
 		(kill-buffer buffer))
+            ;; Set this later to avoid changing dir mtime by lock_file
+            (setq buffer-file-truename
+                  (abbreviate-file-name buffer-file-name))
             (archive-try-jka-compr)     ;Pretty ugly hack :-(
 	    (archive-set-buffer-as-visiting-file ename)
 	    (goto-char (point-min))

bug#30215: Visiting files from zip archives should not modify directory time

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Mon, 22 Jan 2018 23:52:48 +0200
> 
> Visiting a file from zip archive changes the modification time of
> its directory.  It's possible to prevent directory time modification
> by setting ‘buffer-file-truename’ later after the extractor prepares
> the file in its buffer:

Sorry, I don't think I understand what you are saying (and if I did, I
cannot reproduce it).  I visited a random zip archive on my system,
then pressed RET on one of the files, then killed the buffer which
visited that file -- and didn't see any changes in the time stamps
recorded in the zip archive.  Which is what I'd expect, since visiting
a file from an archive doesn't rewrite the archive, nor modifies it in
any other way.

What am I missing?

bug#30215: Visiting files from zip archives should not modify directory time

[Juri Linkov]

>> Visiting a file from zip archive changes the modification time of
>> its directory.  It's possible to prevent directory time modification
>> by setting ‘buffer-file-truename’ later after the extractor prepares
>> the file in its buffer:
>
> Sorry, I don't think I understand what you are saying (and if I did, I
> cannot reproduce it).  I visited a random zip archive on my system,
> then pressed RET on one of the files, then killed the buffer which
> visited that file -- and didn't see any changes in the time stamps
> recorded in the zip archive.  Which is what I'd expect, since visiting
> a file from an archive doesn't rewrite the archive, nor modifies it in
> any other way.
>
> What am I missing?

It modifies the time of the directory where the zip archive is located,
e.g. visiting a file inside ~/dir/file.zip modifies the time of the
directory ~/dir/ because during unzipping in that directory, arc-mode
creates a temporary backup with the file name from ‘buffer-file-truename’.
Setting ‘buffer-file-truename’ later prevents from creating this
unnecessary backup copy that modifies the directory time.

bug#30215: Visiting files from zip archives should not modify directory time

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 30215@debbugs.gnu.org
> Date: Tue, 23 Jan 2018 23:29:34 +0200
> 
> It modifies the time of the directory where the zip archive is located,
> e.g. visiting a file inside ~/dir/file.zip modifies the time of the
> directory ~/dir/ because during unzipping in that directory, arc-mode
> creates a temporary backup with the file name from ‘buffer-file-truename’.
> Setting ‘buffer-file-truename’ later prevents from creating this
> unnecessary backup copy that modifies the directory time.

If that's the problem, then I think it would be cleaner to disable
creation of lock files and backup files by less subtle means -- by
let-binding create-lockfiles and make-backup-files.  Doing that by
leaving buffer-file-truename at nil leaves the code less clear IMO,
and relies on assumptions that might not hold at some future point.

bug#30215: Visiting files from zip archives should not modify directory time

[Juri Linkov]

> If that's the problem, then I think it would be cleaner to disable
> creation of lock files and backup files by less subtle means -- by
> let-binding create-lockfiles and make-backup-files.  Doing that by
> leaving buffer-file-truename at nil leaves the code less clear IMO,
> and relies on assumptions that might not hold at some future point.

Thanks for the suggestion, this is better indeed:

diff --git a/lisp/arc-mode.el b/lisp/arc-mode.el
index adb3669..4fe1b03 100644
--- a/lisp/arc-mode.el
+++ b/lisp/arc-mode.el
@@ -807,7 +807,7 @@ archive-summarize
 Optional argument SHUT-UP, if non-nil, means don't print messages
 when parsing the archive."
   (widen)
-  (let ((buffer-file-truename nil) ; avoid changing dir mtime by lock_file
+  (let ((create-lockfiles nil) ; avoid changing dir mtime by lock_file
 	(inhibit-read-only t))
     (setq archive-proper-file-start (copy-marker (point-min) t))
     (set (make-local-variable 'change-major-mode-hook) 'archive-desummarize)
@@ -1064,7 +1064,9 @@ archive-extract
 		      ;; We read an archive member by no-conversion at
 		      ;; first, then decode appropriately by calling
 		      ;; archive-set-buffer-as-visiting-file later.
-		      (coding-system-for-read 'no-conversion))
+		      (coding-system-for-read 'no-conversion)
+		      ;; Avoid changing dir mtime by lock_file
+		      (create-lockfiles nil))
 		  (condition-case err
 		      (if (fboundp extractor)
 			  (funcall extractor archive ename)

bug#30215: Visiting files from zip archives should not modify directory time

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 30215@debbugs.gnu.org
> Date: Wed, 24 Jan 2018 23:36:10 +0200
> 
> > If that's the problem, then I think it would be cleaner to disable
> > creation of lock files and backup files by less subtle means -- by
> > let-binding create-lockfiles and make-backup-files.  Doing that by
> > leaving buffer-file-truename at nil leaves the code less clear IMO,
> > and relies on assumptions that might not hold at some future point.
> 
> Thanks for the suggestion, this is better indeed:
> 
> diff --git a/lisp/arc-mode.el b/lisp/arc-mode.el
> index adb3669..4fe1b03 100644
> --- a/lisp/arc-mode.el
> +++ b/lisp/arc-mode.el
> @@ -807,7 +807,7 @@ archive-summarize
>  Optional argument SHUT-UP, if non-nil, means don't print messages
>  when parsing the archive."
>    (widen)
> -  (let ((buffer-file-truename nil) ; avoid changing dir mtime by lock_file
> +  (let ((create-lockfiles nil) ; avoid changing dir mtime by lock_file

This LGTM, but wasn't the problem also with creation of a backup file?

bug#30215: Visiting files from zip archives should not modify directory time

[Juri Linkov]

>> > If that's the problem, then I think it would be cleaner to disable
>> > creation of lock files and backup files by less subtle means -- by
>> > let-binding create-lockfiles and make-backup-files.  Doing that by
>> > leaving buffer-file-truename at nil leaves the code less clear IMO,
>> > and relies on assumptions that might not hold at some future point.
>> 
>> Thanks for the suggestion, this is better indeed:
>> 
>> diff --git a/lisp/arc-mode.el b/lisp/arc-mode.el
>> index adb3669..4fe1b03 100644
>> --- a/lisp/arc-mode.el
>> +++ b/lisp/arc-mode.el
>> @@ -807,7 +807,7 @@ archive-summarize
>>  Optional argument SHUT-UP, if non-nil, means don't print messages
>>  when parsing the archive."
>>    (widen)
>> -  (let ((buffer-file-truename nil) ; avoid changing dir mtime by lock_file
>> +  (let ((create-lockfiles nil) ; avoid changing dir mtime by lock_file
>
> This LGTM,

Installed.

> but wasn't the problem also with creation of a backup file?

Creation of a backup file is not a problem, because backups are created
on saving.  Visiting a file from a zip archive doesn't save it, and
thus doesn't create a backup.  But when the user explicitly saves
a file inside a zip archive, only then it should modify the time
of its directory.

bug#30215: Visiting files from zip archives should not modify directory time

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 30215-done@debbugs.gnu.org
> Date: Thu, 25 Jan 2018 23:41:10 +0200
> 
> > but wasn't the problem also with creation of a backup file?
> 
> Creation of a backup file is not a problem, because backups are created
> on saving.

What about auto-saving in general, and the new auto-save-visited-mode
in particular?

bug#30215: Visiting files from zip archives should not modify directory time

[Juri Linkov]

>> Creation of a backup file is not a problem, because backups are created
>> on saving.
>
> What about auto-saving in general, and the new auto-save-visited-mode
> in particular?

The default value of `auto-save-visited-interval' is 5 seconds, but the
time of extracting a file from the archive is measured in microseconds.

BTW, while looking at `auto-save-visited-mode', I noticed that the
docstring of `save-some-buffers' could be improved:

diff --git a/lisp/files.el b/lisp/files.el
index 00622cf..98ad209 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5220,7 +5220,7 @@ save-some-buffers
 This command first saves any buffers where `buffer-save-without-query' is
 non-nil, without asking.
 
-Optional argument (the prefix) non-nil means save all with no questions.
+Optional ARG (the prefix) non-nil means save all with no questions.
 Optional second argument PRED determines which buffers are considered:
 If PRED is nil, all the file-visiting buffers are considered.
 If PRED is t, then certain non-file buffers will also be considered.

bug#30215: Visiting files from zip archives should not modify directory time

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 30215@debbugs.gnu.org
> Date: Sat, 27 Jan 2018 23:16:20 +0200
> 
> >> Creation of a backup file is not a problem, because backups are created
> >> on saving.
> >
> > What about auto-saving in general, and the new auto-save-visited-mode
> > in particular?
> 
> The default value of `auto-save-visited-interval' is 5 seconds, but the
> time of extracting a file from the archive is measured in microseconds.

"Microseconds"?  Does that include very large files? what about
archives on remote machines?

> BTW, while looking at `auto-save-visited-mode', I noticed that the
> docstring of `save-some-buffers' could be improved:

Fixed, thanks.

bug#30215: Visiting files from zip archives should not modify directory time

[Juri Linkov]

>> >> Creation of a backup file is not a problem, because backups are created
>> >> on saving.
>> >
>> > What about auto-saving in general, and the new auto-save-visited-mode
>> > in particular?
>> 
>> The default value of `auto-save-visited-interval' is 5 seconds, but the
>> time of extracting a file from the archive is measured in microseconds.
>
> "Microseconds"?  Does that include very large files? what about
> archives on remote machines?

When enabling ‘auto-save-visited-mode’ doesn't mean the wish to
auto-save everything after 5 seconds of idle time, then
‘auto-save-visited-mode’ should have a filter to exclude some files such
as archives by name or by mode from auto-saving because currently there
is no other way to disable ‘auto-save-visited-mode’ other than stop its
timer.  IOW, this can be addressed in a separate request when necessary.

BTW, I noticed another 2 places that needed the same change and fixed them.