GNU bug report logs - #15418
24.3.50; RCS annotate broken in vc-rcs.el r114131

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 15418 in the body.
You can then email your comments to 15418 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: Richard Copley <rcopley <at> gmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Thu, 19 Sep 2013 15:16:49 +0100

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

For some files under RCS version control, `vc-annotate' now fails with
error `(wrong-type-argument integer-or-marker-p nil)'.

I think the problem is with the variable `b' whose dynamic binding
was used by `gather' and set by `incg' (local functions in
`vc-rcs-parse'), prior to r114131.

Sorry, I haven't been able to craft an example RCS master file that
reproduces the problem. The top of the stack trace looks like this.

Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
  buffer-substring-no-properties(nil 6747)
  (let ((x (buffer-substring-no-properties (cadr pairs) (car pairs))))
(message "%d %d %s" (cadr pairs) (car pairs) x) (setq acc (cons x acc)))
  (while pairs (let ((x (buffer-substring-no-properties (cadr pairs) (car
pairs)))) (message "%d %d %s" (cadr pairs) (car pairs) x) (setq acc (cons x
acc))) (setq pairs (cddr pairs)))
  (let ((pairs (cons e (append @-holes (list b)))) acc) (while pairs (let
((x (buffer-substring-no-properties (cadr pairs) (car pairs)))) (message
"%d %d %s" (cadr pairs) (car pairs) x) (setq acc (cons x acc))) (setq pairs
(cddr pairs))) (apply (quote concat) acc))


In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-09-19 on 57172UHB
Bzr revision: 114395 michael.albinus <at> gmx.de-20130919110801-tl6ne4cxp0bmx0kf
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix c:/emacs/emacs-114395
 --enable-locallisppath=%emacs_dir%/../site-lisp 'CPPFLAGS=-I
 G:/usr/include -I C:/GnuWin32/include' 'LDFLAGS=-L G:/usr/lib -L
 C:/GnuWin32/lib''

Important settings:
  value of $LANG: ENG
  locale-coding-system: cp1252
  default enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x r - e - b <return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils time-date tooltip ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp w32-common-fns disp-table w32-win
w32-vars tool-bar dnd fontset image regexp-opt fringe tabulated-list
newcomment lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer nadvice loaddefs button faces cus-face macroexp
files text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
w32notify w32 multi-tty emacs)

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

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Thu, 19 Sep 2013 17:26:06 -0400

>>>>> "Richard" == Richard Copley <rcopley <at> gmail.com> writes:

> For some files under RCS version control, `vc-annotate' now fails with
> error `(wrong-type-argument integer-or-marker-p nil)'.

> I think the problem is with the variable `b' whose dynamic binding
> was used by `gather' and set by `incg' (local functions in
> `vc-rcs-parse'), prior to r114131.

Indeed, it seems like there was some subtle dynamic scoping at play.
Does the patch below fix your problem?


        Stefan


=== modified file 'lisp/vc/vc-rcs.el'
--- lisp/vc/vc-rcs.el	2013-09-04 21:09:42 +0000
+++ lisp/vc/vc-rcs.el	2013-09-19 21:24:53 +0000
@@ -1234,6 +1234,8 @@
   (vc-do-command "*vc*" 0 "rcs" (vc-name file) (concat "-b" branch))
   (vc-file-setprop file 'vc-rcs-default-branch branch))
 
+(defvar vc-rcs--b)
+
 (defun vc-rcs-parse (&optional buffer)
   "Parse current buffer, presumed to be in RCS-style masterfile format.
 Optional arg BUFFER specifies another buffer to parse.  Return an alist
@@ -1314,7 +1316,7 @@
         tok
         ;; Region (begin and end buffer positions) of the printed
         ;; representation for the current tag.
-        b e
+        vc-rcs--b e
         ;; A list of buffer positions where "@@" can be found within the
         ;; printed representation region.  For each location, we push two
         ;; elements onto the list, 1+ and 2+ the location, respectively,
@@ -1330,11 +1332,11 @@
          (to-eol () (buffer-substring-no-properties
                      (point) (progn (forward-line 1)
                                     (1- (point)))))
-         (to-semi () (setq b (point)
+         (to-semi () (setq vc-rcs--b (point)
                            e (progn (search-forward ";")
                                     (1- (point)))))
          (to-one@ () (setq @-holes nil
-                           b (progn (search-forward "@") (point))
+                           vc-rcs--b (progn (search-forward "@") (point))
                            e (progn (while (and (search-forward "@")
                                                 (= ?@ (char-after))
                                                 (progn
@@ -1349,9 +1351,9 @@
                   (funcall set-b+e)
                   (cons tok (if proc
                                 (funcall proc)
-                              (buffer-substring-no-properties b e))))
+                              (buffer-substring-no-properties vc-rcs--b e))))
          (k-semi (name &optional proc) (tok+val #'to-semi name proc))
-         (gather () (let ((pairs `(,e ,@@-holes ,b))
+         (gather () (let ((pairs `(,e ,@@-holes ,vc-rcs--b))
                           acc)
                       (while pairs
                         (push (buffer-substring-no-properties
@@ -1381,7 +1383,7 @@
                                (setcdr two (cadr two))
                                two))
                            (split-string
-                            (buffer-substring-no-properties b e)))))
+                            (buffer-substring-no-properties vc-rcs--b e)))))
           (hpush 'locks))
         (push `(strict . ,(when (at 'strict)
                             (search-forward ";")
@@ -1403,7 +1405,7 @@
                              (let ((ls (mapcar 'string-to-number
                                                (split-string
                                                 (buffer-substring-no-properties
-                                                 b e)
+                                                 vc-rcs--b e)
                                                 "\\."))))
                                ;; Hack the year -- verified to be the
                                ;; same algorithm used in RCS 5.7.
@@ -1414,7 +1416,7 @@
                   ,(k-semi 'branches
                            (lambda ()
                              (split-string
-                              (buffer-substring-no-properties b e))))
+                              (buffer-substring-no-properties vc-rcs--b e))))
                   ,(k-semi 'next))
                 revs)
           (sw))
@@ -1440,8 +1442,8 @@
               ;; only the former since it behaves identically to the
               ;; latter in the absence of "@@".)
               sub)
-          (cl-flet ((incg (_beg end)
-                          (let ((e end) @-holes)
+          (cl-flet ((incg (beg end)
+                          (let ((vc-rcs--b beg) (e end) @-holes)
                             (while (and asc (< (car asc) e))
                               (push (pop asc) @-holes))
                             ;; Self-deprecate when work is done.
@@ -1471,7 +1473,7 @@
                     (setq asc (nreverse @-holes)
                           sub #'incg)
                   (setq sub #'buffer-substring-no-properties))
-                (goto-char b)
+                (goto-char vc-rcs--b)
                 (setq acc nil)
                 (while (< (point) e)
                   (forward-char 1)

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

From: Richard Copley <rcopley <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Tue, 24 Sep 2013 23:47:45 +0100

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

On 19 September 2013 22:26, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:

> >>>>> "Richard" == Richard Copley <rcopley <at> gmail.com> writes:
>
> > For some files under RCS version control, `vc-annotate' now fails with
> > error `(wrong-type-argument integer-or-marker-p nil)'.
>
> > I think the problem is with the variable `b' whose dynamic binding
> > was used by `gather' and set by `incg' (local functions in
> > `vc-rcs-parse'), prior to r114131.
>
> Indeed, it seems like there was some subtle dynamic scoping at play.
> Does the patch below fix your problem?
>

No, I'm afraid it does not. I get the same error.
Thanks.

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

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Tue, 24 Sep 2013 20:55:13 -0400

>> > For some files under RCS version control, `vc-annotate' now fails with
>> > error `(wrong-type-argument integer-or-marker-p nil)'.
>> > I think the problem is with the variable `b' whose dynamic binding
>> > was used by `gather' and set by `incg' (local functions in
>> > `vc-rcs-parse'), prior to r114131.
>> Indeed, it seems like there was some subtle dynamic scoping at play.
>> Does the patch below fix your problem?
> No, I'm afraid it does not. I get the same error.

Can you show the new backtrace?


        Stefan

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

From: Richard Copley <rcopley <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Wed, 25 Sep 2013 08:08:51 +0100

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

The top of the stack trace is exactly the same as before, except for the
new name for "b".

  buffer-substring-no-properties(nil 6747)
  (cons (buffer-substring-no-properties (cadr pairs) (car pairs)) acc)
  (setq acc (cons (buffer-substring-no-properties (cadr pairs) (car pairs))
acc))
  (while pairs (setq acc (cons (buffer-substring-no-properties (cadr pairs)
(car pairs)) acc)) (setq pairs (cddr pairs)))
  (let ((pairs (cons e (append @-holes (list vc-rcs--b)))) acc) (while
pairs (setq acc (cons (buffer-substring-no-properties (cadr pairs) (car
pairs)) acc)) (setq pairs (cddr pairs))) (apply (quote concat) acc))

(The next line of the backtrace has 159030 characters, most of which are
from the master file. Sorry again that I can't provide an example, but the
file doesn't belong to me.)



On 25 September 2013 01:55, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:

> >> > For some files under RCS version control, `vc-annotate' now fails with
> >> > error `(wrong-type-argument integer-or-marker-p nil)'.
> >> > I think the problem is with the variable `b' whose dynamic binding
> >> > was used by `gather' and set by `incg' (local functions in
> >> > `vc-rcs-parse'), prior to r114131.
> >> Indeed, it seems like there was some subtle dynamic scoping at play.
> >> Does the patch below fix your problem?
> > No, I'm afraid it does not. I get the same error.
>
> Can you show the new backtrace?
>
>
>         Stefan
>

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

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

From: Richard Copley <rcopley <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Mon, 14 Oct 2013 17:14:12 +0100

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

On 25 September 2013 08:08, Richard Copley <rcopley <at> gmail.com> wrote:

> [...] Sorry again that I can't provide an example, but the file doesn't
> belong to me.
>

Attached is a simple RCS master file that reproduces the issue.

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

[vcbug.txt,v (application/octet-stream, attachment)]

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

From: Richard Copley <rcopley <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Mon, 14 Oct 2013 23:33:48 +0100

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

Making `e' and `@-holes' special variables, in addition to `b', as in your
patch, does seem to work.

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

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

From: Richard Copley <rcopley <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Tue, 15 Oct 2013 22:09:35 +0100

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

On 14 October 2013 23:33, Richard Copley <rcopley <at> gmail.com> wrote:

> Making `e' and `@-holes' special variables, in addition to `b', as in your
> patch, does seem to work.
>
> No, it doesn't, sorry. With those changes, `vc-annotate' no longer signals
an error, but it also annotates files wrongly. Some lines are attributed to
version 1 that shouldn't be. Do you see the same for the RCS master files
you have available for testing?

I tried to find the mistake(s) but it's very hard to spot let- and
lambda-bindings intended to be used outside their lexical scope.

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

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Tue, 15 Oct 2013 23:05:37 -0400

>> [...] Sorry again that I can't provide an example, but the file doesn't
>> belong to me.
> Attached is a simple RCS master file that reproduces the issue.

Thanks, I'll take a look,


        Stefan

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

From: Richard Copley <rcopley <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Tue, 5 Nov 2013 10:09:28 +0000

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

On 16 October 2013 03:05, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:

>
> Thanks, I'll take a look,
>

In the meantime, it would be convenient if RCS blame were working on the
trunk. Would you consider reverting vc-rcs.el to the last working version,
r112756?

Richard.

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

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Tue, 05 Nov 2013 10:37:56 -0500

> Attached is a simple RCS master file that reproduces the issue.

Thanks.  I installed the patch below which fixes the problem at least on
this trivial file.  And sorry for taking so long.  Please confirm that
it fixes the problem in real life as well.


        Stefan


=== modified file 'lisp/vc/vc-rcs.el'
--- lisp/vc/vc-rcs.el	2013-10-04 23:47:00 +0000
+++ lisp/vc/vc-rcs.el	2013-11-05 15:25:06 +0000
@@ -1355,18 +1354,18 @@
                                 (funcall proc)
                               (buffer-substring-no-properties b e))))
          (k-semi (name &optional proc) (tok+val #'to-semi name proc))
-         (gather () (let ((pairs `(,e ,@@-holes ,b))
+         (gather (b e @-holes)
+                 (let ((pairs `(,e ,@@-holes ,b))
                           acc)
                       (while pairs
                         (push (buffer-substring-no-properties
                                (cadr pairs) (car pairs))
                               acc)
                         (setq pairs (cddr pairs)))
-                      (apply 'concat acc)))
-         (k-one@ (name &optional later) (tok+val #'to-one@ name
-                                                 (if later
-                                                     (lambda () t)
-                                                   #'gather))))
+                   (apply #'concat acc)))
+         (gather1 () (gather b e @-holes))
+         (k-one@ (name &optional later)
+                 (tok+val #'to-one@ name (if later (lambda () t) #'gather1))))
       (save-excursion
         (goto-char (point-min))
         ;; headers
@@ -1444,9 +1443,10 @@
               ;; only the former since it behaves identically to the
               ;; latter in the absence of "@@".)
               sub)
-          (cl-flet ((incg (_beg end)
-                          (let ((e end) @-holes)
+          (cl-flet ((incg (beg end)
+                          (let ((b beg) (e end) @-holes)
                             (while (and asc (< (car asc) e))
+                              (push (pop asc) @-holes)
                               (push (pop asc) @-holes))
                             ;; Self-deprecate when work is done.
                             ;; Folding many dimensions into one.
@@ -1454,7 +1454,7 @@
                             ;; O beauteous math! --the Unvexed Bum
                             (unless asc
                               (setq sub #'buffer-substring-no-properties))
-                            (gather))))
+                            (gather b e @-holes))))
             (while (and (sw)
                         (not (eobp))
                         (setq context (to-eol)
@@ -1470,7 +1470,7 @@
               ;; other revisions, replace the `text' tag+value with `:insn'
               ;; plus value, always scanning in-place.
               (if (string= context (cdr (assq 'head headers)))
-                  (setcdr (cadr rev) (gather))
+                  (setcdr (cadr rev) (gather b e @-holes))
                 (if @-holes
                     (setq asc (nreverse @-holes)
                           sub #'incg)

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

From: Richard Copley <rcopley <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Wed, 6 Nov 2013 10:56:48 +0000

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

On 5 November 2013 15:37, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:

    > Attached is a simple RCS master file that reproduces the issue.
    Thanks.  I installed the patch below which fixes the problem at least on
    this trivial file.  And sorry for taking so long.  Please confirm that
    it fixes the problem in real life as well.

Thanks. I rebuilt Emacs from trunk r114733 to test. `vc-annotate' no longer
signals an error, but neither does it work correctly. For non-trivial
master files, many lines are wrongly attributed to revision 1.1.

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

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

From: Richard Copley <rcopley <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 15418 <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Wed, 6 Nov 2013 11:19:02 +0000

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

On 6 November 2013 10:56, Richard Copley <rcopley <at> gmail.com> wrote:

> Thanks. I rebuilt Emacs from trunk r114733 to test. `vc-annotate' no
> longer signals an error, but neither does it work correctly. For
> non-trivial master files, many lines are wrongly attributed to revision 1.1.
>

Oops, sorry. I was testing the wrong version (because I didn't notice that
my build had failed).

I've tested again by loading the new version of vc-rcs.el. I think it works
perfectly now. I ran `vc-annotate' on a large, complex master file using
vc-rcs.el revisions 112756 (before lexical-bindings) and 114977 (latest
revision) and got exactly the same results from both versions.

Thanks.

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

Message #46 received at 15418-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 15418-done <at> debbugs.gnu.org
Subject: Re: bug#15418: 24.3.50; RCS annotate broken in vc-rcs.el r114131
Date: Wed, 06 Nov 2013 08:46:17 -0500

> I've tested again by loading the new version of vc-rcs.el. I think it works
> perfectly now. I ran `vc-annotate' on a large, complex master file using
> vc-rcs.el revisions 112756 (before lexical-bindings) and 114977 (latest
> revision) and got exactly the same results from both versions.

Yay!  Thanks for your patience,


        Stefan

Previous Next

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