Emacs isn't resyncing its internal vc state after a commit-patch commit

[dkogan]

This feels like it's probably a regression, but I'm not sure. Problematic behavior I observe:

1. Open some version-controlled file in emacs. I'm using git; probably this doesn't matter

2. (vc-working-revision buffer-file-name) should return either HEAD or maybe the most recent revision of that one file. Either is fine

3. make a change to that file, commit it with commit-patch.

4. (vc-working-revision buffer-file-name) SHOULD now return the new revision but it doesn't for me. This being out-of-date makes vc-annotate and vc-log and others lie to you.

It looks like commit-patch tries to deal with this, but it's broken currently. I'm not patching it because I'd like to know what the intent was, and what you THINK it's doing.

Currently commit-patch-buffer-in-directory looks at the patch buffer, extracts a list of filenames in the patch, and tries to re-synchronize them with emacs. It does this by invoking "lsdiff" on the patch and parsing the output. Problem is, the patch often contains relative paths, so the patch-files variable ends up with strings like "a/file1.c", where "a" isn't a real directory, so the (find-buffer-visiting) call doesn't find the relevant buffer. The code is there. Did this ever work?

If we found a buffer, we'd do this for each one:

                                        (vc-resynch-buffer (buffer-file-name buf) 'revert 'noquery)
                                        ;; stupid vc-revert-buffer1 doesn't call revert-buffer
                                        ;; with preserve-modes which means the CVS version doesn't
                                        ;; get updated, so we do it by hand.
                                        (run-hooks 'find-file-hooks)))

The comment implies that only CVS is affected, but it's all VCSs I'm guessing. At least in this case (vc-resync-buffer) doesn't fix this, but the (run-hooks) call does. The specific call that we need is (vc-file-clearprops buffer-file-name). Maybe we don't need to run ALL the hooks, but you can decide that.

Maybe vc-resynch-buffer should do the vc-clearprops.

[caldwell]

My memory is that the revert buffer stuff was from back before commit-patch supported anything other than CVS. It's specifically because our CVS files had CVS keywords in them—on commit CVS would change the file on disk to update version numbers and even add commit logs (in comments at the end of our C files).

So the point of this code was to do a real revert so the emacs buffer would match what was on the disk. I think at least SVN can do that same keyword stuff. I doubt any of the distributed ones do it (though maybe BZR—it's strange).

But—it does use vc-resynch-buffer which sounds like it might be intending to more. @radford might know, I believe he wrote that part.

[dkogan]

Bug ping. This is still a problem. Are either of you still using this? I am, and it's constantly bugging me. If you're all done with this thing, please tell me, and I'll fix it myself.

Thanks!

[caldwell]

I still use this every day—it probably accounts for 99% of commits I make. However, I never use vc-annotate or vc-log, so I've not noticed this (and it doesn't constantly bug me). I just tested and I do see vc-annotate was wrong for something I just commit-patched.

I looks like it might be as simple as adding 'reset-vc-info to the vc-resynch-buffer call:

(vc-resynch-buffer (buffer-file-name buf) 'revert 'noquery 'reset-vc-info)

Can you try that locally and see if it fixes your issue?

[dkogan]

David Caldwell @.***> writes:

I looks like it might be as simple as adding 'reset-vc-info to the vc-resynch-buffer call:

(vc-resynch-buffer (buffer-file-name buf) 'revert 'noquery 'reset-vc-info)

Can you try that locally and see if it fixes your issue?

Nope. I updated commit-patch-buffer-in-directory with the change, re-evaled. Then made an arbitrary commit to a file. Then

(vc-file-getprop (buffer-file-name) 'vc-working-revision)

I still see the old revision, pre-commit. If I, for instance, (revert-buffer), then I see the new revision.

[caldwell]

I think I found the issue. It was staring me in the face with the message that comes up on every commit:

Patched and committed 1 file(s) and reverted 0.

That "reverted 0" shows that it isn't actually hitting any of that revert code. The crux seems to be when we go looking for buffers earlier in the code. There were 2 things:

1. git uses a/some/file where cvs used some/file.
2. (find-buffer-visiting) doesn't seem to work with relative paths.

I'm not sure how 2 ever worked? Maybe cvs diff returned absolute paths? Anyway, those are both fixable. Can you try this patch?

diff --git a/commit-patch-buffer.el b/commit-patch-buffer.el
index 487e02d..7877996 100644
--- a/commit-patch-buffer.el
+++ b/commit-patch-buffer.el
@@ -90,8 +90,12 @@ one."
                             (split-string (buffer-string))))))
          (log-buffer-name (if amend "*amend*" "*commit*"))
          (f patch-files) visiting-buffers)
-    (while (car f)
-      (let ((buf (find-buffer-visiting (car f))))
+   (while (car f)
+      (let* ((default-directory directory)
+             (buf (or
+                  (find-buffer-visiting (expand-file-name (car f)))
+                  (find-buffer-visiting (expand-file-name (diff-filename-drop-dir (car f))))
+                  )))
         (when buf
           (with-current-buffer buf (vc-buffer-sync))
           (add-to-list 'visiting-buffers buf)))

With that (and without the 'reset-vc-info, which doesn't seem to matter here), it now updates the vc-working-revision correctly for me after commit-patching.

[dkogan]

I like how much this message echoes the original bug report :)

I just tested the patch, and it works for me now. Thanks a lot for fixing it! Ping me when you need to push to Debian.

[caldwell]

Lol, I apparently can't read. My eyes must have been drawn to the nicely formatted code section…