GNU bug report logs - #11490
vc-next-action overwrites changes in non-checked-out RCS file

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11490 in the body.
You can then email your comments to 11490 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jonathan Kamens <jik <at> kamens.us>
To: bug-gnu-emacs <at> gnu.org
Subject: vc-next-action overwrites changes in non-checked-out RCS file
Date: Wed, 16 May 2012 15:29:07 -0400

[Message part 1 (text/plain, inline)]

GNU Emacs 24.0.95.1 (x86_64-redhat-linux-gnu, GTK+ Version 2.24.10) of 
2012-04-06 on x86-13.phx2.fedoraproject.org

Make an RCS file writable with chmod +w without locking it.

Make changes to the file.

Type C-x v v.

The file will be locked and checked out and your changes will be 
overwritten.

This is Bad, Bad, Bad. It needs to check if there are non-checked-out 
changes and ask whether to preserve them.

It used to do this. I have no idea why it's behaving differently now or 
when it started behaving this way, but it's clearly wrong and dangerous, 
given the potential to lose work.

I lost a whole day of work recently as a result of this bug. Yeah, it 
was user error, but that's not really the point. It's easy to protect 
the user from losing work due to this editor, and it's something that 
Emacs used to do, so it should continue to do it.

  jik

[Message part 2 (text/html, inline)]

Message #8 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Jonathan Kamens <jik <at> kamens.us>
Cc: 11490 <at> debbugs.gnu.org
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Wed, 16 May 2012 16:12:40 -0400

Jonathan Kamens wrote:

> GNU Emacs 24.0.95.1 (x86_64-redhat-linux-gnu, GTK+ Version 2.24.10) of
> 2012-04-06 on x86-13.phx2.fedoraproject.org
>
> Make an RCS file writable with chmod +w without locking it.
>
> Make changes to the file.
>
> Type C-x v v.
>
> The file will be locked and checked out and your changes will be
> overwritten.

I cannot reproduce this. I did:


mkdir foo
cd foo
mkdir RCS
echo initial > file
ci -u -t-foo file
chmod +w file
emacs-24.0.96 -Q file
add some text to file
C-x v v

A log buffer appears. I enter some text and press C-c C-c.
I am told the buffer is modified and prompted to save it. I do so.

At this point, RCS returns an error:

  RCS/file,v  <--  file
  ci: RCS/file,v: no lock set by gmorris for revision 1.1

The contents of the file on disk and in the Emacs buffer are unchanged
(ie, the added text is still present).


This was with RCS 5.7. I never normally use RCS, maybe I am missing
something.

Do you have a recipe starting from emacs -Q that shows the problem?

Message #11 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Jonathan Kamens <jik <at> kamens.us>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 11490 <at> debbugs.gnu.org
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Wed, 16 May 2012 21:17:34 -0400

[Message part 1 (text/plain, inline)]

Try it without making the file writable.

This happens, e.g., if a file is edited as root by a user using vim, 
which doesn't care whether it's writable or not, since it's root doing 
the editing, and then by a user using Emacs, which /does/ care because 
it detects VC and thus makes the file read-only in Emacs. The user's 
first impulse will be to type C-x v v, which will overwrite the previous 
editor's changes.

The problem goes away when vc-mistrust-permissions is set to true.

I would argue that C-x v v should always check the status of a file with 
vc-mistrust-permissions set to true (or the logical equivalent) before 
overwriting its contents and potentially losing data.

  jik

[Message part 2 (text/html, inline)]

Message #14 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Jonathan Kamens <jik <at> kamens.us>
Cc: 11490 <at> debbugs.gnu.org
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Thu, 17 May 2012 20:45:46 -0400

Jonathan Kamens wrote:

> Try it without making the file writable.

Well, ok, but your initial report began:

   Make an RCS file writable with chmod +w without locking it.

If I instead use M-x toggle-read-only, make changes, and use C-x v v,
then I see the problem.

> The problem goes away when vc-mistrust-permissions is set to true.

OK. It looks like that variable only affects RCS and SCCS?
In which case I am guessing that few people will care if the default
changes, so maybe we should just do that...

Message #17 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Jonathan Kamens <jik <at> kamens.us>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 11490 <at> debbugs.gnu.org
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Fri, 18 May 2012 10:38:02 -0400

[Message part 1 (text/plain, inline)]

On 05/17/2012 08:45 PM, Glenn Morris wrote:
> Well, ok, but your initial report began:
>
>    Make an RCS file writable with chmod +w without locking it.
>
> If I instead use M-x toggle-read-only, make changes, and use C-x v v,
> then I see the problem.
Yeah, well, you can never trust users to be accurate in bug reports. :-(
Sorry about that.

Seriously, I've encountered this problem twice on two different
computers recently, and the other time it happened might have been under
Cygwin on Windows, where permissions are screwy.
> OK. It looks like that variable only affects RCS and SCCS?
> In which case I am guessing that few people will care if the default
> changes, so maybe we should just do that...
I'm obviously not in any position of authority here, but as for my
personal opinion, I would have no objection whatsoever to making
vc-mistrust-permissions default to true for safety's sake. Might also
want to put a warning in the documentation of the variable about what
might happen if you set it to false and then use vc-next-action on a
file with non-checked-out changes.

  Jon

[Message part 2 (text/html, inline)]

Message #20 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Jonathan Kamens <jik <at> kamens.us>
Cc: 11490 <at> debbugs.gnu.org
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Mon, 21 May 2012 23:57:54 -0400

Jonathan Kamens wrote:

> I'm obviously not in any position of authority here, but as for my
> personal opinion, I would have no objection whatsoever to making
> vc-mistrust-permissions default to true for safety's sake.

Me neither. It seems like the default should be the safest mode of
operation.

However, Emacs 22.3 does not have this issuse; 23.1 onwards does.
Something was lost with the removal of vc-next-action-on-file,
and now vc-buffer-sync does not get called.

Message #27 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Chong Yidong <cyd <at> gnu.org>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Jonathan Kamens <jik <at> kamens.us>, 11490 <at> debbugs.gnu.org
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Fri, 04 Jan 2013 10:57:19 +0800

>> I'm obviously not in any position of authority here, but as for my
>> personal opinion, I would have no objection whatsoever to making
>> vc-mistrust-permissions default to true for safety's sake.
>
> Me neither. It seems like the default should be the safest mode of
> operation.
>
> However, Emacs 22.3 does not have this issuse; 23.1 onwards does.
> Something was lost with the removal of vc-next-action-on-file,
> and now vc-buffer-sync does not get called.

Can someone summarize again a *correct* recipe to see the bug?  What
with the wrong initial bug report, and the imprecise follow-up, I can't
figure out where the problem lies.  If I do

mkdir foo
cd foo
mkdir RCS
echo initial > file
ci -u -t-foo file
[Edit file in root]
emacs-24.0.96 -Q file
C-x v v

Then I get a prompt saying

  File has unlocked changes.  Claim lock retaining changes? (yes or no)

If I type yes, the changes are still there.  What's the problem?

Message #30 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Chong Yidong <cyd <at> gnu.org>
Cc: Jonathan Kamens <jik <at> kamens.us>, 11490 <at> debbugs.gnu.org
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Thu, 03 Jan 2013 22:11:07 -0500

Chong Yidong wrote:

> Can someone summarize again a *correct* recipe to see the bug?

mkdir foo
cd foo
mkdir RCS
echo initial > file
ci -u -t-foo file
emacs-24.2 -Q file

M-x toggle-read-only

Enter some text in the buffer, eg now it looks like:

-----
initial
foobar
-----

Press C-x v v, and "foobar" is deleted with no prompting and no way to
get it back.

I changed vc-mistrust-permissions to t for 24.3 because of this.
But now that I check, it doesn't seem to help...

Message #33 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Jonathan Kamens <jik <at> kamens.us>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 11490 <at> debbugs.gnu.org, Chong Yidong <cyd <at> gnu.org>
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Thu, 03 Jan 2013 22:17:42 -0500

[Message part 1 (text/plain, inline)]

On 01/03/2013 10:11 PM, Glenn Morris wrote:
> Chong Yidong wrote:
>
>> Can someone summarize again a *correct* recipe to see the bug?
> mkdir foo
> cd foo
> mkdir RCS
> echo initial > file
> ci -u -t-foo file
> emacs-24.2 -Q file
>
> M-x toggle-read-only
>
> Enter some text in the buffer, eg now it looks like:
>
> -----
> initial
> foobar
> -----
>
> Press C-x v v, and "foobar" is deleted with no prompting and no way to
> get it back.
>
> I changed vc-mistrust-permissions to t for 24.3 because of this.
> But now that I check, it doesn't seem to help...
The problem described above may indeed be a problem, but it's not the 
problem I reported.

The problem I reported is:

mkdir foo
cd foo
mkdir RCS
echo initial > file
ci -u -t-foo file
chmod +w file
echo second >> file
chmod -w file
emacs -Q file
C-x v v - the changes are overwritten without prompting

I think the step missing from Chong Yidong's recipe was making sure the 
file is read-only before trying to edit it in emacs.

If vc-mistrust-permissions is true by default then this issue doesn't occur.

  jik

[Message part 2 (text/html, inline)]

Message #36 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Jonathan Kamens <jik <at> kamens.us>
Cc: 11490 <at> debbugs.gnu.org, Chong Yidong <cyd <at> gnu.org>
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Thu, 03 Jan 2013 22:21:31 -0500

Jonathan Kamens wrote:

> Make an RCS file writable with chmod +w

I wrote:

> I cannot reproduce this

Jonathan Kamens wrote:

> Try it without making the file writable.

I wrote:

> I see the problem.

(months pass)

Jonathan Kamens wrote:

> The problem described above may indeed be a problem, but it's not the
> problem I reported.
>
> The problem I reported is:
[...]
> chmod +w file


Welp, this sure has been crystal clear.

Message #39 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Jonathan Kamens <jik <at> kamens.us>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 11490 <at> debbugs.gnu.org, Chong Yidong <cyd <at> gnu.org>
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Thu, 03 Jan 2013 22:33:33 -0500

[Message part 1 (text/plain, inline)]

Please read the WHOLE LIST OF COMMANDS I SENT in my test case.

I made the file writable in that list only to modify it. If you read the 
whole list of commands, you will see that I make it unwritable again 
before editing it with emacs. And that's exactly the problem... if the 
file isn't writable, emacs assumes that it's not checked out and not 
modified it and checks it out when you hit C-x v v, overwriting the 
changes in it.

[Message part 2 (text/html, inline)]

Message #42 received at 11490 <at> debbugs.gnu.org (full text, mbox):

From: Chong Yidong <cyd <at> gnu.org>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Jonathan Kamens <jik <at> kamens.us>, 11490 <at> debbugs.gnu.org
Subject: Re: bug#11490: vc-next-action overwrites changes in non-checked-out
	RCS file
Date: Tue, 02 Apr 2013 17:30:54 +0800

Jonathan Kamens <jik <at> kamens.us> writes:

> Please read the WHOLE LIST OF COMMANDS I SENT in my test case.
>
> I made the file writable in that list only to modify it. If you read
> the whole list of commands, you will see that I make it unwritable
> again before editing it with emacs. And that's exactly the problem...
> if the file isn't writable, emacs assumes that it's not checked out
> and not modified it and checks it out when you hit C-x v v,
> overwriting the changes in it.

OK, so the original bug is fixed by changing `vc-mistrust-permissions'
to t, indeed.

As for the problem Glenn just pointed out, it arises because VC caches
the state of the file when it is first visited (via vc-state-refresh).
The following patch (against trunk) should fix this, but I haven't had
time to give it much testing yet.


=== modified file 'lisp/vc/vc-hooks.el'
*** lisp/vc/vc-hooks.el	2013-01-02 16:13:04 +0000
--- lisp/vc/vc-hooks.el	2013-04-02 09:23:22 +0000
***************
*** 703,719 ****
    ;; the state to 'edited and redisplay the mode line.
    (let* ((file buffer-file-name)
           (backend (vc-backend file)))
!     (and backend
! 	 (or (and (equal (vc-file-getprop file 'vc-checkout-time)
! 			 (nth 5 (file-attributes file)))
! 		  ;; File has been saved in the same second in which
! 		  ;; it was checked out.  Clear the checkout-time
! 		  ;; to avoid confusion.
! 		  (vc-file-setprop file 'vc-checkout-time nil))
! 	     t)
!          (eq (vc-checkout-model backend (list file)) 'implicit)
!          (vc-state-refresh file backend)
! 	 (vc-mode-line file backend))
      ;; Try to avoid unnecessary work, a *vc-dir* buffer is
      ;; present if this is true.
      (when vc-dir-buffers
--- 703,723 ----
    ;; the state to 'edited and redisplay the mode line.
    (let* ((file buffer-file-name)
           (backend (vc-backend file)))
!     (when backend
!       (if (eq (vc-checkout-model backend (list file)) 'implicit)
! 	  (progn
! 	    ;; If the file was saved in the same second in which it
! 	    ;; was checked out, clear the checkout-time to avoid
! 	    ;; confusion.
! 	    (if (equal (vc-file-getprop file 'vc-checkout-time)
! 		       (nth 5 (file-attributes file)))
! 		(vc-file-setprop file 'vc-checkout-time nil))
! 	    (if (vc-state-refresh file backend)
! 		(vc-mode-line file backend)))
! 	;; If we saved an unlocked file on a locking based VCS, that
! 	;; file is not longer up-to-date.
! 	(if (eq (vc-file-getprop file 'vc-state) 'up-to-date)
! 	    (vc-file-setprop file 'vc-state nil))))
      ;; Try to avoid unnecessary work, a *vc-dir* buffer is
      ;; present if this is true.
      (when vc-dir-buffers

=== modified file 'lisp/vc/vc.el'
*** lisp/vc/vc.el	2013-01-02 16:13:04 +0000
--- lisp/vc/vc.el	2013-04-02 09:30:28 +0000
***************
*** 1072,1077 ****
--- 1072,1087 ----
           ;; among all the `files'.
  	 (model (nth 4 vc-fileset)))
  
+     ;; If a buffer has unsaved changes, a checkout would discard them.
+     (when (and (not (eq model 'implicit))
+ 	       (eq state 'up-to-date))
+       (let ((files files)
+ 	    buffer)
+ 	(while files
+ 	  (setq buffer (get-file-buffer (car files)))
+ 	  (and buffer (buffer-modified-p buffer)
+ 	       (setq state 'unlocked-changes files nil)))))
+ 
      ;; Do the right thing
      (cond
       ((eq state 'missing)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.