[PATCH 3 of 3] convert: cvs.py - Allow user to use built-in CVS changeset code

[Patrick Mézard]

Frank Kingswood a écrit :
> # HG changeset patch
> # User Frank Kingswood <frank at kingswood-consulting.co.uk>
> # Date 1212833736 -3600
> # Node ID 5a37a8687c4315640c7ab9b9cb1e5f1e60dba102
> # Parent  69b67d6de865c70ecab814faf5a78acac6647627
> convert: cvs.py - Allow user to use built-in CVS changeset code.
> tests: add two testcases for CVS conversion with builtin CVS
> including a testcase for issue 1148.

All the following remarks are about coding style, as noticed in my global reply. I won't annotate every problem, just every *kind* of problem.
 
> diff -r 69b67d6de865 -r 5a37a8687c43 hgext/convert/cvs.py
> --- a/hgext/convert/cvs.py	Thu Apr 24 19:34:28 2008 +0100
> +++ b/hgext/convert/cvs.py	Sat Jun 07 11:15:36 2008 +0100
> @@ -3,8 +3,10 @@
>  import os, locale, re, socket
>  from cStringIO import StringIO
>  from mercurial import util
> +from mercurial.i18n import _
>  
>  from common import NoRepo, commit, converter_source, checktool
> +from cvsps import cvsps_create_log,cvsps_create_changeset

Add a space after the comma.

>  
>  class convert_cvs(converter_source):
>      def __init__(self, ui, path, rev=None):
> @@ -14,10 +16,13 @@
>          if not os.path.exists(cvs):
>              raise NoRepo("%s does not look like a CVS checkout" % path)
>  
> +        checktool('cvs')
>          self.cmd = ui.config('convert', 'cvsps', 'cvsps -A -u --cvs-direct -q')
>          cvspsexe = self.cmd.split(None, 1)[0]
> -        for tool in (cvspsexe, 'cvs'):
> -            checktool(tool)
> +        self.builtin = cvspsexe=='builtin'

Should be "cvspsexe == 'builtin'"

> +
> +        if not self.builtin:
> +            checktool(cvspsexe)

Not sure it will work under Windows, since shebangs are not honored. This could be addressed later.

>  
>          self.changeset = {}
>          self.files = {}
> @@ -28,10 +33,11 @@
>          self.cvsroot = file(os.path.join(cvs, "Root")).read()[:-1]
>          self.cvsrepo = file(os.path.join(cvs, "Repository")).read()[:-1]
>          self.encoding = locale.getpreferredencoding()
> -        self._parse()
> +
> +        self._parse(ui)
>          self._connect()
>  
> -    def _parse(self):
> +    def _parse(self,ui):

Add a space after the separating comma after argument lists

>          if self.changeset:
>              return
>  
> @@ -56,80 +62,114 @@
>              id = None
>              state = 0
>              filerevids = {}

[...]

> -                        oldrev, rev = rev.split("->")
> -                        files[file] = rev
> +            if self.builtin:
> +                # builtin cvsps code
> +                ui.status(_('using builtin cvsps\n'))
>  
> -                        # save some information for identifying branch points
> -                        oldrevs.append("%s:%s" % (oldrev, file))
> -                        filerevids["%s:%s" % (rev, file)] = id
> -                elif state == 3:
> -                    # swallow all input
> -                    continue
> +                db=cvsps_create_log(ui,cache='update')

Spaces after and before assignment, argument lists:
"db = cvsps_create_log(ui, cache='update')"

> +                db=cvsps_create_changeset(ui,db,
> +                      fuzz=int(ui.config('convert','cvsps.fuzz',60)),
> +                      mergeto=ui.config('convert','cvsps.mergeto',None),
> +                      mergefrom=ui.config('convert','cvsps.mergefrom',None))
> +
> +                for cs in db:
> +                    if maxrev and cs.Id>maxrev:
> +                        break
> +                    id = str(cs.Id)
> +                    cs.Author = self.recode(cs.Author)
> +                    self.lastbranch[cs.Branch] = id
> +                    cs.Comment = self.recode(cs.Comment)
> +                    date = util.datestr(cs.Date)
> +                    self.tags.update(dict.fromkeys(cs.Tags,id))
> +
> +                    files = {}
> +                    for f in cs.Entries:
> +                        files[f.File]="%s%s"%('.'.join([str(x) for x in f.Revision]),
> +                                              ['','(DEAD)'][f.Dead])

Spaces around the '%' formatter.

> +
> +                    # add current commit to set
> +                    c=commit(author=cs.Author,date=date,
> +                             parents=[str(p.Id) for p in cs.Parents],
> +                             desc=cs.Comment,branch=cs.Branch or '')
> +                    self.changeset[id]=c
> +                    self.files[id]=files
> +            else:
> +                # external cvsps

--
Patrick Mézard