[PATCH] filemerge: convert a couple of wvfs calls in internal mergetools to contexts
Phillip Cohen
phillip at phillip.io
Wed Jun 28 17:38:56 UTC 2017
> 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
More information about the Mercurial-devel
mailing list