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

Thanks, Giovanni.  I've created issue #483 in the tracker for this.  (You
have to register for a Tigris.org account and request an Observer role in
the ViewVC project to be able to create tracker issues.)

http://viewvc.tigris.org/issues/show_bug.cgi?id=483


On 04/09/2011 05:49 AM, Giovanni Pellicciotta wrote:
> 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/
> 


-- 
C. Michael Pilato <[email protected]>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

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

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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk2jT2cACgkQokEGqRcG/W5ySgCgjMfBlgQPDps5/YiNSe/mSKNz
BUQAn16Q7gT9atT+R2q+u1Gd4fox2EGw
=QB42
-----END PGP SIGNATURE-----