[MERGE] Teach 'bzr remerge' to not use 'iter_conflicts'

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John Arbash Meinel wrote:
> By the way, I'm going to require that we resolve this in one way or the
> other for 0.12. So if you aren't happy with my proposed solution to
> filter the conflict list, then I can revert the change, and we need to
> undeprecate iter_conflicts().

I am glad that you want to solve this problem now.  Yet I am also
frustrated, because I have tried to get people to discuss this problem
in the past, and basically been ignored.

> I personally feel like iter_conflicts() is trying to read too much into
>  filenames, but it is your code. 

I am happy to solve it in terms of the conflict file mechanism for
WorkingTree3.  Doing it that way for WorkingTree2 is double-handling,
but maybe we don't care enough to fix that.

One of my hesitations has been the Python aversion to checking type;
calling isinstance(conflict, ContentsConflict) seemed uncouth, but I
wasn't sure what The Right Way was.

> So if you feel strongly that remerge
> needs to use iter_conflicts(), then I can revert my changes, and
> undeprecate iter_conflicts().

Alternatively, we could create iter_contents_conflict.

> It would be nice to get a few more test cases to indicate *why* we need
> to do this, so we don't break things in a future refactoring.

Sure.

> We can filter based on whether it is a content conflict, but using
> 'iter_conflicts' will also cause us to flag real versioned files that
> happen to end with .BASE, .THIS, or .OTHER. Which is also a bug, though
> perhaps less obvious.

I have no problem with using the conflicts file for WorkingTree3.  But
there's no reason WorkingTree3.iter_conflicts couldn't do that.

> # Remerge only supports resolving contents conflicts
> allowed_conflicts = ('text conflict', 'contents conflict')
> restore_files = [c.path for c in conflicts
>                  if c.typestring in allowed_conflicts]
> 
> This passes the test suite, and should satisfy your 'only allow text or
> content conflicts'.

This seems like a fine approach.

> You claimed remerge only supports 'content conflict', but when I did not
> include 'text conflict' the test suite didn't pass. And I saw that old
> working tree formats used to return both of these types.

Sorry.  I consider text conflicts to be a form of contents conflicts,
and I thought that was reflected in the class hierarchy.  Perhaps we
should turn TextConflict into a subclass of ContentsConflict.

That would mean we could just do [c.path for c in conflicts if
                                  isinstance(c, ContentsConflict)]

> I think _iter_conflicts is broken because it assumes file endings have
> meaning, which is only approximately true. I think we switched to
> 'WorkingTree.conflicts()' for that express purpose.

We switched to WorkingTree.conflicts because it allowed us to record
more kinds of conflicts, in better detail.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFNFHY0F+nu1YWqI0RAl2RAJ9MLvW7dDcdYsbChzE0cHZs7t3dbwCggaVu
7PyHZiHq2WXg7HVAglunrro=
=+3kg
-----END PGP SIGNATURE-----