[MERGE] Implement Tree.plan_merge, deprecate RevisionTree.get_weave
Aaron Bentley
abentley at panoramicfeedback.com
Thu Jul 19 13:40:14 BST 2007
Robert Collins wrote:
> Robert Collins has voted +1 (conditional).
> Status is now: Conditionally approved
> Comment:
>
> + # Disable pending merges, to avoid affecting merge behavior
> + tree.set_parent_ids(parents[:1])
>
> Could you explain more why this is needed?
plan_merge is affected by the list of ancestors. If OTHER is listed as
an ancestor of THIS, that means that all of the changes in OTHER have
already been considered and rejected, so the merge doesn't need to do
anything.
> It seems prone to losing a
> users data e.g. during a power-off or kill -9 situation as well as
> requiring more work at the tree layer to discard and recreate the cached
> parent data.
Yeah, the question is whether we care enough in these situations:
1. remerge is rarely used
2. user files will also be a mess in this situation, so it's only
incrementaly worse
3. if we do lose the parent data, users can just do "revert" and "merge"
instead.
> with our current API
>
> + def _get_ancestors(self, default_revision):
> + ancestors = set([default_revision])
> + for parent_id in self.get_parent_ids():
> + ancestors.update(self.branch.repository.get_ancestry(
> + parent_id, topo_sorted=False))
> + return ancestors
>
> will be faster as
> + ancestors = set([default_revision])
> +
> ancestors.update(self.branch.repository.get_revision_graph(self.get_parent_ids()))
>
> + return ancestors
>
> because that will do just one index traversal.
True, but it's exceptional to perform a merge while there is more than
one ancestor, and it seems clearer to use get_ancestry.
> Its interesting that we grab full ancestry here. This doesn't affect the
> merge, but I wonder about giving a callback here, or even using the file
> merge graph which will be much smaller to reduce the amount of index
> looked at.
Not really necessary. We really just need graph difference.
> With the previously discussed plan_merge -> plan_file_merge I'm happy to
> see this come in.
Thanks.
Aaron
More information about the bazaar
mailing list