[MERGE][1.6] Real --weave merge

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
| John Arbash Meinel wrote:
|
|> I would really like to get this into 1.6, as ATM mysql is using a custom
|> build to get this sort of result, and I'd really like to get them back
|> onto official releases.
|
| Here is a start to the review, focussing on code correctness (rather than
| algorithms).
|
| bb: comment
|
|> +    * ``bzr (re)merge --weave`` will now use a real Weave algorithm,
|> +      rather than the annotation-based merged it was using. It does
so by
|
| s/merged it/merge it/
|

Done

|> +            if child_parents is None:
|> +                import pdb; pdb.set_trace()
|> +            if len(child_parents) != 1:
|> +                # This is not its only parent
|> +                continue
|> +            assert child_parents[0] == node
|
| 1st if statement can go. Assert needs to be replaced by an
| if ... AssertionError().

just removed, it wasn't critical, mostly for testing

|
|> -                include_unchanged=True,
specific_files=self.interesting_files,
|> +                include_unchanged=False,
specific_files=self.interesting_files,
|
| I didn't get as far as working out why this change was made. Can you
please
| confirm it was meant to be permanent and briefly explain it?

reverted for now

|
|> +        if NULL_REVISION in all_revision_keys:
|> +            import pdb; pdb.set_trace()
|
| More droppings to clean up.

Done

|
|> +        ordering = []
|> +        for seq_num, key, depth, eom in
reversed(tsort.merge_sort(parent_map,
|> +
tip_key)):
|> +            if key == tip_key:
|> +                continue
|> +        # for key in tsort.topo_sort(parent_map):
|> +            parent_keys = parent_map[key]
|> +            revision_id = key[-1]
|> +            ordering.append(revision_id)
|> +            parent_ids = [k[-1] for k in parent_keys]
|> +            self._weave.add_lines(revision_id, parent_ids,
|> +                                  all_texts[revision_id])
|> +        mutter('order in weave: %s', ordering)
|
| So "ordering" is never used except for debugging/trace?
| I wonder whether it's worth adding yet another -D flag for the
| numerous mutters like this one throughout this new code? Up to you.

I just removed it. I removed most of the 'uninteresting' ones. So now
there should only be mutters in exceptional cases.


|
|> +            lines =
self.get_lines([self._head_key[-1]])[self._head_key[-1]]
|
| I think it's worth using a local variable here rather than looking up the
| last element twice.

I went ahead and did this, though head_key is a tuple, so there isn't
much of a gain.

|
|> +    def test_collapse_with_multiple_children(self):
|> +        #    7
|> +        #    |
|> +        #    6
|> +        #   / \
|> +        #  4   5
|> +        #  |   |
|> +        #  3   2
|> +        #   \ /
|> +        #    1
|> +        #
|> +        # 4 and 5 cannot be removed because 6 has 2 children
|> +        # 3 and 2 cannot be removed because 1 has 2 parents
|> +        d = {1:[2, 3], 2:[4], 4:[6], 3:[5], 5:[6], 6:[7], 7:[]}
|
| You need to swap 2 and 3 in the text graphic in order for the graphic
| and definition of 'd' to match.

Done

|
|> -        yield 'unchanged', ''           # terminator
|> +        # This doesn't seem to be used anymore
|> +        # yield 'unchanged', ''           # terminator
|
| Hmm. If you haven't already, please ask Aaron about that. It's
| certainly still documented and used inside versionedfiles.py.
|

'unchanged' the keyword is used, but yielding a final terminator is not.

I know that, because when I switched to Weave.plan_merge it started
failing tests because of a trailing ('unchanged', '').

So I went ahead and removed it entirely.

| Sorry I didn't get further on this review. I still need to look at
| test_merge.py in particular before I can vote on it.
|
| Ian C.
|

You've done this, and I'll respond to that mail next.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkh+JzIACgkQJdeBCYSNAAOu0ACgx6437Af/U8sugdBqfHa8I887
V9UAnijQJ6OPHM4QSiEcO7xBIAdVo52m
=g3su
-----END PGP SIGNATURE-----