tree-less merges

[Aaron Bentley]

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

Michael Hudson wrote:
> Aaron Bentley wrote:
> So I did these things and some further hacking (mostly slicing up bits
> of merge.py into smaller methods, attached as a bundle against your
> branch) that lets me write get_diff_as_merged like this:

> and it works for the simple cases, which is pretty nice.

Great!

>  It's not
> tested at all, bad me, but I'd appreciate another sanity check if you
> have time :)

We try to maintain a stable (that is, append-only) API, but your change
to the Merge3Merge constructor is an API break.  Splitting the
functionality into do_merge is fine, but please restore the old behavior
as the default, and provide a new parameter to override it.

> +    def do_merge(self):
> +        self.this_tree.lock_tree_write()

this_tree is intended to be read-only source data.  Especially, it may
be a revision tree, if we can determine that the revision tree has the
same data as the working tree.  Please do not blur the distinction
between this_tree (read-only source data) and working_tree (target for
writing changes to).

> @@ -507,6 +485,52 @@
>              self.this_tree.unlock()
>              self.pb.clear()
>  
> +    def make_preview_transform(self):
> +        self.base_tree.lock_read()
> +        self.other_tree.lock_read()
> +        self.tt = TransformPreview(self.this_tree)
> +        try:
> +            self.pp.next_phase()
> +            self.compute_transform()
> +            self.pp.next_phase()
> +        finally:
> +            self.other_tree.unlock()
> +            self.base_tree.unlock()
> +            self.pb.clear()
> +        return self.tt

You're going to update this to set the conflicts, right?

> @@ -458,7 +459,6 @@
>          file.close()
>          mary_tree.commit("change file2")
>          # john should be able to merge with no conflicts.
> -        merge_type = Merge3Merger

I don't understand why you removed this.

> +        # XXX OW!
> +        self._TreeTransformBase__done = True

Perhaps the best option is to call it _done instead of __done.

> +    def _set_mode(self, trans_id, mode_id, typefunc):
> +        """Set the mode of new file contents.
> +        The mode_id is the existing file to get the mode from (often the same
> +        as trans_id).  The operation is only performed if there's a mode match
> +        according to typefunc.
> +        """
> +        # is it ok to ignore this?  probably
> +        pass

I think this can't be relevant for non-disk-based trees.

> +
> +    def iter_tree_children(self, parent_id):
> +        """Iterate through the entry's tree children, if any"""
> +        # This can't possibly be right :) It's only called from
> +        # _add_tree_children (where the comment is "") which is called
> +        # from find_conflicts.

Actually, it's used in several places.  And the comment in
add_tree_children is actually "ensure that all children are registered
with the transaction"

> +         I don't understand the conflict
> +        # resolution well enough yet to know what the right thing to
> +        # do is, though in the simplest cases this seems to work.
> +        return []

This function is essentially listdir, except that it returns paths
relative to the tree root.  You need to return the paths of the children
of the specified directory in the original tree.

You ought to be able to reuse the original code, except for the listdir.

I think you can replace the listdir with
wt.inventory['file_id'].children.keys()

(probably The Right Thing is using Tree.walkdirs, since that doesn't
require Tree.inventory, but it's a bit like hitting a fly with a
sledgehammer.)

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD4DBQFHTEA/0F+nu1YWqI0RAkK0AJ9DPhmTQoWWzlvRA3aTSQCqs/OTzQCY+dby
FceP3h6eXEKO2OhMiMK5bA==
=ONdv
-----END PGP SIGNATURE-----