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

Martin von Zweigbergk martinvonz at google.com
Sat Mar 11 01:17:10 EST 2017


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".

>>
>> 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 ..
>>


More information about the Mercurial-devel mailing list