[PATCH 1 of 1] Handle prepush logic for named branches

Matt Mackall mpm at selenic.com
Tue May 19 10:42:53 CDT 2009


On Tue, May 19, 2009 at 08:31:10AM +0200, Henrik Stuart wrote:
> # HG changeset patch
> # User Sune Foldager <cryo at cyanite.org>
> # Date 1241528982 -7200
> # Node ID 94ac3cbde1772b46ed4b86bfe140b42b73d4b784
> # Parent  252232621165917755727729c7f0b9a1f1263668
> Handle prepush logic for named branches

Can we break this into a few pieces?

- server branch support + long description
- client branch support
- client branch use + fix broken tests
- new tests

> diff -r 252232621165 -r 94ac3cbde177 mercurial/hgweb/protocol.py
> --- a/mercurial/hgweb/protocol.py	Tue May 19 01:37:38 2009 +0200
> +++ b/mercurial/hgweb/protocol.py	Tue May 05 15:09:42 2009 +0200
> @@ -17,6 +17,7 @@
>  __all__ = [
>     'lookup', 'heads', 'branches', 'between', 'changegroup',
>     'changegroupsubset', 'capabilities', 'unbundle', 'stream_out',
> +   'branchmap',
>  ]
>  
>  HGTYPE = 'application/mercurial-0.1'
> @@ -37,6 +38,12 @@
>      req.respond(HTTP_OK, HGTYPE, length=len(resp))
>      yield resp
>  
> +def branchmap(repo, req):
> +    branches = repo.branchmap()
> +    resp = "\n".join(['%s %s' % (b, " ".join([hex(x) for x in branches[b]])) for b in branches])
> +    req.respond(HTTP_OK, HGTYPE, length=len(resp))
> +    yield resp
> +

Hmm. Can we name this branchheads instead for better parallelism with
heads? Or does that too strongly suggest only heads of one branch?

Looks like there's nothing currently branches from containing nasty
things like spaces or newlines. Hmm.

>  def branches(repo, req):
>      nodes = []
>      if 'nodes' in req.form:
> @@ -97,7 +104,7 @@
>      yield z.flush()
>  
>  def capabilities(repo, req):
> -    caps = ['lookup', 'changegroupsubset']
> +    caps = ['lookup', 'changegroupsubset', 'branchmap']
>      if repo.ui.configbool('server', 'uncompressed', untrusted=True):
>          caps.append('stream=%d' % repo.changelog.version)
>      if changegroupmod.bundlepriority:
> diff -r 252232621165 -r 94ac3cbde177 mercurial/httprepo.py
> --- a/mercurial/httprepo.py	Tue May 19 01:37:38 2009 +0200
> +++ b/mercurial/httprepo.py	Tue May 05 15:09:42 2009 +0200
> @@ -145,6 +145,19 @@
>          except:
>              raise error.ResponseError(_("unexpected response:"), d)
>  
> +    def branchmap(self):
> +        d = self.do_read("branchmap")
> +        try:
> +            branchmap = {}
> +            for branchpart in d.splitlines():
> +                branchheads = branchpart.split(' ')
> +                branchname = branchheads[0]
> +                branchheads = [bin(x) for x in branchheads[1:]]
> +                branchmap[branchname] = branchheads
> +            return branchmap
> +        except:
> +            raise error.ResponseError(_("unexpected response:"), d)
> +
>      def branches(self, nodes):
>          n = " ".join(map(hex, nodes))
>          d = self.do_read("branches", nodes=n)
> diff -r 252232621165 -r 94ac3cbde177 mercurial/localrepo.py
> --- a/mercurial/localrepo.py	Tue May 19 01:37:38 2009 +0200
> +++ b/mercurial/localrepo.py	Tue May 05 15:09:42 2009 +0200
> @@ -18,7 +18,7 @@
>  propertycache = util.propertycache
>  
>  class localrepository(repo.repository):
> -    capabilities = set(('lookup', 'changegroupsubset'))
> +    capabilities = set(('lookup', 'changegroupsubset', 'branchmap'))
>      supported = set('revlogv1 store fncache'.split())
>  
>      def __init__(self, baseui, path=None, create=0):
> @@ -360,6 +360,9 @@
>  
>          return partial
>  
> +    def branchmap(self):
> +        return self._branchheads()
> +
>      def _branchheads(self):
>          tip = self.changelog.tip()
>          if self.branchcache is not None and self._branchcachetip == tip:
> @@ -1429,38 +1432,77 @@
>              self.ui.status(_("no changes found\n"))
>              return None, 1
>          elif not force:
> -            # check if we're creating new remote heads
> -            # to be a remote head after push, node must be either
> +            # 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
> +            #
> +            # New named branches cannot be created without --force.
>  
> -            warn = 0
> +            if remote_heads != [nullid]:
> +                def creates_no_new_heads(lheads, rheads, updatelh):

Is this nested function using local vars? If not, can we at least bump
it back to the top of the function scope? It's way too indented.

Also, I don't like the name. Underbars aside, putting a negation in a
boolean function is not great. This could simply be newheads().

> +                    warn = 0
>  
> -            if remote_heads == [nullid]:
> -                warn = 0
> -            elif not revs and len(heads) > len(remote_heads):
> -                warn = 1
> -            else:
> -                newheads = list(heads)
> -                for r in remote_heads:
> -                    if r in self.changelog.nodemap:
> -                        desc = self.changelog.heads(r, heads)
> -                        l = [h for h in heads if h in desc]
> -                        if not l:
> -                            newheads.append(r)
> +                    if not revs and len(lheads) > len(rheads):
> +                        warn = 1
>                      else:
> -                        newheads.append(r)
> -                if len(newheads) > len(remote_heads):
> -                    warn = 1
> +                        newheads = set(sum([self.changelog.heads(x, lheads) for x in updatelh], []))
> +                        newheads &= set(lheads)
>
These lines are getting too long.

> -            if warn:
> -                self.ui.warn(_("abort: push creates new remote heads!\n"))
> -                self.ui.status(_("(did you forget to merge?"
> -                                 " use push -f to force)\n"))
> -                return None, 0
> -            elif inc:
> +                        if not newheads:
> +                            return True
> +
> +                        for r in rheads:
> +                            if r in self.changelog.nodemap:
> +                                desc = self.changelog.heads(r, heads)
> +                                l = [h for h in heads if h in desc]
> +                                if not l:
> +                                    newheads.add(r)
> +                            else:
> +                                newheads.add(r)
> +                        if len(newheads) > len(rheads):
> +                            warn = 1
> +
> +                    if warn:
> +                        if not rheads: # new branch requires --force
> +                            self.ui.warn(_("abort: push creates new remote branch '%s'!\n" % self[updatelh[0]].branch()))
> +                        else:
> +                            self.ui.warn(_("abort: push creates new remote heads!\n"))
> +
> +                        self.ui.status(_("(did you forget to merge?"
> +                                     " use push -f to force)\n"))
> +                        return False
> +                    return True
> +
> +                if remote.capable('branchmap'):
> +                    local_hds = {}
> +                    if not revs:
> +                        local_hds = self.branchmap()
> +                    else:
> +                        for n in heads:
> +                            branch = self[n].branch()
> +                            if local_hds.has_key(branch):
> +                                local_hds[branch].append(n)
> +                            else:
> +                                local_hds[branch] = [n]
> +
> +                    remote_hds = remote.branchmap()
> +
> +                    for lh in local_hds:
> +                        rheads = remote_hds[lh] if remote_hds.has_key(lh) else []
> +                        lheads = local_hds[lh]
> +                        updatelh = [upd for upd in update if self[upd].branch() == lh]
> +                        if not updatelh:
> +                            continue
> +                        if not creates_no_new_heads(lheads, rheads, updatelh):
> +                            return None, 0
> +                else:
> +                    if not creates_no_new_heads(heads, remote_heads, update):
> +                        return None, 0

I think we could probably arrange for there to be only one call to the
heads function? Doesn't look like heads/remote_heads/update gets used
after this point.

> +            if inc:
>                  self.ui.warn(_("note: unsynced remote changes!\n"))
>  
>  
> diff -r 252232621165 -r 94ac3cbde177 mercurial/sshrepo.py
> --- a/mercurial/sshrepo.py	Tue May 19 01:37:38 2009 +0200
> +++ b/mercurial/sshrepo.py	Tue May 05 15:09:42 2009 +0200
> @@ -166,6 +166,19 @@
>          except:
>              self.abort(error.ResponseError(_("unexpected response:"), d))
>  
> +    def branchmap(self):
> +        d = self.call("branchmap")
> +        try:
> +            branchmap = {}
> +            for branchpart in d.splitlines():
> +                branchheads = branchpart.split(' ')
> +                branchname = branchheads[0]
> +                branchheads = [bin(x) for x in branchheads[1:]]
> +                branchmap[branchname] = branchheads
> +            return branchmap
> +        except:
> +            raise error.ResponseError(_("unexpected response:"), d)
> +
>      def branches(self, nodes):
>          n = " ".join(map(hex, nodes))
>          d = self.call("branches", nodes=n)
> diff -r 252232621165 -r 94ac3cbde177 mercurial/sshserver.py
> --- a/mercurial/sshserver.py	Tue May 19 01:37:38 2009 +0200
> +++ b/mercurial/sshserver.py	Tue May 05 15:09:42 2009 +0200
> @@ -64,6 +64,10 @@
>              success = 0
>          self.respond("%s %s\n" % (success, r))
>  
> +    def do_branchmap(self):
> +        h = self.repo.branchmap()
> +        self.respond("\n".join(['%s %s' % (b, " ".join([hex(x) for x in h[b]])) for b in h]))
> +
>      def do_heads(self):
>          h = self.repo.heads()
>          self.respond(" ".join(map(hex, h)) + "\n")
> @@ -77,7 +81,7 @@
>          capabilities: space separated list of tokens
>          '''
>  
> -        caps = ['unbundle', 'lookup', 'changegroupsubset']
> +        caps = ['unbundle', 'lookup', 'changegroupsubset', 'branchmap']
>          if self.ui.configbool('server', 'uncompressed'):
>              caps.append('stream=%d' % self.repo.changelog.version)
>          self.respond("capabilities: %s\n" % (' '.join(caps),))
> diff -r 252232621165 -r 94ac3cbde177 tests/test-acl.out
> --- a/tests/test-acl.out	Tue May 19 01:37:38 2009 +0200
> +++ b/tests/test-acl.out	Tue May 05 15:09:42 2009 +0200
> @@ -42,6 +42,7 @@
>  pushing to ../b
>  searching for changes
>  common changesets up to 6675d58eff77
> +invalidating branch cache (tip differs)

Why are these appearing? Simply because we're checking the branches now?

>  3 changesets found
>  list of changesets:
>  ef1ea85a6374b77d6da9dcda9541f498f2d17df7
> @@ -74,6 +75,7 @@
>  pushing to ../b
>  searching for changes
>  common changesets up to 6675d58eff77
> +invalidating branch cache (tip differs)
>  3 changesets found
>  list of changesets:
>  ef1ea85a6374b77d6da9dcda9541f498f2d17df7
> @@ -111,6 +113,7 @@
>  pushing to ../b
>  searching for changes
>  common changesets up to 6675d58eff77
> +invalidating branch cache (tip differs)
>  3 changesets found
>  list of changesets:
>  ef1ea85a6374b77d6da9dcda9541f498f2d17df7
> @@ -408,6 +411,7 @@
>  pushing to ../b
>  searching for changes
>  common changesets up to 6675d58eff77
> +invalidating branch cache (tip differs)
>  3 changesets found
>  list of changesets:
>  ef1ea85a6374b77d6da9dcda9541f498f2d17df7
> diff -r 252232621165 -r 94ac3cbde177 tests/test-hgweb-commands.out
> Binary file tests/test-hgweb-commands.out has changed
> diff -r 252232621165 -r 94ac3cbde177 tests/test-push-logic
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/test-push-logic	Tue May 05 15:09:42 2009 +0200

We've already got test-push warn for this. Please extend that test,
with an eye to test run-time. I suspect there's a way to do this test
without building multiple pairs of repos from scratch.

> @@ -0,0 +1,118 @@
> +#!/bin/sh
> +
> +echo push on existing branch and new branch
> +hg init a
> +hg --cwd a branch a
> +echo 0 >> a/foo
> +hg --cwd a add foo
> +hg --cwd a ci -m "0"
> +
> +hg clone a b
> +echo 1 >> b/foo
> +hg --cwd b ci -m "1"
> +hg --cwd b up -r 0 -C
> +hg --cwd b branch b
> +echo 2 >> b/foo
> +hg --cwd b ci -m "2"
> +hg --cwd b push | sed 's/pushing to.*/pushing/'
> +echo
> +
> +rm -rf a b
> +
> +echo push on existing branch and existing branch with multiple heads
> +hg init a
> +hg --cwd a branch a
> +echo 0 >> a/foo
> +hg --cwd a add foo
> +hg --cwd a ci -m "0"
> +hg --cwd a branch b
> +echo 1 >> a/foo
> +hg --cwd a ci -m "1"
> +
> +hg clone a b
> +hg --cwd b up -r 0 -C
> +echo 2 >> b/foo
> +hg --cwd b ci -m "2"
> +hg --cwd b up -r 1 -C
> +echo 3 >> b/foo
> +hg --cwd b ci -m "3"
> +hg --cwd b up -r 1 -C
> +echo 4 >> b/foo
> +hg --cwd b ci -m "4"
> +hg --cwd b push | sed 's/pushing to.*/pushing/'
> +hg --cwd b push -r 2 -r 3 | sed 's/pushing to.*/pushing/'
> +echo
> +
> +rm -rf a b
> +
> +echo fail on multiple head push
> +
> +hg init a
> +echo 0 >> a/foo
> +hg --cwd a branch a
> +hg --cwd a add foo
> +hg --cwd a ci -m "0"
> +
> +hg clone a b
> +echo 1 >> b/foo
> +hg --cwd b ci -m "1"
> +hg --cwd b up -r 0 -C
> +echo 2 >> b/foo
> +hg --cwd b ci -m "2"
> +hg --cwd b push | sed 's/pushing to.*/pushing/'
> +
> +echo
> +
> +rm -rf a b
> +
> +echo merge of branch a to other branch b followed by unrelated push on branch a
> +hg init a
> +echo 0 > a/foo
> +hg --cwd a branch a
> +hg --cwd a add foo
> +hg --cwd a ci -m "0"
> +echo 1 > a/foo
> +hg --cwd a branch b
> +hg --cwd a ci -m "1"
> +hg --cwd a up -r 0 -C
> +echo 2 > a/foo
> +hg --cwd a ci -m "2"
> +
> +hg clone a b
> +hg clone a c
> +
> +HGMERGE=internal:merge hg --cwd b merge -r 1 # merge b down to a
> +hg --cwd b resolve -m foo
> +hg --cwd b ci -m "3 on b"
> +hg --cwd b push | sed 's/pushing to.*/pushing/'
> +
> +hg --cwd c up -r 1
> +echo 3 > c/foo
> +hg --cwd c ci -m "3 on c"
> +hg --cwd c push | sed 's/pushing to.*/pushing/'
> +echo
> +
> +rm -rf a b c
> +
> +echo cheating the count algorithm
> +hg init a
> +echo 0 > a/foo
> +hg --cwd a add foo
> +hg --cwd a ci -m "0"
> +echo 1 > a/foo
> +hg --cwd a ci -m "1"
> +hg --cwd a up -r 0 -C
> +echo 2 > a/foo
> +hg --cwd a ci -m "2"
> +
> +hg clone a b
> +HGMERGE=internal:merge hg --cwd b merge -r 1
> +hg --cwd b resolve -m foo
> +hg --cwd b ci -m "3"
> +hg --cwd b up -r 0
> +echo 4 > b/foo
> +hg --cwd b ci -m "4"
> +hg --cwd b push | sed 's/pushing to.*/pushing/'
> +echo
> +
> +exit 0
> diff -r 252232621165 -r 94ac3cbde177 tests/test-push-logic.out
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/test-push-logic.out	Tue May 05 15:09:42 2009 +0200
> @@ -0,0 +1,92 @@
> +push on existing branch and new branch
> +marked working directory as branch a
> +updating working directory
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +marked working directory as branch b
> +created new head
> +abort: push creates new remote branch 'b'!
> +pushing
> +searching for changes
> +(did you forget to merge? use push -f to force)
> +
> +push on existing branch and existing branch with multiple heads
> +marked working directory as branch a
> +marked working directory as branch b
> +updating working directory
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +created new head
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +created new head
> +abort: push creates new remote heads!
> +pushing
> +searching for changes
> +(did you forget to merge? use push -f to force)
> +pushing
> +searching for changes
> +adding changesets
> +adding manifests
> +adding file changes
> +added 2 changesets with 2 changes to 1 files (+1 heads)
> +
> +fail on multiple head push
> +marked working directory as branch a
> +updating working directory
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +created new head
> +abort: push creates new remote heads!
> +pushing
> +searching for changes
> +(did you forget to merge? use push -f to force)
> +
> +merge of branch a to other branch b followed by unrelated push on branch a
> +marked working directory as branch a
> +marked working directory as branch b
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +created new head
> +updating working directory
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +updating working directory
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +merging foo
> +warning: conflicts during merge.
> +merging foo failed!
> +0 files updated, 0 files merged, 0 files removed, 1 files unresolved
> +use 'hg resolve' to retry unresolved file merges or 'hg up --clean' to abandon
> +pushing
> +searching for changes
> +adding changesets
> +adding manifests
> +adding file changes
> +added 1 changesets with 1 changes to 1 files (-1 heads)
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +note: unsynced remote changes!
> +pushing
> +searching for changes
> +adding changesets
> +adding manifests
> +adding file changes
> +added 1 changesets with 1 changes to 1 files (+1 heads)
> +
> +cheating the count algorithm
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +created new head
> +updating working directory
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +merging foo
> +warning: conflicts during merge.
> +merging foo failed!
> +0 files updated, 0 files merged, 0 files removed, 1 files unresolved
> +use 'hg resolve' to retry unresolved file merges or 'hg up --clean' to abandon
> +1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +created new head
> +pushing
> +searching for changes
> +adding changesets
> +adding manifests
> +adding file changes
> +added 2 changesets with 2 changes to 1 files
> +

-- 
Mathematics is the supreme nostalgia of our time.


More information about the Mercurial-devel mailing list