* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
@ 2012-05-16 19:29 Jonathan Kamens
2012-05-16 20:12 ` Glenn Morris
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Kamens @ 2012-05-16 19:29 UTC (permalink / raw
To: 11490
[-- Attachment #1: Type: text/plain, Size: 868 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 1204 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2012-05-16 19:29 bug#11490: vc-next-action overwrites changes in non-checked-out RCS file Jonathan Kamens
@ 2012-05-16 20:12 ` Glenn Morris
2012-05-17 1:17 ` Jonathan Kamens
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Morris @ 2012-05-16 20:12 UTC (permalink / raw
To: Jonathan Kamens; +Cc: 11490
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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2012-05-16 20:12 ` Glenn Morris
@ 2012-05-17 1:17 ` Jonathan Kamens
2012-05-18 0:45 ` Glenn Morris
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Kamens @ 2012-05-17 1:17 UTC (permalink / raw
To: Glenn Morris; +Cc: 11490
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 975 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2012-05-17 1:17 ` Jonathan Kamens
@ 2012-05-18 0:45 ` Glenn Morris
2012-05-18 14:38 ` Jonathan Kamens
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Morris @ 2012-05-18 0:45 UTC (permalink / raw
To: Jonathan Kamens; +Cc: 11490
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...
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2012-05-18 0:45 ` Glenn Morris
@ 2012-05-18 14:38 ` Jonathan Kamens
2012-05-22 3:57 ` Glenn Morris
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Kamens @ 2012-05-18 14:38 UTC (permalink / raw
To: Glenn Morris; +Cc: 11490
[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 1576 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2012-05-18 14:38 ` Jonathan Kamens
@ 2012-05-22 3:57 ` Glenn Morris
2013-01-04 2:57 ` Chong Yidong
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Morris @ 2012-05-22 3:57 UTC (permalink / raw
To: Jonathan Kamens; +Cc: 11490
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2012-05-22 3:57 ` Glenn Morris
@ 2013-01-04 2:57 ` Chong Yidong
2013-01-04 3:11 ` Glenn Morris
0 siblings, 1 reply; 12+ messages in thread
From: Chong Yidong @ 2013-01-04 2:57 UTC (permalink / raw
To: Glenn Morris; +Cc: Jonathan Kamens, 11490
>> 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?
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2013-01-04 2:57 ` Chong Yidong
@ 2013-01-04 3:11 ` Glenn Morris
2013-01-04 3:17 ` Jonathan Kamens
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Morris @ 2013-01-04 3:11 UTC (permalink / raw
To: Chong Yidong; +Cc: Jonathan Kamens, 11490
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...
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2013-01-04 3:11 ` Glenn Morris
@ 2013-01-04 3:17 ` Jonathan Kamens
2013-01-04 3:21 ` Glenn Morris
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Kamens @ 2013-01-04 3:17 UTC (permalink / raw
To: Glenn Morris; +Cc: 11490, Chong Yidong
[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 1618 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2013-01-04 3:17 ` Jonathan Kamens
@ 2013-01-04 3:21 ` Glenn Morris
2013-01-04 3:33 ` Jonathan Kamens
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Morris @ 2013-01-04 3:21 UTC (permalink / raw
To: Jonathan Kamens; +Cc: 11490, Chong Yidong
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2013-01-04 3:21 ` Glenn Morris
@ 2013-01-04 3:33 ` Jonathan Kamens
2013-01-05 9:35 ` Chong Yidong
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan Kamens @ 2013-01-04 3:33 UTC (permalink / raw
To: Glenn Morris; +Cc: 11490, Chong Yidong
[-- Attachment #1: Type: text/plain, Size: 435 bytes --]
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.
[-- Attachment #2: Type: text/html, Size: 651 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#11490: vc-next-action overwrites changes in non-checked-out RCS file
2013-01-04 3:33 ` Jonathan Kamens
@ 2013-01-05 9:35 ` Chong Yidong
0 siblings, 0 replies; 12+ messages in thread
From: Chong Yidong @ 2013-01-05 9:35 UTC (permalink / raw
To: Glenn Morris; +Cc: Jonathan Kamens, 11490
Jonathan Kamens <jik@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)
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-01-05 9:35 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-16 19:29 bug#11490: vc-next-action overwrites changes in non-checked-out RCS file Jonathan Kamens
2012-05-16 20:12 ` Glenn Morris
2012-05-17 1:17 ` Jonathan Kamens
2012-05-18 0:45 ` Glenn Morris
2012-05-18 14:38 ` Jonathan Kamens
2012-05-22 3:57 ` Glenn Morris
2013-01-04 2:57 ` Chong Yidong
2013-01-04 3:11 ` Glenn Morris
2013-01-04 3:17 ` Jonathan Kamens
2013-01-04 3:21 ` Glenn Morris
2013-01-04 3:33 ` Jonathan Kamens
2013-01-05 9:35 ` Chong Yidong
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).