Got some comments for you.

I'm not sure about hardcoding the pserver method here.  Perhaps it'll do for now.

This seems really fragile, given that there's a datestring in there.  Can you convert this to a regular expression, or at least split on whitespace?

Instead of the (truly ugly) isfloat method below, can you just do this?

if not re.match('\d\\.\d', revision):
    raise SCMException('Unable to parse diff revision header "%s"' % \
                       revision_str)

This is a lot more complicated than it needs to be.  Here's what I recommend:

for line in file.splitlines():
    m = re.match('^RCS file: %s/(.*),v$' % self.repopath, line) or \
        re.match('^RCS file: (.+)$', line)
    if m:
        self.extrapath.append(m.group(1))
        return
    if line[:10] == 'RCS file: ':
        raise SCMException('Unable to parse RCS line "%s"' % line)

I'm not sure those last two lines are even necessary.  Will
cvs diff -u ever do that?

As far as .cvspass is concerned, maybe this could read the file in CVSTool.__init__ and add the necessary line yourself?

Some more comments.

Might be time for us to come up with some sort of authmethod field in the database.

Excess Whitespace. I'm only going to mark this one, but it'd be nice to go through the diff and remove the others.

I can't remember.. Is there a case where we actually pass in a path that's None?

Is there any exception we can catch that would give us a FileNotFound?

You can condense this down to:

return file_str, PRE_CREATION

This can be condensed to:

return self.extrapath.pop(0), revision

I've been thinking about this parse_extra_info_headers thing a bit more. Passing the entire diff and parsing it a second time is kind of excessive.

Perhaps we should go a slightly different route. Parse the file diff, store everything we don't recognize, and pass that to the SCMTool? Of course, this possibly opens up additional problems where we have to suddenly understand every kind of diff. This is food for thought, not something that would have to be done for this change :)

This is looking pretty good!  Just a couple comments on the code:

OK, very minor style comment.  Can you write these lines like this:

import re

import pycvs

from reviewboard.scmtools.core import SCMException, FileNotFoundException, \
                                      SCMTool, HEAD, PRE_CREATION


This is how PEP-8 recommends formatting imports.

Don't you need \d+ here?  This regex won't match "1.15" or "10.2"

What more needs to be done with this before it can go into SVN?

As David mentioned, this won't match multiple digits.  It also needs to match branch revisions (1.2.3.4), and you should make sure that the .* doesn't get carried away.

You can use r'' to avoid the extra escapes.

My suggestion: r'^.*?(\d+(\.\d+)+)$'

I don't like this general FIFO mechanism you're using, because it assumes that parse_diff_revision will always be called in the exact same order of the diff file.  This may be true now, but that's a dangerous assumption.

Can you at least throw in an assertion that the popped value .endswith(file_str)?

This should be:

  raise SCMException("File '%s' doesn't match the parsed file '%s'" %\
  (file_str, fullfile))

The CVS server may be accessed using something other than pserver (ext, for example). There should be a way to configure that, I suppose.

This regex doesn't work with diffs created on a Windows machine. I managed to fix that with the following:

  self.regex_rev   = re.compile(r'^.*?(\d+(\.\d+)+)\r?$')

Got some more comments for you on this.

Add a blank line after this as per PEP-8

1 space between each token of variable = X.  Aligning them up like this is useful in some cases but for disparate data like this, just makes it hard to maintain.  Notice that you've already got one that ended up misaligned during your revisions ;)

We don't want to compile this regex each time an instance is initialized, since SCMTools are short-lived.  Make this class data like this:

class CVSTool(SCMTool):
    regex_rev = re.compile(...)

    def __init__(self, repository):
        ...

That way the regexs are compiled once for the entire program.

Two blank lines between classes.

Same deal as above with class data.

One way you could fix the problem with /dev/null is to have this rewrite origFile, origInfo as something like (file.newFile, 'PRE-CREATION') if origFile == '/dev/null'.  Then above in parse_diff_revision, check that the info string is 'PRE-CREATION'

Add another blank line here.

Please wrap these to 80 columns.

This is looking very good -- I'm already using it for our internal site.  I have a few more comments, but I hope to see it committed soon.

This member variable is never used.

It's a bit painful to scan every line for RCS info.  I think extrapath is not needed anymore -- see below.

Rather than relying on extrapath, which assumes things happen in an exact order, I think you can now just parse for the RCS line directly.  Just scan for a few lines before linenum for the matching RCS.  My patches always seem to have RCS at (linenum-3), but you can add a little slop to that.

Rather than format-printing '-d%s', just pass it as two separate arguments: '-d', self.repository.  Same for '-r%s'.

This looks great.  Is it ready to be committed?