all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] autorevert.el -- revert fix for Windows platform
@ 2007-03-22 11:27 Jari Aalto
  2007-03-22 15:00 ` Chong Yidong
  2007-03-22 22:50 ` Richard Stallman
  0 siblings, 2 replies; 32+ messages in thread
From: Jari Aalto @ 2007-03-22 11:27 UTC (permalink / raw)
  To: emacs-devel


For some reason the `verify-visited-file-modtime' test in
auto-revert-buffers is not enough to notice a file change under
Windows / native Emacs 21.3. Here is patch to compare the bufer size
against files size on disk. It cures the revert problem.

The patch is against CVS 2007-03-22

2007-03-22 Thu Jari Aalto <jari aalto A T cante net>

	* autorevert.el
	(auto-revert-size-equal-p): New function.
	(auto-revert-handler): Use `auto-revert-size-equal-p' to
	test file change as well.

=== modified file 'autorevert.el'
--- autorevert.el       2007-03-22 11:05:43 +0000
+++ autorevert.el       2007-03-22 11:24:06 +0000
@@ -302,7 +302,6 @@
     (auto-revert-buffers)
     (setq auto-revert-tail-mode nil)))

-
 ;;;###autoload
 (defun turn-on-auto-revert-mode ()
   "Turn on Auto-Revert Mode.
@@ -396,6 +395,11 @@
        (not (memq major-mode
                  global-auto-revert-ignore-modes)))))

+(defun auto-revert-size-equal-p ()
+  (let ((bsize (buffer-size))
+        (fsize (nth 7 (file-attributes (buffer-file-name)))))
+    (eq bsize fsize)))
+
 (defun auto-revert-handler ()
   "Revert current buffer, if appropriate.
 This is an internal function used by Auto-Revert Mode."
@@ -405,7 +409,9 @@
            (or (and buffer-file-name
                     (not (file-remote-p buffer-file-name))
                     (file-readable-p buffer-file-name)
-                    (not (verify-visited-file-modtime buffer)))
+                     (or
+                      (not (verify-visited-file-modtime buffer))
+                      (not (auto-revert-size-equal-p))))
                (and (or auto-revert-mode
                         global-auto-revert-non-file-buffers)
                     revert-buffer-function

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-22 11:27 [PATCH] autorevert.el -- revert fix for Windows platform Jari Aalto
@ 2007-03-22 15:00 ` Chong Yidong
  2007-03-22 15:56   ` Lennart Borgman (gmail)
                     ` (2 more replies)
  2007-03-22 22:50 ` Richard Stallman
  1 sibling, 3 replies; 32+ messages in thread
From: Chong Yidong @ 2007-03-22 15:00 UTC (permalink / raw)
  To: Jari Aalto; +Cc: emacs-devel

Jari Aalto <jari.aalto@cante.net> writes:

> For some reason the `verify-visited-file-modtime' test in
> auto-revert-buffers is not enough to notice a file change under
> Windows / native Emacs 21.3. Here is patch to compare the bufer size
> against files size on disk. It cures the revert problem.

That's not foolproof, since changes need not alter the file size.

Could you try to find out why verify-visited-file-modtime doesn't work
on Windows?  You said that the problem exists on Emacs 21---does it
still exist under Emacs 22?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-22 15:00 ` Chong Yidong
@ 2007-03-22 15:56   ` Lennart Borgman (gmail)
  2007-03-22 17:49     ` Jason Rumney
  2007-03-23  7:03   ` Jari Aalto
  2007-03-23 13:25   ` Eli Zaretskii
  2 siblings, 1 reply; 32+ messages in thread
From: Lennart Borgman (gmail) @ 2007-03-22 15:56 UTC (permalink / raw)
  To: Chong Yidong; +Cc: emacs-devel, Jari Aalto

Chong Yidong wrote:
> Jari Aalto <jari.aalto@cante.net> writes:
> 
>> For some reason the `verify-visited-file-modtime' test in
>> auto-revert-buffers is not enough to notice a file change under
>> Windows / native Emacs 21.3. Here is patch to compare the bufer size
>> against files size on disk. It cures the revert problem.
> 
> That's not foolproof, since changes need not alter the file size.
> 
> Could you try to find out why verify-visited-file-modtime doesn't work
> on Windows?  You said that the problem exists on Emacs 21---does it
> still exist under Emacs 22?


I did a test on latest CVS Emacs on w32:

- opened a file in Emacs.
- (verify-visited-file-modetime (current-buffer)) => t
- did "touch file" (outside of Emacs)

Now I got

- (verify-visited-file-modetime (current-buffer)) => nil

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-22 15:56   ` Lennart Borgman (gmail)
@ 2007-03-22 17:49     ` Jason Rumney
  2007-03-23 13:26       ` Eli Zaretskii
  0 siblings, 1 reply; 32+ messages in thread
From: Jason Rumney @ 2007-03-22 17:49 UTC (permalink / raw)
  To: Lennart Borgman (gmail); +Cc: Chong Yidong, Jari Aalto, emacs-devel

Lennart Borgman (gmail) wrote:
> I did a test on latest CVS Emacs on w32:
>
> - opened a file in Emacs.
> - (verify-visited-file-modetime (current-buffer)) => t
> - did "touch file" (outside of Emacs)
>
> Now I got
>
> - (verify-visited-file-modetime (current-buffer)) => nil

Looking at the code, this will probably fail on network drives that are 
accessed by UNC paths. The stat information is populated with dummy data 
in that case. I'm sure there is a reason for this, but maybe it will be 
possible to at least get the modification timestamp right.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-22 11:27 [PATCH] autorevert.el -- revert fix for Windows platform Jari Aalto
  2007-03-22 15:00 ` Chong Yidong
@ 2007-03-22 22:50 ` Richard Stallman
  1 sibling, 0 replies; 32+ messages in thread
From: Richard Stallman @ 2007-03-22 22:50 UTC (permalink / raw)
  To: Jari Aalto; +Cc: emacs-devel

Is this change needed in the Emacs in CVS?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-22 15:00 ` Chong Yidong
  2007-03-22 15:56   ` Lennart Borgman (gmail)
@ 2007-03-23  7:03   ` Jari Aalto
  2007-03-23 14:10     ` Eli Zaretskii
  2007-03-23 13:25   ` Eli Zaretskii
  2 siblings, 1 reply; 32+ messages in thread
From: Jari Aalto @ 2007-03-23  7:03 UTC (permalink / raw)
  To: emacs-devel

Chong Yidong <cyd@stupidchicken.com> writes:

> Jari Aalto <jari.aalto@cante.net> writes:
> 
> > For some reason the `verify-visited-file-modtime' test in
> > auto-revert-buffers is not enough to notice a file change under
> > Windows / native Emacs 21.3. Here is patch to compare the bufer size
> > against files size on disk. It cures the revert problem.
> 
> That's not foolproof, since changes need not alter the file size.

True. Do you know any use case of such a situation?

> Could you try to find out why verify-visited-file-modtime doesn't
> work on Windows? You said that the problem exists on Emacs 21---does
> it still exist under Emacs 22?

I could test more if you point me to a location where I can download
native windows version of 22.x

I downloaded only the lisp code from CVS to submit a patch that would apply
cleanly.

Jari

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-22 15:00 ` Chong Yidong
  2007-03-22 15:56   ` Lennart Borgman (gmail)
  2007-03-23  7:03   ` Jari Aalto
@ 2007-03-23 13:25   ` Eli Zaretskii
  2 siblings, 0 replies; 32+ messages in thread
From: Eli Zaretskii @ 2007-03-23 13:25 UTC (permalink / raw)
  To: Chong Yidong; +Cc: emacs-devel, jari.aalto

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Thu, 22 Mar 2007 11:00:32 -0400
> Cc: emacs-devel@gnu.org
> 
> Jari Aalto <jari.aalto@cante.net> writes:
> 
> > For some reason the `verify-visited-file-modtime' test in
> > auto-revert-buffers is not enough to notice a file change under
> > Windows / native Emacs 21.3. Here is patch to compare the bufer size
> > against files size on disk. It cures the revert problem.
> 
> That's not foolproof, since changes need not alter the file size.

Right, it's certainly not a good idea to install such a kludgey
solution.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-22 17:49     ` Jason Rumney
@ 2007-03-23 13:26       ` Eli Zaretskii
  2007-03-23 17:31         ` Jari Aalto
  0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2007-03-23 13:26 UTC (permalink / raw)
  To: Jason Rumney; +Cc: cyd, emacs-devel, lennart.borgman, jari.aalto

> Date: Thu, 22 Mar 2007 17:49:21 +0000
> From: Jason Rumney <jasonr@gnu.org>
> Cc: Chong Yidong <cyd@stupidchicken.com>, Jari Aalto <jari.aalto@cante.net>,
> 	emacs-devel@gnu.org
> 
> Looking at the code, this will probably fail on network drives that are 
> accessed by UNC paths.

Jari, can you confirm that this problem happens for you only with UNC
style file names?

If not, please provide a self-contained test case that we could try to
see what is going wrong.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23  7:03   ` Jari Aalto
@ 2007-03-23 14:10     ` Eli Zaretskii
  2007-03-23 15:47       ` Kim F. Storm
  0 siblings, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2007-03-23 14:10 UTC (permalink / raw)
  To: Jari Aalto; +Cc: emacs-devel

> From: Jari Aalto <jari.aalto@cante.net>
> Date: 23 Mar 2007 09:03:20 +0200
> 
> > That's not foolproof, since changes need not alter the file size.
> 
> True. Do you know any use case of such a situation?

Yes: replace one characters with another, and make no other changes.

> > Could you try to find out why verify-visited-file-modtime doesn't
> > work on Windows? You said that the problem exists on Emacs 21---does
> > it still exist under Emacs 22?
> 
> I could test more if you point me to a location where I can download
> native windows version of 22.x

Here's one place:

  http://ourcomments.org/Emacs/EmacsW32SetupUtilities.html

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 14:10     ` Eli Zaretskii
@ 2007-03-23 15:47       ` Kim F. Storm
  2007-03-23 16:51         ` Jason Rumney
                           ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Kim F. Storm @ 2007-03-23 15:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, Jari Aalto

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jari Aalto <jari.aalto@cante.net>
>> Date: 23 Mar 2007 09:03:20 +0200
>> 
>> > That's not foolproof, since changes need not alter the file size.
>> 
>> True. Do you know any use case of such a situation?
>
> Yes: replace one characters with another, and make no other changes.

How often does an external file change in that way?

FWIW, auto-revert mode is often used to track changes to files that do
change size, e.g. log file, compilation output, etc, and in such cases,
Jari's patch is a significant improvement, even if it is not a general
solution to the problem.

For the release, I'd say Jari's patch is good enough!

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 15:47       ` Kim F. Storm
@ 2007-03-23 16:51         ` Jason Rumney
  2007-03-23 18:48           ` Luc Teirlinck
  2007-03-23 22:41           ` Stefan Monnier
  2007-03-23 16:57         ` Stefan Monnier
  2007-03-24 14:49         ` Eli Zaretskii
  2 siblings, 2 replies; 32+ messages in thread
From: Jason Rumney @ 2007-03-23 16:51 UTC (permalink / raw)
  To: Kim F. Storm; +Cc: Eli Zaretskii, Jari Aalto, emacs-devel

Kim F. Storm wrote:
> For the release, I'd say Jari's patch is good enough!
>   

I'm not sure, do we really expect buffer-size to be the same as the 
number of bytes in the file, potentially in the presence of multibyte 
characters? This change is quite likely to break things if that 
assumption is not correct.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 15:47       ` Kim F. Storm
  2007-03-23 16:51         ` Jason Rumney
@ 2007-03-23 16:57         ` Stefan Monnier
  2007-03-24 14:49         ` Eli Zaretskii
  2 siblings, 0 replies; 32+ messages in thread
From: Stefan Monnier @ 2007-03-23 16:57 UTC (permalink / raw)
  To: Kim F. Storm; +Cc: Eli Zaretskii, Jari Aalto, emacs-devel

> For the release, I'd say Jari's patch is good enough!

Agreed.


        Stefan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 13:26       ` Eli Zaretskii
@ 2007-03-23 17:31         ` Jari Aalto
  2007-03-23 20:36           ` Jason Rumney
  2007-03-24 14:46           ` Eli Zaretskii
  0 siblings, 2 replies; 32+ messages in thread
From: Jari Aalto @ 2007-03-23 17:31 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:
> > Looking at the code, this will probably fail on network drives
> > that are accessed by UNC paths.
> 
> Jari, can you confirm that this problem happens for you only with
> UNC style file names?

This happens in local PC disk. No UNC involved.

> If not, please provide a self-contained test case that we could try to
> see what is going wrong.

Have some process that keeps a file open while the data is being
written to it. The file is never closed (only if process terminated).
This is happens e.g with firewalls(*) that do not close the file in
between log writes.

Jari

(*) In Windows:
1. Download Jetico Personal Firewall 2.x, install, configure, start 
   http://www.jetico.com/ (Free)
2. Open Emacs, M-x global-auto-revert-mode
3. Load Jetico's firewall log C-x C-f path/to/jpfsrv.log

Watch the file grow on disk (Windows Eplorer), but no revert happens
in Emacs buffer.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 16:51         ` Jason Rumney
@ 2007-03-23 18:48           ` Luc Teirlinck
  2007-03-24 14:59             ` Eli Zaretskii
  2007-03-23 22:41           ` Stefan Monnier
  1 sibling, 1 reply; 32+ messages in thread
From: Luc Teirlinck @ 2007-03-23 18:48 UTC (permalink / raw)
  To: jasonr; +Cc: eliz, jari.aalto, emacs-devel, storm

Jason Rumney wrote:

   I'm not sure, do we really expect buffer-size to be the same as the 
   number of bytes in the file, potentially in the presence of multibyte 
   characters?

Of course not and I actually got files for which the two are very
different.

   This change is quite likely to break things if that 
   assumption is not correct.

Could cause 100% CPU usage and continuous overshouting of other
minibuffer messages with revert messages, making Emacs completely
unusable.

On top of that, if Emacs has trouble checking when a file has changed
on disk on MS Windows, then how is this a problem that is specific to
Auto Revert Mode?  What about any other part of Emacs that needs to
know this?

Sincerely,

Luc Teirlinck.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 17:31         ` Jari Aalto
@ 2007-03-23 20:36           ` Jason Rumney
  2007-03-23 23:07             ` Jason Rumney
  2007-03-24 18:21             ` Eli Zaretskii
  2007-03-24 14:46           ` Eli Zaretskii
  1 sibling, 2 replies; 32+ messages in thread
From: Jason Rumney @ 2007-03-23 20:36 UTC (permalink / raw)
  To: Jari Aalto; +Cc: emacs-devel

Jari Aalto wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
>   
>>> Looking at the code, this will probably fail on network drives
>>> that are accessed by UNC paths.
>>>       
>> Jari, can you confirm that this problem happens for you only with
>> UNC style file names?
>>     
>
> This happens in local PC disk. No UNC involved.
>   

I went back and looked at the code I thought looked suspicious, and it 
turns out it only applies to the root of a UNC shared drive, not normal 
files on shared drives.

I will try to find out if other methods can be used that give more 
reliable modification timestamps.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 16:51         ` Jason Rumney
  2007-03-23 18:48           ` Luc Teirlinck
@ 2007-03-23 22:41           ` Stefan Monnier
  2007-03-24 23:50             ` Kim F. Storm
  1 sibling, 1 reply; 32+ messages in thread
From: Stefan Monnier @ 2007-03-23 22:41 UTC (permalink / raw)
  To: Jason Rumney; +Cc: Eli Zaretskii, Jari Aalto, emacs-devel, Kim F. Storm

>> For the release, I'd say Jari's patch is good enough!

> I'm not sure, do we really expect buffer-size to be the same as the number
> of bytes in the file, potentially in the presence of multibyte characters?
> This change is quite likely to break things if that assumption is
> not correct.

Duh, I didn't even look at the patch.  So no clearly the patch is
unacceptable as is.  But the principle of checking whether the file changes
size is the thing I agreed to.


        Stefan

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 20:36           ` Jason Rumney
@ 2007-03-23 23:07             ` Jason Rumney
  2007-03-24 10:52               ` martin rudalics
  2007-03-24 18:21             ` Eli Zaretskii
  1 sibling, 1 reply; 32+ messages in thread
From: Jason Rumney @ 2007-03-23 23:07 UTC (permalink / raw)
  To: Jason Rumney; +Cc: emacs-devel, Jari Aalto

It seems to me that auto-revert-tail-mode already exists to deal with 
this specific use case.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 23:07             ` Jason Rumney
@ 2007-03-24 10:52               ` martin rudalics
  2007-03-24 11:11                 ` Jason Rumney
  0 siblings, 1 reply; 32+ messages in thread
From: martin rudalics @ 2007-03-24 10:52 UTC (permalink / raw)
  To: Jason Rumney; +Cc: Jari Aalto, emacs-devel

 > It seems to me that auto-revert-tail-mode already exists to deal with
 > this specific use case.

Doesn't this

(defun auto-revert-tail-handler ()
   (let ((size (nth 7 (file-attributes buffer-file-name)))
     ...
     (when (> size auto-revert-tail-pos)
           ...
	  (insert-file-contents file nil auto-revert-tail-pos size)))
       (undo-boundary)
       (setq auto-revert-tail-pos size)
       ...

suffer from the buffer-size file-size dichotomy as well?  Or are we on
the safe side here?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-24 10:52               ` martin rudalics
@ 2007-03-24 11:11                 ` Jason Rumney
  2007-03-24 11:25                   ` Johan Bockgård
  2007-03-24 11:32                   ` martin rudalics
  0 siblings, 2 replies; 32+ messages in thread
From: Jason Rumney @ 2007-03-24 11:11 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel, Jari Aalto

martin rudalics wrote:

> Doesn't this
>
> (defun auto-revert-tail-handler ()
>   (let ((size (nth 7 (file-attributes buffer-file-name)))
>     ...
>     (when (> size auto-revert-tail-pos)
>           ...
>       (insert-file-contents file nil auto-revert-tail-pos size)))
>       (undo-boundary)
>       (setq auto-revert-tail-pos size)
>       ...
>
> suffer from the buffer-size file-size dichotomy as well?  Or are we on
> the safe side here?

No. buffer-size is not used, both auto-revert-tail-pos and size are 
measuring bytes in the file, not characters in the buffer.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-24 11:11                 ` Jason Rumney
@ 2007-03-24 11:25                   ` Johan Bockgård
  2007-03-24 11:46                     ` Jason Rumney
  2007-03-24 11:32                   ` martin rudalics
  1 sibling, 1 reply; 32+ messages in thread
From: Johan Bockgård @ 2007-03-24 11:25 UTC (permalink / raw)
  To: emacs-devel

Jason Rumney <jasonr@gnu.org> writes:

> No. buffer-size is not used, both auto-revert-tail-pos and size are
> measuring bytes in the file, not characters in the buffer.

There are however two occurrences of

      (set (make-local-variable 'auto-revert-tail-pos)
            (save-restriction (widen) (1- (point-max))))

in autorevert.el.

-- 
Johan Bockgård

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-24 11:11                 ` Jason Rumney
  2007-03-24 11:25                   ` Johan Bockgård
@ 2007-03-24 11:32                   ` martin rudalics
  1 sibling, 0 replies; 32+ messages in thread
From: martin rudalics @ 2007-03-24 11:32 UTC (permalink / raw)
  To: Jason Rumney; +Cc: emacs-devel, Jari Aalto

 > No. buffer-size is not used, both auto-revert-tail-pos and size are
 > measuring bytes in the file, not characters in the buffer.

But `auto-revert-tail-mode' has this:

       (or (local-variable-p 'auto-revert-tail-pos) ; don't lose prior position
	  (set (make-local-variable 'auto-revert-tail-pos)
	       (save-restriction (widen) (1- (point-max)))))

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-24 11:25                   ` Johan Bockgård
@ 2007-03-24 11:46                     ` Jason Rumney
  0 siblings, 0 replies; 32+ messages in thread
From: Jason Rumney @ 2007-03-24 11:46 UTC (permalink / raw)
  To: emacs-devel

Johan Bockgård wrote:
> Jason Rumney <jasonr@gnu.org> writes:
>
>   
>> No. buffer-size is not used, both auto-revert-tail-pos and size are
>> measuring bytes in the file, not characters in the buffer.
>>     
>
> There are however two occurrences of
>
>       (set (make-local-variable 'auto-revert-tail-pos)
>             (save-restriction (widen) (1- (point-max))))
>
> in autorevert.el.
>   

I think this patch should fix that. Though maybe we need to handle a 
user trying to use this mode on a buffer without an associated file.


*** autorevert.el    26 Jan 2007 22:30:51 +0000    1.56
--- autorevert.el    24 Mar 2007 11:42:53 +0000   
***************
*** 278,284 ****
  (add-hook 'find-file-hook
        (lambda ()
          (set (make-local-variable 'auto-revert-tail-pos)
!          (save-restriction (widen) (1- (point-max))))))
 
  ;; Functions:
 
--- 278,284 ----
  (add-hook 'find-file-hook
        (lambda ()
          (set (make-local-variable 'auto-revert-tail-pos)
!          (nth 7 (file-attributes buffer-file-name)))))
 
  ;; Functions:
 
***************
*** 341,347 ****
        (add-hook 'before-save-hook (lambda () (auto-revert-tail-mode 
0)) nil t)
        (or (local-variable-p 'auto-revert-tail-pos) ; don't lose prior 
position
        (set (make-local-variable 'auto-revert-tail-pos)
!            (save-restriction (widen) (1- (point-max)))))
        ;; let auto-revert-mode set up the mechanism for us if it isn't 
already
        (or auto-revert-mode
        (let ((auto-revert-tail-mode t))
--- 341,347 ----
        (add-hook 'before-save-hook (lambda () (auto-revert-tail-mode 
0)) nil t)
        (or (local-variable-p 'auto-revert-tail-pos) ; don't lose prior 
position
        (set (make-local-variable 'auto-revert-tail-pos)
!            (nth 7 (file-attributes buffer-file-name))))
        ;; let auto-revert-mode set up the mechanism for us if it isn't 
already
        (or auto-revert-mode
        (let ((auto-revert-tail-mode t))

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 17:31         ` Jari Aalto
  2007-03-23 20:36           ` Jason Rumney
@ 2007-03-24 14:46           ` Eli Zaretskii
  1 sibling, 0 replies; 32+ messages in thread
From: Eli Zaretskii @ 2007-03-24 14:46 UTC (permalink / raw)
  To: Jari Aalto; +Cc: emacs-devel

> From: Jari Aalto <jari.aalto@cante.net>
> Date: 23 Mar 2007 19:31:41 +0200
> 
> Have some process that keeps a file open while the data is being
> written to it. The file is never closed (only if process terminated).
> This is happens e.g with firewalls(*) that do not close the file in
> between log writes.
> 
> Jari
> 
> (*) In Windows:
> 1. Download Jetico Personal Firewall 2.x, install, configure, start 
>    http://www.jetico.com/ (Free)
> 2. Open Emacs, M-x global-auto-revert-mode
> 3. Load Jetico's firewall log C-x C-f path/to/jpfsrv.log
> 
> Watch the file grow on disk (Windows Eplorer), but no revert happens
> in Emacs buffer.

In what ways is this specific to MS-Windows?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 15:47       ` Kim F. Storm
  2007-03-23 16:51         ` Jason Rumney
  2007-03-23 16:57         ` Stefan Monnier
@ 2007-03-24 14:49         ` Eli Zaretskii
  2 siblings, 0 replies; 32+ messages in thread
From: Eli Zaretskii @ 2007-03-24 14:49 UTC (permalink / raw)
  To: Kim F. Storm; +Cc: emacs-devel, jari.aalto

> Cc: Jari Aalto <jari.aalto@cante.net>,  emacs-devel@gnu.org
> From: storm@cua.dk (Kim F. Storm)
> Date: Fri, 23 Mar 2007 16:47:42 +0100
> 
> For the release, I'd say Jari's patch is good enough!

I'm not convinced that a better patch is impossible.

Jari's patch tries to work around a problem that we do not yet
understand.  When we do understand it, then we could reason whether
his patch is the best solution we can reasonably provide.  Until then,
arguing about the patch is just hand-waiving without any real basis.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 18:48           ` Luc Teirlinck
@ 2007-03-24 14:59             ` Eli Zaretskii
  0 siblings, 0 replies; 32+ messages in thread
From: Eli Zaretskii @ 2007-03-24 14:59 UTC (permalink / raw)
  To: Luc Teirlinck; +Cc: jari.aalto, emacs-devel, storm, jasonr

> Date: Fri, 23 Mar 2007 13:48:01 -0500 (CDT)
> From: Luc Teirlinck <teirllm@dms.auburn.edu>
> Cc: eliz@gnu.org, jari.aalto@cante.net, emacs-devel@gnu.org, storm@cua.dk
> 
> On top of that, if Emacs has trouble checking when a file has changed
> on disk on MS Windows, then how is this a problem that is specific to
> Auto Revert Mode?

You are right: the proper change to fix this is in `stat' and other
related functions used by Emacs primitives to check for file changes.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 20:36           ` Jason Rumney
  2007-03-23 23:07             ` Jason Rumney
@ 2007-03-24 18:21             ` Eli Zaretskii
  2007-03-24 22:56               ` Kim F. Storm
  1 sibling, 1 reply; 32+ messages in thread
From: Eli Zaretskii @ 2007-03-24 18:21 UTC (permalink / raw)
  To: Jason Rumney; +Cc: emacs-devel, jari.aalto

> Date: Fri, 23 Mar 2007 20:36:45 +0000
> From: Jason Rumney <jasonr@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> I will try to find out if other methods can be used that give more 
> reliable modification timestamps.

My limited testing seems to indicate that, for a newly created file,
as long as it is not closed by the application that writes to it, all
3 of the file's times are identical, set to the time the file was
created, and do not change.  Only when the file is closed, the
last-write and last-access times are updated.

This is on Windows XP with NTFS volumes.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-24 18:21             ` Eli Zaretskii
@ 2007-03-24 22:56               ` Kim F. Storm
  2007-03-25  4:13                 ` Eli Zaretskii
  0 siblings, 1 reply; 32+ messages in thread
From: Kim F. Storm @ 2007-03-24 22:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: jari.aalto, emacs-devel, Jason Rumney


IIRC, on Windows 98 with VFAT, the size of a newly created file
was 0 until the file was closed.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-23 22:41           ` Stefan Monnier
@ 2007-03-24 23:50             ` Kim F. Storm
  2007-03-25  0:24               ` Luc Teirlinck
  0 siblings, 1 reply; 32+ messages in thread
From: Kim F. Storm @ 2007-03-24 23:50 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, Jari Aalto, emacs-devel, Jason Rumney

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> For the release, I'd say Jari's patch is good enough!
>
>> I'm not sure, do we really expect buffer-size to be the same as the number
>> of bytes in the file, potentially in the presence of multibyte characters?
>> This change is quite likely to break things if that assumption is
>> not correct.

>
> Duh, I didn't even look at the patch.  So no clearly the patch is
> unacceptable as is.  But the principle of checking whether the file changes
> size is the thing I agreed to.

Me too.

I didn't think deeply about the implications of using (buffer-size)
directly, but what about using (position-bytes (buffer-size)) instead?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-24 23:50             ` Kim F. Storm
@ 2007-03-25  0:24               ` Luc Teirlinck
  2007-03-25  1:09                 ` Kim F. Storm
  0 siblings, 1 reply; 32+ messages in thread
From: Luc Teirlinck @ 2007-03-25  0:24 UTC (permalink / raw)
  To: storm; +Cc: eliz, emacs-devel, jasonr, monnier, jari.aalto

Kim Storm wrote:

   I didn't think deeply about the implications of using (buffer-size)
   directly, but what about using (position-bytes (buffer-size)) instead?

Just as wrong, and again, I have actually files for which
(position-bytes (buffer-size)) is very different from the size of the
file on disk.  The internal contents of an Emacs buffer are _not_ the
same as the contents of the file on disk, unless `find-file-literally'
was used (see for instance `(elisp)Coding Systems').

Sincerely,

Luc Teirlinck.
 LocalWords:  elisp

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-25  0:24               ` Luc Teirlinck
@ 2007-03-25  1:09                 ` Kim F. Storm
  2007-03-25 19:09                   ` Stefan Monnier
  0 siblings, 1 reply; 32+ messages in thread
From: Kim F. Storm @ 2007-03-25  1:09 UTC (permalink / raw)
  To: Luc Teirlinck; +Cc: jari.aalto, eliz, jasonr, monnier, emacs-devel

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Kim Storm wrote:
>
>    I didn't think deeply about the implications of using (buffer-size)
>    directly, but what about using (position-bytes (buffer-size)) instead?
>
> Just as wrong, and again, I have actually files for which
> (position-bytes (buffer-size)) is very different from the size of the
> file on disk.  The internal contents of an Emacs buffer are _not_ the
> same as the contents of the file on disk, unless `find-file-literally'
> was used (see for instance `(elisp)Coding Systems').

I see, thanks!

Still, I don't see why auto-revert-mode cannot just track changes
to the file size, just like auto-revert-tail-mode does (i.e. store
the previous file size in buffer-local var auto-revert-tail-pos).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-24 22:56               ` Kim F. Storm
@ 2007-03-25  4:13                 ` Eli Zaretskii
  0 siblings, 0 replies; 32+ messages in thread
From: Eli Zaretskii @ 2007-03-25  4:13 UTC (permalink / raw)
  To: Kim F. Storm; +Cc: jari.aalto, emacs-devel, jasonr

> Cc: Jason Rumney <jasonr@gnu.org>,  emacs-devel@gnu.org,  jari.aalto@cante.net
> From: storm@cua.dk (Kim F. Storm)
> Date: Sat, 24 Mar 2007 23:56:31 +0100
> 
> IIRC, on Windows 98 with VFAT, the size of a newly created file
> was 0 until the file was closed.

Yes, I think so.  I don't have an easy access to Windows 9x anymore,
but I can test on a FAT32 volume if necessary.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] autorevert.el -- revert fix for Windows platform
  2007-03-25  1:09                 ` Kim F. Storm
@ 2007-03-25 19:09                   ` Stefan Monnier
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Monnier @ 2007-03-25 19:09 UTC (permalink / raw)
  To: Kim F. Storm; +Cc: jari.aalto, eliz, jasonr, Luc Teirlinck, emacs-devel

> Still, I don't see why auto-revert-mode cannot just track changes
> to the file size, just like auto-revert-tail-mode does (i.e. store
> the previous file size in buffer-local var auto-revert-tail-pos).

I'd argue that we should replace buffer-file-number with
buffer-file-attributes.


        Stefan

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2007-03-25 19:09 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-22 11:27 [PATCH] autorevert.el -- revert fix for Windows platform Jari Aalto
2007-03-22 15:00 ` Chong Yidong
2007-03-22 15:56   ` Lennart Borgman (gmail)
2007-03-22 17:49     ` Jason Rumney
2007-03-23 13:26       ` Eli Zaretskii
2007-03-23 17:31         ` Jari Aalto
2007-03-23 20:36           ` Jason Rumney
2007-03-23 23:07             ` Jason Rumney
2007-03-24 10:52               ` martin rudalics
2007-03-24 11:11                 ` Jason Rumney
2007-03-24 11:25                   ` Johan Bockgård
2007-03-24 11:46                     ` Jason Rumney
2007-03-24 11:32                   ` martin rudalics
2007-03-24 18:21             ` Eli Zaretskii
2007-03-24 22:56               ` Kim F. Storm
2007-03-25  4:13                 ` Eli Zaretskii
2007-03-24 14:46           ` Eli Zaretskii
2007-03-23  7:03   ` Jari Aalto
2007-03-23 14:10     ` Eli Zaretskii
2007-03-23 15:47       ` Kim F. Storm
2007-03-23 16:51         ` Jason Rumney
2007-03-23 18:48           ` Luc Teirlinck
2007-03-24 14:59             ` Eli Zaretskii
2007-03-23 22:41           ` Stefan Monnier
2007-03-24 23:50             ` Kim F. Storm
2007-03-25  0:24               ` Luc Teirlinck
2007-03-25  1:09                 ` Kim F. Storm
2007-03-25 19:09                   ` Stefan Monnier
2007-03-23 16:57         ` Stefan Monnier
2007-03-24 14:49         ` Eli Zaretskii
2007-03-23 13:25   ` Eli Zaretskii
2007-03-22 22:50 ` Richard Stallman

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.