[MERGE][BUG #187169] rollback an invalid delta

[Martin Pool]

Martin Pool has voted tweak.
Status is now: Conditionally approved
Comment:
This is useful robustness, thanks for doing it.  I agree with you about 
not putting this in the other state fields.

I think _consistency is too vague or general a word for this purpose. 
If it means "broken" or "aborted" then say so.  Otherwise it raises a 
question about "consistent with what?"  Then it could just hold a 
boolean, rather than an enumeration - particularly because this 
enumeration is I think at risk of people confusing it with the 
_dirblock_state and other fields.



+class CorruptDirstate(BzrError):
+
+    _fmt = ("There is an inconsistency with your dirstate file at path"
+            " %(dirstate)s.\n"
+            "Error: %(description)s")

This is implying the problem is specifically with the file on disk, 
which is probably fine but I wonder if that will really always be true.

This error class is tested in test_errors with a path string as the 
first parameter.  However, it's actually called with a dirstate 
instance.

I have mixed feelings about the general issue of constructing exceptions 
with objects vs strings.  The object lets you get more attributes out of 
it from a debugger, or interrogate or poke it from a finally clause. 
But passing the string lets the object be gc'd, is more likely to avoid 
bad messages for objects with no repr or secondary failures when we try 
to use the object (which may already be broken.)

To catch this kind of thing we need to check the str of the exception 
returned from assertRaises, which is unfortunately similar to what 
test_errors does.

             if real_add and entry[1][1][0] not in absent:
-                raise errors.BzrError('dirstate: inconsistent delta, 
with '
-                    'tree 0. %r %r' % (new_path, file_id))
+                self._consistency = DirState.INCONSISTENT
+                raise errors.BzrError(new_path, file_id,
+                    'The entry was considered to be a genuinely new 
record,'
+                    ' but there was already an old record for it.')

Shouldn't this be a InconsistentDelta too?

So in this particular case I would change the exception-raising code to 
just pass the path.

If you could either resubmit this, or indicate you're ok for me to make 
those changes, I'll merge it before 1.2rc1.

For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C47A7810F.2000300%40arbash-meinel.com%3E