[Patch try#2] convert: Separate trunk detection from branch layout detection

[Patrick Mézard]

Hello,

Edouard Gomez a écrit :
> # HG changeset patch
> # User Edouard Gomez <ed.gomez at free.fr>
> # Date 1199494281 -3600
> # Node ID 0033a21bd8f7d66acffaa173efac07049e2f0f49
> # Parent  a9f7897466cd6d0bda119c17f795343d3d14f690
> convert: Separate trunk detection from branch layout detection
> 
> In some subversion repositories, trunk is present but no branches
> are used. The current code is assuming that both trunk and branches
> must exist before adding trunk's head to the heads list.
> 
> It's just better to separate the branch layout stuff from the trunk one.
> 
> diff -r a9f7897466cd -r 0033a21bd8f7 hgext/convert/subversion.py
> --- a/hgext/convert/subversion.py	Sat Jan 05 01:51:21 2008 +0100
> +++ b/hgext/convert/subversion.py	Sat Jan 05 01:51:21 2008 +0100
> @@ -177,47 +177,52 @@ class svn_source(converter_source):
>          except SubversionException, err:
>              return False
>  
> +    def getcfgpath(self, name, rev):
> +        cfgpath = self.ui.config('convert', 'svn.' + name)
> +        path = (cfgpath or name).strip('/')
> +        if not self.exists(path, rev):
> +            if cfgpath:
> +                raise util.Abort(_('expected %s to be at %r, but not found')
> +                                 % (name, path))
> +            return None
> +        self.ui.note(_('found %s at %r\n') % (name, path))
> +        return path

I would make a local function instead.

> +
>      def getheads(self):
>          # detect standard /branches, /tags, /trunk layout
>          rev = optrev(self.last_changed)
> -        rpath = self.url.strip('/')
> -        cfgtrunk = self.ui.config('convert', 'svn.trunk')
> -        cfgbranches = self.ui.config('convert', 'svn.branches')
> -        cfgtags = self.ui.config('convert', 'svn.tags')
> -        trunk = (cfgtrunk or 'trunk').strip('/')
> -        branches = (cfgbranches or 'branches').strip('/')
> -        tags = (cfgtags or 'tags').strip('/')
> -        if self.exists(trunk, rev) and self.exists(branches, rev) and self.exists(tags, rev):
> -            self.ui.note('found trunk at %r, branches at %r and tags at %r\n' %
> -                         (trunk, branches, tags))
> +
> +        oldmodule = ''
> +        trunk = self.getcfgpath('trunk', rev)
> +        tags = self.getcfgpath('tags', rev)
> +        branches = self.getcfgpath('branches', rev)
> +
> +        # Till here, project's root is considered to be the module
> +        # and its last revision is considered to be the head.
> +        # In the case the project has a trunk, both head and module
> +        # must be updated accordingly
> +        if trunk:
>              oldmodule = self.module

What about:

oldmodule = self.module or ''

and avoid the two following "(oldmodule or '')" ?

>              self.module += '/' + trunk
>              lt = self.latest(self.module, self.last_changed)
>              self.head = self.revid(lt)
> -            self.heads = [self.head]
> +
> +        # First head in the list is the module's head
> +        self.heads = [self.head]
> +        self.tags = '%s/%s' % ((oldmodule or ''), (tags or 'tags'))

Here the new behaviour differs from the old one. Without a trunk/branch/tags layout, "self.tags = tags", without "/" prefix. Still I cannot make a single example where this would impact the conversion in any way so I think we can keep it.

If you agree with the changes above, I can edit them and push the patch directly, as you wish.

--
Patrick Mézard