In context.py, basefilectx.cmp explicitly calls it with None, so it has to be
supported. Specifically, this breaks "hg absorb -i" currently.
Details
- Reviewers
- None
- Group Reviewers
hg-reviewers - Commits
- rHGeed9a62e1f10: remotefilelog: accepting a None node to cmp
rHG4e17679c336b: remotefilelog: accepting a None node to cmp
Diff Detail
- Repository
- rHG Mercurial
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
In context.py, basefilectx.cmp explicitly calls it with None, so it has to be supported. Specifically, this breaks "hg absorb -i" currently.
IIUC, self._filenode should never be None if the given fctx._filenode
is None. If None were passed down to the filelog layer, exception would
be raised.
returns True if text is different than what is stored. """
- if node == nullid:
+ if not node or node == nullid:
return True
Are we sure that the working-directory data is different from the given text?
TLDR: I don't know, to be honest, I don't understand RFL deeply enough to understand all the edge cases.
About it being None - notice the block that calls this:
if (fctx._filenode is None and (self._repo._encodefilterpats # if file data starts with '\1\n', empty metadata block is # prepended, which adds 4 bytes to filelog.size(). or self.size() - 4 == fctx.size()) or self.size() == fctx.size()): return self._filelog.cmp(self._filenode, fctx.data())
which is essentially:
if f == None: return filelog.cmp(f)
so I assumed it must be the intention to actually pass None, and with so much handling, that None was an ok value?
Returning True seemed like the safe approach, rather than claiming the file was unmodified, but I'm not confident that it covers 100% of the cases.
If someone has a better understanding of all the remotefilelog overriding/interaction here, I'm happy to propose or look at a different patch, but this one does fix the immediate issue (absorb -i crashing).
Oops, my oversight - what's passed to cmp is not the same that's compared to None in that if :) I have even less confidence on this now.
Oops, my oversight - what's passed to cmp is not the same that's compared to None in that if :) I have even less confidence on this now.
And workingfilectx.cmp(self, fctx) flips self and fctx as it knows
self._filenode is None, and the other's wouldn't be None. I don't like it,
but that's how fctx.cmp() is working.
I'm dropping this since I found a bug in memfilectx. That's probably why
absorb crashed.