[viewvc-dev] Fwd: Re: Improvement on newphrase handling in cvs2svn_rcsparse/common.py ?

Hi,

I've found an issue in file viewvc-1.0.12/lib/vclib/ccvs/rcsparse/common.py while using it as part of cvs2svn.
Newphrases between the admin and deltas section of an RCS section are not completely skipped: skipping stops when the actual newphrase contains a digit or any of the other special 
values being checked inside parse_rcs_admin.

I'd like to propose following patch:
       # Chew up "newphrase"
       else:
         # warn("Unexpected RCS token: $token\n")
         while self.ts.get() != ';':
           pass
         pass

as a replacement for the current code on lines 150-153:
       # Chew up "newphrase"
       else:
         pass
         # warn("Unexpected RCS token: $token\n")

I'm sorry if this is not the right method or place to report this kind of patch request, but I've been unable to open a ticket in your Issue tracker (as mentioned in 
http://www.viewvc.org/contributing.html).

Regards,
Giovanni Pellicciotta


-------- Original Message --------
Subject: 	Re: Improvement on newphrase handling in cvs2svn_rcsparse/common.py ?
Date: 	Thu, 07 Apr 2011 15:34:14 +0200
From: 	Michael Haggerty <[email protected]>
To: 	Giovanni Pellicciotta <[email protected]>
CC: 	[email protected]



On 04/07/2011 01:36 PM, Giovanni Pellicciotta wrote:
>  I'm not sure whether you consider this a bug or not, but I encountered a
>  number of 'ERROR: 'xxx.c,v' is not a valid ,v file' error messages when
>  converting a CVS repository with cvs2svn 2.3.0.

Thanks a lot for the excellent bug report, analysis, and patch!  I
indeed consider this a bug.

>  [...] This code implicitly assumes that a newphrase is a single token that can
>  be discarded (the lines starting with # Chew up "newphrase"):

...or multiple tokens, provided that none of them are in admin_token_map
or start with a digit...

>    def parse_rcs_admin(self):
>      while 1:
>        # Read initial token at beginning of line
>        token = self.ts.get()
>
>        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:
>            # Chew up "newphrase"
>            # warn("Unexpected RCS token: $token\n")
>            pass
>        else:
>          if f is None:
>            self.ts.unget(token)
>            return
>          else:
>            f(self, token)
>
>  However, a newphrase can consist of multiple tokens and ends with a
>  semicolon, so IMHO the code could be improved to handle this better, as
>  follows:
>
>    def _parse_admin_newphrase(self):
>      while 1:
>        token = self.ts.get()
>        if token == ';':
>          break
>
>    def parse_rcs_admin(self):
>      while 1:
>        # Read initial token at beginning of line
>        token = self.ts.get()
>        print "FOUND TOKEN: %s" % (token)
>        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:
>            # Chew up "newphrase"
>            self._parse_admin_newphrase()
>        else:
>          if f is None:
>            self.ts.unget(token)
>            return
>          else:
>            f(self, token)

Yes, this looks very reasonable.  It would make sense to remove the
"print" statement, and the _parse_admin_newphrase() method could be
shortened to

     while self.ts.get() != ';':
       pass

and probably inlined.

It would also be helpful if you would include your RCS file (or a
suitably simplified/anonymized version), for testing purposes.

But...the rcsparse code is actually taken from the ViewVC project [1,2].
  It would be preferable that you submit your bug fix to that project.
Once it is in the upstream project, the cvs2svn project would upgrade to
the fixed version.

Michael

[1] http://www.viewvc.org/
[2] http://www.viewvc.org/contributing.html

-- 
Michael Haggerty
[email protected]
http://softwareswirl.blogspot.com/

------------------------------------------------------
http://viewvc.tigris.org/ds/viewMessage.do?dsForumId=4251&dsMessageId=2718387

To unsubscribe from this discussion, e-mail: [[email protected]].