[PATCH] subrepo: share instead of clone if the parent repo is shared (issue5675) (BC)

[Yuya Nishihara]

On Mon, 16 Oct 2017 09:21:24 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison at yahoo.com>
> # Date 1508122082 14400
> #      Sun Oct 15 22:48:02 2017 -0400
> # Node ID d0c2b68fedb27184337af6392ecc1f03dab39522
> # Parent  718adb1bf3a9a1ee509a803c7512c14296a1db79
> subrepo: share instead of clone if the parent repo is shared (issue5675) (BC)

Generally looks good to me, but a few questions.

> --- a/mercurial/hg.py
> +++ b/mercurial/hg.py
> @@ -255,6 +255,7 @@
>      r = repository(ui, destwvfs.base)
>      postshare(srcrepo, r, bookmarks=bookmarks, defaultpath=defaultpath)
>      _postshareupdate(r, update, checkout=checkout)
> +    return r
>  
>  def postshare(sourcerepo, destrepo, bookmarks=True, defaultpath=None):
>      """Called after a new shared repo is created.
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -857,28 +857,40 @@
>  
>      def _get(self, state):
>          source, revision, kind = state
> +        parentrepo = self._repo._subparent
> +
>          if revision in self._repo.unfiltered():
> -            return True
> +            # Allow shared subrepos tracked at null to setup the sharedpath
> +            if revision != node.nullhex or not parentrepo.shared():

'revisions != node.nullhex' looks a bit tricky, which seemed to assume a
certain sequence how the current subrepo clone works under the hood.

Can we check 'len(self._repo) != 0' instead?

> +                return True
>          self._repo._subsource = source
>          srcurl = _abssource(self._repo)
>          other = hg.peer(self._repo, {}, srcurl)
>          if len(self._repo) == 0:
> -            self.ui.status(_('cloning subrepo %s from %s\n')
> -                           % (subrelpath(self), srcurl))
> -            parentrepo = self._repo._subparent
>              # use self._repo.vfs instead of self.wvfs to remove .hg only
>              self._repo.vfs.rmtree()
> -            other, cloned = hg.clone(self._repo._subparent.baseui, {},
> -                                     other, self._repo.root,
> -                                     update=False)
> -            self._repo = cloned.local()
> +            if parentrepo.shared():
> +                self.ui.status(_('sharing subrepo %s from %s\n')
> +                               % (subrelpath(self), srcurl))
> +                shared = hg.share(self._repo._subparent.baseui,
> +                                  other, self._repo.root,
> +                                  update=False, bookmarks=False)

Perhaps bookmarks option should follow the parentrepo's, but that can be
addressed later.

> +                self._repo = shared.local()
> +            else:
> +                self.ui.status(_('cloning subrepo %s from %s\n')
> +                               % (subrelpath(self), srcurl))
> +                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._cachestorehash(srcurl)
>          else:
> -            self.ui.status(_('pulling subrepo %s from %s\n')
> -                           % (subrelpath(self), srcurl))
>              cleansub = self.storeclean(srcurl)
> -            exchange.pull(self._repo, other)
> +            if not parentrepo.shared():

self._repo.shared() seems better since self._repo could be an existing
unshared repo.

> +                self.ui.status(_('pulling subrepo %s from %s\n')
> +                               % (subrelpath(self), srcurl))
> +                exchange.pull(self._repo, other)