[PATCH stable treemanifest] changegroup: fix treemanifest exchange code (issue5061)

[Martin von Zweigbergk]

On Wed, Jan 27, 2016 at 7:25 AM Augie Fackler <raf at durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <augie at google.com>
> # Date 1453908265 18000
> #      Wed Jan 27 10:24:25 2016 -0500
> # Branch stable
> # Node ID 7f7e25855eac3dafb388d63fb0b7f566334ff732
> # Parent  4511e8dac4c798e5fed91e629aa9802b01c2b6c3
> changegroup: fix treemanifest exchange code (issue5061)
>
> There were two mistakes: one was accidental reuse of the fclnode
> variable from the loop gathering file nodes, and the other (masked by
> that bug) was not correctly handling deleted directories. Both cases
> are now fixed and the test passes.
>

Nice, that takes "hg clone --pull" on a test repo I had a little further.
It still fails a little later, but this seems better than before, thanks.


>
> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
> --- a/mercurial/changegroup.py
> +++ b/mercurial/changegroup.py
> @@ -778,12 +778,15 @@ class cg1packer(object):
>                  if 'treemanifest' in repo.requirements:
>                      submfs = {'/': mdata}
>                      for dn, bn in _moddirs(mfchangedfiles[x]):
> -                        submf = submfs[dn]
> -                        submf = submf._dirs[bn]
> +                        try:
> +                            submf = submfs[dn]
> +                            submf = submf._dirs[bn]
> +                        except KeyError:
> +                            continue # deleted directory, so nothing to
> send
>                          submfs[submf.dir()] = submf
>                          tmfclnodes = tmfnodes.setdefault(submf.dir(), {})
> -                        tmfclnodes.setdefault(submf._node, clnode)
> -                        if clrevorder[clnode] < clrevorder[fclnode]:
> +                        tmfclnode = tmfclnodes.setdefault(submf._node,
> clnode)
> +                        if clrevorder[clnode] < clrevorder[tmfclnode]:
>                              tmfclnodes[n] = clnode
>                  return clnode
>
> diff --git a/tests/test-treemanifest.t b/tests/test-treemanifest.t
> --- a/tests/test-treemanifest.t
> +++ b/tests/test-treemanifest.t
> @@ -332,6 +332,21 @@ Pushing from treemanifest repo to an emp
>    $ grep treemanifest empty-repo/.hg/requires
>    treemanifest
>
> +Pushing to an empty repo makes that a treemanifest repo
>

This seems like it's taken from the test case I recently added, but it
doesn't seem accurate for your test. What is really the point of this test
case? I think it's pretty much the same thing as the test case further down
using "deepclone", but it just happens to hit some other conditions in the
lookup function. Also, you could probably switch to "hg clone --pull"
instead if you prefer. Either way, can I ask for at least a new
description? Either as a resend or just a text for me to paste.


> +
> +  $ hg --config experimental.treemanifest=1 init clone
> +  $ grep treemanifest clone/.hg/requires
> +  treemanifest
> +  $ hg push -R repo clone
> +  pushing to clone
> +  searching for changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 11 changesets with 15 changes to 10 files (+3 heads)
> +  $ grep treemanifest clone/.hg/requires
> +  treemanifest
> +
>  Create deeper repo with tree manifests.
>
>    $ hg --config experimental.treemanifest=True init deeprepo
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20160127/8fd523a2/attachment.html>