Rev 6059: (vila) Generate a 'duplicate' conflict instead of a 'content' conflict when in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/2.4/
Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Oct 27 15:29:58 UTC 2011
At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/2.4/
------------------------------------------------------------
revno: 6059 [merge]
revision-id: pqm at pqm.ubuntu.com-20111027152957-gfc64dky77azio9v
parent: pqm at pqm.ubuntu.com-20111027144157-4hksvwpd93mfylia
parent: v.ladeuil+lp at free.fr-20111027150435-12spmboagxaiqq3n
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.4
timestamp: Thu 2011-10-27 15:29:57 +0000
message:
(vila) Generate a 'duplicate' conflict instead of a 'content' conflict when
the same path is involved in both branches. (Vincent Ladeuil)
modified:
bzrlib/errors.py errors.py-20050309040759-20512168c4e14fbd
bzrlib/merge.py merge.py-20050513021216-953b65a438527106
bzrlib/tests/test_conflicts.py test_conflicts.py-20051006031059-e2dad9bbeaa5891f
bzrlib/transform.py transform.py-20060105172343-dd99e54394d91687
doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py 2011-08-01 06:24:48 +0000
+++ b/bzrlib/errors.py 2011-10-24 15:15:09 +0000
@@ -1964,7 +1964,7 @@
self.prefix = prefix
-class MalformedTransform(BzrError):
+class MalformedTransform(InternalBzrError):
_fmt = "Tree transform is malformed %(conflicts)r"
=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py 2011-07-06 09:22:00 +0000
+++ b/bzrlib/merge.py 2011-10-27 12:26:48 +0000
@@ -591,11 +591,11 @@
else:
self.base_rev_id = self.revision_graph.find_unique_lca(
*lcas)
- sorted_lca_keys = self.revision_graph.find_merge_order(
+ sorted_lca_keys = self.revision_graph.find_merge_order(
revisions[0], lcas)
if self.base_rev_id == _mod_revision.NULL_REVISION:
self.base_rev_id = sorted_lca_keys[0]
-
+
if self.base_rev_id == _mod_revision.NULL_REVISION:
raise errors.UnrelatedBranches()
if self._is_criss_cross:
@@ -604,7 +604,8 @@
trace.mutter('Criss-cross lcas: %r' % lcas)
if self.base_rev_id in lcas:
trace.mutter('Unable to find unique lca. '
- 'Fallback %r as best option.' % self.base_rev_id)
+ 'Fallback %r as best option.'
+ % self.base_rev_id)
interesting_revision_ids = set(lcas)
interesting_revision_ids.add(self.base_rev_id)
interesting_trees = dict((t.get_revision_id(), t)
@@ -689,7 +690,8 @@
continue
sub_merge = Merger(sub_tree.branch, this_tree=sub_tree)
sub_merge.merge_type = self.merge_type
- other_branch = self.other_branch.reference_parent(file_id, relpath)
+ other_branch = self.other_branch.reference_parent(file_id,
+ relpath)
sub_merge.set_other_revision(other_revision, other_branch)
base_revision = self.base_tree.get_reference_revision(file_id)
sub_merge.base_tree = \
@@ -1387,18 +1389,54 @@
if hook_status != 'not_applicable':
# Don't try any more hooks, this one applies.
break
+ # If the merge ends up replacing the content of the file, we get rid of
+ # it at the end of this method (this variable is used to track the
+ # exceptions to this rule).
+ keep_this = False
result = "modified"
if hook_status == 'not_applicable':
- # This is a contents conflict, because none of the available
- # functions could merge it.
+ # No merge hook was able to resolve the situation. Two cases exist:
+ # a content conflict or a duplicate one.
result = None
name = self.tt.final_name(trans_id)
parent_id = self.tt.final_parent(trans_id)
- if self.this_tree.has_id(file_id):
- self.tt.unversion_file(trans_id)
- file_group = self._dump_conflicts(name, parent_id, file_id,
- set_version=True)
- self._raw_conflicts.append(('contents conflict', file_group))
+ duplicate = False
+ inhibit_content_conflict = False
+ if params.this_kind is None: # file_id is not in THIS
+ # Is the name used for a different file_id ?
+ dupe_path = self.other_tree.id2path(file_id)
+ this_id = self.this_tree.path2id(dupe_path)
+ if this_id is not None:
+ # Two entries for the same path
+ keep_this = True
+ # versioning the merged file will trigger a duplicate
+ # conflict
+ self.tt.version_file(file_id, trans_id)
+ transform.create_from_tree(
+ self.tt, trans_id, self.other_tree, file_id,
+ filter_tree_path=self._get_filter_tree_path(file_id))
+ inhibit_content_conflict = True
+ elif params.other_kind is None: # file_id is not in OTHER
+ # Is the name used for a different file_id ?
+ dupe_path = self.this_tree.id2path(file_id)
+ other_id = self.other_tree.path2id(dupe_path)
+ if other_id is not None:
+ # Two entries for the same path again, but here, the other
+ # entry will also be merged. We simply inhibit the
+ # 'content' conflict creation because we know OTHER will
+ # create (or has already created depending on ordering) an
+ # entry at the same path. This will trigger a 'duplicate'
+ # conflict later.
+ keep_this = True
+ inhibit_content_conflict = True
+ if not inhibit_content_conflict:
+ if params.this_kind is not None:
+ self.tt.unversion_file(trans_id)
+ # This is a contents conflict, because none of the available
+ # functions could merge it.
+ file_group = self._dump_conflicts(name, parent_id, file_id,
+ set_version=True)
+ self._raw_conflicts.append(('contents conflict', file_group))
elif hook_status == 'success':
self.tt.create_file(lines, trans_id)
elif hook_status == 'conflicted':
@@ -1420,36 +1458,23 @@
raise AssertionError('unknown hook_status: %r' % (hook_status,))
if not self.this_tree.has_id(file_id) and result == "modified":
self.tt.version_file(file_id, trans_id)
- # The merge has been performed, so the old contents should not be
- # retained.
- self.tt.delete_contents(trans_id)
+ if not keep_this:
+ # The merge has been performed and produced a new content, so the
+ # old contents should not be retained.
+ self.tt.delete_contents(trans_id)
return result
def _default_other_winner_merge(self, merge_hook_params):
"""Replace this contents with other."""
file_id = merge_hook_params.file_id
trans_id = merge_hook_params.trans_id
- file_in_this = self.this_tree.has_id(file_id)
if self.other_tree.has_id(file_id):
# OTHER changed the file
- wt = self.this_tree
- if wt.supports_content_filtering():
- # We get the path from the working tree if it exists.
- # That fails though when OTHER is adding a file, so
- # we fall back to the other tree to find the path if
- # it doesn't exist locally.
- try:
- filter_tree_path = wt.id2path(file_id)
- except errors.NoSuchId:
- filter_tree_path = self.other_tree.id2path(file_id)
- else:
- # Skip the id2path lookup for older formats
- filter_tree_path = None
- transform.create_from_tree(self.tt, trans_id,
- self.other_tree, file_id,
- filter_tree_path=filter_tree_path)
+ transform.create_from_tree(
+ self.tt, trans_id, self.other_tree, file_id,
+ filter_tree_path=self._get_filter_tree_path(file_id))
return 'done', None
- elif file_in_this:
+ elif self.this_tree.has_id(file_id):
# OTHER deleted the file
return 'delete', None
else:
@@ -1529,6 +1554,20 @@
other_lines)
file_group.append(trans_id)
+
+ def _get_filter_tree_path(self, file_id):
+ if self.this_tree.supports_content_filtering():
+ # We get the path from the working tree if it exists.
+ # That fails though when OTHER is adding a file, so
+ # we fall back to the other tree to find the path if
+ # it doesn't exist locally.
+ try:
+ return self.this_tree.id2path(file_id)
+ except errors.NoSuchId:
+ return self.other_tree.id2path(file_id)
+ # Skip the id2path lookup for older formats
+ return None
+
def _dump_conflicts(self, name, parent_id, file_id, this_lines=None,
base_lines=None, other_lines=None, set_version=False,
no_base=False):
@@ -1652,10 +1691,12 @@
for trans_id in conflict[1]:
file_id = self.tt.final_file_id(trans_id)
if file_id is not None:
+ # Ok we found the relevant file-id
break
path = fp.get_path(trans_id)
for suffix in ('.BASE', '.THIS', '.OTHER'):
if path.endswith(suffix):
+ # Here is the raw path
path = path[:-len(suffix)]
break
c = _mod_conflicts.Conflict.factory(conflict_type,
=== modified file 'bzrlib/tests/test_conflicts.py'
--- a/bzrlib/tests/test_conflicts.py 2011-07-07 10:20:59 +0000
+++ b/bzrlib/tests/test_conflicts.py 2011-10-26 15:22:24 +0000
@@ -677,6 +677,14 @@
('fileb_created',
dict(actions='create_file_b', check='file_content_b',
path='file', file_id='file-b-id')),),
+ # File created with different file-ids but deleted on one side
+ (dict(_base_actions='create_file_a'),
+ ('filea_replaced',
+ dict(actions='replace_file_a_by_b', check='file_content_b',
+ path='file', file_id='file-b-id')),
+ ('filea_modified',
+ dict(actions='modify_file_a', check='file_new_content',
+ path='file', file_id='file-a-id')),),
])
def do_nothing(self):
@@ -694,6 +702,16 @@
def check_file_content_b(self):
self.assertFileEqual('file b content\n', 'branch/file')
+ def do_replace_file_a_by_b(self):
+ return [('unversion', 'file-a-id'),
+ ('add', ('file', 'file-b-id', 'file', 'file b content\n'))]
+
+ def do_modify_file_a(self):
+ return [('modify', ('file-a-id', 'new content\n'))]
+
+ def check_file_new_content(self):
+ self.assertFileEqual('new content\n', 'branch/file')
+
def _get_resolve_path_arg(self, wt, action):
return self._this['path']
@@ -1043,7 +1061,8 @@
# This is nearly like TestResolveNonDirectoryParent but with branch and
# trunk switched. As such it should certainly produce the same
# conflict.
- self.run_script("""
+ self.assertRaises(errors.MalformedTransform,
+ self.run_script,"""
$ bzr init trunk
...
$ cd trunk
=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py 2011-07-06 20:52:00 +0000
+++ b/bzrlib/transform.py 2011-10-26 15:21:29 +0000
@@ -3068,7 +3068,7 @@
existing_file, new_file = conflict[2], conflict[1]
else:
existing_file, new_file = conflict[1], conflict[2]
- new_name = tt.final_name(existing_file)+'.moved'
+ new_name = tt.final_name(existing_file) + '.moved'
tt.adjust_path(new_name, final_parent, existing_file)
new_conflicts.add((c_type, 'Moved existing file to',
existing_file, new_file))
=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt 2011-10-27 14:16:10 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt 2011-10-27 15:04:35 +0000
@@ -32,6 +32,17 @@
.. Fixes for situations where bzr would previously crash or give incorrect
or undesirable results.
+* During merges, when two entries end up using the same path for two
+ different file-ids (the same file being 'bzr added' in two different
+ branches) , 'duplicate' conflicts are created instead of 'content'
+ ones. This was previously leading to a 'Malformed tramsform' exception.
+ (Vincent Ladeuil, #880701)
+
+* 'Malformed transform' exceptions are now recognized as internal errors
+ instead of user errors and report a traceback. This will reduce user
+ confusion as there is generally nothing users can do about them.
+ (Vincent Ladeuil, #880701)
+
Documentation
*************
@@ -102,6 +113,7 @@
* Return early from create_delta_index_from_delta given tiny inputs. This
avoids raising a spurious MemoryError on certain platforms such as AIX.
(John Arbash Meinel, #856731)
+
Documentation
*************
More information about the bazaar-commits
mailing list