D674: filemerge: use arbitraryfilectx for backup files

[phillco (Phil Cohen)]

phillco added a subscriber: sid0.
phillco added inline comments.

INLINE COMMENTS

> martinvonz wrote in test-dirstate-race.t:85
> Why did this change?

It's caused by the addition of this block into `abstractfilectx`.

  if isinstance(fctx, abstractfilectx):
      return self.data() != fctx.data()

When `fctx` is a `workingfilectx`, and has been replaced by a directory, calling `data()` on it raises an `IOError` instead of returning `True` like this function used to do.

Interestingly, the test behavior is caused by this error being caught by an existing block inside `workingctx._checklookup()`:

  except (IOError, OSError):
          # A file become inaccessible in between? Mark it as deleted,
          # matching dirstate behavior (issue5584).
          # The dirstate has more complex behavior around whether a
          # missing file matches a directory, etc, but we don't need to
          # bother with that: if f has made it to this point, we're sure
          # it's in the dirstate.
          deleted.append(f)

Which seems to somewhat predict the case of files being unavailable and raising raw `IOError`s, although not this case specifically.

Also, @sid0's comment around the test case says:

  XXX Note that this returns M for files that got replaced by directories. This is
  definitely a bug, but the fix for that is hard and the next status run is fine
  anyway.

Which is in fact the case for `e`. So maybe continuing to throw is actually the better behavior here, given that it would also throw if `fctx` was missing so the idea isn't unprecedented.

On the other hand, just catching the error and returning `True` is the most conservative path forward, so I might just do that here.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D674

To: phillco, #hg-reviewers
Cc: sid0, martinvonz, mercurial-devel