[PATCH 2 of 2] rebase: use unfiltered repo and remove complex unhiding code (issue5219)

[Jun Wu]

Excerpts from Martin von Zweigbergk's message of 2017-03-10 22:17:10 -0800:
> On Fri, Mar 10, 2017 at 6:06 PM, Jun Wu <quark at fb.com> wrote:
> > I like this change. It removes a dynamic blocker.
> >
> > I also did some manual tests and the logic seems good to me.
> >
> > Excerpts from Martin von Zweigbergk's message of 2017-03-10 17:15:52 -0800:
> >> # HG changeset patch
> >> # User Martin von Zweigbergk <martinvonz at google.com>
> >> # Date 1485968584 28800
> >> #      Wed Feb 01 09:03:04 2017 -0800
> >> # Node ID 0c89a4fb256390123fd543d73cbd5f01450e6da7
> >> # Parent  460068415dc97223cce156c70f7f8a6b659c4dd2
> >> rebase: use unfiltered repo and remove complex unhiding code (issue5219)
> >>
> >> Instead of dynamically unhiding commits, just work with the unfiltered
> >> repo. The deleted _setrebasesetvisibility() code seems very
> >> intentional, but I can't figure out what I'm breaking with this
> >> patch. Hopefully reviewers can tell me.
> 
> Pierre-Yves expressed unspecified concern that this patch might make
> some operations use the unfiltered repo that shouldn't be. So I looked
> a little closer and I think I found a case that confirms his
> suspicion. When allowunstable is not set, I think the revset
> "first(children(<rebaseset>) - rebaseset)" would report an obsolete
> child as unstable. I'll try to resend this series with a more narrowly
> scoped use of repo.unfiltered(), or maybe with an updated set of
> "dynamic blockers".

I wish we could kill the "allowunstable" feature eventually. That'll likely
simplify a lot of things. It's not that unrealistic since the "archived"
phase could be a thing, and will help people who want safe history rewriting
without using the obsstore.

Since some caches (branch, tags) depend on "dynamic blockers", making it
more complex may invalidate those caches. I think just checking
"allowunstable" and using "unfiltered" is better if we want better
performance and less complex code.

> >>
> >> diff -r 460068415dc9 -r 0c89a4fb2563 hgext/rebase.py
> >> --- a/hgext/rebase.py    Wed Feb 01 09:18:44 2017 -0800
> >> +++ b/hgext/rebase.py    Wed Feb 01 09:03:04 2017 -0800
> >> @@ -44,7 +44,6 @@
> >>      phases,
> >>      registrar,
> >>      repair,
> >> -    repoview,
> >>      revset,
> >>      scmutil,
> >>      smartset,
> >> @@ -128,7 +127,7 @@
> >>          if opts is None:
> >>              opts = {}
> >>
> >> -        self.repo = repo
> >> +        self.repo = repo.unfiltered()
> >>          self.ui = ui
> >>          self.opts = opts
> >>          self.originalwd = None
> >> @@ -250,7 +249,6 @@
> >>          repo.ui.debug('computed skipped revs: %s\n' %
> >>                          (' '.join(str(r) for r in sorted(skipped)) or None))
> >>          repo.ui.debug('rebase status resumed\n')
> >> -        _setrebasesetvisibility(repo, state.keys())
> >>
> >>          self.originalwd = originalwd
> >>          self.target = target
> >> @@ -1119,7 +1117,6 @@
> >>
> >>  def clearstatus(repo):
> >>      'Remove the status files'
> >> -    _clearrebasesetvisibiliy(repo)
> >>      util.unlinkpath(repo.join("rebasestate"), ignoremissing=True)
> >>
> >>  def needupdate(repo, state):
> >> @@ -1203,7 +1200,6 @@
> >>      dest: context
> >>      rebaseset: set of rev
> >>      '''
> >> -    _setrebasesetvisibility(repo, rebaseset)
> >>
> >>      # This check isn't strictly necessary, since mq detects commits over an
> >>      # applied patch. But it prevents messing up the working directory when
> >> @@ -1383,31 +1379,6 @@
> >>
> >>      return ret
> >>
> >> -def _setrebasesetvisibility(repo, revs):
> >> -    """store the currently rebased set on the repo object
> >> -
> >> -    This is used by another function to prevent rebased revision to because
> >> -    hidden (see issue4504)"""
> >> -    repo = repo.unfiltered()
> >> -    revs = set(revs)
> >> -    repo._rebaseset = revs
> >> -    # invalidate cache if visibility changes
> >> -    hiddens = repo.filteredrevcache.get('visible', set())
> >> -    if revs & hiddens:
> >> -        repo.invalidatevolatilesets()
> >> -
> >> -def _clearrebasesetvisibiliy(repo):
> >> -    """remove rebaseset data from the repo"""
> >> -    repo = repo.unfiltered()
> >> -    if '_rebaseset' in vars(repo):
> >> -        del repo._rebaseset
> >> -
> >> -def _rebasedvisible(orig, repo):
> >> -    """ensure rebased revs stay visible (see issue4504)"""
> >> -    blockers = orig(repo)
> >> -    blockers.update(getattr(repo, '_rebaseset', ()))
> >> -    return blockers
> >> -
> >>  def _filterobsoleterevs(repo, revs):
> >>      """returns a set of the obsolete revisions in revs"""
> >>      return set(r for r in revs if repo[r].obsolete())
> >> @@ -1479,5 +1450,3 @@
> >>           _("use 'hg rebase --continue' or 'hg rebase --abort'")])
> >>      cmdutil.afterresolvedstates.append(
> >>          ['rebasestate', _('hg rebase --continue')])
> >> -    # ensure rebased rev are not hidden
> >> -    extensions.wrapfunction(repoview, '_getdynamicblockers', _rebasedvisible)
> >> diff -r 460068415dc9 -r 0c89a4fb2563 tests/test-rebase-obsolete.t
> >> --- a/tests/test-rebase-obsolete.t    Wed Feb 01 09:18:44 2017 -0800
> >> +++ b/tests/test-rebase-obsolete.t    Wed Feb 01 09:03:04 2017 -0800
> >> @@ -279,9 +279,27 @@
> >>    $ hg --hidden up -qr 'first(hidden())'
> >>    $ hg rebase --rev 13 --dest 15
> >>    rebasing 13:98f6af4ee953 "C"
> >> -  abort: hidden revision '1'!
> >> -  (use --hidden to access hidden revisions)
> >> -  [255]
> >> +  $ hg log -G
> >> +  o  16:294a2b93eb4d C
> >> +  |
> >> +  o  15:627d46148090 D
> >> +  |
> >> +  | o  12:462a34d07e59 B
> >> +  | |
> >> +  | o  11:4596109a6a43 D
> >> +  | |
> >> +  | o  7:02de42196ebe H
> >> +  | |
> >> +  +---o  6:eea13746799a G
> >> +  | |/
> >> +  | o  5:24b6387c8c8c F
> >> +  | |
> >> +  o |  4:9520eea781bc E
> >> +  |/
> >> +  | @  1:42ccdea3bb16 B
> >> +  |/
> >> +  o  0:cd010b8cd998 A
> >> +
> >>
> >>    $ cd ..
> >>