[MERGE][0.15] a few pointers needed for solving a bug

[Martin Pool]

On 3/24/07, Martin Pool <mbp at sourcefrog.net> wrote:
> This fails with

OK, that traceback was fairly obviously just a need for lock/unlock
calls around the validation.   However, the new assertion added in
John's code does actually trigger in these two tests.  It fails in
http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/move_children_correctly
 too.

FAIL: test_merge_core.FunctionalMergeTest.test_merge_swapping_renames
    a rename points to a record which points to a different location.
(('', 'tmp', 'un'), [('r', 'deux', 0, False, ''), ('r', 'un', 0,
False, '')]) => (('', 'deux', 'un'),
 [('f',
   'c04d888cef268cfc704ac73209596e3d0d0f2721',
   2L,
   False,
   'AAAAAkYGWn1GBlp9AAD+AwGpB8QAAIGk'),
  ('r', 'un', 0, False, '')])

FAIL: blackbox.test_merge.TestMerge.test_merge_uncommitted
    a rename points to a record which doesn't point back. (('',
'file_ii', 'file_2-20070325111817-xj11v0rn7wx4uud2-239'),
 [('r', 'file_2', 0, False, ''), ('r', 'file_2', 0, False, '')]) =>
(('', 'file_2', 'file_2-20070325111817-xj11v0rn7wx4uud2-239'),
 [('f', '', 0, False, 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'),
  ('f',
   'f82ab652155d01023ad7dad14013e892cda8c58a',
   21,
   False,
   'mbp at limpid-20070325111827-xo1fb82ih7idkbwg')])

+        # Make sure that all renamed entries point to the correct location.
+        for entry in self._iter_entries():
+            for tree_index, tree_state in enumerate(entry[1]):
+                if tree_state[0] == 'r': # Renamed entry
+                    target_location = tree_state[1]
+                    other_entry = self._get_entry(tree_index,
+                                                  path_utf8=target_location)
+                    this_path = osutils.pathjoin(entry[0][0], entry[0][1])
+                    other_path = osutils.pathjoin(other_entry[0][0],
+                                                  other_entry[0][1])
+                    assert entry[0][2] == other_entry[0][2], \
+                        ('A rename entry points to a record with a different'
+                         ' file id. %s => %s'
+                         % (pformat(entry), pformat(other_entry)))
+                    if len(entry[1]) == 2: # Check the rename is symmetric
+                        if tree_index == 0:
+                            other_index = 1
+                        else:
+                            other_index = 0
+                        assert other_entry[1][other_index][0] == 'r', \
+                            ('a rename points to a record which doesn\'t'
+                             ' point back. %s => %s'
+                             % (pformat(entry), pformat(other_entry)))
+                        assert other_entry[1][other_index][1] == this_path, \
+                            ('a rename points to a record which points to a'
+                             ' different location. %s => %s'
+                             % (pformat(entry), pformat(other_entry)))
+

I am not sure these last two assertions are correct.  As I understand
it, the 'r' records just tell you where, within one tree, a particular
file id can be found, and they don't necessarily require back pointers
across trees.

I think the correct test would be something like this:  (pseudocode)

  for all file ids:
    for all trees:
      if the file id is not in the tree at all: pass
      the file id must occur exactly once with minikind != r
      every other occurrence must have minikind 'r', and point to the
real record within this tree

I do think, though. that having a _validate for nontrivial data
sttructures that states their invariants in a mechanical way is very
useful.

-- 
Martin