bug#694: 23.0.60; wrong behaviour of vc-workfile-unchanged-p

bug#694: 23.0.60; wrong behaviour of vc-workfile-unchanged-p

[Alex Harsanyi]

(I posted this on emacs-devel in December 2007, the problem is still  
present
in the latest CVS version)

When calling `vc-next-action' (C-x v v) on a file which has no  
changes, vc.el
will revert to the master version for RCS or say that the file is up  
to date
for CVS.  Unfortunately, this does not work corectly when the file is
unchanged but its timestamp is more recent than the checkout time.   
Consider
this scenario:

  1/ Create a new file and register it with RCS

  2/ Checkout the file (C-x v v).

  3/ Type C-x v v again.  Emacs will revert to the master version,  
since the
  file is unchanged.

  4/ Modify the file, save it, undo the changes and save it again.   
This will
  have the effect that the file will have the same contents as when  
checked
  out but a more recent modification time.

  5/ Type C-x v v again.  Emacs will pop-up the VC-Log buffer and  
checkin the
  file.

The problem seems to be in `vc-workfile-unchanged-p': if the checkout  
time is
available for a file, it is compared against the modification time  
and the
"unchanged " decision is made on that alone, without consulting the  
backed
specific function (vc-rcs-workfile-unchanged-p or
vc-default-workfile-unchanged-p)

I believe vc-workfile-unchanged-p and if the two timestamsps are the  
same, it
can conclude that the file is unchanged, but if the timestamps are  
different,
it should call the backend specific workfile-unchanged-p function.

The following patch is one way to fix the problem:

--- vc-hooks.el.~1.261.~	2008-07-25 17:08:10.000000000 +0800
+++ vc-hooks.el	2008-08-11 18:22:04.000000000 +0800
@@ -561,8 +561,9 @@ and does not employ any heuristic at all
          (lastmod (nth 5 (file-attributes file))))
      (if (and checkout-time
               ;; Tramp and Ange-FTP return this when they don't know  
the time.
-             (not (equal lastmod '(0 0))))
-        (equal checkout-time lastmod)
+             (not (equal lastmod '(0 0)))
+             (equal checkout-time lastmod))
+        t
        (let ((unchanged (vc-call workfile-unchanged-p file)))
          (vc-file-setprop file 'vc-checkout-time (if unchanged  
lastmod 0))
          unchanged))))

Best Regards,
Alex.

bug#694: marked as done (23.0.60; wrong behaviour of vc-workfile-unchanged-p)

[Emacs bug Tracking System]

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


Your message dated Wed, 27 Aug 2008 16:42:02 -0400
with message-id <87ljyiyyud.fsf@cyd.mit.edu>
and subject line Re: 23.0.60; wrong behaviour of vc-workfile-unchanged-p
has caused the Emacs bug report #694,
regarding 23.0.60; wrong behaviour of vc-workfile-unchanged-p
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
694: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=694
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 4459 bytes --]

From: Alex Harsanyi <harsanyi@mac.com>
To: emacs-pretest-bug@gnu.org
Subject: 23.0.60; wrong behaviour of vc-workfile-unchanged-p
Date: Mon, 11 Aug 2008 18:24:43 +0800
Message-ID: <D1E985FF-F665-40A8-BAF9-ED9800EAD336@mac.com>

(I posted this on emacs-devel in December 2007, the problem is still  
present
in the latest CVS version)

When calling `vc-next-action' (C-x v v) on a file which has no  
changes, vc.el
will revert to the master version for RCS or say that the file is up  
to date
for CVS.  Unfortunately, this does not work corectly when the file is
unchanged but its timestamp is more recent than the checkout time.   
Consider
this scenario:

  1/ Create a new file and register it with RCS

  2/ Checkout the file (C-x v v).

  3/ Type C-x v v again.  Emacs will revert to the master version,  
since the
  file is unchanged.

  4/ Modify the file, save it, undo the changes and save it again.   
This will
  have the effect that the file will have the same contents as when  
checked
  out but a more recent modification time.

  5/ Type C-x v v again.  Emacs will pop-up the VC-Log buffer and  
checkin the
  file.

The problem seems to be in `vc-workfile-unchanged-p': if the checkout  
time is
available for a file, it is compared against the modification time  
and the
"unchanged " decision is made on that alone, without consulting the  
backed
specific function (vc-rcs-workfile-unchanged-p or
vc-default-workfile-unchanged-p)

I believe vc-workfile-unchanged-p and if the two timestamsps are the  
same, it
can conclude that the file is unchanged, but if the timestamps are  
different,
it should call the backend specific workfile-unchanged-p function.

The following patch is one way to fix the problem:

--- vc-hooks.el.~1.261.~	2008-07-25 17:08:10.000000000 +0800
+++ vc-hooks.el	2008-08-11 18:22:04.000000000 +0800
@@ -561,8 +561,9 @@ and does not employ any heuristic at all
          (lastmod (nth 5 (file-attributes file))))
      (if (and checkout-time
               ;; Tramp and Ange-FTP return this when they don't know  
the time.
-             (not (equal lastmod '(0 0))))
-        (equal checkout-time lastmod)
+             (not (equal lastmod '(0 0)))
+             (equal checkout-time lastmod))
+        t
        (let ((unchanged (vc-call workfile-unchanged-p file)))
          (vc-file-setprop file 'vc-checkout-time (if unchanged  
lastmod 0))
          unchanged))))

Best Regards,
Alex.




[-- Attachment #3: Type: message/rfc822, Size: 1632 bytes --]

From: Chong Yidong <cyd@stupidchicken.com>
To: Alex Harsanyi <harsanyi@mac.com>
Cc: 694-done@emacsbugs.donarmstrong.com
Subject: Re: 23.0.60; wrong behaviour of vc-workfile-unchanged-p
Date: Wed, 27 Aug 2008 16:42:02 -0400
Message-ID: <87ljyiyyud.fsf@cyd.mit.edu>

> The problem seems to be in `vc-workfile-unchanged-p': if the checkout
> time is available for a file, it is compared against the modification
> time and the "unchanged " decision is made on that alone, without
> consulting the backed specific function (vc-rcs-workfile-unchanged-p or
> vc-default-workfile-unchanged-p)
>
> I believe vc-workfile-unchanged-p and if the two timestamsps are the
> same, it can conclude that the file is unchanged, but if the timestamps
> are different, it should call the backend specific workfile-unchanged-p
> function.

Sounds reasonable.  I've checked in your patch (with a minor
modification) into the CVS trunk.  Thanks.

bug#694: marked as done (23.0.60; wrong behaviour of

[Stefan Monnier]

> I believe vc-workfile-unchanged-p and if the two timestamsps are the
> same, it can conclude that the file is unchanged, but if the
> timestamps are different, it should call the backend specific
> workfile-unchanged-p function.

For CVS if the user requested to `stay-local', this is not quite right
either since it will connect to the server.  Admittedly, there's no way
to get the right answer without connecting to the server, but this
behavior has been with us for years and I usually consider it a feature
(most/all the CVS servers I use are usually remote/slow/unreachable, so
it's important to have control over when VC contacts the server and when
it doesn't).


        Stefan

bug#694: marked as done (23.0.60; wrong behaviour of

[Chong Yidong]

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

>> I believe vc-workfile-unchanged-p and if the two timestamsps are the
>> same, it can conclude that the file is unchanged, but if the
>> timestamps are different, it should call the backend specific
>> workfile-unchanged-p function.
>
> For CVS if the user requested to `stay-local', this is not quite right
> either since it will connect to the server.  Admittedly, there's no way
> to get the right answer without connecting to the server, but this
> behavior has been with us for years and I usually consider it a feature
> (most/all the CVS servers I use are usually remote/slow/unreachable, so
> it's important to have control over when VC contacts the server and when
> it doesn't).

Should I revert the patch then?

bug#694: marked as done (23.0.60; wrong behaviour of

[Stefan Monnier]

>>> I believe vc-workfile-unchanged-p and if the two timestamsps are the
>>> same, it can conclude that the file is unchanged, but if the
>>> timestamps are different, it should call the backend specific
>>> workfile-unchanged-p function.
>> 
>> For CVS if the user requested to `stay-local', this is not quite right
>> either since it will connect to the server.  Admittedly, there's no way
>> to get the right answer without connecting to the server, but this
>> behavior has been with us for years and I usually consider it a feature
>> (most/all the CVS servers I use are usually remote/slow/unreachable, so
>> it's important to have control over when VC contacts the server and when
>> it doesn't).

> Should I revert the patch then?

I'm not sure, but I'd tend to think so, yes.


        Stefan

bug#694: marked as done (23.0.60; wrong behaviour of

[Chong Yidong]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>>> I believe vc-workfile-unchanged-p and if the two timestamsps are the
>>>> same, it can conclude that the file is unchanged, but if the
>>>> timestamps are different, it should call the backend specific
>>>> workfile-unchanged-p function.
>>> 
>>> For CVS if the user requested to `stay-local', this is not quite right
>>> either since it will connect to the server.  Admittedly, there's no way
>>> to get the right answer without connecting to the server, but this
>>> behavior has been with us for years and I usually consider it a feature
>>> (most/all the CVS servers I use are usually remote/slow/unreachable, so
>>> it's important to have control over when VC contacts the server and when
>>> it doesn't).
>
>> Should I revert the patch then?
>
> I'm not sure, but I'd tend to think so, yes.

Okay, reverted.  I inserted a comment in the code linking to this
discussion.