[PATCH 2 of 2] subrepo: Subversion support

Matt Mackall mpm at selenic.com
Tue Dec 8 16:11:03 CST 2009


On Tue, 2009-12-08 at 15:15 -0600, durin42 at gmail.com wrote:
> # HG changeset patch
> # User Augie Fackler <durin42 at gmail.com>
> # Date 1260296745 21600
> # Node ID e7cf66df1d49d1f48822b6be5ca77ab3f66afcc8
> # Parent  7bb991c4c3da3996272858798959de9de48e4f99
> subrepo: Subversion support

Hurray! Looks like a good start.

> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -5,12 +5,21 @@
>  # This software may be used and distributed according to the terms of the
>  # GNU General Public License version 2, incorporated herein by reference.
>  
> -import errno, os
> +import errno, os, re
>  from i18n import _
>  import config, util, node, error
>  hg = None
>  
> -nullstate = ('', '')
> +class proxysubrepo(object):
> +    def __init__(self, *args):
> +        self.args = args
> +
> +    def get(self, state):
> +        self.__class__ = state[2]
> +        self.__init__(*self.args)
> +        return self.get(state)
> +
> +nullstate = ('', '', proxysubrepo)

?

>  def state(ctx):
>      p = config.config()
> @@ -34,8 +43,14 @@
>                  raise
>  
>      state = {}
> +    sections = set(p.sections())
> +    map(sections.discard, ['svn', ''])
> +    if sections:
> +        raise util.Abort(_('unrecognized subrepo types %s') % ', '.join(sections))
>      for path, src in p[''].items():
> -        state[path] = (src, rev.get(path, ''))
> +        state[path] = (src, rev.get(path, ''), hgsubrepo)
> +    for path, src in p['svn'].items():
> +        state[path] = (src, rev.get(path, ''), svnsubrepo)

Why don't you just put a type string here ('hg') and have a class lookup
function later? 
 
>      return state
>  
> @@ -56,7 +71,7 @@
>  
>      def debug(s, msg, r=""):
>          if r:
> -            r = "%s:%s" % r
> +            r = "%s:%s" % r[:-1]
>          repo.ui.debug("  subrepo %s: %s %s\n" % (s, msg, r))
>  
>      for s, l in s1.items():
> @@ -145,9 +160,86 @@
>  
>      util.path_auditor(ctx._repo.root)(path)
>      state = ctx.substate.get(path, nullstate)
> -    if state[0].startswith('['): # future expansion
> -        raise error.Abort('unknown subrepo source %s' % state[0])

Hmm, apparently I'd planned to prefix non-hg paths with '[<type>]'. I'm
afraid we should probably use that for backward compatibility with 1.3.
1.3 will thus refuse to checkout revs where SVN support is needed.

This also suggests that my plan was just to leave the config parser as
is (src/path pairs) and deal with selecting the subrepo type here.

> -    return hgsubrepo(ctx, path, state)
> +    return state[2](ctx, path, state[:2])

I think the way to do things here is to be 'table-driven'. This will
allow easy plug-ins. For instance:

types = {
  'hg': hgsubrepo,
  'svn': svnsubrepo,
}

...

    src, type = parsesrctype(state[0])
    return types[type](ctx, path, state)

Then when someone finally gets around to doing Git/CVS/... support, they
add one line to the types table and a class and they're done.

If you go down this path, you should probably send a patch that does the
table lookup with one item, followed by a patch that adds SVN.

> +class svnsubrepo(object):
> +    def __init__(self, ctx, path, state):
> +        self._path = path
> +        self._state = state
> +        self._ctx = ctx
> +        self._ui = ctx._repo.ui
> +
> +    def _svncommand(self, commands):
> +        cmd = ['svn'] + commands + [self._path]
> +        cmd = [util.shellquote(arg) for arg in cmd]
> +        cmd = util.quotecommand(' '.join(cmd))
> +        write, read, err = util.popen3(cmd)
> +        retdata = read.read()
> +        err = err.read().strip()
> +        if err:
> +            raise util.Abort(err)
> +        return retdata
> +
> +    def _wcrev(self):
> +        info = self._svncommand(['info'])
> +        mat = re.search('Revision: ([\d]+)\n', info)
> +        if not mat:
> +            return 0
> +        return mat.groups()[0]
> +
> +    def _url(self):
> +        info = self._svncommand(['info'])
> +        mat = re.search('URL: ([^\n]+)\n', info)
> +        if not mat:
> +            return 0
> +        return mat.groups()[0]
> +
> +    def _wcclean(self):
> +        status = self._svncommand(['status'])
> +        status = '\n'.join([s for s in status.splitlines() if s[0] != '?'])
> +        if status.strip():
> +            return False
> +        return True
> +
> +    def dirty(self):
> +        if self._wcrev() == self._state[1] and self._wcclean():
> +            return False
> +        return True
> +
> +    def commit(self, text, user, date):
> +        # user and date are out of our hands since svn is centralized
> +        if self._wcclean():
> +            return self._wcrev()
> +        commitinfo = self._svncommand(['commit', '-m', text])
> +        self._ui.status(commitinfo)
> +        newrev = re.search('Committed revision ([\d]+).', commitinfo)
> +        if not newrev:
> +            raise util.Abort(commitinfo.splitlines()[-1])
> +        newrev = newrev.groups()[0]
> +        self._ui.status(self._svncommand(['update', '-r', newrev]))
> +        return newrev
> +
> +    def remove(self):
> +        if self.dirty():
> +            self._repo.ui.warn('Not removing repo %s because'
> +                               'it has changes.\n' % self._path)
> +            return
> +        self._repo.ui.note('removing subrepo %s\n' % self._path)
> +        shutil.rmtree(self._ctx.repo.join(self._path))
> +
> +    def get(self, state):
> +        status = self._svncommand(['checkout', state[0], '--revision', state[1]])
> +        if not re.search('Checked out revision [\d]+.', status):
> +            raise util.Abort(status.splitlines()[-1])
> +        self._ui.status()

Looks sane. We should probably briefly document what the expected
semantics are for the basic subrepo methods.

> +    def merge(self, state):
> +        # Pretty sure we should not implement merge on svn subrepos
> +        raise NotImplementedError()

Seems weird, but I've studiously avoided knowing much about SVN. I
suspect we want some update to happen here.

-- 
http://selenic.com : development and support for Mercurial and Linux




More information about the Mercurial-devel mailing list