Patchwork D1747: repoview: add visibilityexception argument to filterrevs() and related fns

login
register
mail settings
Submitter phabricator
Date Dec. 22, 2017, 5:56 p.m.
Message ID <differential-rev-PHID-DREV-wkcku7rsubqaojp7pym3-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26405/
State Superseded
Headers show

Comments

phabricator - Dec. 22, 2017, 5:56 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  After this patch, filterrevs() can take an optional argument
  visibilityexceptions which is a set of revs which should be exception to
  being hidden. The visibilityexceptions will be passed to the function computing
  hidden revisions for that filtername and are considered there while calculating
  the set of hidden revs.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1747

AFFECTED FILES
  mercurial/repoview.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 25, 2017, 12:54 p.m.
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> repoview.py:157
>          func = filtertable[filtername]
> -        repo.filteredrevcache[filtername] = func(repo.unfiltered())
> +        repo.filteredrevcache[filtername] = func(repo.unfiltered(),
> +                                                 visibilityexceptions)

Since revisions are dynamically unhidden, we can't cache the result
to the (unfiltered) repo per `filterename`.

Perhaps we could apply `visibilityexceptions` at repoview.changelog().

  revs = filterrevs(unfi, self.filtername)
  revs = revs - self._visibilityexceptions

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1747

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 25, 2017, 1:03 p.m.
yuja added a comment.


  > Since revisions are dynamically unhidden, we can't cache the result
  >  to the (unfiltered) repo per filterename.
  
  FWIW, if the cache don't include "unhidden" revisions, maybe we don't
  need new filter names like 'visible-hidden"?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1747

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 25, 2017, 2:45 p.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D1747#29916, @yuja wrote:
  
  > > Since revisions are dynamically unhidden, we can't cache the result
  > >  to the (unfiltered) repo per filterename.
  >
  > FWIW, if the cache don't include "unhidden" revisions, maybe we don't
  >  need new filter names like 'visible-hidden"?
  
  
  Yep I agree, but I wanted be on the safe side to prevent unknown bugs by using the same filtername.

INLINE COMMENTS

> yuja wrote in repoview.py:157
> Since revisions are dynamically unhidden, we can't cache the result
> to the (unfiltered) repo per `filterename`.
> 
> Perhaps we could apply `visibilityexceptions` at repoview.changelog().
> 
>   revs = filterrevs(unfi, self.filtername)
>   revs = revs - self._visibilityexceptions

One reason to have this considered in computehidden() is we want this to be considered in `_revealancestors()` otherwise things which needs parent of hidden revision will throw an error like `export`. I will make it not cache the result if visibilityexceptions is present.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1747

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 26, 2017, 1:47 p.m.
yuja added a comment.


  >> FWIW, if the cache don't include "unhidden" revisions, maybe we don't
  >>  need new filter names like 'visible-hidden"?
  > 
  > Yep I agree, but I wanted be on the safe side to prevent unknown bugs by using the same filtername.
  
  Actually we have to use new filtername to separate branch/tags caches until
  we can disable these caches when revisions are dynamically pinned.
  
  Queued the series, thanks.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1747

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/repoview.py b/mercurial/repoview.py
--- a/mercurial/repoview.py
+++ b/mercurial/repoview.py
@@ -65,24 +65,26 @@ 
                 hidden.remove(p)
                 stack.append(p)
 
-def computehidden(repo):
+def computehidden(repo, visibilityexceptions=None):
     """compute the set of hidden revision to filter
 
     During most operation hidden should be filtered."""
     assert not repo.changelog.filteredrevs
 
     hidden = hideablerevs(repo)
     if hidden:
         hidden = set(hidden - pinnedrevs(repo))
+        if visibilityexceptions:
+            hidden -= visibilityexceptions
         pfunc = repo.changelog.parentrevs
         mutablephases = (phases.draft, phases.secret)
         mutable = repo._phasecache.getrevset(repo, mutablephases)
 
         visible = mutable - hidden
         _revealancestors(pfunc, hidden, visible)
     return frozenset(hidden)
 
-def computeunserved(repo):
+def computeunserved(repo, visibilityexceptions=None):
     """compute the set of revision that should be filtered when used a server
 
     Secret and hidden changeset should not pretend to be here."""
@@ -100,16 +102,16 @@ 
     else:
         return hiddens
 
-def computemutable(repo):
+def computemutable(repo, visibilityexceptions=None):
     assert not repo.changelog.filteredrevs
     # fast check to avoid revset call on huge repo
     if any(repo._phasecache.phaseroots[1:]):
         getphase = repo._phasecache.phase
         maymutable = filterrevs(repo, 'base')
         return frozenset(r for r in maymutable if getphase(repo, r))
     return frozenset()
 
-def computeimpactable(repo):
+def computeimpactable(repo, visibilityexceptions=None):
     """Everything impactable by mutable revision
 
     The immutable filter still have some chance to get invalidated. This will
@@ -145,11 +147,15 @@ 
                'immutable':  computemutable,
                'base':  computeimpactable}
 
-def filterrevs(repo, filtername):
-    """returns set of filtered revision for this filter name"""
+def filterrevs(repo, filtername, visibilityexceptions=None):
+    """returns set of filtered revision for this filter name
+
+    visibilityexceptions is a set of revs which must are exceptions for
+    hidden-state and must be visible"""
     if filtername not in repo.filteredrevcache:
         func = filtertable[filtername]
-        repo.filteredrevcache[filtername] = func(repo.unfiltered())
+        repo.filteredrevcache[filtername] = func(repo.unfiltered(),
+                                                 visibilityexceptions)
     return repo.filteredrevcache[filtername]
 
 class repoview(object):
@@ -209,7 +215,7 @@ 
         unfilen = len(unfiindex) - 1
         unfinode = unfiindex[unfilen - 1][7]
 
-        revs = filterrevs(unfi, self.filtername)
+        revs = filterrevs(unfi, self.filtername, self._visibilityexceptions)
         cl = self._clcache
         newkey = (unfilen, unfinode, hash(revs), unfichangelog._delayed)
         # if cl.index is not unfiindex, unfi.changelog would be