Patchwork repoview: introduce a `experimental.extra-filter-revs` config

login
register
mail settings
Submitter Pierre-Yves David
Date April 16, 2019, 10:41 p.m.
Message ID <86eac5d2d7a1386dca59.1555454509@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/39673/
State Superseded
Headers show

Comments

Pierre-Yves David - April 16, 2019, 10:41 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1554565579 -7200
#      Sat Apr 06 17:46:19 2019 +0200
# Node ID 86eac5d2d7a1386dca5970126506d07201058200
# Parent  eebf78724d94649de84f921ff5bd39cbc0f48cb6
# EXP-Topic repoview
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 86eac5d2d7a1
repoview: introduce a `experimental.extra-filter-revs` config

The option define revisions to additionally filter out of all repository "view".
The end goal is to provide and easy to way to serve multiple subset of the same
repository using multiple "shares".

The simplest use case of this feature is to have one view serving the public
changesets and one view also serving the draft. This is currently achievable
using the new `server.view` option introduced recently by Joerg Sonnenberger.
However, more advanced use cases need more advanced definitions. For example
some needs a view dedicated to some release branches, or view that hides
security fixes to be released. Joerg Sonnenberger and I discussed this topic at
the recent mini-sprint and the both of us have seen real life use cases for
this. (This series got written during the same mini-sprint).

The feature is fully functional, and use similar cache-fallback mechanism to
ensure decent performance. However,there remaining room to ensure each share
caches and hooks collaborate with each others. This will come at a later time
once users start to actually test this feature on real usecase.
Gregory Szorc - April 17, 2019, 11:22 a.m.
On Tue, Apr 16, 2019 at 4:54 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1554565579 -7200
> #      Sat Apr 06 17:46:19 2019 +0200
> # Node ID 86eac5d2d7a1386dca5970126506d07201058200
> # Parent  eebf78724d94649de84f921ff5bd39cbc0f48cb6
> # EXP-Topic repoview
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> 86eac5d2d7a1
> repoview: introduce a `experimental.extra-filter-revs` config
>
> The option define revisions to additionally filter out of all repository
> "view".
> The end goal is to provide and easy to way to serve multiple subset of the
> same
> repository using multiple "shares".
>
> The simplest use case of this feature is to have one view serving the
> public
> changesets and one view also serving the draft. This is currently
> achievable
> using the new `server.view` option introduced recently by Joerg
> Sonnenberger.
> However, more advanced use cases need more advanced definitions. For
> example
> some needs a view dedicated to some release branches, or view that hides
> security fixes to be released. Joerg Sonnenberger and I discussed this
> topic at
> the recent mini-sprint and the both of us have seen real life use cases for
> this. (This series got written during the same mini-sprint).
>
> The feature is fully functional, and use similar cache-fallback mechanism
> to
> ensure decent performance. However,there remaining room to ensure each
> share
> caches and hooks collaborate with each others. This will come at a later
> time
> once users start to actually test this feature on real usecase.
>
> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> --- a/mercurial/configitems.py
> +++ b/mercurial/configitems.py
> @@ -529,6 +529,13 @@ coreconfigitem('experimental', 'evolutio
>  coreconfigitem('experimental', 'evolution.track-operation',
>      default=True,
>  )
> +# repo-level config to exclude a revset visibility
> +#
> +# The target use case is to use `share` to expose different subset of the
> same
> +# repository, especially server side. See also `server.view`.
> +coreconfigitem('experimental', 'extra-filter-revs',
> +    default=None,
> +)
>  coreconfigitem('experimental', 'maxdeltachainspan',
>      default=-1,
>  )
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1050,6 +1050,8 @@ class localrepository(object):
>          # Signature to cached matcher instance.
>          self._sparsematchercache = {}
>
> +        self._extrafilterid = repoview.extrafilter(ui)
> +
>      def _getvfsward(self, origfunc):
>          """build a ward for self.vfs"""
>          rref = weakref.ref(self)
> @@ -1197,6 +1199,9 @@ class localrepository(object):
>
>          In other word, there is always only one level of `repoview`
> "filtering".
>          """
> +        if self._extrafilterid is not None:
> +            name = name + '%'  + self._extrafilterid
> +
>          cls = repoview.newtype(self.unfiltered().__class__)
>          return cls(self, name, visibilityexceptions)
>
> diff --git a/mercurial/repoview.py b/mercurial/repoview.py
> --- a/mercurial/repoview.py
> +++ b/mercurial/repoview.py
> @@ -17,6 +17,10 @@ from . import (
>      phases,
>      pycompat,
>      tags as tagsmod,
> +    util,
> +)
> +from .utils import (
> +    repoviewutil,
>  )
>
>  def hideablerevs(repo):
> @@ -154,6 +158,35 @@ filtertable = {'visible': computehidden,
>                 'immutable':  computemutable,
>                 'base':  computeimpactable}
>
> +_basefiltername = list(filtertable)
> +
> +def extrafilter(ui):
> +    """initialize extra filter and return its id
> +
> +    If extra filtering is configured, we make sure the associated
> filtered view
> +    are declared and return the associated id.
> +    """
> +    frevs = ui.config('experimental', 'extra-filter-revs')
> +    if frevs is None:
> +        return None
> +
> +    fid = util.DIGESTS['sha512'](frevs).hexdigest()
>

Why are we computing a hash here? Why not store the raw query? Is it
because the filter names can be persisted to disk in the cache and must
therefore be valid filenames? Is % an allowed character on all filesystems?
Maybe '-' would be safer?


> +
> +    combine = lambda fname: fname + '%' + fid
> +
> +    subsettable = repoviewutil.subsettable
> +
> +    if combine('base') not in filtertable:
> +        for name in _basefiltername:
> +            def extrafilteredrevs(repo, *args, **kwargs):
> +                baserevs = filtertable[name](repo, *args, **kwargs)
> +                extrarevs = frozenset(repo.revs(frevs))
> +                return baserevs | extrarevs
> +            filtertable[combine(name)] = extrafilteredrevs
> +            if name in subsettable:
> +                subsettable[combine(name)] = combine(subsettable[name])
> +    return fid
> +
>  def filterrevs(repo, filtername, visibilityexceptions=None):
>      """returns set of filtered revision for this filter name
>
> diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
> --- a/mercurial/statichttprepo.py
> +++ b/mercurial/statichttprepo.py
> @@ -155,6 +155,7 @@ class statichttprepository(localrepo.loc
>
>          self.names = namespaces.namespaces()
>          self.filtername = None
> +        self._extrafilterid = None
>
>          try:
>              requirements = set(self.vfs.read(b'requires').splitlines())
> diff --git a/tests/test-server-view.t b/tests/test-server-view.t
> --- a/tests/test-server-view.t
> +++ b/tests/test-server-view.t
> @@ -34,5 +34,21 @@
>    date:        Thu Jan 01 00:00:00 1970 +0000
>    summary:     r0
>
> +
> +Check same result using `experimental.extra-filter-revs`
> +
> +  $ hg -R test --config experimental.extra-filter-revs='not public()'
> serve -p $HGPORT1 -d --pid-file=hg2.pid -E errors.log
> +  $ cat hg2.pid >> $DAEMON_PIDS
> +  $ hg -R test2 incoming http://foo:xyzzy@localhost:$HGPORT1/
> +  comparing with http://foo:***@localhost:$HGPORT1/
> +  changeset:   0:1ea73414a91b
> +  tag:         tip
> +  user:        debugbuilddag
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  summary:     r0
> +
> +
> +cleanup
> +
>    $ cat errors.log
>    $ killdaemons.py
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - April 17, 2019, 11:34 a.m.
On 4/17/19 1:22 PM, Gregory Szorc wrote:
> On Tue, Apr 16, 2019 at 4:54 PM Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> 
> wrote:
> 
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@octobus.net
>     <mailto:pierre-yves.david@octobus.net>>
>     # Date 1554565579 -7200
>     #      Sat Apr 06 17:46:19 2019 +0200
>     # Node ID 86eac5d2d7a1386dca5970126506d07201058200
>     # Parent  eebf78724d94649de84f921ff5bd39cbc0f48cb6
>     # EXP-Topic repoview
>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>     #              hg pull
>     https://bitbucket.org/octobus/mercurial-devel/ -r 86eac5d2d7a1
>     repoview: introduce a `experimental.extra-filter-revs` config
> 
>     The option define revisions to additionally filter out of all
>     repository "view".
>     The end goal is to provide and easy to way to serve multiple subset
>     of the same
>     repository using multiple "shares".
> 
>     The simplest use case of this feature is to have one view serving
>     the public
>     changesets and one view also serving the draft. This is currently
>     achievable
>     using the new `server.view` option introduced recently by Joerg
>     Sonnenberger.
>     However, more advanced use cases need more advanced definitions. For
>     example
>     some needs a view dedicated to some release branches, or view that hides
>     security fixes to be released. Joerg Sonnenberger and I discussed
>     this topic at
>     the recent mini-sprint and the both of us have seen real life use
>     cases for
>     this. (This series got written during the same mini-sprint).
> 
>     The feature is fully functional, and use similar cache-fallback
>     mechanism to
>     ensure decent performance. However,there remaining room to ensure
>     each share
>     caches and hooks collaborate with each others. This will come at a
>     later time
>     once users start to actually test this feature on real usecase.
> 
>     diff --git a/mercurial/configitems.py b/mercurial/configitems.py
>     --- a/mercurial/configitems.py
>     +++ b/mercurial/configitems.py
>     @@ -529,6 +529,13 @@ coreconfigitem('experimental', 'evolutio
>       coreconfigitem('experimental', 'evolution.track-operation',
>           default=True,
>       )
>     +# repo-level config to exclude a revset visibility
>     +#
>     +# The target use case is to use `share` to expose different subset
>     of the same
>     +# repository, especially server side. See also `server.view`.
>     +coreconfigitem('experimental', 'extra-filter-revs',
>     +    default=None,
>     +)
>       coreconfigitem('experimental', 'maxdeltachainspan',
>           default=-1,
>       )
>     diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>     --- a/mercurial/localrepo.py
>     +++ b/mercurial/localrepo.py
>     @@ -1050,6 +1050,8 @@ class localrepository(object):
>               # Signature to cached matcher instance.
>               self._sparsematchercache = {}
> 
>     +        self._extrafilterid = repoview.extrafilter(ui)
>     +
>           def _getvfsward(self, origfunc):
>               """build a ward for self.vfs"""
>               rref = weakref.ref(self)
>     @@ -1197,6 +1199,9 @@ class localrepository(object):
> 
>               In other word, there is always only one level of
>     `repoview` "filtering".
>               """
>     +        if self._extrafilterid is not None:
>     +            name = name + '%'  + self._extrafilterid
>     +
>               cls = repoview.newtype(self.unfiltered().__class__)
>               return cls(self, name, visibilityexceptions)
> 
>     diff --git a/mercurial/repoview.py b/mercurial/repoview.py
>     --- a/mercurial/repoview.py
>     +++ b/mercurial/repoview.py
>     @@ -17,6 +17,10 @@ from . import (
>           phases,
>           pycompat,
>           tags as tagsmod,
>     +    util,
>     +)
>     +from .utils import (
>     +    repoviewutil,
>       )
> 
>       def hideablerevs(repo):
>     @@ -154,6 +158,35 @@ filtertable = {'visible': computehidden,
>                      'immutable':  computemutable,
>                      'base':  computeimpactable}
> 
>     +_basefiltername = list(filtertable)
>     +
>     +def extrafilter(ui):
>     +    """initialize extra filter and return its id
>     +
>     +    If extra filtering is configured, we make sure the associated
>     filtered view
>     +    are declared and return the associated id.
>     +    """
>     +    frevs = ui.config('experimental', 'extra-filter-revs')
>     +    if frevs is None:
>     +        return None
>     +
>     +    fid = util.DIGESTS['sha512'](frevs).hexdigest()
> 
> 
> Why are we computing a hash here? Why not store the raw query? Is it 
> because the filter names can be persisted to disk in the cache and must 
> therefore be valid filenames?

Since query can be arbitrarily long, complex with any char. It seems 
simple to keep the ID simple. Especially because we persist it on disk

> Is % an allowed character on all 
> filesystems? Maybe '-' would be safer?

% is a valid filename character on all system but RT-11. I think we are 
safe. I would rather keep the "-" char for something more meaningful 
(like what we are doing for ".hidden").

In the final design of this feature, we might end up with each query 
having an explicitly declared name.

> 
>     +
>     +    combine = lambda fname: fname + '%' + fid
>     +
>     +    subsettable = repoviewutil.subsettable
>     +
>     +    if combine('base') not in filtertable:
>     +        for name in _basefiltername:
>     +            def extrafilteredrevs(repo, *args, **kwargs):
>     +                baserevs = filtertable[name](repo, *args, **kwargs)
>     +                extrarevs = frozenset(repo.revs(frevs))
>     +                return baserevs | extrarevs
>     +            filtertable[combine(name)] = extrafilteredrevs
>     +            if name in subsettable:
>     +                subsettable[combine(name)] = combine(subsettable[name])
>     +    return fid
>     +
>       def filterrevs(repo, filtername, visibilityexceptions=None):
>           """returns set of filtered revision for this filter name
> 
>     diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
>     --- a/mercurial/statichttprepo.py
>     +++ b/mercurial/statichttprepo.py
>     @@ -155,6 +155,7 @@ class statichttprepository(localrepo.loc
> 
>               self.names = namespaces.namespaces()
>               self.filtername = None
>     +        self._extrafilterid = None
> 
>               try:
>                   requirements =
>     set(self.vfs.read(b'requires').splitlines())
>     diff --git a/tests/test-server-view.t b/tests/test-server-view.t
>     --- a/tests/test-server-view.t
>     +++ b/tests/test-server-view.t
>     @@ -34,5 +34,21 @@
>         date:        Thu Jan 01 00:00:00 1970 +0000
>         summary:     r0
> 
>     +
>     +Check same result using `experimental.extra-filter-revs`
>     +
>     +  $ hg -R test --config experimental.extra-filter-revs='not
>     public()' serve -p $HGPORT1 -d --pid-file=hg2.pid -E errors.log
>     +  $ cat hg2.pid >> $DAEMON_PIDS
>     +  $ hg -R test2 incoming http://foo:xyzzy@localhost:$HGPORT1/
>     +  comparing with http://foo:***@localhost:$HGPORT1/
>     +  changeset:   0:1ea73414a91b
>     +  tag:         tip
>     +  user:        debugbuilddag
>     +  date:        Thu Jan 01 00:00:00 1970 +0000
>     +  summary:     r0
>     +
>     +
>     +cleanup
>     +
>         $ cat errors.log
>         $ killdaemons.py
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@mercurial-scm.org
>     <mailto:Mercurial-devel@mercurial-scm.org>
>     https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Gregory Szorc - April 17, 2019, 12:08 p.m.
> On Apr 17, 2019, at 13:34, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
>> On 4/17/19 1:22 PM, Gregory Szorc wrote:
>> On Tue, Apr 16, 2019 at 4:54 PM Pierre-Yves David <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>>    # HG changeset patch
>>    # User Pierre-Yves David <pierre-yves.david@octobus.net
>>    <mailto:pierre-yves.david@octobus.net>>
>>    # Date 1554565579 -7200
>>    #      Sat Apr 06 17:46:19 2019 +0200
>>    # Node ID 86eac5d2d7a1386dca5970126506d07201058200
>>    # Parent  eebf78724d94649de84f921ff5bd39cbc0f48cb6
>>    # EXP-Topic repoview
>>    # Available At https://bitbucket.org/octobus/mercurial-devel/
>>    #              hg pull
>>    https://bitbucket.org/octobus/mercurial-devel/ -r 86eac5d2d7a1
>>    repoview: introduce a `experimental.extra-filter-revs` config
>>    The option define revisions to additionally filter out of all
>>    repository "view".
>>    The end goal is to provide and easy to way to serve multiple subset
>>    of the same
>>    repository using multiple "shares".
>>    The simplest use case of this feature is to have one view serving
>>    the public
>>    changesets and one view also serving the draft. This is currently
>>    achievable
>>    using the new `server.view` option introduced recently by Joerg
>>    Sonnenberger.
>>    However, more advanced use cases need more advanced definitions. For
>>    example
>>    some needs a view dedicated to some release branches, or view that hides
>>    security fixes to be released. Joerg Sonnenberger and I discussed
>>    this topic at
>>    the recent mini-sprint and the both of us have seen real life use
>>    cases for
>>    this. (This series got written during the same mini-sprint).
>>    The feature is fully functional, and use similar cache-fallback
>>    mechanism to
>>    ensure decent performance. However,there remaining room to ensure
>>    each share
>>    caches and hooks collaborate with each others. This will come at a
>>    later time
>>    once users start to actually test this feature on real usecase.
>>    diff --git a/mercurial/configitems.py b/mercurial/configitems.py
>>    --- a/mercurial/configitems.py
>>    +++ b/mercurial/configitems.py
>>    @@ -529,6 +529,13 @@ coreconfigitem('experimental', 'evolutio
>>      coreconfigitem('experimental', 'evolution.track-operation',
>>          default=True,
>>      )
>>    +# repo-level config to exclude a revset visibility
>>    +#
>>    +# The target use case is to use `share` to expose different subset
>>    of the same
>>    +# repository, especially server side. See also `server.view`.
>>    +coreconfigitem('experimental', 'extra-filter-revs',
>>    +    default=None,
>>    +)
>>      coreconfigitem('experimental', 'maxdeltachainspan',
>>          default=-1,
>>      )
>>    diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>>    --- a/mercurial/localrepo.py
>>    +++ b/mercurial/localrepo.py
>>    @@ -1050,6 +1050,8 @@ class localrepository(object):
>>              # Signature to cached matcher instance.
>>              self._sparsematchercache = {}
>>    +        self._extrafilterid = repoview.extrafilter(ui)
>>    +
>>          def _getvfsward(self, origfunc):
>>              """build a ward for self.vfs"""
>>              rref = weakref.ref(self)
>>    @@ -1197,6 +1199,9 @@ class localrepository(object):
>>              In other word, there is always only one level of
>>    `repoview` "filtering".
>>              """
>>    +        if self._extrafilterid is not None:
>>    +            name = name + '%'  + self._extrafilterid
>>    +
>>              cls = repoview.newtype(self.unfiltered().__class__)
>>              return cls(self, name, visibilityexceptions)
>>    diff --git a/mercurial/repoview.py b/mercurial/repoview.py
>>    --- a/mercurial/repoview.py
>>    +++ b/mercurial/repoview.py
>>    @@ -17,6 +17,10 @@ from . import (
>>          phases,
>>          pycompat,
>>          tags as tagsmod,
>>    +    util,
>>    +)
>>    +from .utils import (
>>    +    repoviewutil,
>>      )
>>      def hideablerevs(repo):
>>    @@ -154,6 +158,35 @@ filtertable = {'visible': computehidden,
>>                     'immutable':  computemutable,
>>                     'base':  computeimpactable}
>>    +_basefiltername = list(filtertable)
>>    +
>>    +def extrafilter(ui):
>>    +    """initialize extra filter and return its id
>>    +
>>    +    If extra filtering is configured, we make sure the associated
>>    filtered view
>>    +    are declared and return the associated id.
>>    +    """
>>    +    frevs = ui.config('experimental', 'extra-filter-revs')
>>    +    if frevs is None:
>>    +        return None
>>    +
>>    +    fid = util.DIGESTS['sha512'](frevs).hexdigest()
>> Why are we computing a hash here? Why not store the raw query? Is it because the filter names can be persisted to disk in the cache and must therefore be valid filenames?
> 
> Since query can be arbitrarily long, complex with any char. It seems simple to keep the ID simple. Especially because we persist it on disk
> 
>> Is % an allowed character on all filesystems? Maybe '-' would be safer?
> 
> % is a valid filename character on all system but RT-11. I think we are safe. I would rather keep the "-" char for something more meaningful (like what we are doing for ".hidden").
> 
> In the final design of this feature, we might end up with each query having an explicitly declared name.

Fair enough.

Could you please send a v2 using sha-256 (sha-512 is overkill) and with the necessary pycompat bits to make the hexdigest() bytes so things work on Python 3?

> 
>>    +
>>    +    combine = lambda fname: fname + '%' + fid
>>    +
>>    +    subsettable = repoviewutil.subsettable
>>    +
>>    +    if combine('base') not in filtertable:
>>    +        for name in _basefiltername:
>>    +            def extrafilteredrevs(repo, *args, **kwargs):
>>    +                baserevs = filtertable[name](repo, *args, **kwargs)
>>    +                extrarevs = frozenset(repo.revs(frevs))
>>    +                return baserevs | extrarevs
>>    +            filtertable[combine(name)] = extrafilteredrevs
>>    +            if name in subsettable:
>>    +                subsettable[combine(name)] = combine(subsettable[name])
>>    +    return fid
>>    +
>>      def filterrevs(repo, filtername, visibilityexceptions=None):
>>          """returns set of filtered revision for this filter name
>>    diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
>>    --- a/mercurial/statichttprepo.py
>>    +++ b/mercurial/statichttprepo.py
>>    @@ -155,6 +155,7 @@ class statichttprepository(localrepo.loc
>>              self.names = namespaces.namespaces()
>>              self.filtername = None
>>    +        self._extrafilterid = None
>>              try:
>>                  requirements =
>>    set(self.vfs.read(b'requires').splitlines())
>>    diff --git a/tests/test-server-view.t b/tests/test-server-view.t
>>    --- a/tests/test-server-view.t
>>    +++ b/tests/test-server-view.t
>>    @@ -34,5 +34,21 @@
>>        date:        Thu Jan 01 00:00:00 1970 +0000
>>        summary:     r0
>>    +
>>    +Check same result using `experimental.extra-filter-revs`
>>    +
>>    +  $ hg -R test --config experimental.extra-filter-revs='not
>>    public()' serve -p $HGPORT1 -d --pid-file=hg2.pid -E errors.log
>>    +  $ cat hg2.pid >> $DAEMON_PIDS
>>    +  $ hg -R test2 incoming http://foo:xyzzy@localhost:$HGPORT1/
>>    +  comparing with http://foo:***@localhost:$HGPORT1/
>>    +  changeset:   0:1ea73414a91b
>>    +  tag:         tip
>>    +  user:        debugbuilddag
>>    +  date:        Thu Jan 01 00:00:00 1970 +0000
>>    +  summary:     r0
>>    +

Could we also please get a test that the on-disk cache file(s) are crested as expected?

Regarding those on-disk cache files, are there any cache invalidation issues since revsets are dynamic? (I think we’re OK but I want to make sure we have thought things through.)

>>    +
>>    +cleanup
>>    +
>>        $ cat errors.log
>>        $ killdaemons.py
>>    _______________________________________________
>>    Mercurial-devel mailing list
>>    Mercurial-devel@mercurial-scm.org
>>    <mailto:Mercurial-devel@mercurial-scm.org>
>>    https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> 
> -- 
> Pierre-Yves David
Pierre-Yves David - April 17, 2019, 2:09 p.m.
On 4/17/19 2:08 PM, Gregory Szorc wrote:
> 
> 
>> On Apr 17, 2019, at 13:34, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>>
>>
>>
>>> On 4/17/19 1:22 PM, Gregory Szorc wrote:
>>> On Tue, Apr 16, 2019 at 4:54 PM Pierre-Yves David <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>>>     # HG changeset patch
>>>     # User Pierre-Yves David <pierre-yves.david@octobus.net
>>>     <mailto:pierre-yves.david@octobus.net>>
>>>     # Date 1554565579 -7200
>>>     #      Sat Apr 06 17:46:19 2019 +0200
>>>     # Node ID 86eac5d2d7a1386dca5970126506d07201058200
>>>     # Parent  eebf78724d94649de84f921ff5bd39cbc0f48cb6
>>>     # EXP-Topic repoview
>>>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>>>     #              hg pull
>>>     https://bitbucket.org/octobus/mercurial-devel/ -r 86eac5d2d7a1
>>>     repoview: introduce a `experimental.extra-filter-revs` config
>>>     The option define revisions to additionally filter out of all
>>>     repository "view".
>>>     The end goal is to provide and easy to way to serve multiple subset
>>>     of the same
>>>     repository using multiple "shares".
>>>     The simplest use case of this feature is to have one view serving
>>>     the public
>>>     changesets and one view also serving the draft. This is currently
>>>     achievable
>>>     using the new `server.view` option introduced recently by Joerg
>>>     Sonnenberger.
>>>     However, more advanced use cases need more advanced definitions. For
>>>     example
>>>     some needs a view dedicated to some release branches, or view that hides
>>>     security fixes to be released. Joerg Sonnenberger and I discussed
>>>     this topic at
>>>     the recent mini-sprint and the both of us have seen real life use
>>>     cases for
>>>     this. (This series got written during the same mini-sprint).
>>>     The feature is fully functional, and use similar cache-fallback
>>>     mechanism to
>>>     ensure decent performance. However,there remaining room to ensure
>>>     each share
>>>     caches and hooks collaborate with each others. This will come at a
>>>     later time
>>>     once users start to actually test this feature on real usecase.
>>>     diff --git a/mercurial/configitems.py b/mercurial/configitems.py
>>>     --- a/mercurial/configitems.py
>>>     +++ b/mercurial/configitems.py
>>>     @@ -529,6 +529,13 @@ coreconfigitem('experimental', 'evolutio
>>>       coreconfigitem('experimental', 'evolution.track-operation',
>>>           default=True,
>>>       )
>>>     +# repo-level config to exclude a revset visibility
>>>     +#
>>>     +# The target use case is to use `share` to expose different subset
>>>     of the same
>>>     +# repository, especially server side. See also `server.view`.
>>>     +coreconfigitem('experimental', 'extra-filter-revs',
>>>     +    default=None,
>>>     +)
>>>       coreconfigitem('experimental', 'maxdeltachainspan',
>>>           default=-1,
>>>       )
>>>     diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>>>     --- a/mercurial/localrepo.py
>>>     +++ b/mercurial/localrepo.py
>>>     @@ -1050,6 +1050,8 @@ class localrepository(object):
>>>               # Signature to cached matcher instance.
>>>               self._sparsematchercache = {}
>>>     +        self._extrafilterid = repoview.extrafilter(ui)
>>>     +
>>>           def _getvfsward(self, origfunc):
>>>               """build a ward for self.vfs"""
>>>               rref = weakref.ref(self)
>>>     @@ -1197,6 +1199,9 @@ class localrepository(object):
>>>               In other word, there is always only one level of
>>>     `repoview` "filtering".
>>>               """
>>>     +        if self._extrafilterid is not None:
>>>     +            name = name + '%'  + self._extrafilterid
>>>     +
>>>               cls = repoview.newtype(self.unfiltered().__class__)
>>>               return cls(self, name, visibilityexceptions)
>>>     diff --git a/mercurial/repoview.py b/mercurial/repoview.py
>>>     --- a/mercurial/repoview.py
>>>     +++ b/mercurial/repoview.py
>>>     @@ -17,6 +17,10 @@ from . import (
>>>           phases,
>>>           pycompat,
>>>           tags as tagsmod,
>>>     +    util,
>>>     +)
>>>     +from .utils import (
>>>     +    repoviewutil,
>>>       )
>>>       def hideablerevs(repo):
>>>     @@ -154,6 +158,35 @@ filtertable = {'visible': computehidden,
>>>                      'immutable':  computemutable,
>>>                      'base':  computeimpactable}
>>>     +_basefiltername = list(filtertable)
>>>     +
>>>     +def extrafilter(ui):
>>>     +    """initialize extra filter and return its id
>>>     +
>>>     +    If extra filtering is configured, we make sure the associated
>>>     filtered view
>>>     +    are declared and return the associated id.
>>>     +    """
>>>     +    frevs = ui.config('experimental', 'extra-filter-revs')
>>>     +    if frevs is None:
>>>     +        return None
>>>     +
>>>     +    fid = util.DIGESTS['sha512'](frevs).hexdigest()
>>> Why are we computing a hash here? Why not store the raw query? Is it because the filter names can be persisted to disk in the cache and must therefore be valid filenames?
>>
>> Since query can be arbitrarily long, complex with any char. It seems simple to keep the ID simple. Especially because we persist it on disk
>>
>>> Is % an allowed character on all filesystems? Maybe '-' would be safer?
>>
>> % is a valid filename character on all system but RT-11. I think we are safe. I would rather keep the "-" char for something more meaningful (like what we are doing for ".hidden").
>>
>> In the final design of this feature, we might end up with each query having an explicitly declared name.
> 
> Fair enough.
> 
> Could you please send a v2 using sha-256 (sha-512 is overkill) and with the necessary pycompat bits to make the hexdigest() bytes so things work on Python 3?

I am assuming you mean sha1 since we don't have sha-236 digest in the 
standard mercurial.util.DIGESTS dict.

(tests running),

Cheers,
Pierre-Yves David - April 17, 2019, 2:13 p.m.
On 4/17/19 2:08 PM, Gregory Szorc wrote:
> 
> 
>> On Apr 17, 2019, at 13:34, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>>
>>
>>
>>> On 4/17/19 1:22 PM, Gregory Szorc wrote:
>>>     diff --git a/tests/test-server-view.t b/tests/test-server-view.t
>>>     --- a/tests/test-server-view.t
>>>     +++ b/tests/test-server-view.t
>>>     @@ -34,5 +34,21 @@
>>>         date:        Thu Jan 01 00:00:00 1970 +0000
>>>         summary:     r0
>>>     +
>>>     +Check same result using `experimental.extra-filter-revs`
>>>     +
>>>     +  $ hg -R test --config experimental.extra-filter-revs='not
>>>     public()' serve -p $HGPORT1 -d --pid-file=hg2.pid -E errors.log
>>>     +  $ cat hg2.pid >> $DAEMON_PIDS
>>>     +  $ hg -R test2 incoming http://foo:xyzzy@localhost:$HGPORT1/
>>>     +  comparing with http://foo:***@localhost:$HGPORT1/
>>>     +  changeset:   0:1ea73414a91b
>>>     +  tag:         tip
>>>     +  user:        debugbuilddag
>>>     +  date:        Thu Jan 01 00:00:00 1970 +0000
>>>     +  summary:     r0
>>>     +
> 
> Could we also please get a test that the on-disk cache file(s) are crested as expected?
> 
> Regarding those on-disk cache files, are there any cache invalidation issues since revsets are dynamic? (I think we’re OK but I want to make sure we have thought things through.)

The cache validated themselves based on the skipped revision, so they 
will not use an invalid caches whichever it comes from (disk, subset, etc…).

Patch

diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -529,6 +529,13 @@  coreconfigitem('experimental', 'evolutio
 coreconfigitem('experimental', 'evolution.track-operation',
     default=True,
 )
+# repo-level config to exclude a revset visibility
+#
+# The target use case is to use `share` to expose different subset of the same
+# repository, especially server side. See also `server.view`.
+coreconfigitem('experimental', 'extra-filter-revs',
+    default=None,
+)
 coreconfigitem('experimental', 'maxdeltachainspan',
     default=-1,
 )
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1050,6 +1050,8 @@  class localrepository(object):
         # Signature to cached matcher instance.
         self._sparsematchercache = {}
 
+        self._extrafilterid = repoview.extrafilter(ui)
+
     def _getvfsward(self, origfunc):
         """build a ward for self.vfs"""
         rref = weakref.ref(self)
@@ -1197,6 +1199,9 @@  class localrepository(object):
 
         In other word, there is always only one level of `repoview` "filtering".
         """
+        if self._extrafilterid is not None:
+            name = name + '%'  + self._extrafilterid
+
         cls = repoview.newtype(self.unfiltered().__class__)
         return cls(self, name, visibilityexceptions)
 
diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -17,6 +17,10 @@  from . import (
     phases,
     pycompat,
     tags as tagsmod,
+    util,
+)
+from .utils import (
+    repoviewutil,
 )
 
 def hideablerevs(repo):
@@ -154,6 +158,35 @@  filtertable = {'visible': computehidden,
                'immutable':  computemutable,
                'base':  computeimpactable}
 
+_basefiltername = list(filtertable)
+
+def extrafilter(ui):
+    """initialize extra filter and return its id
+
+    If extra filtering is configured, we make sure the associated filtered view
+    are declared and return the associated id.
+    """
+    frevs = ui.config('experimental', 'extra-filter-revs')
+    if frevs is None:
+        return None
+
+    fid = util.DIGESTS['sha512'](frevs).hexdigest()
+
+    combine = lambda fname: fname + '%' + fid
+
+    subsettable = repoviewutil.subsettable
+
+    if combine('base') not in filtertable:
+        for name in _basefiltername:
+            def extrafilteredrevs(repo, *args, **kwargs):
+                baserevs = filtertable[name](repo, *args, **kwargs)
+                extrarevs = frozenset(repo.revs(frevs))
+                return baserevs | extrarevs
+            filtertable[combine(name)] = extrafilteredrevs
+            if name in subsettable:
+                subsettable[combine(name)] = combine(subsettable[name])
+    return fid
+
 def filterrevs(repo, filtername, visibilityexceptions=None):
     """returns set of filtered revision for this filter name
 
diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
--- a/mercurial/statichttprepo.py
+++ b/mercurial/statichttprepo.py
@@ -155,6 +155,7 @@  class statichttprepository(localrepo.loc
 
         self.names = namespaces.namespaces()
         self.filtername = None
+        self._extrafilterid = None
 
         try:
             requirements = set(self.vfs.read(b'requires').splitlines())
diff --git a/tests/test-server-view.t b/tests/test-server-view.t
--- a/tests/test-server-view.t
+++ b/tests/test-server-view.t
@@ -34,5 +34,21 @@ 
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     r0
   
+
+Check same result using `experimental.extra-filter-revs`
+
+  $ hg -R test --config experimental.extra-filter-revs='not public()' serve -p $HGPORT1 -d --pid-file=hg2.pid -E errors.log
+  $ cat hg2.pid >> $DAEMON_PIDS
+  $ hg -R test2 incoming http://foo:xyzzy@localhost:$HGPORT1/
+  comparing with http://foo:***@localhost:$HGPORT1/
+  changeset:   0:1ea73414a91b
+  tag:         tip
+  user:        debugbuilddag
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     r0
+  
+
+cleanup
+
   $ cat errors.log
   $ killdaemons.py