Re: [PATCH] manifest: don't store None in fulltextcache

2016-10-18 Thread Martin von Zweigbergk via Mercurial-devel 
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

2016-10-18 Thread Durham Goode 


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

2016-10-18 Thread Pierre-Yves David 



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