Rev 4623: Fix bug #529968 by renaming the kept file on content conflicts. in file:///home/vila/src/bzr/experimental/conflict-manager/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Mon Mar 1 14:51:07 GMT 2010
At file:///home/vila/src/bzr/experimental/conflict-manager/
------------------------------------------------------------
revno: 4623
revision-id: v.ladeuil+lp at free.fr-20100301145106-v79eogiexrbv08ko
parent: v.ladeuil+lp at free.fr-20100301135549-gxwcs9fyk4t8fitb
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: cleanup
timestamp: Mon 2010-03-01 15:51:06 +0100
message:
Fix bug #529968 by renaming the kept file on content conflicts.
* bzrlib/conflicts.py:
(ContentsConflict._take_it): Helper to resolve the conflict,
factored out of the two symmetric cases. Use a tree transform to
transparently handle more cases.
(ContentsConflict.action_take_this,
ContentsConflict.action_take_other):
* bzrlib/tests/test_conflicts.py:
(load_tests): Start parametrizing tests.
(content_conflict_scenarios): First scenarios for ContenConflict.
(TestResolveContentConflicts.setUp): Actions are parameters.
(TestResolveContentConflicts.test_resolve_taking_this,
TestResolveContentConflicts.test_resolve_taking_other): Finer
checks are parameters.
(TestResolveContentConflicts._get_actions)
(TestResolveContentConflicts._get_check): Helpers.
(TestResolveContentConflicts.do_modify_file,
TestResolveContentConflicts.do_delete_file): Specific actions.
(TestResolveContentConflicts.check_file_has_more_content,
TestResolveContentConflicts.check_file_doesnt_exist): Specific
checks.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2010-02-26 04:43:31 +0000
+++ b/NEWS 2010-03-01 14:51:06 +0000
@@ -79,6 +79,10 @@
* ``bzr remove-tree`` can now remove multiple working trees.
(Jared Hance, Andrew Bennetts, #253137)
+* ``bzr resolve --take-{this|other}`` now correctly rename the kept file
+ on content conflicts where one side deleted the file.
+ (Vincent Ladeuil, #529968)
+
* ``bzr upgrade`` now names backup directory as ``backup.bzr.~N~`` instead
of ``backup.bzr``. This directory is ignored by bzr commands such as
``add``.
=== modified file 'bzrlib/conflicts.py'
--- a/bzrlib/conflicts.py 2010-02-23 07:43:11 +0000
+++ b/bzrlib/conflicts.py 2010-03-01 14:51:06 +0000
@@ -479,16 +479,42 @@
def associated_filenames(self):
return [self.path + suffix for suffix in ('.BASE', '.OTHER')]
- # FIXME: I smell something weird here and it seems we should be able to be
- # more coherent with some other conflict ? bzr *did* a choice there but
- # neither action_take_this nor action_take_other reflect that...
- # -- vila 20091224
+ def _take_it(self, tree, suffix):
+ """Resolve the conflict.
+
+ :param tree: The working tree where the confict is resolved.
+ :param suffix: Either 'THIS' or 'OTHER'
+
+ The resolution is symmetric, when taking THIS, OTHER is deleted and
+ item.THIS is renamed into item and vice-versa.
+
+ Note that suffix='OTHER' really means takes 'THIS' and vice-versa.
+ """
+ tt = transform.TreeTransform(tree)
+ try:
+ try:
+ # Delete 'item.THIS' or 'item.OTHER' depending on suffix
+ tt.delete_contents(
+ tt.trans_id_tree_path(self.path + '.' + suffix))
+ except errors.NoSuchFile:
+ # There are valid cases where 'item.suffix' either never
+ # existed or was already deleted (including the case where the
+ # user deleted it)
+ pass
+ # Rename 'item.suffix' (note that if 'item.suffix' has been
+ # deleted, this is a no-op)
+ this_tid = tt.trans_id_file_id(self.file_id)
+ parent_tid = tt.get_tree_parent(this_tid)
+ tt.adjust_path(self.path, parent_tid, this_tid)
+ tt.apply()
+ finally:
+ tt.finalize()
+
def action_take_this(self, tree):
- tree.remove([self.path + '.OTHER'], force=True, keep_files=False)
+ self._take_it(tree, 'OTHER')
def action_take_other(self, tree):
- tree.remove([self.path], force=True, keep_files=False)
-
+ self._take_it(tree, 'THIS')
# FIXME: TextConflict is about a single file-id, there never is a conflict_path
=== modified file 'bzrlib/tests/test_conflicts.py'
--- a/bzrlib/tests/test_conflicts.py 2010-03-01 13:55:49 +0000
+++ b/bzrlib/tests/test_conflicts.py 2010-03-01 14:51:06 +0000
@@ -29,6 +29,21 @@
from bzrlib.tests import script
+def load_tests(standard_tests, module, loader):
+ result = loader.suiteClass()
+
+ sp_tests, remaining_tests = tests.split_suite_by_condition(
+ standard_tests, tests.condition_isinstance((
+ TestResolveContentConflicts,
+ )))
+ tests.multiply_tests(sp_tests, content_conflict_scenarios(), result)
+
+ # No parametrization for the remaining tests
+ result.addTests(remaining_tests)
+
+ return result
+
+
# TODO: Test commit with some added, and added-but-missing files
# RBC 20060124 is that not tested in test_commit.py ?
@@ -194,10 +209,25 @@
pass
+def content_conflict_scenarios():
+ return [('file,None', dict(_this_actions='modify_file',
+ _check_this='file_has_more_content',
+ _other_actions='delete_file',
+ _check_other='file_doesnt_exist',
+ )),
+ ('None,file', dict(_this_actions='delete_file',
+ _check_this='file_doesnt_exist',
+ _other_actions='modify_file',
+ _check_other='file_has_more_content',
+ )),
+ ]
+
+
class TestResolveContentConflicts(tests.TestCaseWithTransport):
- # FIXME: We need to add the reverse case (delete in trunk, modify in
- # branch) but that could wait until the resolution mechanism is implemented.
+ # Set by load_tests
+ this_actions = None
+ other_actions = None
def setUp(self):
super(TestResolveContentConflicts, self).setUp()
@@ -210,14 +240,32 @@
builder.build_snapshot('base', ['start'], [
('add', ('file', 'file-id', 'file', 'trunk content\n'))])
# Modify the base content in branch
- builder.build_snapshot('other', ['base'], [
- ('unversion', 'file-id')])
+ other_actions = self._get_actions(self._other_actions)
+ builder.build_snapshot('other', ['base'], other_actions())
# Modify the base content in trunk
- builder.build_snapshot('this', ['base'], [
- ('modify', ('file-id', 'trunk content\nmore content\n'))])
+ this_actions = self._get_actions(self._this_actions)
+ builder.build_snapshot('this', ['base'], this_actions())
builder.finish_series()
self.builder = builder
+ def _get_actions(self, name):
+ return getattr(self, 'do_%s' % name)
+
+ def _get_check(self, name):
+ return getattr(self, 'check_%s' % name)
+
+ def do_modify_file(self):
+ return [('modify', ('file-id', 'trunk content\nmore content\n'))]
+
+ def check_file_has_more_content(self):
+ self.assertFileEqual('trunk content\nmore content\n', 'branch/file')
+
+ def do_delete_file(self):
+ return [('unversion', 'file-id')]
+
+ def check_file_doesnt_exist(self):
+ self.failIfExists('branch/file')
+
def _merge_other_into_this(self):
b = self.builder.get_branch()
wt = b.bzrdir.sprout('branch').open_workingtree()
@@ -244,14 +292,16 @@
self.assertConflict(wt, conflicts.ContentsConflict,
path='file', file_id='file-id',)
self.assertResolved(wt, 'file', 'take_this')
- self.assertFileEqual('trunk content\nmore content\n', 'branch/file')
+ check_this = self._get_check(self._check_this)
+ check_this()
def test_resolve_taking_other(self):
wt = self._merge_other_into_this()
self.assertConflict(wt, conflicts.ContentsConflict,
path='file', file_id='file-id',)
self.assertResolved(wt, 'file', 'take_other')
- self.failIfExists('branch/file')
+ check_other = self._get_check(self._check_other)
+ check_other()
class TestResolveDuplicateEntry(TestResolveConflicts):
More information about the bazaar-commits
mailing list