Re: [PATCH 10 of 14] obsstore: pass a repository object for initialisation

[Augie Fackler]

> On Jul 14, 2017, at 14:27, Boris Feld  wrote:
> 
> On Fri, 2017-07-14 at 14:16 -0400, Augie Fackler wrote:
>>> On Jul 14, 2017, at 14:15, Boris Feld 
>>> wrote:
>>> 
>>> On Fri, 2017-07-14 at 14:11 -0400, Augie Fackler wrote:
 On Sun, Jul 09, 2017 at 07:55:22PM +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld 
> # Date 1495197862 -7200
> #  Fri May 19 14:44:22 2017 +0200
> # Node ID 985d753d4f5799f2a332140adedb06efd465d62b
> # Parent  63214f4d9a766761259b650539eede424413e6a2
> # EXP-Topic obs-cache
> obsstore: pass a repository object for initialisation
> 
> The cache will needs a repository object (to grab a 'vfs'), so
> we
> pass a repo object instead of just the 'svfs' and we grab the
> 'svfs'
> from there.
 
 I suspect I'll get to it, but why does this cache want to know
 about
 anything outside of svfs?
 
 I'm pretty uncomfortable (architecturally) with passing all of
 `self`
 into the cache layer.
>>> 
>>> The obscache need the vfs and not the svfs because caches lives in
>>> .hg
>>> and not in .hg/store.
>>> 
>>> Passing the whole repo and grabbing what we need seemed simpler.
>> 
>> It's simpler today, but more of a potential headache later if someone
>> decides to just retain the whole repo in the cache. I'd rather not do
>> it.
> 
> I understand the danger.
> 
> Should we pass vfs and svfs explicitly in the V2 then?

Yep. But let's wait until after the freeze, we've got enough other stuff 
pouring through the list I don't know that we have time to get through another 
round on this series.

(If you can do the first two patches and move branch cache to that 
infrastructure, feel free to send that though)

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 10 of 14] obsstore: pass a repository object for initialisation

[Boris Feld]

On Fri, 2017-07-14 at 14:16 -0400, Augie Fackler wrote:
> > On Jul 14, 2017, at 14:15, Boris Feld 
> > wrote:
> > 
> > On Fri, 2017-07-14 at 14:11 -0400, Augie Fackler wrote:
> > > On Sun, Jul 09, 2017 at 07:55:22PM +0200, Boris Feld wrote:
> > > > # HG changeset patch
> > > > # User Boris Feld 
> > > > # Date 1495197862 -7200
> > > > #  Fri May 19 14:44:22 2017 +0200
> > > > # Node ID 985d753d4f5799f2a332140adedb06efd465d62b
> > > > # Parent  63214f4d9a766761259b650539eede424413e6a2
> > > > # EXP-Topic obs-cache
> > > > obsstore: pass a repository object for initialisation
> > > > 
> > > > The cache will needs a repository object (to grab a 'vfs'), so
> > > > we
> > > > pass a repo object instead of just the 'svfs' and we grab the
> > > > 'svfs'
> > > > from there.
> > > 
> > > I suspect I'll get to it, but why does this cache want to know
> > > about
> > > anything outside of svfs?
> > > 
> > > I'm pretty uncomfortable (architecturally) with passing all of
> > > `self`
> > > into the cache layer.
> > 
> > The obscache need the vfs and not the svfs because caches lives in
> > .hg
> > and not in .hg/store.
> > 
> > Passing the whole repo and grabbing what we need seemed simpler.
> 
> It's simpler today, but more of a potential headache later if someone
> decides to just retain the whole repo in the cache. I'd rather not do
> it.

I understand the danger.

Should we pass vfs and svfs explicitly in the V2 then?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 10 of 14] obsstore: pass a repository object for initialisation

[Augie Fackler]

> On Jul 14, 2017, at 14:15, Boris Feld  wrote:
> 
> On Fri, 2017-07-14 at 14:11 -0400, Augie Fackler wrote:
>> On Sun, Jul 09, 2017 at 07:55:22PM +0200, Boris Feld wrote:
>>> # HG changeset patch
>>> # User Boris Feld 
>>> # Date 1495197862 -7200
>>> #  Fri May 19 14:44:22 2017 +0200
>>> # Node ID 985d753d4f5799f2a332140adedb06efd465d62b
>>> # Parent  63214f4d9a766761259b650539eede424413e6a2
>>> # EXP-Topic obs-cache
>>> obsstore: pass a repository object for initialisation
>>> 
>>> The cache will needs a repository object (to grab a 'vfs'), so we
>>> pass a repo object instead of just the 'svfs' and we grab the
>>> 'svfs'
>>> from there.
>> 
>> I suspect I'll get to it, but why does this cache want to know about
>> anything outside of svfs?
>> 
>> I'm pretty uncomfortable (architecturally) with passing all of `self`
>> into the cache layer.
> 
> The obscache need the vfs and not the svfs because caches lives in .hg
> and not in .hg/store.
> 
> Passing the whole repo and grabbing what we need seemed simpler.

It's simpler today, but more of a potential headache later if someone decides 
to just retain the whole repo in the cache. I'd rather not do it.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 10 of 14] obsstore: pass a repository object for initialisation

[Boris Feld]

On Fri, 2017-07-14 at 14:11 -0400, Augie Fackler wrote:
> On Sun, Jul 09, 2017 at 07:55:22PM +0200, Boris Feld wrote:
> > # HG changeset patch
> > # User Boris Feld 
> > # Date 1495197862 -7200
> > #  Fri May 19 14:44:22 2017 +0200
> > # Node ID 985d753d4f5799f2a332140adedb06efd465d62b
> > # Parent  63214f4d9a766761259b650539eede424413e6a2
> > # EXP-Topic obs-cache
> > obsstore: pass a repository object for initialisation
> > 
> > The cache will needs a repository object (to grab a 'vfs'), so we
> > pass a repo object instead of just the 'svfs' and we grab the
> > 'svfs'
> > from there.
> 
> I suspect I'll get to it, but why does this cache want to know about
> anything outside of svfs?
> 
> I'm pretty uncomfortable (architecturally) with passing all of `self`
> into the cache layer.

The obscache need the vfs and not the svfs because caches lives in .hg
and not in .hg/store.

Passing the whole repo and grabbing what we need seemed simpler.

> 
> > 
> > diff -r 63214f4d9a76 -r 985d753d4f57 contrib/perf.py
> > --- a/contrib/perf.py   Fri May 19 14:46:26 2017 +0200
> > +++ b/contrib/perf.py   Fri May 19 14:44:22 2017 +0200
> > @@ -1391,8 +1391,7 @@
> > 
> >  Result is the number of markers in the repo."""
> >  timer, fm = gettimer(ui)
> > -svfs = getsvfs(repo)
> > -timer(lambda: len(obsolete.obsstore(svfs)))
> > +timer(lambda: len(obsolete.obsstore(repo)))
> >  fm.end()
> > 
> >  @command('perflrucachedict', formatteropts +
> > diff -r 63214f4d9a76 -r 985d753d4f57 mercurial/obsolete.py
> > --- a/mercurial/obsolete.py Fri May 19 14:46:26 2017 +0200
> > +++ b/mercurial/obsolete.py Fri May 19 14:44:22 2017 +0200
> > @@ -519,10 +519,10 @@
> >  # 1024 byte should be about 10 markers in average
> >  _obskeyspan = 1024
> > 
> > -def __init__(self, svfs, defaultformat=_fm1version,
> > readonly=False):
> > +def __init__(self, repo, defaultformat=_fm1version,
> > readonly=False):
> >  # caches for various obsolescence related cache
> >  self.caches = {}
> > -self.svfs = svfs
> > +self.svfs = repo.svfs
> >  self._defaultformat = defaultformat
> >  self._readonly = readonly
> > 
> > @@ -799,7 +799,7 @@
> >  if defaultformat is not None:
> >  kwargs['defaultformat'] = defaultformat
> >  readonly = not isenabled(repo, createmarkersopt)
> > -store = obsstore(repo.svfs, readonly=readonly, **kwargs)
> > +store = obsstore(repo, readonly=readonly, **kwargs)
> >  if store and readonly:
> >  ui.warn(_('obsolete feature not enabled but %i markers
> > found!\n')
> >  % len(list(store)))
> > ___
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 10 of 14] obsstore: pass a repository object for initialisation

[Augie Fackler]

On Sun, Jul 09, 2017 at 07:55:22PM +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld 
> # Date 1495197862 -7200
> #  Fri May 19 14:44:22 2017 +0200
> # Node ID 985d753d4f5799f2a332140adedb06efd465d62b
> # Parent  63214f4d9a766761259b650539eede424413e6a2
> # EXP-Topic obs-cache
> obsstore: pass a repository object for initialisation
>
> The cache will needs a repository object (to grab a 'vfs'), so we
> pass a repo object instead of just the 'svfs' and we grab the 'svfs'
> from there.

I suspect I'll get to it, but why does this cache want to know about
anything outside of svfs?

I'm pretty uncomfortable (architecturally) with passing all of `self`
into the cache layer.

>
> diff -r 63214f4d9a76 -r 985d753d4f57 contrib/perf.py
> --- a/contrib/perf.py Fri May 19 14:46:26 2017 +0200
> +++ b/contrib/perf.py Fri May 19 14:44:22 2017 +0200
> @@ -1391,8 +1391,7 @@
>
>  Result is the number of markers in the repo."""
>  timer, fm = gettimer(ui)
> -svfs = getsvfs(repo)
> -timer(lambda: len(obsolete.obsstore(svfs)))
> +timer(lambda: len(obsolete.obsstore(repo)))
>  fm.end()
>
>  @command('perflrucachedict', formatteropts +
> diff -r 63214f4d9a76 -r 985d753d4f57 mercurial/obsolete.py
> --- a/mercurial/obsolete.py   Fri May 19 14:46:26 2017 +0200
> +++ b/mercurial/obsolete.py   Fri May 19 14:44:22 2017 +0200
> @@ -519,10 +519,10 @@
>  # 1024 byte should be about 10 markers in average
>  _obskeyspan = 1024
>
> -def __init__(self, svfs, defaultformat=_fm1version, readonly=False):
> +def __init__(self, repo, defaultformat=_fm1version, readonly=False):
>  # caches for various obsolescence related cache
>  self.caches = {}
> -self.svfs = svfs
> +self.svfs = repo.svfs
>  self._defaultformat = defaultformat
>  self._readonly = readonly
>
> @@ -799,7 +799,7 @@
>  if defaultformat is not None:
>  kwargs['defaultformat'] = defaultformat
>  readonly = not isenabled(repo, createmarkersopt)
> -store = obsstore(repo.svfs, readonly=readonly, **kwargs)
> +store = obsstore(repo, readonly=readonly, **kwargs)
>  if store and readonly:
>  ui.warn(_('obsolete feature not enabled but %i markers found!\n')
>  % len(list(store)))
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel