> The basic aim to here sounds like the right thing, content conflicts for files
> that happen to share the same path after merging is never going to be useful.
> Some code style nits you can act on or not as you wish, but needs fixing for
> improving the release notes a little.
> 
> 
> +        keep_this = False
> 
> An alternative to this flag and the check at the end would be to `return None`

return result you mean, but I see your point.

... crap !... it really returns None. That's bad :-( Yet another point to
address ...

Anyway, I don't really agree with returning early as this method is already
very hard to read and I'm afraid adding early returns, while locally making
it more understandable, will make it harder to grasp globally.

It's 120 lines long and really need a better refactoring that I don't want
to do now :-/

If you look at annotations, you'll see that postponing the
self.tt.delete_contents(trans_id) was introduced recently and it took me a
while to understand its true meaning and *this* proposal introducing an
exception for it makes me think that we'd better put it back in all relevant
parts instead of delaying it.

As it stands, I think it makes more sense to leave it this way so people
realize that its intent is to get rid of the content of the file *before*
the merge *because* it's obsoleted by the merge producing a different
content (I've clarified the associated comment).

Also, there is:

        if not self.this_tree.has_id(file_id) and result == "modified":
            self.tt.version_file(file_id, trans_id)

that is still needed (or may be not but the fact that I can't tell is
another indication we still have more work to do here).

> early inside these two blocks. This also avoids the need for the
> `inhibit_content_conflict` flag.
> 
> The remainder of the logic added here looks sensible to me, but it's hard to
> comprehend the exact implications.

Yeah, I know :-/ The conflicts being handled in various parts under various
circumstances is part of the problem. I don't have a good answer for that
(yet) as it's strongly related to tree transform features.

> 
> 
> +    def _get_filter_tree_path(self, file_id):
> 
> Now you've split this to a separate function, may as well return early.

Fixed.

> 
> +            # File created with different file-ids but deleted on one side
> +            (dict(_base_actions='create_file_a'),
> 

> I like functional programming languages, but the way these test_conflict
> cases are spelt with scenarios like this is... not very easy to read. Per
> the prevailing style though, they look about right.

Yeah, this has been mentioned in the past.

These tests are... hard to describe.

An alternative would be to generate test scripts instead but I refrained
from doing so in the past as that turned them into blackbox tests which
couldn't be debugged easily. This is not true anymore.

Otherwise, their main interest is that they allow to really run 4 different
tests with a single description:
- merging from one branch into the other (and vice-versa)
- resolving in both directions (--take-this and --take-other)

They also encompass running two bzr commands (merge and resolve) which is
not encountered elsewhere in the test code base (AFAIK).

> 
> 
> In the release notes:
> 
> +* Fixed an infinite loop when creating a repo at the root of the filesystem,
> 
> Moving this is right, but:
> 
> +* 'Duplicate' conflicts are now created instead of 'Content' ones when a
> +  merge tries to create two entries for the same path.
> +  (Vincent Ladeuil, #880701)
> 
> 'D' should come before 'F'?

Oh ? Nobody never tells me anything ! :)

> 
> Also, saying something about the what effect this fix has directly would be
> good, as this is a user-facing bug. So, mention the exception that will no
> longer be raised in which circumstances or similar.

Done.

> 
> General review note, fixing code formatting nits as you go when working on
> something like this is a good habit, but pulling them out to a separate,
> prerequisite if needed, branch would make life easier for diff readers.

Yeah, this has been discussed numerous times in the past. I generally revert
such nits to avoid this (especially on older stable releases). Sorry about
that.