[viewvc-dev] [PATCHES] A pile of cleanup patches for rcsparse

parse-rcs-file.diff -p0
add-tests.diff -p0
fix-warning.diff -p0
sort-sink-callbacks.diff -p0
separate-strict.diff -p0
token-lookup.diff -p0
remove-dead-eof-check.diff -p0
allow-empty-head.diff -p0
allow-empty-tree.diff -p0
simplify-match.diff -p0
better-rcs-expected.diff -p0
parse-raises-rcs-expected.diff -p0
simplify-semicolon-handling.diff -p0
rename-locals.diff -p0
simplify-parse-admin-access.diff -p0
parse-rcs-tree-entry.diff -p0
simplify-tree-entry-parsing.diff -p0
simplify-date-processing.diff -p0
read-until-semicolon.diff -p0

Add a new test/demonstration script.

Submitted by: Michael Haggerty <[email protected]>

This script was taken from the cvs2svn project, http://cvs2svn.tigris.org.

* lib/vclib/ccvs/rcsparse/parse_rcs_file.py: New demonstration script;
  illustrates the use of the rcsparse API.
Index: lib/vclib/ccvs/rcsparse/parse_rcs_file.py
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ lib/vclib/ccvs/rcsparse/parse_rcs_file.py	2007-11-10 15:40:09.000000000 +0100
@@ -0,0 +1,74 @@
+#! /usr/bin/python
+
+# (Be in -*- python -*- mode.)
+#
+# ====================================================================
+# Copyright (c) 2006-2007 CollabNet.  All rights reserved.
+#
+# This software is licensed as described in the file COPYING, which
+# you should have received as part of this distribution.  The terms
+# are also available at http://subversion.tigris.org/license-1.html.
+# If newer versions of this license are posted there, you may use a
+# newer version instead, at your option.
+#
+# This software consists of voluntary contributions made by many
+# individuals.  For exact contribution history, see the revision
+# history and logs, available at http://cvs2svn.tigris.org/.
+# ====================================================================
+
+"""Parse an RCS file, showing the rcsparse callbacks that are called.
+
+This program is useful to see whether an RCS file has a problem (in
+the sense of not being parseable by rcsparse) and also to illuminate
+the correspondence between RCS file contents and rcsparse callbacks.
+
+The output of this program can also be considered to be a kind of
+'canonical' format for RCS files, at least in so far as rcsparse
+returns all relevant information in the file and provided that the
+order of callbacks is always the same."""
+
+
+import sys
+import os
+
+
+class Logger:
+  def __init__(self, f, name):
+    self.f = f
+    self.name = name
+
+  def __call__(self, *args):
+    self.f.write(
+        '%s(%s)\n' % (self.name, ', '.join(['%r' % arg for arg in args]),)
+        )
+
+
+class LoggingSink:
+  def __init__(self, f):
+    self.f = f
+
+  def __getattr__(self, name):
+    return Logger(self.f, name)
+
+
+if __name__ == '__main__':
+  # Since there is nontrivial logic in __init__.py, we can't just
+  # import things directly out of this directory.  Therefore, we
+  # have to add the parent directory to the path, then import
+  # "rcsparse".
+  sys.path.insert(0, os.path.join(os.path.dirname(sys.argv[0]), '..'))
+
+  import rcsparse
+
+  if sys.argv[1:]:
+    for path in sys.argv[1:]:
+      if os.path.isfile(path) and path.endswith(',v'):
+        rcsparse.parse(
+            open(path, 'rb'), LoggingSink(sys.stdout)
+            )
+      else:
+        sys.stderr.write('%r is being ignored.\n' % path)
+  else:
+    rcsparse.parse(sys.stdin, LoggingSink(sys.stdout))
+
+

Add a very primitive testing infrastructure for rcsparse, and the first test.

* lib/vclib/ccvs/rcsparse/run-tests.py: New file.

* lib/vclib/ccvs/rcsparse/test-data/default,v,
  lib/vclib/ccvs/rcsparse/test-data/default.out: Test data for a
  simple test.
Index: lib/vclib/ccvs/rcsparse/run-tests.py
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ lib/vclib/ccvs/rcsparse/run-tests.py	2007-11-10 15:48:40.000000000 +0100
@@ -0,0 +1,70 @@
+#! /usr/bin/python
+
+# (Be in -*- python -*- mode.)
+#
+# ====================================================================
+# Copyright (c) 2007 CollabNet.  All rights reserved.
+#
+# This software is licensed as described in the file COPYING, which
+# you should have received as part of this distribution.  The terms
+# are also available at http://subversion.tigris.org/license-1.html.
+# If newer versions of this license are posted there, you may use a
+# newer version instead, at your option.
+#
+# This software consists of voluntary contributions made by many
+# individuals.  For exact contribution history, see the revision
+# history and logs, available at http://viewvc.tigris.org/.
+# ====================================================================
+
+"""Run tests of rcsparse code."""
+
+import sys
+import os
+import glob
+from cStringIO import StringIO
+from difflib import Differ
+
+script_dir = os.path.dirname(sys.argv[0])
+sys.path.insert(0, os.path.join(script_dir, '..'))
+
+from rcsparse import parse
+from rcsparse.parse_rcs_file import LoggingSink
+
+
+test_dir = os.path.join(script_dir, 'test-data')
+
+filelist = glob.glob(os.path.join(test_dir, '*,v'))
+filelist.sort()
+
+all_tests_ok = 1
+
+for filename in filelist:
+    sys.stderr.write('%s: ' % (filename,))
+    f = StringIO()
+    try:
+        parse(open(filename, 'rb'), LoggingSink(f))
+    except Exception, e:
+        sys.stderr.write('Error parsing file: %s!\n' % (e,))
+        all_tests_ok = 0
+    else:
+        output = f.getvalue()
+
+        expected_output_filename = filename[:-2] + '.out'
+        expected_output = open(expected_output_filename, 'rb').read()
+
+        if output == expected_output:
+            sys.stderr.write('OK\n')
+        else:
+            sys.stderr.write('Output does not match expected output!\n')
+            differ = Differ()
+            for diffline in differ.compare(
+                expected_output.splitlines(1), output.splitlines(1)
+                ):
+                sys.stderr.write(diffline)
+            all_tests_ok = 0
+
+if all_tests_ok:
+    sys.exit(0)
+else:
+    sys.exit(1)
+
Index: lib/vclib/ccvs/rcsparse/test-data/default,v
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ lib/vclib/ccvs/rcsparse/test-data/default,v	2007-11-10 15:48:50.000000000 +0100
@@ -0,0 +1,102 @@
+head	1.2;
+access;
+symbols
+	B_SPLIT:1.2.0.4
+	B_MIXED:1.2.0.2
+	T_MIXED:1.2
+	B_FROM_INITIALS_BUT_ONE:1.1.1.1.0.4
+	B_FROM_INITIALS:1.1.1.1.0.2
+	T_ALL_INITIAL_FILES_BUT_ONE:1.1.1.1
+	T_ALL_INITIAL_FILES:1.1.1.1
+	vendortag:1.1.1.1
+	vendorbranch:1.1.1;
+locks; strict;
+comment	@# @;
+
+
+1.2
+date	2003.05.23.00.17.53;	author jrandom;	state Exp;
+branches
+	1.2.2.1
+	1.2.4.1;
+next	1.1;
+
+1.1
+date	2003.05.22.23.20.19;	author jrandom;	state Exp;
+branches
+	1.1.1.1;
+next	;
+
+1.1.1.1
+date	2003.05.22.23.20.19;	author jrandom;	state Exp;
+branches;
+next	;
+
+1.2.2.1
+date	2003.05.23.00.31.36;	author jrandom;	state Exp;
+branches;
+next	;
+
+1.2.4.1
+date	2003.06.03.03.20.31;	author jrandom;	state Exp;
+branches;
+next	;
+
+
+desc
+@@
+
+
+1.2
+log
+@Second commit to proj, affecting all 7 files.
+@
+text
+@This is the file `default' in the top level of the project.
+
+Every directory in the `proj' project has a file named `default'.
+
+This line was added in the second commit (affecting all 7 files).
+@
+
+
+1.2.4.1
+log
+@First change on branch B_SPLIT.
+
+This change excludes sub3/default, because it was not part of this
+commit, and sub1/subsubB/default, which is not even on the branch yet.
+@
+text
+@a5 2
+
+First change on branch B_SPLIT.
+@
+
+
+1.2.2.1
+log
+@Modify three files, on branch B_MIXED.
+@
+text
+@a5 2
+
+This line was added on branch B_MIXED only (affecting 3 files).
+@
+
+
+1.1
+log
+@Initial revision
+@
+text
+@d4 2
+@
+
+
+1.1.1.1
+log
+@Initial import.
+@
+text
+@@
Index: lib/vclib/ccvs/rcsparse/test-data/default.out
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ lib/vclib/ccvs/rcsparse/test-data/default.out	2007-11-10 15:49:19.000000000 +0100
@@ -0,0 +1,26 @@
+set_head_revision('1.2')
+define_tag('B_SPLIT', '1.2.0.4')
+define_tag('B_MIXED', '1.2.0.2')
+define_tag('T_MIXED', '1.2')
+define_tag('B_FROM_INITIALS_BUT_ONE', '1.1.1.1.0.4')
+define_tag('B_FROM_INITIALS', '1.1.1.1.0.2')
+define_tag('T_ALL_INITIAL_FILES_BUT_ONE', '1.1.1.1')
+define_tag('T_ALL_INITIAL_FILES', '1.1.1.1')
+define_tag('vendortag', '1.1.1.1')
+define_tag('vendorbranch', '1.1.1')
+set_locking('strict')
+set_comment('# ')
+admin_completed()
+define_revision('1.2', 1053649073, 'jrandom', 'Exp', ['1.2.2.1', '1.2.4.1'], '1.1')
+define_revision('1.1', 1053645619, 'jrandom', 'Exp', ['1.1.1.1'], None)
+define_revision('1.1.1.1', 1053645619, 'jrandom', 'Exp', [], None)
+define_revision('1.2.2.1', 1053649896, 'jrandom', 'Exp', [], None)
+define_revision('1.2.4.1', 1054610431, 'jrandom', 'Exp', [], None)
+tree_completed()
+set_description('')
+set_revision_info('1.2', 'Second commit to proj, affecting all 7 files.\n', "This is the file `default' in the top level of the project.\n\nEvery directory in the `proj' project has a file named `default'.\n\nThis line was added in the second commit (affecting all 7 files).\n")
+set_revision_info('1.2.4.1', 'First change on branch B_SPLIT.\n\nThis change excludes sub3/default, because it was not part of this\ncommit, and sub1/subsubB/default, which is not even on the branch yet.\n', 'a5 2\n\nFirst change on branch B_SPLIT.\n')
+set_revision_info('1.2.2.1', 'Modify three files, on branch B_MIXED.\n', 'a5 2\n\nThis line was added on branch B_MIXED only (affecting 3 files).\n')
+set_revision_info('1.1', 'Initial revision\n', 'd4 2\n')
+set_revision_info('1.1.1.1', 'Initial import.\n', '')
+parse_completed()

Fix a syntax error that was preventing compilation.

* tparse/tparse.h (rcstoken::init): Fix declaration.
Index: tparse/tparse.h
===================================================================
--- tparse/tparse.h.orig	2007-11-05 21:35:38.000000000 +0100
+++ tparse/tparse.h	2007-11-05 21:35:58.000000000 +0100
@@ -121,7 +121,7 @@
         free(data);
       data = NULL;
     };
-  void rcstoken::init(const char *mydata, size_t mylen);
+  void init(const char *mydata, size_t mylen);
   int null_token()
     {
       return data == NULL;

Reorder definitions in the approximate order that they will be called.

Order the callbacks in Sink and the "if" branched in parse_rcs_admin()
in the approximate order that they will be encountered.

* lib/vclib/ccvs/rcsparse/common.py (Sink): Reorder method definitions.

  (_Parser.parse_rcs_admin): Reorder "if" statement branches.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-05 22:23:18.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-05 23:21:20.000000000 +0100
@@ -18,35 +18,49 @@
 class Sink:
   def set_head_revision(self, revision):
     pass
+
   def set_principal_branch(self, branch_name):
     pass
-  def define_tag(self, name, revision):
-    pass
+
   def set_access(self, accessors):
     pass
-  def set_expansion(self, mode):
+
+  def define_tag(self, name, revision):
+    pass
+
+  def set_locker(self, revision, locker):
     pass
+
   def set_locking(self, mode):
     """Used to signal locking mode.
 
     Called with mode argument 'strict' if strict locking
     Not called when no locking used."""
+
     pass
-  def set_locker(self, revision, locker):
-    pass
+
   def set_comment(self, comment):
     pass
-  def set_description(self, description):
+
+  def set_expansion(self, mode):
     pass
+
+  def admin_completed(self):
+    pass
+
   def define_revision(self, revision, timestamp, author, state,
                       branches, next):
     pass
-  def set_revision_info(self, revision, log, text):
+
+  def tree_completed(self):
     pass
-  def admin_completed(self):
+
+  def set_description(self, description):
     pass
-  def tree_completed(self):
+
+  def set_revision_info(self, revision, log, text):
     pass
+
   def parse_completed(self):
     pass
 
@@ -100,6 +114,15 @@
             self.ts.unget(semi);
           else:
             raise RCSExpected(semi, ';')
+      elif token == "access":
+        accessors = []
+        while 1:
+          tag = self.ts.get()
+          if tag == ';':
+            if accessors != []:
+              self.sink.set_access(accessors)
+            break
+          accessors = accessors + [ tag ]
       elif token == "symbols":
         while 1:
           tag = self.ts.get()
@@ -109,16 +132,6 @@
           tag_name = tag
           tag_rev = self.ts.get()
           self.sink.define_tag(tag_name, tag_rev)
-      elif token == "comment":
-        semi, comment = self.ts.mget(2)
-        self.sink.set_comment(comment)
-        if semi != ';':
-          raise RCSExpected(semi, ';')
-      elif token == "expand":
-        semi, expand_mode = self.ts.mget(2)
-        self.sink.set_expansion(expand_mode)
-        if semi != ';':
-          raise RCSExpected(semi, ';')
       elif token == "locks":
         while 1:
           tag = self.ts.get()
@@ -135,15 +148,16 @@
           self.ts.match(';')
         else:
           self.ts.unget(tag)
-      elif token == "access":
-        accessors = []
-        while 1:
-          tag = self.ts.get()
-          if tag == ';':
-            if accessors != []:
-              self.sink.set_access(accessors)
-            break
-          accessors = accessors + [ tag ]
+      elif token == "comment":
+        semi, comment = self.ts.mget(2)
+        self.sink.set_comment(comment)
+        if semi != ';':
+          raise RCSExpected(semi, ';')
+      elif token == "expand":
+        semi, expand_mode = self.ts.mget(2)
+        self.sink.set_expansion(expand_mode)
+        if semi != ';':
+          raise RCSExpected(semi, ';')
 
       # Chew up "newphrase"
       else:

Separate handling of the "strict" keyword into a separate "if" branch.

This makes the keyword handling more uniform.

* lib/vclib/ccvs/rcsparse/common.py (_Parser.parse_rcs_admin):
  Separate handling of the "strict" keyword into a separate "if"
  branch.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-05 23:22:21.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-05 23:22:36.000000000 +0100
@@ -141,13 +141,9 @@
           locker = tag
           rev = self.ts.get()
           self.sink.set_locker(rev, locker)
-
-        tag = self.ts.get()
-        if tag == "strict":
-          self.sink.set_locking("strict")
-          self.ts.match(';')
-        else:
-          self.ts.unget(tag)
+      elif token == "strict":
+        self.sink.set_locking("strict")
+        self.ts.match(';')
       elif token == "comment":
         semi, comment = self.ts.mget(2)
         self.sink.set_comment(comment)

Rewrite _Parser.parse_rcs_admin() to use a token lookup table.

* lib/vclib/ccvs/rcsparse/common.py (_Parser._parse_admin_head,
  _Parser._parse_admin_branch, _Parser._parse_admin_access,
  _Parser._parse_admin_symbols, _Parser._parse_admin_locks,
  _Parser._parse_admin_strict, _Parser._parse_admin_comment,
  _Parser._parse_admin_expand): New methods (one for each keyword that
  can appear in the admin section).

  (_Parser.admin_token_map): A map from keyword name to the method
  used to handle that keyword.

  (_Parser.parse_rcs_admin): Look up the keyword tokens in the map
  instead of using a big "if" statement.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-05 23:22:36.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-05 23:56:40.000000000 +0100
@@ -90,75 +90,97 @@
 class _Parser:
   stream_class = None   # subclasses need to define this
 
+  def _parse_admin_head(self, token):
+    semi, rev = self.ts.mget(2)
+    self.sink.set_head_revision(rev)
+    if semi != ';':
+      raise RCSExpected(semi, ';')
+
+  def _parse_admin_branch(self, token):
+    semi, branch = self.ts.mget(2)
+    if semi == ';':
+      self.sink.set_principal_branch(branch)
+    else:
+      if branch == ';':
+        self.ts.unget(semi);
+      else:
+        raise RCSExpected(semi, ';')
+
+  def _parse_admin_access(self, token):
+    accessors = []
+    while 1:
+      tag = self.ts.get()
+      if tag == ';':
+        if accessors != []:
+          self.sink.set_access(accessors)
+        return
+      accessors = accessors + [ tag ]
+
+  def _parse_admin_symbols(self, token):
+    while 1:
+      tag = self.ts.get()
+      if tag == ';':
+        break
+      self.ts.match(':')
+      tag_name = tag
+      tag_rev = self.ts.get()
+      self.sink.define_tag(tag_name, tag_rev)
+
+  def _parse_admin_locks(self, token):
+    while 1:
+      tag = self.ts.get()
+      if tag == ';':
+        break
+      self.ts.match(':')
+      locker = tag
+      rev = self.ts.get()
+      self.sink.set_locker(rev, locker)
+
+  def _parse_admin_strict(self, token):
+    self.sink.set_locking("strict")
+    self.ts.match(';')
+
+  def _parse_admin_comment(self, token):
+    semi, comment = self.ts.mget(2)
+    self.sink.set_comment(comment)
+    if semi != ';':
+      raise RCSExpected(semi, ';')
+
+  def _parse_admin_expand(self, token):
+    semi, expand_mode = self.ts.mget(2)
+    self.sink.set_expansion(expand_mode)
+    if semi != ';':
+      raise RCSExpected(semi, ';')
+
+  admin_token_map = {
+      'head' : _parse_admin_head,
+      'branch' : _parse_admin_branch,
+      'access' : _parse_admin_access,
+      'symbols' : _parse_admin_symbols,
+      'locks' : _parse_admin_locks,
+      'strict' : _parse_admin_strict,
+      'comment' : _parse_admin_comment,
+      'expand' : _parse_admin_expand,
+      }
+
   def parse_rcs_admin(self):
     while 1:
       # Read initial token at beginning of line
       token = self.ts.get()
 
-      # We're done once we reach the description of the RCS tree
-      if token[0] in string.digits:
-        self.ts.unget(token)
-        return
-
-      if token == "head":
-        semi, rev = self.ts.mget(2)
-        self.sink.set_head_revision(rev)
-        if semi != ';':
-          raise RCSExpected(semi, ';')
-      elif token == "branch":
-        semi, branch = self.ts.mget(2)
-        if semi == ';':
-          self.sink.set_principal_branch(branch)
+      try:
+        f = self.admin_token_map[token]
+      except KeyError:
+        # We're done once we reach the description of the RCS tree
+        if token[0] in string.digits:
+          self.ts.unget(token)
+          return
         else:
-          if branch == ';':
-            self.ts.unget(semi);
-          else:
-            raise RCSExpected(semi, ';')
-      elif token == "access":
-        accessors = []
-        while 1:
-          tag = self.ts.get()
-          if tag == ';':
-            if accessors != []:
-              self.sink.set_access(accessors)
-            break
-          accessors = accessors + [ tag ]
-      elif token == "symbols":
-        while 1:
-          tag = self.ts.get()
-          if tag == ';':
-            break
-          self.ts.match(':')
-          tag_name = tag
-          tag_rev = self.ts.get()
-          self.sink.define_tag(tag_name, tag_rev)
-      elif token == "locks":
-        while 1:
-          tag = self.ts.get()
-          if tag == ';':
-            break
-          self.ts.match(':')
-          locker = tag
-          rev = self.ts.get()
-          self.sink.set_locker(rev, locker)
-      elif token == "strict":
-        self.sink.set_locking("strict")
-        self.ts.match(';')
-      elif token == "comment":
-        semi, comment = self.ts.mget(2)
-        self.sink.set_comment(comment)
-        if semi != ';':
-          raise RCSExpected(semi, ';')
-      elif token == "expand":
-        semi, expand_mode = self.ts.mget(2)
-        self.sink.set_expansion(expand_mode)
-        if semi != ';':
-          raise RCSExpected(semi, ';')
-
-      # Chew up "newphrase"
+          # Chew up "newphrase"
+          # warn("Unexpected RCS token: $token\n")
+          pass
       else:
-        pass
-        # warn("Unexpected RCS token: $token\n")
+        f(self, token)
 
     raise RuntimeError, "Unexpected EOF"

Remove dead code.

* lib/vclib/ccvs/rcsparse/common.py (_Parser.parse_rcs_admin): Remove
  dead code (the only way to escape from the loop is via "return").
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 14:02:04.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 14:02:05.000000000 +0100
@@ -182,8 +182,6 @@
       else:
         f(self, token)
 
-    raise RuntimeError, "Unexpected EOF"
-
   def parse_rcs_tree(self):
     while 1:
       revision = self.ts.get()

Handle a "head" keyword with no revision number.

According to rcsfile(5), this is allowed.  In this case, don't invoke
any callbacks.

* lib/vclib/ccvs/rcsparse/common.py (_Parser._parse_admin_head): Handle
  the case that the "head" keyword doesn't list a revision.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 16:00:35.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 16:02:10.000000000 +0100
@@ -91,10 +91,16 @@
   stream_class = None   # subclasses need to define this
 
   def _parse_admin_head(self, token):
-    semi, rev = self.ts.mget(2)
-    self.sink.set_head_revision(rev)
-    if semi != ';':
-      raise RCSExpected(semi, ';')
+    rev = self.ts.get()
+    if rev == ';':
+      # The head revision is not specified.  Just drop the semicolon
+      # on the floor.
+      pass
+    else:
+      self.sink.set_head_revision(rev)
+      semi = self.ts.get()
+      if semi != ';':
+        raise RCSExpected(semi, ';')
 
   def _parse_admin_branch(self, token):
     semi, branch = self.ts.mget(2)

Handle files with empty "delta" (a.k.a., "tree") part.

According to rcsfile(5), there don't have to be any deltas in an RCS
file.  These files can be created using "rcs -i -t- foofile".  This
change allows rcsparse to handle such files.

This change might help fix cvs2svn Issue #80:
http://cvs2svn.tigris.org/issues/show_bug.cgi?id=80

* lib/vclib/ccvs/rcsparse/common.py (_Parser.admin_token_map): Add an
  entry for "desc" (because if the tree is empty, the next thing
  encountered after the administrative information is "desc").

  (_Parser.parse_rcs_admin): Terminate loop if "desc" is seen.

* lib/vclib/ccvs/rcsparse/test-data/empty-file,v,
  lib/vclib/ccvs/rcsparse/test-data/empty-file.out: Add a test RCS
  file with no contents to test data, and the results expected when
  parsing the file.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 16:02:10.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 16:03:46.000000000 +0100
@@ -167,6 +167,7 @@
       'strict' : _parse_admin_strict,
       'comment' : _parse_admin_comment,
       'expand' : _parse_admin_expand,
+      'desc' : None,
       }
 
   def parse_rcs_admin(self):
@@ -186,7 +187,11 @@
           # warn("Unexpected RCS token: $token\n")
           pass
       else:
-        f(self, token)
+        if f is None:
+          self.ts.unget(token)
+          return
+        else:
+          f(self, token)
 
   def parse_rcs_tree(self):
     while 1:
Index: lib/vclib/ccvs/rcsparse/test-data/empty-file,v
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ lib/vclib/ccvs/rcsparse/test-data/empty-file,v	2007-11-10 16:03:46.000000000 +0100
@@ -0,0 +1,10 @@
+head	;
+access;
+symbols;
+locks; strict;
+comment	@# @;
+
+
+
+desc
+@@
Index: lib/vclib/ccvs/rcsparse/test-data/empty-file.out
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ lib/vclib/ccvs/rcsparse/test-data/empty-file.out	2007-11-10 16:04:28.000000000 +0100
@@ -0,0 +1,6 @@
+set_locking('strict')
+set_comment('# ')
+admin_completed()
+tree_completed()
+set_description('')
+parse_completed()

Simplify the logic in texttools._mxTokenStream.match() (remove duplicate code).

* lib/vclib/ccvs/rcsparse/texttools.py (_mxTokenStream.match):
  Remove duplicate code.
Index: lib/vclib/ccvs/rcsparse/texttools.py
===================================================================
--- lib/vclib/ccvs/rcsparse/texttools.py.orig	2007-11-10 14:14:43.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/texttools.py	2007-11-10 14:15:12.000000000 +0100
@@ -323,16 +323,13 @@
   def match(self, match):
     if self.tokens:
       token = self.tokens.pop()
-      if token != match:
-        raise RuntimeError, ('Unexpected parsing error in RCS file.\n'
-                             'Expected token: %s, but saw: %s'
-                             % (match, token))
     else:
       token = self.get()
-      if token != match:
-        raise RuntimeError, ('Unexpected parsing error in RCS file.\n'
-                             'Expected token: %s, but saw: %s'
-                             % (match, token))
+
+    if token != match:
+      raise RuntimeError, ('Unexpected parsing error in RCS file.\n'
+                           'Expected token: %s, but saw: %s'
+                           % (match, token))
 
   def unget(self, token):
     self.tokens.append(token)

Improve the error message for RCSExpected exceptions.

* lib/vclib/ccvs/rcsparse/common.py (RCSExpected.__init__): Improve
  the error message passed to the base class constructor.  Also
  improve spacing around the exception class definitions.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 14:46:28.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 14:47:46.000000000 +0100
@@ -72,16 +72,26 @@
 
 class RCSParseError(Exception):
   pass
+
+
 class RCSIllegalCharacter(RCSParseError):
   pass
-### need more work on this one
+
+
 class RCSExpected(RCSParseError):
   def __init__(self, got, wanted):
-    RCSParseError.__init__(self, got, wanted)
+    RCSParseError.__init__(
+        self,
+        'Unexpected parsing error in RCS file.\n'
+        'Expected token: %s, but saw: %s'
+        % (wanted, got)
+        )
+
 
 class RCSStopParser(Exception):
   pass
 
+
 # --------------------------------------------------------------------------
 #
 # STANDARD TOKEN STREAM-BASED PARSER

Change the token stream match() methods to raise RCSExpected exceptions.

Previously, they raised a naked RuntimeError.  Nobody seems to catch
these exceptions specifically, anyway.

* lib/vclib/ccvs/rcsparse/default.py (_TokenStream.match): Raise
  RCSExpected exception in the case of a mismatch.

* lib/vclib/ccvs/rcsparse/texttools.py (_mxTokenStream.match): Raise
  RCSExpected exception in the case of a mismatch.
Index: lib/vclib/ccvs/rcsparse/default.py
===================================================================
--- lib/vclib/ccvs/rcsparse/default.py.orig	2007-11-10 14:14:28.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/default.py	2007-11-10 14:15:17.000000000 +0100
@@ -134,8 +134,7 @@
 
     token = self.get()
     if token != match:
-      raise RuntimeError, ('Unexpected parsing error in RCS file.\n' +
-                           'Expected token: %s, but saw: %s' % (match, token))
+      raise common.RCSExpected(token, match)
 
   def unget(self, token):
     "Put this token back, for the next get() to return."
Index: lib/vclib/ccvs/rcsparse/texttools.py
===================================================================
--- lib/vclib/ccvs/rcsparse/texttools.py.orig	2007-11-10 14:15:21.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/texttools.py	2007-11-10 14:15:43.000000000 +0100
@@ -327,9 +327,7 @@
       token = self.get()
 
     if token != match:
-      raise RuntimeError, ('Unexpected parsing error in RCS file.\n'
-                           'Expected token: %s, but saw: %s'
-                           % (match, token))
+      raise common.RCSExpected(token, match)
 
   def unget(self, token):
     self.tokens.append(token)

Use match() to handle semicolons when possible.

This simplifies the code without a measureable performance cost.

* lib/vclib/ccvs/rcsparse/common.py (_Parser._parse_admin_head,
  _Parser._parse_admin_branch, _Parser._parse_admin_comment,
  _Parser._parse_admin_expand): Use the token scanner's match() method
  to handle semicolons.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 16:32:48.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 16:42:40.000000000 +0100
@@ -108,19 +108,13 @@
       pass
     else:
       self.sink.set_head_revision(rev)
-      semi = self.ts.get()
-      if semi != ';':
-        raise RCSExpected(semi, ';')
+      self.ts.match(';')
 
   def _parse_admin_branch(self, token):
-    semi, branch = self.ts.mget(2)
-    if semi == ';':
+    branch = self.ts.get()
+    if branch != ';':
       self.sink.set_principal_branch(branch)
-    else:
-      if branch == ';':
-        self.ts.unget(semi);
-      else:
-        raise RCSExpected(semi, ';')
+      self.ts.match(';')
 
   def _parse_admin_access(self, token):
     accessors = []
@@ -157,16 +151,13 @@
     self.ts.match(';')
 
   def _parse_admin_comment(self, token):
-    semi, comment = self.ts.mget(2)
-    self.sink.set_comment(comment)
-    if semi != ';':
-      raise RCSExpected(semi, ';')
+    self.sink.set_comment(self.ts.get())
+    self.ts.match(';')
 
   def _parse_admin_expand(self, token):
-    semi, expand_mode = self.ts.mget(2)
+    expand_mode = self.ts.get()
     self.sink.set_expansion(expand_mode)
-    if semi != ';':
-      raise RCSExpected(semi, ';')
+    self.ts.match(';')
 
   admin_token_map = {
       'head' : _parse_admin_head,

Rename some local variables according to their uses.

* lib/vclib/ccvs/rcsparse/common.py (_Parser._parse_admin_access,
  _Parser._parse_admin_symbols, _Parser._parse_admin_locks): Rename
  local variables.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 17:03:46.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 17:07:14.000000000 +0100
@@ -119,30 +119,28 @@
   def _parse_admin_access(self, token):
     accessors = []
     while 1:
-      tag = self.ts.get()
-      if tag == ';':
+      accessor = self.ts.get()
+      if accessor == ';':
         if accessors != []:
           self.sink.set_access(accessors)
         return
-      accessors = accessors + [ tag ]
+      accessors = accessors + [ accessor ]
 
   def _parse_admin_symbols(self, token):
     while 1:
-      tag = self.ts.get()
-      if tag == ';':
+      tag_name = self.ts.get()
+      if tag_name == ';':
         break
       self.ts.match(':')
-      tag_name = tag
       tag_rev = self.ts.get()
       self.sink.define_tag(tag_name, tag_rev)
 
   def _parse_admin_locks(self, token):
     while 1:
-      tag = self.ts.get()
-      if tag == ';':
+      locker = self.ts.get()
+      if locker == ';':
         break
       self.ts.match(':')
-      locker = tag
       rev = self.ts.get()
       self.sink.set_locker(rev, locker)

Simplify _Parser._parse_admin_access().

* lib/vclib/ccvs/rcsparse/common.py (_Parser._parse_admin_access):
  Simplify logic and use list.append(x) instead of "list = list +
  [x]".
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 16:47:18.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 16:47:23.000000000 +0100
@@ -121,10 +121,11 @@
     while 1:
       accessor = self.ts.get()
       if accessor == ';':
-        if accessors != []:
-          self.sink.set_access(accessors)
-        return
-      accessors = accessors + [ accessor ]
+        break
+      accessors.append(accessor)
+
+    if accessors:
+      self.sink.set_access(accessors)
 
   def _parse_admin_symbols(self, token):
     while 1:

Extract method _Parser._parse_rcs_tree_entry().

* lib/vclib/ccvs/rcsparse/common.py (_Parser._parse_rcs_tree_entry):
  New method, extracted from parse_rcs_tree().

  (_Parser.parse_rcs_tree): Use new method.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 16:50:23.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 16:50:26.000000000 +0100
@@ -193,6 +193,88 @@
         else:
           f(self, token)
 
+  def _parse_rcs_tree_entry(self, revision):
+    # Parse date
+    semi, date, sym = self.ts.mget(3)
+    if sym != 'date':
+      raise RCSExpected(sym, 'date')
+    if semi != ';':
+      raise RCSExpected(semi, ';')
+
+    # Convert date into timestamp
+    date_fields = string.split(date, '.') + ['0', '0', '0']
+    date_fields = map(string.atoi, date_fields)
+    # need to make the date four digits for timegm
+    EPOCH = 1970
+    if date_fields[0] < EPOCH:
+        if date_fields[0] < 70:
+            date_fields[0] = date_fields[0] + 2000
+        else:
+            date_fields[0] = date_fields[0] + 1900
+        if date_fields[0] < EPOCH:
+            raise ValueError, 'invalid year'
+
+    timestamp = calendar.timegm(tuple(date_fields))
+
+    # Parse author
+    ### NOTE: authors containing whitespace are violations of the
+    ### RCS specification.  We are making an allowance here because
+    ### CVSNT is known to produce these sorts of authors.
+    self.ts.match('author')
+    author = ''
+    while 1:
+      token = self.ts.get()
+      if token == ';':
+        break
+      author = author + token + ' '
+    author = author[:-1]   # toss the trailing space
+
+    # Parse state
+    self.ts.match('state')
+    state = ''
+    while 1:
+      token = self.ts.get()
+      if token == ';':
+        break
+      state = state + token + ' '
+    state = state[:-1]   # toss the trailing space
+
+    # Parse branches
+    self.ts.match('branches')
+    branches = [ ]
+    while 1:
+      token = self.ts.get()
+      if token == ';':
+        break
+      branches.append(token)
+
+    # Parse revision of next delta in chain
+    next, sym = self.ts.mget(2)
+    if sym != 'next':
+      raise RCSExpected(sym, 'next')
+    if next == ';':
+      next = None
+    else:
+      self.ts.match(';')
+
+    # there are some files with extra tags in them. for example:
+    #    owner	640;
+    #    group	15;
+    #    permissions	644;
+    #    hardlinks	@configure.in@;
+    # this is "newphrase" in RCSFILE(5). we just want to skip over these.
+    while 1:
+      token = self.ts.get()
+      if token == 'desc' or token[0] in string.digits:
+        self.ts.unget(token)
+        break
+      # consume everything up to the semicolon
+      while self.ts.get() != ';':
+        pass
+
+    self.sink.define_revision(revision, timestamp, author, state, branches,
+                              next)
+
   def parse_rcs_tree(self):
     while 1:
       revision = self.ts.get()
@@ -202,86 +284,7 @@
         self.ts.unget(revision)
         return
 
-      # Parse date
-      semi, date, sym = self.ts.mget(3)
-      if sym != 'date':
-        raise RCSExpected(sym, 'date')
-      if semi != ';':
-        raise RCSExpected(semi, ';')
-
-      # Convert date into timestamp
-      date_fields = string.split(date, '.') + ['0', '0', '0']
-      date_fields = map(string.atoi, date_fields)
-      # need to make the date four digits for timegm
-      EPOCH = 1970
-      if date_fields[0] < EPOCH:
-          if date_fields[0] < 70:
-              date_fields[0] = date_fields[0] + 2000
-          else:
-              date_fields[0] = date_fields[0] + 1900
-          if date_fields[0] < EPOCH:
-              raise ValueError, 'invalid year'
-
-      timestamp = calendar.timegm(tuple(date_fields))
-
-      # Parse author
-      ### NOTE: authors containing whitespace are violations of the
-      ### RCS specification.  We are making an allowance here because
-      ### CVSNT is known to produce these sorts of authors.
-      self.ts.match('author')
-      author = ''
-      while 1:
-        token = self.ts.get()
-        if token == ';':
-          break
-        author = author + token + ' '
-      author = author[:-1]   # toss the trailing space
-
-      # Parse state
-      self.ts.match('state')
-      state = ''
-      while 1:
-        token = self.ts.get()
-        if token == ';':
-          break
-        state = state + token + ' '
-      state = state[:-1]   # toss the trailing space
-
-      # Parse branches
-      self.ts.match('branches')
-      branches = [ ]
-      while 1:
-        token = self.ts.get()
-        if token == ';':
-          break
-        branches.append(token)
-
-      # Parse revision of next delta in chain
-      next, sym = self.ts.mget(2)
-      if sym != 'next':
-        raise RCSExpected(sym, 'next')
-      if next == ';':
-        next = None
-      else:
-        self.ts.match(';')
-
-      # there are some files with extra tags in them. for example:
-      #    owner	640;
-      #    group	15;
-      #    permissions	644;
-      #    hardlinks	@configure.in@;
-      # this is "newphrase" in RCSFILE(5). we just want to skip over these.
-      while 1:
-        token = self.ts.get()
-        if token == 'desc' or token[0] in string.digits:
-          self.ts.unget(token)
-          break
-        # consume everything up to the semicolon
-        while self.ts.get() != ';':
-          pass
-
-      self.sink.define_revision(revision, timestamp, author, state, branches,
-                                next)
+      self._parse_rcs_tree_entry(revision)
 
   def parse_rcs_description(self):
     self.ts.match('desc')

Simplify _Parser._parse_rcs_tree_entry().

* lib/vclib/ccvs/rcsparse/common.py (_Parser._parse_rcs_tree_entry):
  Simplify logic.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 16:51:23.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 16:52:38.000000000 +0100
@@ -195,11 +195,9 @@
 
   def _parse_rcs_tree_entry(self, revision):
     # Parse date
-    semi, date, sym = self.ts.mget(3)
-    if sym != 'date':
-      raise RCSExpected(sym, 'date')
-    if semi != ';':
-      raise RCSExpected(semi, ';')
+    self.ts.match('date')
+    date = self.ts.get()
+    self.ts.match(';')
 
     # Convert date into timestamp
     date_fields = string.split(date, '.') + ['0', '0', '0']
@@ -249,9 +247,8 @@
       branches.append(token)
 
     # Parse revision of next delta in chain
-    next, sym = self.ts.mget(2)
-    if sym != 'next':
-      raise RCSExpected(sym, 'next')
+    self.ts.match('next')
+    next = self.ts.get()
     if next == ';':
       next = None
     else:

Improve the handling of dates in _Parser._parse_rcs_tree_entry().

* lib/vclib/ccvs/rcsparse/common.py (_Parser._parse_rcs_tree_entry):
  Adjust the handling of dates to conform more strictly to
  rcsparse(5), and simplify slightly.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 16:50:32.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 16:50:34.000000000 +0100
@@ -200,19 +200,17 @@
     self.ts.match(';')
 
     # Convert date into timestamp
-    date_fields = string.split(date, '.') + ['0', '0', '0']
+    date_fields = string.split(date, '.')
+    # According to rcsfile(5): the year "contains just the last two
+    # digits of the year for years from 1900 through 1999, and all the
+    # digits of years thereafter".
+    if len(date_fields[0]) == 2:
+      date_fields[0] = '19' + date_fields[0]
     date_fields = map(string.atoi, date_fields)
-    # need to make the date four digits for timegm
     EPOCH = 1970
     if date_fields[0] < EPOCH:
-        if date_fields[0] < 70:
-            date_fields[0] = date_fields[0] + 2000
-        else:
-            date_fields[0] = date_fields[0] + 1900
-        if date_fields[0] < EPOCH:
-            raise ValueError, 'invalid year'
-
-    timestamp = calendar.timegm(tuple(date_fields))
+      raise ValueError, 'invalid year'
+    timestamp = calendar.timegm(tuple(date_fields + [0, 0, 0]))
 
     # Parse author
     ### NOTE: authors containing whitespace are violations of the

Add method _Parser._read_until_semicolon(), and use it in other methods.

* lib/vclib/ccvs/rcsparse/common.py (_Parser._read_until_semicolon):
  New method.

  (_Parser._parse_admin_access, _Parser._parse_rcs_tree_entry): Use
  the new method.
Index: lib/vclib/ccvs/rcsparse/common.py
===================================================================
--- lib/vclib/ccvs/rcsparse/common.py.orig	2007-11-10 16:55:30.000000000 +0100
+++ lib/vclib/ccvs/rcsparse/common.py	2007-11-10 17:00:13.000000000 +0100
@@ -100,6 +100,22 @@
 class _Parser:
   stream_class = None   # subclasses need to define this
 
+  def _read_until_semicolon(self):
+    """Read all tokens up to (but not including) the next semicolon token.
+
+    Return the tokens as a list.  Consume the terminating
+    semicolon."""
+
+    tokens = []
+
+    while 1:
+      token = self.ts.get()
+      if token == ';':
+        break
+      tokens.append(token)
+
+    return tokens
+
   def _parse_admin_head(self, token):
     rev = self.ts.get()
     if rev == ';':
@@ -117,13 +133,7 @@
       self.ts.match(';')
 
   def _parse_admin_access(self, token):
-    accessors = []
-    while 1:
-      accessor = self.ts.get()
-      if accessor == ';':
-        break
-      accessors.append(accessor)
-
+    accessors = self._read_until_semicolon()
     if accessors:
       self.sink.set_access(accessors)
 
@@ -217,13 +227,7 @@
     ### RCS specification.  We are making an allowance here because
     ### CVSNT is known to produce these sorts of authors.
     self.ts.match('author')
-    author = ''
-    while 1:
-      token = self.ts.get()
-      if token == ';':
-        break
-      author = author + token + ' '
-    author = author[:-1]   # toss the trailing space
+    author = ' '.join(self._read_until_semicolon())
 
     # Parse state
     self.ts.match('state')
@@ -237,12 +241,7 @@
 
     # Parse branches
     self.ts.match('branches')
-    branches = [ ]
-    while 1:
-      token = self.ts.get()
-      if token == ';':
-        break
-      branches.append(token)
+    branches = self._read_until_semicolon()
 
     # Parse revision of next delta in chain
     self.ts.match('next')
@@ -264,8 +263,7 @@
         self.ts.unget(token)
         break
       # consume everything up to the semicolon
-      while self.ts.get() != ';':
-        pass
+      self._read_until_semicolon()
 
     self.sink.define_revision(revision, timestamp, author, state, branches,
                               next)