Re: [PATCH] manifest: don't store None in fulltextcache
On Tue, Oct 18, 2016 at 1:09 PM Durham Goode wrote: > > > On 10/17/16, 10:58 PM, "Martin von Zweigbergk" > wrote: > > ># HG changeset patch > ># User Martin von Zweigbergk > ># Date 1476769882 25200 > ># Mon Oct 17 22:51:22 2016 -0700 > ># Node ID d2c313417026d76cb19534277df3f3a8b6b22425 > ># Parent 87a7c0d403ff29dcae2a41e0516c75bbd9f6a5a8 > >manifest: don't store None in fulltextcache > > > >When we read a value from fulltextcache, we expect it to be an array, > >so we should not store None in it. Found while working on narrowhg. > > > >diff -r 87a7c0d403ff -r d2c313417026 mercurial/manifest.py > >--- a/mercurial/manifest.pyTue Oct 18 02:09:08 2016 +0200 > >+++ b/mercurial/manifest.pyMon Oct 17 22:51:22 2016 -0700 > >@@ -1210,7 +1210,8 @@ > > n = self.addrevision(text, transaction, link, p1, p2) > > arraytext = array.array('c', text) > > > >-self.fulltextcache[n] = arraytext > >+if arraytext is not None: > >+self.fulltextcache[n] = arraytext > > > > return n > > > >@@ -1506,7 +1507,8 @@ > > m = self._newmanifest(text) > > arraytext = array.array('c', text) > > self._mancache[node] = m > >-self.fulltextcache[node] = arraytext > >+if arraytext is not None: > >+self.fulltextcache[node] = arraytext > > return m > > > > def readshallow(self, node): > > LGTM. Is there a tree code path that hits this that is not covered by > tests? > > Yes, what our tests in narrowhg triggered was a line in bundlemanifest: self.fulltextcache[node].tostring(). The test case was trying to pull from a bundle. We do have a test case for that in test-treemanifest.t. I don't know what the difference is. Perhaps something means it gets cached in our narrowhg test case, but not in the test-treemanifest.t one. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] manifest: don't store None in fulltextcache
On 10/17/16, 10:58 PM, "Martin von Zweigbergk" wrote: ># HG changeset patch ># User Martin von Zweigbergk ># Date 1476769882 25200 ># Mon Oct 17 22:51:22 2016 -0700 ># Node ID d2c313417026d76cb19534277df3f3a8b6b22425 ># Parent 87a7c0d403ff29dcae2a41e0516c75bbd9f6a5a8 >manifest: don't store None in fulltextcache > >When we read a value from fulltextcache, we expect it to be an array, >so we should not store None in it. Found while working on narrowhg. > >diff -r 87a7c0d403ff -r d2c313417026 mercurial/manifest.py >--- a/mercurial/manifest.pyTue Oct 18 02:09:08 2016 +0200 >+++ b/mercurial/manifest.pyMon Oct 17 22:51:22 2016 -0700 >@@ -1210,7 +1210,8 @@ > n = self.addrevision(text, transaction, link, p1, p2) > arraytext = array.array('c', text) > >-self.fulltextcache[n] = arraytext >+if arraytext is not None: >+self.fulltextcache[n] = arraytext > > return n > >@@ -1506,7 +1507,8 @@ > m = self._newmanifest(text) > arraytext = array.array('c', text) > self._mancache[node] = m >-self.fulltextcache[node] = arraytext >+if arraytext is not None: >+self.fulltextcache[node] = arraytext > return m > > def readshallow(self, node): LGTM. Is there a tree code path that hits this that is not covered by tests? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] manifest: don't store None in fulltextcache
On 10/18/2016 07:58 AM, Martin von Zweigbergk via Mercurial-devel wrote: # HG changeset patch # User Martin von Zweigbergk # Date 1476769882 25200 # Mon Oct 17 22:51:22 2016 -0700 # Node ID d2c313417026d76cb19534277df3f3a8b6b22425 # Parent 87a7c0d403ff29dcae2a41e0516c75bbd9f6a5a8 manifest: don't store None in fulltextcache Apparently it was still the october 17th when you mailed that ;-) I've pushed it. -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel