| Submitter | Durham Goode |
|---|---|
| Date | Aug. 31, 2016, 8:30 p.m. |
| Message ID | <59bc68e1d78538bb83f6.1472675420@dev8486.prn1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/16517/ |
| State | Accepted |
| Headers | show |
Comments
On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1472518929 25200 > # Mon Aug 29 18:02:09 2016 -0700 > # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf > # Parent abce9af35512d8589683d94f34f6d8aa21163568 > manifest: make uses of _mancache aware of contexts > --- a/mercurial/manifest.py > +++ b/mercurial/manifest.py > @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): > if node == revlog.nullid: > return self._newmanifest() # don't upset local cache > if node in self._mancache: > - return self._mancache[node] > + cached = self._mancache[node] > + if (isinstance(cached, manifestctx) or > + isinstance(cached, treemanifestctx)): > + cached = cached.read() manifestctx.read() is added by the next patch, so tests fail at this revision. But that wouldn't be a good reason enough to rework this series.
On 09/03/2016 05:51 PM, Yuya Nishihara wrote: > On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote: >> # HG changeset patch >> # User Durham Goode <durham@fb.com> >> # Date 1472518929 25200 >> # Mon Aug 29 18:02:09 2016 -0700 >> # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf >> # Parent abce9af35512d8589683d94f34f6d8aa21163568 >> manifest: make uses of _mancache aware of contexts > >> --- a/mercurial/manifest.py >> +++ b/mercurial/manifest.py >> @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): >> if node == revlog.nullid: >> return self._newmanifest() # don't upset local cache >> if node in self._mancache: >> - return self._mancache[node] >> + cached = self._mancache[node] >> + if (isinstance(cached, manifestctx) or >> + isinstance(cached, treemanifestctx)): >> + cached = cached.read() > > manifestctx.read() is added by the next patch, so tests fail at this revision. > But that wouldn't be a good reason enough to rework this series. This changeset or the next also introduce a strange failure in evolve tests where a manifest hash changes (eg: test-evolve.t). please hold on during investigation.
On 09/07/2016 05:35 PM, Pierre-Yves David wrote: > > > On 09/03/2016 05:51 PM, Yuya Nishihara wrote: >> On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote: >>> # HG changeset patch >>> # User Durham Goode <durham@fb.com> >>> # Date 1472518929 25200 >>> # Mon Aug 29 18:02:09 2016 -0700 >>> # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf >>> # Parent abce9af35512d8589683d94f34f6d8aa21163568 >>> manifest: make uses of _mancache aware of contexts >> >>> --- a/mercurial/manifest.py >>> +++ b/mercurial/manifest.py >>> @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): >>> if node == revlog.nullid: >>> return self._newmanifest() # don't upset local cache >>> if node in self._mancache: >>> - return self._mancache[node] >>> + cached = self._mancache[node] >>> + if (isinstance(cached, manifestctx) or >>> + isinstance(cached, treemanifestctx)): >>> + cached = cached.read() >> >> manifestctx.read() is added by the next patch, so tests fail at this >> revision. >> But that wouldn't be a good reason enough to rework this series. > > This changeset or the next also introduce a strange failure in evolve > tests where a manifest hash changes (eg: test-evolve.t). please hold on > during investigation. So, this is strange, this changeset or the next affect various test in evolve. I've digged in details for test-evolve.t where evolve create a changeset with a different hash. I've added various debug data to the test to track how they change and: * the operation is a bump fix * the changeset have a different hash because manifest hash change (kinda expected), * the manifest have a different hash because one of the file hash change, * file content and commit diff are similar * the file hash change because the revlog gains a p1 (from a previous null ID), * The previous value (no parent for the filerev) is most probably wrong as the file appear "Modified" in the commit. So, something in this changes seems to be magically "fix" evolve. That's suspicious and I would be much more comfortable if we understood what is going on here. I'm about to switch out of work mode for the evening, can someone else look into this (ideal Durham as he is familliar with his change. I've pushed a draft for evolve that display the various debug data here: https://www.mercurial-scm.org/repo/users/marmoute/evolve/rev/7a76f9a43e89 If runs fine with: # Node ID f3157d8a10fb02a1a051ddaaf6fe996f7334f8f1 manifest: add treemanifestctx class And fails with: # Node ID d130a38ef41f3c9e2d2f26df3535d89a93f87301 manifest: change manifestctx to not inherit from manifestdict I can't test the other change because it is foobar: # Node ID ff884112d0efa24f60cd8e3334adc8ef36f5da0f manifest: make uses of _mancache aware of contexts The test failure are: > --- /home/marmoute/src/mutable-history/tests/test-evolve.t > +++ /home/marmoute/src/mutable-history/tests/test-evolve.t.err > @@ -401,8 +401,8 @@ > recreate:[8] another feature that rox > atop:[7] another feature (child of ba0ec09b1bab) > computing new diff > - committed as 2d8c5414e9f0 > - working directory is now at 2d8c5414e9f0 > + committed as 6707c5e1c49d > + working directory is now at 6707c5e1c49d > $ hg status --change . > M main-file-1 > $ hg export . > @@ -410,7 +410,7 @@ > # User test > # Date 0 0 > # Thu Jan 01 00:00:00 1970 +0000 > - # Node ID 2d8c5414e9f0cabf0286a53ec526ab7cda76468c > + # Node ID 6707c5e1c49d54b986803331aaf78eae02764fc9 > # Parent 99833d22b0c6526806fe26fe79b61ad51a72458d > bumped update to 99833d22b0c6: > > @@ -423,13 +423,13 @@ > -Zwei > +deux > $ hg log --debug --rev . --config ui.logtemplate= > - changeset: 9:2d8c5414e9f0cabf0286a53ec526ab7cda76468c > + changeset: 9:6707c5e1c49d54b986803331aaf78eae02764fc9 > bookmark: feature-B > tag: tip > phase: draft > parent: 7:99833d22b0c6526806fe26fe79b61ad51a72458d > parent: -1:0000000000000000000000000000000000000000 > - manifest: 9:9dee7532189f6d1b2eadb9cad8f3407a3ed02162 > + manifest: 9:4f07da51791a798dcf81546115081323819b7a83 > user: test > date: Thu Jan 01 00:00:00 1970 +0000 > files: main-file-1 > @@ -443,7 +443,7 @@ > $ hg manifest --debug -r . > 669c54abb32b6f06b395c85feca4511e433ad00b 644 file-from-A > 94f2f82a0c3f341ead55ea8d2b130f972ed56666 644 file-from-B > - 006a74077e75026fda3ebe59f330125443f13e8c 644 main-file-1 > + 64ad1b503f917abc9118376ab94f7d762466d9e7 644 main-file-1 > 94f2f82a0c3f341ead55ea8d2b130f972ed56666 644 main-file-2 > $ hg debugindex -m --debug > rev offset length delta linkrev nodeid p1 p2 > @@ -456,7 +456,7 @@ > 6 522 101 0 6 e656c939e387427a6b077ef798ccebd237d03ef5 7ec8c19bea7d58a3435e518951d344f145f00f21 0000000000000000000000000000000000000000 > 7 623 103 6 7 bf464c9d2093ac0524289f031f6c2e906041afb5 e656c939e387427a6b077ef798ccebd237d03ef5 0000000000000000000000000000000000000000 > 8 726 102 6 8 1da1c04471dbf03a362b0fdfeacb18989e6ec370 e656c939e387427a6b077ef798ccebd237d03ef5 0000000000000000000000000000000000000000 > - 9 828 129 -1 9 9dee7532189f6d1b2eadb9cad8f3407a3ed02162 bf464c9d2093ac0524289f031f6c2e906041afb5 0000000000000000000000000000000000000000 > + 9 828 131 -1 9 4f07da51791a798dcf81546115081323819b7a83 bf464c9d2093ac0524289f031f6c2e906041afb5 0000000000000000000000000000000000000000 > $ hg cat -r . main-file-1 > Un > > @@ -474,7 +474,7 @@ > 6 86 0 5 6 d6ada519b989 492807de722e 000000000000 > 7 86 17 -1 7 8d64e54db1c4 d6ada519b989 000000000000 > 8 103 17 -1 8 ba9aab3c7032 d6ada519b989 000000000000 > - 9 120 0 8 9 006a74077e75 000000000000 000000000000 > + 9 120 0 8 9 64ad1b503f91 8d64e54db1c4 000000000000 > $ hg glog > @ 9 feature-B: bumped update to 99833d22b0c6: - test > |
On 9/7/16 9:10 AM, Pierre-Yves David wrote: > > > On 09/07/2016 05:35 PM, Pierre-Yves David wrote: >> >> >> On 09/03/2016 05:51 PM, Yuya Nishihara wrote: >>> On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote: >>>> # HG changeset patch >>>> # User Durham Goode <durham@fb.com> >>>> # Date 1472518929 25200 >>>> # Mon Aug 29 18:02:09 2016 -0700 >>>> # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf >>>> # Parent abce9af35512d8589683d94f34f6d8aa21163568 >>>> manifest: make uses of _mancache aware of contexts >>> >>>> --- a/mercurial/manifest.py >>>> +++ b/mercurial/manifest.py >>>> @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): >>>> if node == revlog.nullid: >>>> return self._newmanifest() # don't upset local cache >>>> if node in self._mancache: >>>> - return self._mancache[node] >>>> + cached = self._mancache[node] >>>> + if (isinstance(cached, manifestctx) or >>>> + isinstance(cached, treemanifestctx)): >>>> + cached = cached.read() >>> >>> manifestctx.read() is added by the next patch, so tests fail at this >>> revision. >>> But that wouldn't be a good reason enough to rework this series. >> >> This changeset or the next also introduce a strange failure in evolve >> tests where a manifest hash changes (eg: test-evolve.t). please hold on >> during investigation. > > So, this is strange, this changeset or the next affect various test in > evolve. I've digged in details for test-evolve.t where evolve create a > changeset with a different hash. > > I've added various debug data to the test to track how they change and: > > * the operation is a bump fix > > * the changeset have a different hash because manifest hash change > (kinda expected), > > * the manifest have a different hash because one of the file hash change, > > * file content and commit diff are similar > > * the file hash change because the revlog gains a p1 (from a previous > null ID), > > * The previous value (no parent for the filerev) is most probably > wrong as the file appear "Modified" in the commit. > > So, something in this changes seems to be magically "fix" evolve. > > That's suspicious and I would be much more comfortable if we > understood what is going on here. I'm about to switch out of work mode > for the evening, can someone else look into this (ideal Durham as he > is familliar with his change. I'll look into this when I get a moment. Feel free to drop the series until then.
Dropped. On Thu, Sep 8, 2016 at 11:19 AM, Durham Goode <durham@fb.com> wrote: > > > On 9/7/16 9:10 AM, Pierre-Yves David wrote: >> >> >> >> On 09/07/2016 05:35 PM, Pierre-Yves David wrote: >>> >>> >>> >>> On 09/03/2016 05:51 PM, Yuya Nishihara wrote: >>>> >>>> On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote: >>>>> >>>>> # HG changeset patch >>>>> # User Durham Goode <durham@fb.com> >>>>> # Date 1472518929 25200 >>>>> # Mon Aug 29 18:02:09 2016 -0700 >>>>> # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf >>>>> # Parent abce9af35512d8589683d94f34f6d8aa21163568 >>>>> manifest: make uses of _mancache aware of contexts >>>> >>>> >>>>> --- a/mercurial/manifest.py >>>>> +++ b/mercurial/manifest.py >>>>> @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): >>>>> if node == revlog.nullid: >>>>> return self._newmanifest() # don't upset local cache >>>>> if node in self._mancache: >>>>> - return self._mancache[node] >>>>> + cached = self._mancache[node] >>>>> + if (isinstance(cached, manifestctx) or >>>>> + isinstance(cached, treemanifestctx)): >>>>> + cached = cached.read() >>>> >>>> >>>> manifestctx.read() is added by the next patch, so tests fail at this >>>> revision. >>>> But that wouldn't be a good reason enough to rework this series. >>> >>> >>> This changeset or the next also introduce a strange failure in evolve >>> tests where a manifest hash changes (eg: test-evolve.t). please hold on >>> during investigation. >> >> >> So, this is strange, this changeset or the next affect various test in >> evolve. I've digged in details for test-evolve.t where evolve create a >> changeset with a different hash. >> >> I've added various debug data to the test to track how they change and: >> >> * the operation is a bump fix >> >> * the changeset have a different hash because manifest hash change (kinda >> expected), >> >> * the manifest have a different hash because one of the file hash change, >> >> * file content and commit diff are similar >> >> * the file hash change because the revlog gains a p1 (from a previous null >> ID), >> >> * The previous value (no parent for the filerev) is most probably wrong as >> the file appear "Modified" in the commit. >> >> So, something in this changes seems to be magically "fix" evolve. >> >> That's suspicious and I would be much more comfortable if we understood >> what is going on here. I'm about to switch out of work mode for the evening, >> can someone else look into this (ideal Durham as he is familliar with his >> change. > > I'll look into this when I get a moment. Feel free to drop the series until > then.
On 09/08/2016 09:16 PM, Martin von Zweigbergk via Mercurial-devel wrote: > Dropped. Okay, apparently they made it back into the repository and got published. After chatting with Martin and Durham I've pushed a backout of these two. If some other reviewers could give them a second acceptance stamp (Martin did one) that would be great. > > On Thu, Sep 8, 2016 at 11:19 AM, Durham Goode <durham@fb.com> wrote: >> >> >> On 9/7/16 9:10 AM, Pierre-Yves David wrote: >>> >>> >>> >>> On 09/07/2016 05:35 PM, Pierre-Yves David wrote: >>>> >>>> >>>> >>>> On 09/03/2016 05:51 PM, Yuya Nishihara wrote: >>>>> >>>>> On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote: >>>>>> >>>>>> # HG changeset patch >>>>>> # User Durham Goode <durham@fb.com> >>>>>> # Date 1472518929 25200 >>>>>> # Mon Aug 29 18:02:09 2016 -0700 >>>>>> # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf >>>>>> # Parent abce9af35512d8589683d94f34f6d8aa21163568 >>>>>> manifest: make uses of _mancache aware of contexts >>>>> >>>>> >>>>>> --- a/mercurial/manifest.py >>>>>> +++ b/mercurial/manifest.py >>>>>> @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): >>>>>> if node == revlog.nullid: >>>>>> return self._newmanifest() # don't upset local cache >>>>>> if node in self._mancache: >>>>>> - return self._mancache[node] >>>>>> + cached = self._mancache[node] >>>>>> + if (isinstance(cached, manifestctx) or >>>>>> + isinstance(cached, treemanifestctx)): >>>>>> + cached = cached.read() >>>>> >>>>> >>>>> manifestctx.read() is added by the next patch, so tests fail at this >>>>> revision. >>>>> But that wouldn't be a good reason enough to rework this series. >>>> >>>> >>>> This changeset or the next also introduce a strange failure in evolve >>>> tests where a manifest hash changes (eg: test-evolve.t). please hold on >>>> during investigation. >>> >>> >>> So, this is strange, this changeset or the next affect various test in >>> evolve. I've digged in details for test-evolve.t where evolve create a >>> changeset with a different hash. >>> >>> I've added various debug data to the test to track how they change and: >>> >>> * the operation is a bump fix >>> >>> * the changeset have a different hash because manifest hash change (kinda >>> expected), >>> >>> * the manifest have a different hash because one of the file hash change, >>> >>> * file content and commit diff are similar >>> >>> * the file hash change because the revlog gains a p1 (from a previous null >>> ID), >>> >>> * The previous value (no parent for the filerev) is most probably wrong as >>> the file appear "Modified" in the commit. >>> >>> So, something in this changes seems to be magically "fix" evolve. >>> >>> That's suspicious and I would be much more comfortable if we understood >>> what is going on here. I'm about to switch out of work mode for the evening, >>> can someone else look into this (ideal Durham as he is familliar with his >>> change. >> >> I'll look into this when I get a moment. Feel free to drop the series until >> then. > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >
On 9/7/16 9:10 AM, Pierre-Yves David wrote: > > > On 09/07/2016 05:35 PM, Pierre-Yves David wrote: >> >> >> On 09/03/2016 05:51 PM, Yuya Nishihara wrote: >>> On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote: >>>> # HG changeset patch >>>> # User Durham Goode <durham@fb.com> >>>> # Date 1472518929 25200 >>>> # Mon Aug 29 18:02:09 2016 -0700 >>>> # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf >>>> # Parent abce9af35512d8589683d94f34f6d8aa21163568 >>>> manifest: make uses of _mancache aware of contexts >>> >>>> --- a/mercurial/manifest.py >>>> +++ b/mercurial/manifest.py >>>> @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): >>>> if node == revlog.nullid: >>>> return self._newmanifest() # don't upset local cache >>>> if node in self._mancache: >>>> - return self._mancache[node] >>>> + cached = self._mancache[node] >>>> + if (isinstance(cached, manifestctx) or >>>> + isinstance(cached, treemanifestctx)): >>>> + cached = cached.read() >>> >>> manifestctx.read() is added by the next patch, so tests fail at this >>> revision. >>> But that wouldn't be a good reason enough to rework this series. >> >> This changeset or the next also introduce a strange failure in evolve >> tests where a manifest hash changes (eg: test-evolve.t). please hold on >> during investigation. > > So, this is strange, this changeset or the next affect various test in > evolve. I've digged in details for test-evolve.t where evolve create a > changeset with a different hash. > > I've added various debug data to the test to track how they change and: > > * the operation is a bump fix > > * the changeset have a different hash because manifest hash change > (kinda expected), > > * the manifest have a different hash because one of the file hash change, > > * file content and commit diff are similar > > * the file hash change because the revlog gains a p1 (from a previous > null ID), > > * The previous value (no parent for the filerev) is most probably > wrong as the file appear "Modified" in the commit. > > So, something in this changes seems to be magically "fix" evolve. > > That's suspicious and I would be much more comfortable if we > understood what is going on here. I'm about to switch out of work mode > for the evening, can someone else look into this (ideal Durham as he > is familliar with his change. I think I've found the issue here. The bump code accesses the parent manifest and modifies the manifest dictionary (precmanifest in _solvebumped()). Because that manifestdict is in the mancache, those changes are persisted and the next thing to access the manifest (which is repo.commit()) gets the modified version which causes the new commit to be incorrect since it sees a bad p1.manifest(). With my code, since the manifest cache gets broken into two, the version being modified is in a different cache from the version being accessed later, so it accidentally "fixes" the problem. If I add a ".copy()" to the bump code to copy the manifest before editing it, I get the exact same failures as if my manifest patches are applied. I'll send an evolve patch with the .copy() and the test updates. This manifestdict editing code has been around since 2014 at least. We may want to make non-copied manifests be read-only to prevent such errors in the future.
On 09/12/2016 07:36 PM, Durham Goode wrote: > > > On 9/7/16 9:10 AM, Pierre-Yves David wrote: >> >> >> On 09/07/2016 05:35 PM, Pierre-Yves David wrote: >>> >>> >>> On 09/03/2016 05:51 PM, Yuya Nishihara wrote: >>>> On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote: >>>>> # HG changeset patch >>>>> # User Durham Goode <durham@fb.com> >>>>> # Date 1472518929 25200 >>>>> # Mon Aug 29 18:02:09 2016 -0700 >>>>> # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf >>>>> # Parent abce9af35512d8589683d94f34f6d8aa21163568 >>>>> manifest: make uses of _mancache aware of contexts >>>> >>>>> --- a/mercurial/manifest.py >>>>> +++ b/mercurial/manifest.py >>>>> @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): >>>>> if node == revlog.nullid: >>>>> return self._newmanifest() # don't upset local cache >>>>> if node in self._mancache: >>>>> - return self._mancache[node] >>>>> + cached = self._mancache[node] >>>>> + if (isinstance(cached, manifestctx) or >>>>> + isinstance(cached, treemanifestctx)): >>>>> + cached = cached.read() >>>> >>>> manifestctx.read() is added by the next patch, so tests fail at this >>>> revision. >>>> But that wouldn't be a good reason enough to rework this series. >>> >>> This changeset or the next also introduce a strange failure in evolve >>> tests where a manifest hash changes (eg: test-evolve.t). please hold on >>> during investigation. >> >> So, this is strange, this changeset or the next affect various test in >> evolve. I've digged in details for test-evolve.t where evolve create a >> changeset with a different hash. >> >> I've added various debug data to the test to track how they change and: >> >> * the operation is a bump fix >> >> * the changeset have a different hash because manifest hash change >> (kinda expected), >> >> * the manifest have a different hash because one of the file hash change, >> >> * file content and commit diff are similar >> >> * the file hash change because the revlog gains a p1 (from a previous >> null ID), >> >> * The previous value (no parent for the filerev) is most probably >> wrong as the file appear "Modified" in the commit. >> >> So, something in this changes seems to be magically "fix" evolve. >> >> That's suspicious and I would be much more comfortable if we >> understood what is going on here. I'm about to switch out of work mode >> for the evening, can someone else look into this (ideal Durham as he >> is familliar with his change. > I think I've found the issue here. The bump code accesses the parent > manifest and modifies the manifest dictionary (precmanifest in > _solvebumped()). Because that manifestdict is in the mancache, those > changes are persisted and the next thing to access the manifest (which > is repo.commit()) gets the modified version which causes the new commit > to be incorrect since it sees a bad p1.manifest(). > > With my code, since the manifest cache gets broken into two, the version > being modified is in a different cache from the version being accessed > later, so it accidentally "fixes" the problem. > > If I add a ".copy()" to the bump code to copy the manifest before > editing it, I get the exact same failures as if my manifest patches are > applied. > > I'll send an evolve patch with the .copy() and the test updates. This > manifestdict editing code has been around since 2014 at least. We may > want to make non-copied manifests be read-only to prevent such errors in > the future. Thanks for digging this out. I agree we should make any cached manifest read only to prevent to this class of error. Can you handle this as part of your current rework ? Cheers,
Patch
diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py --- a/mercurial/bundlerepo.py +++ b/mercurial/bundlerepo.py @@ -204,8 +204,8 @@ class bundlemanifest(bundlerevlog, manif if isinstance(node, int): node = self.node(node) - if node in self._mancache: - result = self._mancache[node].text() + if node in self.fulltextcache: + result = self.fulltextcache[node].tostring() else: result = manifest.manifest.revision(self, nodeorrev) return result diff --git a/mercurial/manifest.py b/mercurial/manifest.py --- a/mercurial/manifest.py +++ b/mercurial/manifest.py @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog): if node == revlog.nullid: return self._newmanifest() # don't upset local cache if node in self._mancache: - return self._mancache[node] + cached = self._mancache[node] + if (isinstance(cached, manifestctx) or + isinstance(cached, treemanifestctx)): + cached = cached.read() + return cached if self._treeondisk: def gettext(): return self.revision(node)