GNU bug report logs - #13547
svn annotate - incorrect previous/next revision

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 13547 in the body.
You can then email your comments to 13547 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: Lars Ljung <lars <at> matholka.se>
To: bug-gnu-emacs <at> gnu.org
Subject: svn annotate - incorrect previous/next revision
Date: Fri, 25 Jan 2013 10:29:11 +0100

Hi,

The functions vc-svn-previous-revision and vc-svn-next-revision just add
or subtract 1 from the revision number. The is usually not correct.

This patch uses "svn log" to get the correct previous/next revision of
the file.

Kind regards,
Lars Ljung

=== modified file 'lisp/vc/vc-svn.el'
*** lisp/vc/vc-svn.el	2013-01-02 16:13:04 +0000
--- lisp/vc/vc-svn.el	2013-01-25 08:26:53 +0000
*************** RESULT is a list of conses (FILE . STATE
*** 259,279 ****
  ;; works just fine.

  (defun vc-svn-previous-revision (file rev)
!   (let ((newrev (1- (string-to-number rev))))
!     (when (< 0 newrev)
!       (number-to-string newrev))))

  (defun vc-svn-next-revision (file rev)
!   (let ((newrev (1+ (string-to-number rev))))
!     ;; The "working revision" is an uneasy conceptual fit under
Subversion;
!     ;; we use it as the upper bound until a better idea comes along.
If the
!     ;; workfile version W coincides with the tree's latest revision R,
then
!     ;; this check prevents a "no such revision: R+1" error.  Otherwise, it
!     ;; inhibits showing of W+1 through R, which could be considered
anywhere
!     ;; from gracious to impolite.
!     (unless (< (string-to-number (vc-file-getprop file
'vc-working-revision))
!                newrev)
!       (number-to-string newrev))))


  ;;;
--- 259,280 ----
  ;; works just fine.

  (defun vc-svn-previous-revision (file rev)
!   (with-temp-buffer
!     (vc-svn-command t 0 file "log" "-q" "--limit" "2"
! 		    (format "-r%s:1" rev))
!     (let ((revision-list '()))
!       (while (re-search-backward "^r\\([0-9]+\\)" nil t)
! 	(setq revision-list (cons (match-string 1) revision-list)))
!       (cadr revision-list))))

  (defun vc-svn-next-revision (file rev)
!   (with-temp-buffer
!     (vc-svn-command t 0 file "log" "-q" "--limit" "2"
! 		    (format "-r%s:HEAD" rev))
!     (let ((revision-list '()))
!       (while (re-search-backward "^r\\([0-9]+\\)" nil t)
! 	(setq revision-list (cons (match-string 1) revision-list)))
!       (cadr revision-list))))


  ;;;

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

From: Glenn Morris <rgm <at> gnu.org>
To: Lars Ljung <lars <at> matholka.se>
Cc: 13547 <at> debbugs.gnu.org
Subject: Re: bug#13547: svn annotate - incorrect previous/next revision
Date: Sat, 26 Jan 2013 21:24:03 -0500

Lars Ljung wrote:

> The functions vc-svn-previous-revision and vc-svn-next-revision just add
> or subtract 1 from the revision number. The is usually not correct.
>
> This patch uses "svn log" to get the correct previous/next revision of
> the file.

Thanks. I guess the drawback here is, that will contact the svn
repository, which may be on a remote server, so could be slow.

It seems this is used by vc-annotate, vc-rollback, and vc-diff.

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

From: Lars Ljung <lars <at> matholka.se>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 13547 <at> debbugs.gnu.org
Subject: Re: bug#13547: svn annotate - incorrect previous/next revision
Date: Sun, 27 Jan 2013 12:32:47 +0100

2013-01-27 03:24, Glenn Morris skrev:
> Thanks. I guess the drawback here is, that will contact the svn
> repository, which may be on a remote server, so could be slow.
> 
> It seems this is used by vc-annotate, vc-rollback, and vc-diff. 

Yes, it will be slower. But as far as I know there is no other way to
get the previous/next revision of a file.

I suppose the output from an unlimited "svn log" could be saved to speed
up future operations. vc-annotate-warp-revision might actually call
these functions multiple times.

vc-rollback is not supported by the svn backend.

I don'f fully understand how vc-diff works, but these functions are not
called for "C-x v =". This is good since that operation should not
require server access at all.

vc-annotate-show-diff-revision-at-line calls vc-svn-previous-revision.
In that case the server will be contacted twice for no good reason, a
simple "svn diff -r rev" would suffice (isn't this true for all backends?).

vc-annotate-show-changeset-diff-revision-at-line calls
vc-svn-previous-revision with filename set to nil. It that case it makes
sense to return rev-1. The modified patch below takes care of that.

=== modified file 'lisp/vc/vc-svn.el'
*** lisp/vc/vc-svn.el	2013-01-02 16:13:04 +0000
--- lisp/vc/vc-svn.el	2013-01-27 11:04:07 +0000
***************
*** 259,279 ****
  ;; works just fine.

  (defun vc-svn-previous-revision (file rev)
!   (let ((newrev (1- (string-to-number rev))))
!     (when (< 0 newrev)
!       (number-to-string newrev))))

  (defun vc-svn-next-revision (file rev)
!   (let ((newrev (1+ (string-to-number rev))))
!     ;; The "working revision" is an uneasy conceptual fit under
Subversion;
!     ;; we use it as the upper bound until a better idea comes along.
If the
!     ;; workfile version W coincides with the tree's latest revision R,
then
!     ;; this check prevents a "no such revision: R+1" error.  Otherwise, it
!     ;; inhibits showing of W+1 through R, which could be considered
anywhere
!     ;; from gracious to impolite.
!     (unless (< (string-to-number (vc-file-getprop file
'vc-working-revision))
!                newrev)
!       (number-to-string newrev))))


  ;;;
--- 259,286 ----
  ;; works just fine.

  (defun vc-svn-previous-revision (file rev)
!   (if file
!       (with-temp-buffer
! 	(let (process-file-side-effects)
! 	  (vc-svn-command t 0 file "log" "-q" "--limit" "2"
! 			  (format "-r%s:1" rev)))
! 	(let ((revision-list '()))
! 	  (while (re-search-backward "^r\\([0-9]+\\)" nil t)
! 	    (push (match-string 1) revision-list))
! 	  (cadr revision-list)))
!     (let ((newrev (1- (string-to-number rev))))
!       (when (< 0 newrev)
! 	(number-to-string newrev)))))

  (defun vc-svn-next-revision (file rev)
!   (with-temp-buffer
!     (let (process-file-side-effects)
!       (vc-svn-command t 0 file "log" "-q" "--limit" "2"
! 		      (format "-r%s:HEAD" rev)))
!     (let ((revision-list '()))
!       (while (re-search-backward "^r\\([0-9]+\\)" nil t)
! 	(push (match-string 1) revision-list))
!       (cadr revision-list))))


  ;;;

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Lars Ljung <lars <at> matholka.se>
Cc: Glenn Morris <rgm <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 13547 <at> debbugs.gnu.org
Subject: Re: bug#13547: svn annotate - incorrect previous/next revision
Date: Thu, 15 Jul 2021 11:01:13 +0200

Lars Ljung <lars <at> matholka.se> writes:

>> Thanks. I guess the drawback here is, that will contact the svn
>> repository, which may be on a remote server, so could be slow.
>> 
>> It seems this is used by vc-annotate, vc-rollback, and vc-diff. 
>
> Yes, it will be slower. But as far as I know there is no other way to
> get the previous/next revision of a file.

I haven't used svn in many years, so I don't have much of an opinion
here.  I've respun the patch for Emacs 28; included below.

Anybody got an opinion here?

diff --git a/lisp/vc/vc-svn.el b/lisp/vc/vc-svn.el
index c30920dd15..0cc7bda1ba 100644
--- a/lisp/vc/vc-svn.el
+++ b/lisp/vc/vc-svn.el
@@ -248,23 +248,29 @@ vc-svn-working-revision
 ;; vc-svn-mode-line-string doesn't exist because the default implementation
 ;; works just fine.
 
-(defun vc-svn-previous-revision (_file rev)
-  (let ((newrev (1- (string-to-number rev))))
-    (when (< 0 newrev)
-      (number-to-string newrev))))
+(defun vc-svn-previous-revision (file rev)
+  (if file
+      (with-temp-buffer
+ 	(let (process-file-side-effects)
+ 	  (vc-svn-command t 0 file "log" "-q" "--limit" "2"
+ 			  (format "-r%s:1" rev)))
+ 	(let ((revision-list '()))
+ 	  (while (re-search-backward "^r\\([0-9]+\\)" nil t)
+ 	    (push (match-string 1) revision-list))
+ 	  (cadr revision-list)))
+    (let ((newrev (1- (string-to-number rev))))
+      (when (< 0 newrev)
+ 	(number-to-string newrev)))))
 
 (defun vc-svn-next-revision (file rev)
-  (let ((newrev (1+ (string-to-number rev))))
-    ;; The "working revision" is an uneasy conceptual fit under Subversion;
-    ;; we use it as the upper bound until a better idea comes along.  If the
-    ;; workfile version W coincides with the tree's latest revision R, then
-    ;; this check prevents a "no such revision: R+1" error.  Otherwise, it
-    ;; inhibits showing of W+1 through R, which could be considered anywhere
-    ;; from gracious to impolite.
-    (unless (< (string-to-number (vc-file-getprop file 'vc-working-revision))
-               newrev)
-      (number-to-string newrev))))
-
+  (with-temp-buffer
+    (let (process-file-side-effects)
+      (vc-svn-command t 0 file "log" "-q" "--limit" "2"
+ 		      (format "-r%s:HEAD" rev)))
+    (let ((revision-list '()))
+      (while (re-search-backward "^r\\([0-9]+\\)" nil t)
+ 	(push (match-string 1) revision-list))
+      (cadr revision-list))))
 
 ;;;
 ;;; State-changing functions


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Lars Ljung <lars <at> matholka.se>
Cc: Glenn Morris <rgm <at> gnu.org>, Dmitry Gutov <dgutov <at> yandex.ru>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 13547 <at> debbugs.gnu.org
Subject: Re: bug#13547: svn annotate - incorrect previous/next revision
Date: Fri, 30 Jul 2021 13:54:52 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>>> Thanks. I guess the drawback here is, that will contact the svn
>>> repository, which may be on a remote server, so could be slow.
>>> 
>>> It seems this is used by vc-annotate, vc-rollback, and vc-diff. 
>>
>> Yes, it will be slower. But as far as I know there is no other way to
>> get the previous/next revision of a file.
>
> I haven't used svn in many years, so I don't have much of an opinion
> here.  I've respun the patch for Emacs 28; included below.
>
> Anybody got an opinion here?

I forgot to put Dmitry in the CCs; perhaps he'll have an opinion.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Lars Ljung <lars <at> matholka.se>
Cc: Glenn Morris <rgm <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 13547 <at> debbugs.gnu.org
Subject: Re: bug#13547: svn annotate - incorrect previous/next revision
Date: Sat, 31 Jul 2021 05:40:37 +0300

Hi!

On 30.07.2021 14:54, Lars Ingebrigtsen wrote:
> Lars Ingebrigtsen<larsi <at> gnus.org>  writes:
> 
>>>> Thanks. I guess the drawback here is, that will contact the svn
>>>> repository, which may be on a remote server, so could be slow.
>>>>
>>>> It seems this is used by vc-annotate, vc-rollback, and vc-diff.
>>> Yes, it will be slower. But as far as I know there is no other way to
>>> get the previous/next revision of a file.
>> I haven't used svn in many years, so I don't have much of an opinion
>> here.  I've respun the patch for Emacs 28; included below.
>>
>> Anybody got an opinion here?
> I forgot to put Dmitry in the CCs; perhaps he'll have an opinion.

Since this is about SVN, it's hard for me to have an opinion as well: 
all practical recollections are from ~10 years ago. How slow are SVN 
servers to respond to requests like this these days?

Could someone also describe the effect that our current "not correct" 
revision numbers have? Do they trigger errors? Do them cause 
'vc-annotate' to show wrong file contents?

If it's either of these, then we probably should go ahead with the patch.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Glenn Morris <rgm <at> gnu.org>, Lars Ljung <lars <at> matholka.se>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 13547 <at> debbugs.gnu.org
Subject: Re: bug#13547: svn annotate - incorrect previous/next revision
Date: Wed, 23 Mar 2022 20:57:09 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> Since this is about SVN, it's hard for me to have an opinion as well:
> all practical recollections are from ~10 years ago. How slow are SVN
> servers to respond to requests like this these days?
>
> Could someone also describe the effect that our current "not correct"
> revision numbers have? Do they trigger errors? Do them cause
> 'vc-annotate' to show wrong file contents?
>
> If it's either of these, then we probably should go ahead with the patch.

This was half a year ago, and nobody's piped up, so I'm closing this bug
report.  (If somebody that uses svn wants this feature, they can
resurrect the patch, probably.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

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