This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2069

revlog: resolve lfs rawtext to vanilla rawtext before applying delta
ClosedPublic

Authored by quark on Feb 6 2018, 8:14 PM.

Details

Reviewers
indygreg
ryanmce
Group Reviewers
hg-reviewers
Commits
rHG369aadf7a326: revlog: resolve lfs rawtext to vanilla rawtext before applying delta
Summary

This happens when a LFS delta base gets a non-LFS delta from another client.
In that case, the LFS delta base needs to be converted to non-LFS version
before applying the delta.

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

quark created this revision.Feb 6 2018, 8:14 PM
quark updated this revision to Diff 5267.Feb 6 2018, 8:19 PM
quark updated this revision to Diff 5271.Feb 6 2018, 9:13 PM
indygreg requested changes to this revision.Feb 7 2018, 5:36 PM

I'd like to see the next versions of hte previous 2 patches before looking at this because this patch will likely get some minor rework as well.

This revision now requires changes to proceed.Feb 7 2018, 5:36 PM
quark updated this revision to Diff 5652.Feb 13 2018, 2:41 PM
ryanmce accepted this revision.Mar 3 2018, 2:37 PM
quark updated this revision to Diff 6471.Mar 3 2018, 3:27 PM
indygreg accepted this revision.Mar 6 2018, 1:53 PM

This change is a bit difficult to reason about. But I think it is fine.

Essentially, we're ensuring that the delta stored in the revlog is applied on top of the fulltext after flag transformation rather than before. In the common case where we have no flag processors, this doesn't matter. In the case of LFS, it ensures LFS content resolution is performed first so we can delta against that.

On the addrevision side, we're now taking the size of the flag resolved content rather than the raw content. In the common case of no flag processors, the new behavior is identical. In the case of LFS, we compute the size of the LFS resolved data rather than the pointer record.

This revision is now accepted and ready to land.Mar 6 2018, 1:53 PM
Closed by commit rHG369aadf7a326: revlog: resolve lfs rawtext to vanilla rawtext before applying delta (authored by quark). · Explain WhyMar 6 2018, 1:55 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/revlog.py (9 lines)
Mtests/test-lfs-bundle.t (2 lines)

Diff 6674

mercurial/revlog.py

# special case deltas which replace entire base; no need to decode # special case deltas which replace entire base; no need to decode
# base revision. this neatly avoids censored bases, which throw when # base revision. this neatly avoids censored bases, which throw when
# they're decoded. # they're decoded.
hlen = struct.calcsize(">lll") hlen = struct.calcsize(">lll")
if delta[:hlen] == mdiff.replacediffheader(revlog.rawsize(baserev), if delta[:hlen] == mdiff.replacediffheader(revlog.rawsize(baserev),
len(delta) - hlen): len(delta) - hlen):
btext[0] = delta[hlen:] btext[0] = delta[hlen:]
else: else:
basetext = revlog.revision(baserev, _df=fh, raw=True) # deltabase is rawtext before changed by flag processors, which is
# equivalent to non-raw text
basetext = revlog.revision(baserev, _df=fh, raw=False)
btext[0] = mdiff.patch(basetext, delta) btext[0] = mdiff.patch(basetext, delta)
try: try:
res = revlog._processflags(btext[0], flags, 'read', raw=True) res = revlog._processflags(btext[0], flags, 'read', raw=True)
btext[0], validatehash = res btext[0], validatehash = res
if validatehash: if validatehash:
revlog.checkhash(btext[0], node, p1=revinfo.p1, p2=revinfo.p2) revlog.checkhash(btext[0], node, p1=revinfo.p1, p2=revinfo.p2)
if flags & REVIDX_ISCENSORED: if flags & REVIDX_ISCENSORED:
curr = len(self) curr = len(self)
prev = curr - 1 prev = curr - 1
offset = self.end(prev) offset = self.end(prev)
p1r, p2r = self.rev(p1), self.rev(p2) p1r, p2r = self.rev(p1), self.rev(p2)
# full versions are inserted when the needed deltas # full versions are inserted when the needed deltas
# become comparable to the uncompressed text # become comparable to the uncompressed text
if rawtext is None: if rawtext is None:
textlen = mdiff.patchedsize(self.rawsize(cachedelta[0]), # need rawtext size, before changed by flag processors, which is
# the non-raw size. use revlog explicitly to avoid filelog's extra
# logic that might remove metadata size.
textlen = mdiff.patchedsize(revlog.size(self, cachedelta[0]),
cachedelta[1]) cachedelta[1])
else: else:
textlen = len(rawtext) textlen = len(rawtext)
if deltacomputer is None: if deltacomputer is None:
deltacomputer = _deltacomputer(self) deltacomputer = _deltacomputer(self)
revinfo = _revisioninfo(node, p1, p2, btext, textlen, cachedelta, flags) revinfo = _revisioninfo(node, p1, p2, btext, textlen, cachedelta, flags)

tests/test-lfs-bundle.t

> else > else
> echo CRASHED > echo CRASHED
> fi > fi
> done > done
> done > done
---- Applying src-normal.bundle to dst-normal ---- ---- Applying src-normal.bundle to dst-normal ----
OK OK
---- Applying src-normal.bundle to dst-lfs ---- ---- Applying src-normal.bundle to dst-lfs ----
CRASHED OK
---- Applying src-lfs.bundle to dst-normal ---- ---- Applying src-lfs.bundle to dst-normal ----
OK OK
---- Applying src-lfs.bundle to dst-lfs ---- ---- Applying src-lfs.bundle to dst-lfs ----
OK OK