discovery: diet discovery.prepush from non-discovery code
authorPierre-Yves David <pierre-yves.david@logilab.fr>
Thu, 19 Jan 2012 15:50:55 +0100
changeset 15932 4154338f0bc0
parent 15931 44b5de2d1876
child 15933 b8696a6676be
discovery: diet discovery.prepush from non-discovery code The ``discovery.prepush`` function was doing multiple things not related to discovery. This changeset move some code into the ``localrepo.push`` method. The old ``discovery.prepush`` function jobs is now restricted to checking for multple head creation. It was then renamed ``discovery.checkheads``. This new ``discovery.checkheads`` function may receive several other changes in the future but we are a bit too much near the freeze for a wider refactoring.
mercurial/discovery.py
mercurial/localrepo.py
tests/test-check-code-hg.t
--- a/mercurial/discovery.py	Thu Jan 19 11:30:37 2012 +0100
+++ b/mercurial/discovery.py	Thu Jan 19 15:50:55 2012 +0100
@@ -132,146 +132,108 @@
 
     return og
 
-def prepush(repo, remote, force, revs, newbranch):
-    '''Analyze the local and remote repositories and determine which
-    changesets need to be pushed to the remote. Return value depends
-    on circumstances:
-
-    If we are not going to push anything, return a tuple (None, 1,
-    common) The third element "common" is the list of heads of the
-    common set between local and remote.
+def checkheads(repo, remote, outgoing, remoteheads, newbranch=False):
+    """Check that a push won't add any outgoing head
 
-    Otherwise, return a tuple (changegroup, remoteheads, futureheads),
-    where changegroup is a readable file-like object whose read()
-    returns successive changegroup chunks ready to be sent over the
-    wire, remoteheads is the list of remote heads and futureheads is
-    the list of heads of the common set between local and remote to
-    be after push completion.
-    '''
-    commoninc = findcommonincoming(repo, remote, force=force)
-    outgoing = findcommonoutgoing(repo, remote, onlyheads=revs,
-                                      commoninc=commoninc, force=force)
-    _common, inc, remoteheads = commoninc
+    raise Abort error and display ui message as needed.
+    """
+    if remoteheads == [nullid]:
+        # remote is empty, nothing to check.
+        return
 
     cl = repo.changelog
-    outg = outgoing.missing
-    common = outgoing.commonheads
+    if remote.capable('branchmap'):
+        # Check for each named branch if we're creating new remote heads.
+        # To be a remote head after push, node must be either:
+        # - unknown locally
+        # - a local outgoing head descended from update
+        # - a remote head that's known locally and not
+        #   ancestral to an outgoing head
 
-    if not outg:
-        if outgoing.excluded:
-            repo.ui.status(_("no changes to push but %i secret changesets\n")
-                           % len(outgoing.excluded))
-        else:
-            repo.ui.status(_("no changes found\n"))
-        return None, 1, common
+        # 1. Create set of branches involved in the push.
+        branches = set(repo[n].branch() for n in outgoing.missing)
 
-    if not force and remoteheads != [nullid]:
-        if remote.capable('branchmap'):
-            # Check for each named branch if we're creating new remote heads.
-            # To be a remote head after push, node must be either:
-            # - unknown locally
-            # - a local outgoing head descended from update
-            # - a remote head that's known locally and not
-            #   ancestral to an outgoing head
-
-            # 1. Create set of branches involved in the push.
-            branches = set(repo[n].branch() for n in outg)
+        # 2. Check for new branches on the remote.
+        remotemap = remote.branchmap()
+        newbranches = branches - set(remotemap)
+        if newbranches and not newbranch: # new branch requires --new-branch
+            branchnames = ', '.join(sorted(newbranches))
+            raise util.Abort(_("push creates new remote branches: %s!")
+                               % branchnames,
+                             hint=_("use 'hg push --new-branch' to create"
+                                    " new remote branches"))
+        branches.difference_update(newbranches)
 
-            # 2. Check for new branches on the remote.
-            remotemap = remote.branchmap()
-            newbranches = branches - set(remotemap)
-            if newbranches and not newbranch: # new branch requires --new-branch
-                branchnames = ', '.join(sorted(newbranches))
-                raise util.Abort(_("push creates new remote branches: %s!")
-                                   % branchnames,
-                                 hint=_("use 'hg push --new-branch' to create"
-                                        " new remote branches"))
-            branches.difference_update(newbranches)
+        # 3. Construct the initial oldmap and newmap dicts.
+        # They contain information about the remote heads before and
+        # after the push, respectively.
+        # Heads not found locally are not included in either dict,
+        # since they won't be affected by the push.
+        # unsynced contains all branches with incoming changesets.
+        oldmap = {}
+        newmap = {}
+        unsynced = set()
+        for branch in branches:
+            remotebrheads = remotemap[branch]
+            prunedbrheads = [h for h in remotebrheads if h in cl.nodemap]
+            oldmap[branch] = prunedbrheads
+            newmap[branch] = list(prunedbrheads)
+            if len(remotebrheads) > len(prunedbrheads):
+                unsynced.add(branch)
 
-            # 3. Construct the initial oldmap and newmap dicts.
-            # They contain information about the remote heads before and
-            # after the push, respectively.
-            # Heads not found locally are not included in either dict,
-            # since they won't be affected by the push.
-            # unsynced contains all branches with incoming changesets.
-            oldmap = {}
-            newmap = {}
-            unsynced = set()
-            for branch in branches:
-                remotebrheads = remotemap[branch]
-                prunedbrheads = [h for h in remotebrheads if h in cl.nodemap]
-                oldmap[branch] = prunedbrheads
-                newmap[branch] = list(prunedbrheads)
-                if len(remotebrheads) > len(prunedbrheads):
-                    unsynced.add(branch)
-
-            # 4. Update newmap with outgoing changes.
-            # This will possibly add new heads and remove existing ones.
-            ctxgen = (repo[n] for n in outg)
-            repo._updatebranchcache(newmap, ctxgen)
+        # 4. Update newmap with outgoing changes.
+        # This will possibly add new heads and remove existing ones.
+        ctxgen = (repo[n] for n in outgoing.missing)
+        repo._updatebranchcache(newmap, ctxgen)
 
-        else:
-            # 1-4b. old servers: Check for new topological heads.
-            # Construct {old,new}map with branch = None (topological branch).
-            # (code based on _updatebranchcache)
-            oldheads = set(h for h in remoteheads if h in cl.nodemap)
-            newheads = oldheads.union(outg)
-            if len(newheads) > 1:
-                for latest in reversed(outg):
-                    if latest not in newheads:
-                        continue
-                    minhrev = min(cl.rev(h) for h in newheads)
-                    reachable = cl.reachable(latest, cl.node(minhrev))
-                    reachable.remove(latest)
-                    newheads.difference_update(reachable)
-            branches = set([None])
-            newmap = {None: newheads}
-            oldmap = {None: oldheads}
-            unsynced = inc and branches or set()
+    else:
+        # 1-4b. old servers: Check for new topological heads.
+        # Construct {old,new}map with branch = None (topological branch).
+        # (code based on _updatebranchcache)
+        oldheads = set(h for h in remoteheads if h in cl.nodemap)
+        newheads = oldheads.union(outg)
+        if len(newheads) > 1:
+            for latest in reversed(outg):
+                if latest not in newheads:
+                    continue
+                minhrev = min(cl.rev(h) for h in newheads)
+                reachable = cl.reachable(latest, cl.node(minhrev))
+                reachable.remove(latest)
+                newheads.difference_update(reachable)
+        branches = set([None])
+        newmap = {None: newheads}
+        oldmap = {None: oldheads}
+        unsynced = inc and branches or set()
 
-        # 5. Check for new heads.
-        # If there are more heads after the push than before, a suitable
-        # error message, depending on unsynced status, is displayed.
-        error = None
-        for branch in branches:
-            newhs = set(newmap[branch])
-            oldhs = set(oldmap[branch])
-            if len(newhs) > len(oldhs):
-                dhs = list(newhs - oldhs)
-                if error is None:
-                    if branch not in ('default', None):
-                        error = _("push creates new remote head %s "
-                                  "on branch '%s'!") % (short(dhs[0]), branch)
-                    else:
-                        error = _("push creates new remote head %s!"
-                                  ) % short(dhs[0])
-                    if branch in unsynced:
-                        hint = _("you should pull and merge or "
-                                 "use push -f to force")
-                    else:
-                        hint = _("did you forget to merge? "
-                                 "use push -f to force")
-                if branch is not None:
-                    repo.ui.note(_("new remote heads on branch '%s'\n") % branch)
-                for h in dhs:
-                    repo.ui.note(_("new remote head %s\n") % short(h))
-        if error:
-            raise util.Abort(error, hint=hint)
+    # 5. Check for new heads.
+    # If there are more heads after the push than before, a suitable
+    # error message, depending on unsynced status, is displayed.
+    error = None
+    for branch in branches:
+        newhs = set(newmap[branch])
+        oldhs = set(oldmap[branch])
+        if len(newhs) > len(oldhs):
+            dhs = list(newhs - oldhs)
+            if error is None:
+                if branch not in ('default', None):
+                    error = _("push creates new remote head %s "
+                              "on branch '%s'!") % (short(dhs[0]), branch)
+                else:
+                    error = _("push creates new remote head %s!"
+                              ) % short(dhs[0])
+                if branch in unsynced:
+                    hint = _("you should pull and merge or "
+                             "use push -f to force")
+                else:
+                    hint = _("did you forget to merge? "
+                             "use push -f to force")
+            if branch is not None:
+                repo.ui.note(_("new remote heads on branch '%s'\n") % branch)
+            for h in dhs:
+                repo.ui.note(_("new remote head %s\n") % short(h))
+    if error:
+        raise util.Abort(error, hint=hint)
 
-        # 6. Check for unsynced changes on involved branches.
-        if unsynced:
-            repo.ui.warn(_("note: unsynced remote changes!\n"))
-
-    if revs is None and not outgoing.excluded:
-        # push everything,
-        # use the fast path, no race possible on push
-        cg = repo._changegroup(outg, 'push')
-    else:
-        cg = repo.getlocalbundle('push', outgoing)
-    # no need to compute outg ancestor. All node in outg have either:
-    # - parents in outg
-    # - parents in common
-    # - nullid parent
-    rset = repo.set('heads(%ln + %ln)', common, outg)
-    futureheads = [ctx.node() for ctx in rset]
-    return cg, remoteheads, futureheads
+    # 6. Check for unsynced changes on involved branches.
+    if unsynced:
+        repo.ui.warn(_("note: unsynced remote changes!\n"))
--- a/mercurial/localrepo.py	Thu Jan 19 11:30:37 2012 +0100
+++ b/mercurial/localrepo.py	Thu Jan 19 15:50:55 2012 +0100
@@ -1606,26 +1606,59 @@
             # get local lock as we might write phase data
             locallock = self.lock()
             try:
-                cg, remote_heads, fut = discovery.prepush(self, remote, force,
-                                                           revs, newbranch)
-                ret = remote_heads
-                # create a callback for addchangegroup.
-                # If will be used branch of the conditionnal too.
-                if cg is not None:
+                # discovery
+                fci = discovery.findcommonincoming
+                commoninc = fci(self, remote, force=force)
+                common, inc, remoteheads = commoninc
+                fco = discovery.findcommonoutgoing
+                outgoing = fco(self, remote, onlyheads=revs,
+                               commoninc=commoninc, force=force)
+
+
+                if not outgoing.missing:
+                    # nothing to push
+                    if outgoing.excluded:
+                        msg = "no changes to push but %i secret changesets\n"
+                        self.ui.status(_(msg) % len(outgoing.excluded))
+                    else:
+                        self.ui.status(_("no changes found\n"))
+                    fut = outgoing.common
+                    ret = 1
+                else:
+                    # something to push
+                    if not force:
+                        discovery.checkheads(self, remote, outgoing,
+                                             remoteheads, newbranch)
+
+                    # create a changegroup from local
+                    if revs is None and not outgoing.excluded:
+                        # push everything,
+                        # use the fast path, no race possible on push
+                        cg = self._changegroup(outgoing.missing, 'push')
+                    else:
+                        cg = self.getlocalbundle('push', outgoing)
+
+                    # apply changegroup to remote
                     if unbundle:
                         # local repo finds heads on server, finds out what
                         # revs it must push. once revs transferred, if server
                         # finds it has different heads (someone else won
                         # commit/push race), server aborts.
                         if force:
-                            remote_heads = ['force']
+                            remoteheads = ['force']
                         # ssh: return remote's addchangegroup()
                         # http: return remote's addchangegroup() or 0 for error
-                        ret = remote.unbundle(cg, remote_heads, 'push')
+                        ret = remote.unbundle(cg, remoteheads, 'push')
                     else:
                         # we return an integer indicating remote head count change
                         ret = remote.addchangegroup(cg, 'push', self.url())
 
+                # compute what should be the now common
+                #
+                # XXX If push failed we should use strict common and not
+                # future to avoid pushing phase data on unknown changeset.
+                # This is to done later.
+                fut = outgoing.commonheads + outgoing.missingheads
                 # even when we don't push, exchanging phase data is useful
                 remotephases = remote.listkeys('phases')
                 if not remotephases: # old server or public only repo
@@ -1641,10 +1674,6 @@
                         phases.advanceboundary(self, phases.public, pheads)
                         phases.advanceboundary(self, phases.draft, fut)
                     ### Apply local phase on remote
-                    #
-                    # XXX If push failed we should use strict common and not
-                    # future to avoid pushing phase data on unknown changeset.
-                    # This is to done later.
 
                     # Get the list of all revs draft on remote by public here.
                     # XXX Beware that revset break if droots is not strictly
--- a/tests/test-check-code-hg.t	Thu Jan 19 11:30:37 2012 +0100
+++ b/tests/test-check-code-hg.t	Thu Jan 19 15:50:55 2012 +0100
@@ -438,9 +438,6 @@
    >                 if not st is None and not getkind(st.st_mode) in (regkind, lnkkind):
    warning: line over 80 characters
   mercurial/discovery.py:0:
-   >                     repo.ui.note(_("new remote heads on branch '%s'\n") % branch)
-   warning: line over 80 characters
-  mercurial/discovery.py:0:
    >     If onlyheads is given, only nodes ancestral to nodes in onlyheads (inclusive)
    warning: line over 80 characters
   mercurial/discovery.py:0: