[PATCH] filemerge: convert a couple of wvfs calls in internal mergetools to contexts

[Phillip Cohen]

> It looks like in the test suite, we only call isabsent() after calling
> remove().

I was wrong. The existing merge code relies on isabsent() still
returning True after writing to that path in the working copy. So, I
think this patch is probably the best we can do for now without
over-complicating things.

On Mon, Jun 26, 2017 at 11:12 PM, Phillip Cohen <phillip at phillip.io> wrote:
>> I discussed with Sidd about just having the getters
> raise RuntimeErrors after a mutator has been called, but we actually call
> isabsent() in merge.py after running the internal merge tools.
>
> It looks like in the test suite, we only call isabsent() after calling
> remove(). So perhaps a reasonable option is to make calling write() on
> an absentfilectx marked it as non-absent (and cause flags(), size(),
> data(), e.g. to pass-through to the underlying filectx). Calling
> remove() would mark it as absent, if it wasn't. It would default to
> absent. Then, we can do the writes on the absentcontext instance
> (which feels better than going around it and writing directly to the
> working copy) without breaking any existing behavior.
>
> (Also, hooray, I reinvented Optional[] :P)
>
> On Mon, Jun 26, 2017 at 11:03 PM, Phil Cohen <phillco at fb.com> wrote:
>> # HG changeset patch
>> # User Phil Cohen <phillco at fb.com>
>> # Date 1498542735 25200
>> #      Mon Jun 26 22:52:15 2017 -0700
>> # Node ID 016ce3d8fee0968dd30894bf78c3812a19a240d9
>> # Parent  d2f2b5a60476e18e69fdcd76ac296d37bb69b112
>> filemerge: convert a couple of wvfs calls in internal mergetools to contexts
>>
>> One hitch is that sometimes fcd is actually an absentfilectx which does not
>> expose any mutator functions. In order to still use the context functions,
>> we look up the underlying workingfilectx to perform the write there.
>>
>> One alternate way would be to put the write functions on the absentfilectx and
>> have them pass-through. While this makes the callsites cleaner, we would need
>> to decide what its getter functions would return after this point, since
>> returning None for `data` (and True for `isabsent()`) might no longer be
>> correct after a write. I discussed with Sidd about just having the getters
>> raise RuntimeErrors after a mutator has been called, but we actually call
>> isabsent() in merge.py after running the internal merge tools.
>>
>> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
>> --- a/mercurial/filemerge.py
>> +++ b/mercurial/filemerge.py
>> @@ -298,10 +298,10 @@
>>      """Uses the other `p2()` version of files as the merged version."""
>>      if fco.isabsent():
>>          # local changed, remote deleted -- 'deleted' picked
>> -        repo.wvfs.unlinkpath(fcd.path())
>> +        _underlyingfctxifabsent(fcd).remove()
>>          deleted = True
>>      else:
>> -        repo.wwrite(fcd.path(), fco.data(), fco.flags())
>> +        _underlyingfctxifabsent(fcd).write(fco.data(), fco.flags())
>>          deleted = False
>>      return 0, deleted
>>
>> @@ -313,9 +313,19 @@
>>      used to resolve these conflicts."""
>>      # for change/delete conflicts write out the changed version, then fail
>>      if fcd.isabsent():
>> -        repo.wwrite(fcd.path(), fco.data(), fco.flags())
>> +        _underlyingfctxifabsent(fcd).write(fco.data(), fco.flags())
>>      return 1, False
>>
>> +def _underlyingfctxifabsent(filectx):
>> +    """Sometimes when resolving, our fcd is actually an absentfilectx, but
>> +    we want to write to it (to do the resolve). This helper returns the
>> +    underyling workingfilectx in that case.
>> +    """
>> +    if filectx.isabsent():
>> +        return filectx.changectx()[filectx.path()]
>> +    else:
>> +        return filectx
>> +
>>  def _premerge(repo, fcd, fco, fca, toolconf, files, labels=None):
>>      tool, toolpath, binary, symlink = toolconf
>>      if symlink or fcd.isabsent() or fco.isabsent():
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel