[PATCH 1 of 5] pull: add --subrev option

Pierre-Yves David pierre-yves.david at ens-lyon.org
Wed Jan 15 16:15:37 CST 2014


On Sat, Nov 23, 2013 at 09:28:00PM +0100, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra at gmail.com>
> # Date 1372976561 -7200
> #      Fri Jul 05 00:22:41 2013 +0200
> # Node ID ecd1ca99f1b3e4d0f6ea8abd606990ef24667779
> # Parent  1c46b18b0e1c47fa4cecf21b78c083a54ae9903f
> pull: add --subrev option

Why not --subrepos as for other -S option ?

> 
> The purpose of this new option is to make it possible to pull subrepos at the
> same time that you pull the parent repository, instead of having to update to
> the newly pulled revision to pull the corresponding subrepos.

I love that!

> This has multiple advantages. First of all it lets you pull the subrepos from
> a non default path (when subrepos are pulled during udpate they are always
> pulled from the default path). Also, it lets you pull subrepos referenced in all
> new revisions in one go, rather than potentially having to update to every new
> revision.
> 
> The main use case for this new flag is to be able to ensure that your
> repositories are alwasys "self contained".

This is one of the biggest pain point of subrepo so its great to have it solved.

> That is, that you will be able to
> update to any incoming revision without requiring any network access. This can
> be done by adding "--subrev .:tip" to your pull command (it would be nicer if
> there was an "incoming()" revset expression, akin to the existing
> "outgoing(path)" expression).

Wait what? your new -S argument takes an argument to expression which revision
you want to synchronise? why don't he just check all incoming one by default ?

> When the --subrev flag is enabled, pull will also pull (or clone) all subrepos
> that are present on the revisions matching the --subrev revset(s). Pulls are
> recursive (i.e. subrepos withing subrepos will be also pulled as needed), but
> clones are not recursive yet (it requires adding a similar --subrev flag to
> clone, which will be done on another patch).
> 
> If the selected --subrev revisions refer to subrepos that are not on the working
> directory yet they will be cloned.

What is the clone destination then? somewhere in .hg/ ?

> If any of the subrepositories changes its
> pull source (as defined on the .hgsub file) it will be pulled from the current
> and the new source. If a pulled subrepo also refers to one of its own subrepos
> on any of the incoming subrepo revisions those "sub-subrepos" will be pulled
> too.
> 
> This first patch only supports mercurial subrepos (a NotImplementedError
> exception will be raised for all other subrepo types). Future patches will add
> support for other subrepo types.
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -4563,6 +4563,7 @@
>      ('B', 'bookmark', [], _("bookmark to pull"), _('BOOKMARK')),
>      ('b', 'branch', [], _('a specific branch you would like to pull'),
>       _('BRANCH')),
> +    ('S', 'subrev', [], _('pull subrepos from these revisions')),
>      ] + remoteopts,
>      _('[-u] [-f] [-r REV]... [-e CMD] [--remotecmd CMD] [SOURCE]'))
>  def pull(ui, repo, source="default", **opts):
> @@ -4608,7 +4609,8 @@
>                      "so a rev cannot be specified.")
>              raise util.Abort(err)
>  
> -    modheads = repo.pull(other, heads=revs, force=opts.get('force'))
> +    modheads = repo.pull(other, heads=revs, force=opts.get('force'),
> +        subrev=opts.get('subrev'))
>      bookmarks.updatefromremote(ui, repo, remotebookmarks, source)
>      if checkout:
>          checkout = str(repo.changelog.rev(other.lookup(checkout)))
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -11,6 +11,7 @@
>  import lock as lockmod
>  import transaction, store, encoding
>  import scmutil, util, extensions, hook, error, revset
> +import config, urllib2
>  import match as matchmod
>  import merge as mergemod
>  import tags as tagsmod
> @@ -1666,7 +1667,7 @@
>  
>          return r
>  
> -    def pull(self, remote, heads=None, force=False):
> +    def pull(self, remote, heads=None, force=False, subrev=None):
>          if remote.local():
>              missing = set(remote.requirements) - self.supported
>              if missing:
> @@ -1757,8 +1758,49 @@
>                  tr.release()
>              lock.release()
>  
> +        # update current and new subrepos
> +        if subrev:
> +            # pull (or clone) the subrepos that are on any of the revisions
> +            # on the subrev list
> +            self.getsubrepos(subrev=subrev, source=remote.url())
> +
>          return result
>  
> +    def getsubrepos(self, subrev=None, source=None):

1) DO NOT ADD NEW FUNCTION IN LOCALREPOSITORY CLASS!

If you function is not to be called all over the place by a lot of people it belong somewhere else. Please make it a `getsubrepos(repo, subrev=None, source=None)` function in the `mercurial.subrepo` module.

I also think the name in unfortunate: by "get" I would expect something just
returning the list of subrepo object. while you don't return anything but
change their content.

I would rename that `pullsubrepos` (or something related)

> +        """Get (clone or pull) the subrepos that are referenced
> +        on any of the revisions on the given revision list
> +        """
> +        if not subrev:
> +            return
> +        try:
> +            revs = self.revs('%lr', subrev)
> +        except error.RepoLookupError, ex:
> +            # No matches to the subrev revset
> +            self.ui.note(_("no revisions found matching subrev revset '%s'\n"))

I think this should go to standard error output

> +            return
> +        revs.sort()
> +        # use a sortdict to make sure that we get the subrepos
> +        # in the order they are found
> +        substopull = config.sortdict()
> +        for rev in revs:
> +            ctx = self[rev]
> +            # read the substate items in alphabetical order to ensure
> +            # that we always process the subrepos in the same order
> +            for sname in sorted(ctx.substate):
> +                sinfo = ctx.substate[sname]
> +                substopull[(sname, sinfo[0], sinfo[2])] = ctx
> +        try:
> +            self._subtoppath = source
> +            for (sname, ssource, stype), ctx in substopull.items():
> +                try:
> +                    ctx.sub(sname).pull(ssource)
> +                except (error.RepoError, urllib2.HTTPError,
> +                        subrepo.SubrepoAbort), ex:
> +                    self.ui.warn(_('could not pull subrepo %s from %s (%s)\n')
> +                                 % (sname, ssource, str(ex)))

consider honoring the --traceback option here.

> +        finally:
> +            del self._subtoppath
> +
>      def checkpush(self, force, revs):
>          """Extensions can override this function if additional checks have
>          to be performed before pushing, or call it if they override push
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -380,6 +380,11 @@
>          """
>          raise NotImplementedError
>  
> +    def pull(self, source):
> +        """pull from the given parent repository source
> +        """
> +        raise NotImplementedError
> +
>      def get(self, state, overwrite=False):
>          """run whatever commands are needed to put the subrepo into
>          this state
> @@ -646,34 +651,49 @@
>          self._repo.ui.note(_('removing subrepo %s\n') % subrelpath(self))
>          hg.clean(self._repo, node.nullid, False)
>  
> -    def _get(self, state):
> +    def pull(self, source, heads=None, subrev=None):
> +        '''pull (or clone) the subrepository and its subrepos'''
> +        if subrev is None:
> +            # Pull subrepos referenced by all incoming changesets
> +            revs = list(self._repo.changelog.revs())
> +            if revs:
> +                tip = revs[-1]
> +                subrev = ['%d:' % (tip + 1)]
> +            else:
> +                subrev = ['0:']
> +        return self._pullorclone(source, heads=heads, subrev=subrev)
> +
> +    def _get(self, state, subrev=None):
>          source, revision, kind = state
>          if revision not in self._repo:
> -            self._repo._subsource = source
> -            srcurl = _abssource(self._repo)
> -            other = hg.peer(self._repo, {}, srcurl)
> -            if len(self._repo) == 0:
> -                self._repo.ui.status(_('cloning subrepo %s from %s\n')
> -                                     % (subrelpath(self), srcurl))
> -                parentrepo = self._repo._subparent
> -                shutil.rmtree(self._repo.path)
> -                other, cloned = hg.clone(self._repo._subparent.baseui, {},
> -                                         other, self._repo.root,
> -                                         update=False)
> -                self._repo = cloned.local()
> -                self._initrepo(parentrepo, source, create=True)
> +            self._pullorclone(source, subrev=subrev)
> +
> +    def _pullorclone(self, source, heads=None, subrev=None):
> +        self._repo._subsource = source
> +        srcurl = _abssource(self._repo)
> +        other = hg.peer(self._repo, {}, srcurl)
> +        if len(self._repo) == 0:
> +            self._repo.ui.status(_('cloning subrepo %s from %s\n')
> +                                 % (subrelpath(self), srcurl))
> +            parentrepo = self._repo._subparent
> +            shutil.rmtree(self._repo.path)
> +            other, cloned = hg.clone(self._repo._subparent.baseui, {},
> +                                     other, self._repo.root,
> +                                     update=False, heads=heads)
> +            self._repo = cloned.local()
> +            self._initrepo(parentrepo, source, create=True)
> +            self._cachestorehash(srcurl)
> +        else:
> +            self._repo.ui.status(_('pulling subrepo %s from %s\n')
> +                                 % (subrelpath(self), srcurl))
> +            cleansub = self.storeclean(srcurl)
> +            remotebookmarks = other.listkeys('bookmarks')
> +            self._repo.pull(other, heads=heads, subrev=subrev)
> +            bookmarks.updatefromremote(self._repo.ui, self._repo,
> +                                       remotebookmarks, srcurl)
> +            if cleansub:
> +                # keep the repo clean after pull
>                  self._cachestorehash(srcurl)
> -            else:
> -                self._repo.ui.status(_('pulling subrepo %s from %s\n')
> -                                     % (subrelpath(self), srcurl))
> -                cleansub = self.storeclean(srcurl)
> -                remotebookmarks = other.listkeys('bookmarks')
> -                self._repo.pull(other)
> -                bookmarks.updatefromremote(self._repo.ui, self._repo,
> -                                           remotebookmarks, srcurl)
> -                if cleansub:
> -                    # keep the repo clean after pull
> -                    self._cachestorehash(srcurl)
>  
>      @annotatesubrepoerror
>      def get(self, state, overwrite=False):
> diff --git a/tests/test-completion.t b/tests/test-completion.t
> --- a/tests/test-completion.t
> +++ b/tests/test-completion.t
> @@ -206,7 +206,7 @@
>    init: ssh, remotecmd, insecure
>    log: follow, follow-first, date, copies, keyword, rev, removed, only-merges, user, only-branch, branch, prune, patch, git, limit, no-merges, stat, graph, style, template, include, exclude
>    merge: force, rev, preview, tool
> -  pull: update, force, rev, bookmark, branch, ssh, remotecmd, insecure
> +  pull: update, force, rev, bookmark, branch, subrev, ssh, remotecmd, insecure
>    push: force, rev, bookmark, branch, new-branch, ssh, remotecmd, insecure
>    remove: after, force, include, exclude
>    serve: accesslog, daemon, daemon-pipefds, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate

> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra at gmail.com>
> # Date 1372976561 -7200
> #      Fri Jul 05 00:22:41 2013 +0200
> # Node ID ecd1ca99f1b3e4d0f6ea8abd606990ef24667779
> # Parent  1c46b18b0e1c47fa4cecf21b78c083a54ae9903f
> pull: add --subrev option
> 
> The purpose of this new option is to make it possible to pull subrepos at the
> same time that you pull the parent repository, instead of having to update to
> the newly pulled revision to pull the corresponding subrepos.
> 
> This has multiple advantages. First of all it lets you pull the subrepos from
> a non default path (when subrepos are pulled during udpate they are always
> pulled from the default path). Also, it lets you pull subrepos referenced in all
> new revisions in one go, rather than potentially having to update to every new
> revision.
> 
> The main use case for this new flag is to be able to ensure that your
> repositories are alwasys "self contained". That is, that you will be able to
> update to any incoming revision without requiring any network access. This can
> be done by adding "--subrev .:tip" to your pull command (it would be nicer if
> there was an "incoming()" revset expression, akin to the existing
> "outgoing(path)" expression).
> 
> When the --subrev flag is enabled, pull will also pull (or clone) all subrepos
> that are present on the revisions matching the --subrev revset(s). Pulls are
> recursive (i.e. subrepos withing subrepos will be also pulled as needed), but
> clones are not recursive yet (it requires adding a similar --subrev flag to
> clone, which will be done on another patch).
> 
> If the selected --subrev revisions refer to subrepos that are not on the working
> directory yet they will be cloned. If any of the subrepositories changes its
> pull source (as defined on the .hgsub file) it will be pulled from the current
> and the new source. If a pulled subrepo also refers to one of its own subrepos
> on any of the incoming subrepo revisions those "sub-subrepos" will be pulled
> too.
> 
> This first patch only supports mercurial subrepos (a NotImplementedError
> exception will be raised for all other subrepo types). Future patches will add
> support for other subrepo types.
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -4563,6 +4563,7 @@
>      ('B', 'bookmark', [], _("bookmark to pull"), _('BOOKMARK')),
>      ('b', 'branch', [], _('a specific branch you would like to pull'),
>       _('BRANCH')),
> +    ('S', 'subrev', [], _('pull subrepos from these revisions')),
>      ] + remoteopts,
>      _('[-u] [-f] [-r REV]... [-e CMD] [--remotecmd CMD] [SOURCE]'))
>  def pull(ui, repo, source="default", **opts):
> @@ -4608,7 +4609,8 @@
>                      "so a rev cannot be specified.")
>              raise util.Abort(err)
>  
> -    modheads = repo.pull(other, heads=revs, force=opts.get('force'))
> +    modheads = repo.pull(other, heads=revs, force=opts.get('force'),
> +        subrev=opts.get('subrev'))
>      bookmarks.updatefromremote(ui, repo, remotebookmarks, source)
>      if checkout:
>          checkout = str(repo.changelog.rev(other.lookup(checkout)))
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -11,6 +11,7 @@
>  import lock as lockmod
>  import transaction, store, encoding
>  import scmutil, util, extensions, hook, error, revset
> +import config, urllib2
>  import match as matchmod
>  import merge as mergemod
>  import tags as tagsmod
> @@ -1666,7 +1667,7 @@
>  
>          return r
>  
> -    def pull(self, remote, heads=None, force=False):
> +    def pull(self, remote, heads=None, force=False, subrev=None):
>          if remote.local():
>              missing = set(remote.requirements) - self.supported
>              if missing:
> @@ -1757,8 +1758,49 @@
>                  tr.release()
>              lock.release()
>  
> +        # update current and new subrepos
> +        if subrev:
> +            # pull (or clone) the subrepos that are on any of the revisions
> +            # on the subrev list
> +            self.getsubrepos(subrev=subrev, source=remote.url())
> +
>          return result
>  
> +    def getsubrepos(self, subrev=None, source=None):
> +        """Get (clone or pull) the subrepos that are referenced
> +        on any of the revisions on the given revision list
> +        """
> +        if not subrev:
> +            return
> +        try:
> +            revs = self.revs('%lr', subrev)
> +        except error.RepoLookupError, ex:
> +            # No matches to the subrev revset
> +            self.ui.note(_("no revisions found matching subrev revset '%s'\n"))
> +            return
> +        revs.sort()
> +        # use a sortdict to make sure that we get the subrepos
> +        # in the order they are found
> +        substopull = config.sortdict()
> +        for rev in revs:
> +            ctx = self[rev]
> +            # read the substate items in alphabetical order to ensure
> +            # that we always process the subrepos in the same order
> +            for sname in sorted(ctx.substate):
> +                sinfo = ctx.substate[sname]
> +                substopull[(sname, sinfo[0], sinfo[2])] = ctx
> +        try:
> +            self._subtoppath = source
> +            for (sname, ssource, stype), ctx in substopull.items():
> +                try:
> +                    ctx.sub(sname).pull(ssource)
> +                except (error.RepoError, urllib2.HTTPError,
> +                        subrepo.SubrepoAbort), ex:
> +                    self.ui.warn(_('could not pull subrepo %s from %s (%s)\n')
> +                                 % (sname, ssource, str(ex)))
> +        finally:
> +            del self._subtoppath

This _subtoppath stuff is weird. can't we do something cleaner ?

>      def checkpush(self, force, revs):
>          """Extensions can override this function if additional checks have
>          to be performed before pushing, or call it if they override push
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -380,6 +380,11 @@
>          """
>          raise NotImplementedError
>  
> +    def pull(self, source):
> +        """pull from the given parent repository source
> +        """
> +        raise NotImplementedError
> +
>      def get(self, state, overwrite=False):
>          """run whatever commands are needed to put the subrepo into
>          this state
> @@ -646,34 +651,49 @@
>          self._repo.ui.note(_('removing subrepo %s\n') % subrelpath(self))
>          hg.clean(self._repo, node.nullid, False)
>  
> -    def _get(self, state):
> +    def pull(self, source, heads=None, subrev=None):
> +        '''pull (or clone) the subrepository and its subrepos'''
> +        if subrev is None:
> +            # Pull subrepos referenced by all incoming changesets
> +            revs = list(self._repo.changelog.revs())
> +            if revs:
> +                tip = revs[-1]
> +                subrev = ['%d:' % (tip + 1)]
> +            else:
> +                subrev = ['0:']
> +        return self._pullorclone(source, heads=heads, subrev=subrev)
> +
> +    def _get(self, state, subrev=None):
>          source, revision, kind = state
>          if revision not in self._repo:
> -            self._repo._subsource = source
> -            srcurl = _abssource(self._repo)
> -            other = hg.peer(self._repo, {}, srcurl)
> -            if len(self._repo) == 0:
> -                self._repo.ui.status(_('cloning subrepo %s from %s\n')
> -                                     % (subrelpath(self), srcurl))
> -                parentrepo = self._repo._subparent
> -                shutil.rmtree(self._repo.path)
> -                other, cloned = hg.clone(self._repo._subparent.baseui, {},
> -                                         other, self._repo.root,
> -                                         update=False)
> -                self._repo = cloned.local()
> -                self._initrepo(parentrepo, source, create=True)
> +            self._pullorclone(source, subrev=subrev)
> +
> +    def _pullorclone(self, source, heads=None, subrev=None):
> +        self._repo._subsource = source
> +        srcurl = _abssource(self._repo)
> +        other = hg.peer(self._repo, {}, srcurl)
> +        if len(self._repo) == 0:
> +            self._repo.ui.status(_('cloning subrepo %s from %s\n')
> +                                 % (subrelpath(self), srcurl))
> +            parentrepo = self._repo._subparent
> +            shutil.rmtree(self._repo.path)
> +            other, cloned = hg.clone(self._repo._subparent.baseui, {},
> +                                     other, self._repo.root,
> +                                     update=False, heads=heads)
> +            self._repo = cloned.local()
> +            self._initrepo(parentrepo, source, create=True)
> +            self._cachestorehash(srcurl)
> +        else:
> +            self._repo.ui.status(_('pulling subrepo %s from %s\n')
> +                                 % (subrelpath(self), srcurl))
> +            cleansub = self.storeclean(srcurl)
> +            remotebookmarks = other.listkeys('bookmarks')
> +            self._repo.pull(other, heads=heads, subrev=subrev)
> +            bookmarks.updatefromremote(self._repo.ui, self._repo,
> +                                       remotebookmarks, srcurl)
> +            if cleansub:
> +                # keep the repo clean after pull
>                  self._cachestorehash(srcurl)
> -            else:
> -                self._repo.ui.status(_('pulling subrepo %s from %s\n')
> -                                     % (subrelpath(self), srcurl))
> -                cleansub = self.storeclean(srcurl)
> -                remotebookmarks = other.listkeys('bookmarks')
> -                self._repo.pull(other)
> -                bookmarks.updatefromremote(self._repo.ui, self._repo,
> -                                           remotebookmarks, srcurl)
> -                if cleansub:
> -                    # keep the repo clean after pull
> -                    self._cachestorehash(srcurl)
>  
>      @annotatesubrepoerror
>      def get(self, state, overwrite=False):
> diff --git a/tests/test-completion.t b/tests/test-completion.t
> --- a/tests/test-completion.t
> +++ b/tests/test-completion.t
> @@ -206,7 +206,7 @@
>    init: ssh, remotecmd, insecure
>    log: follow, follow-first, date, copies, keyword, rev, removed, only-merges, user, only-branch, branch, prune, patch, git, limit, no-merges, stat, graph, style, template, include, exclude
>    merge: force, rev, preview, tool
> -  pull: update, force, rev, bookmark, branch, ssh, remotecmd, insecure
> +  pull: update, force, rev, bookmark, branch, subrev, ssh, remotecmd, insecure
>    push: force, rev, bookmark, branch, new-branch, ssh, remotecmd, insecure
>    remove: after, force, include, exclude
>    serve: accesslog, daemon, daemon-pipefds, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20140115/de92cfc8/attachment.pgp>


More information about the Mercurial-devel mailing list