| Submitter | Simon Farnsworth |
|---|---|
| Date | July 19, 2016, 10:39 a.m. |
| Message ID | <39ca6ff7183e4a42fea8.1468924783@devvm631.lla1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/15954/ |
| State | Accepted |
| Headers | show |
Comments
LGTM. Thanks for the detailed explanation. Excerpts from Simon Farnsworth's message of 2016-07-19 03:39:43 -0700: > # HG changeset patch > # User Simon Farnsworth <simonfar@fb.com> > # Date 1468924193 25200 > # Tue Jul 19 03:29:53 2016 -0700 > # Branch stable > # Node ID 39ca6ff7183e4a42fea832e0fedfe65de9b70ffa > # Parent 02a8fea4289b51992b2495a06d4b12cbda876cf1 > rebase: turn rebase revs into set before filtering obsolete > > When the inhibit extension from mutable-history is enabled, it attempts to > iterate over the rebaseset to prevent the nodes being rebased from being > marked obsolete. This happens at the same time as rebase's > _filterobsoleterevs function trying to iterate over the rebaseset to figure > out which ones are obsolete. The two of these iterating over the same > revset generatorset cause a 'generator already executing' exception. This is > probably a flaw in the revset implementation, since iterating over the same > set twice should be supported. > > This regression was introduced in 5d16ebe7b14, since it changed > _filterobsoleterevs to be called before the rebaseset was turned into a > set(). For now let’s just make the rebaseset an actual set again before > calling that function. This was caught by the inhibit tests. > > The relevant call stack from test-inhibit.t: > > File "/tmp/hgtests.jgjrN5/install/lib/python/hgext/rebase.py", line 285, in _preparenewrebase > obsrevs = _filterobsoleterevs(self.repo, rebaseset) > File "/data/hgbuild/facebook-hg-rpms/mutable-history/hgext/inhibit.py", line 197, in _filterobsoleterevswrap > r = orig(repo, rebasesetrevs, *args, **kwargs) > File "/tmp/hgtests.jgjrN5/install/lib/python/hgext/rebase.py", line 1380, in _filterobsoleterevs > return set(r for r in revs if repo[r].obsolete()) > File "/tmp/hgtests.jgjrN5/install/lib/python/hgext/rebase.py", line 1380, in <genexpr> > return set(r for r in revs if repo[r].obsolete()) > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3079, in _iterordered > val2 = next(iter2) > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3417, in gen > yield nextrev() > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3424, in _consumegen > for item in self._gen: > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 71, in iterate > cl = repo.changelog > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 319, in changelog > revs = filterrevs(unfi, self.filtername) > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 261, in filterrevs > repo.filteredrevcache[filtername] = func(repo.unfiltered()) > File "/data/hgbuild/facebook-hg-rpms/mutable-history/hgext/directaccess.py", line 65, in _computehidden > hidden = repoview.filterrevs(repo, 'visible') > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 261, in filterrevs > repo.filteredrevcache[filtername] = func(repo.unfiltered()) > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 175, in computehidden > hideable = hideablerevs(repo) > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 33, in hideablerevs > return obsolete.getrevs(repo, 'obsolete') > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/obsolete.py", line 1097, in getrevs > repo.obsstore.caches[name] = cachefuncs[name](repo) > File "/data/hgbuild/facebook-hg-rpms/mutable-history/hgext/inhibit.py", line 255, in _computeobsoleteset > if getrev(n) not in blacklist: > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3264, in __contains__ > return x in self._r1 or x in self._r2 > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3348, in __contains__ > for l in self._consumegen(): > File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3424, in _consumegen > for item in self._gen: > ValueError: generator already executing > > diff --git a/hgext/rebase.py b/hgext/rebase.py > --- a/hgext/rebase.py > +++ b/hgext/rebase.py > @@ -281,7 +281,7 @@ > " unrebased descendants"), > hint=_('use --keep to keep original changesets')) > > - obsrevs = _filterobsoleterevs(self.repo, rebaseset) > + obsrevs = _filterobsoleterevs(self.repo, set(rebaseset)) > self._handleskippingobsolete(rebaseset, obsrevs, dest) > > result = buildstate(self.repo, dest, rebaseset, self.collapsef,
On Tue, 19 Jul 2016 03:39:43 -0700, Simon Farnsworth wrote: > # HG changeset patch > # User Simon Farnsworth <simonfar@fb.com> > # Date 1468924193 25200 > # Tue Jul 19 03:29:53 2016 -0700 > # Branch stable > # Node ID 39ca6ff7183e4a42fea832e0fedfe65de9b70ffa > # Parent 02a8fea4289b51992b2495a06d4b12cbda876cf1 > rebase: turn rebase revs into set before filtering obsolete > > When the inhibit extension from mutable-history is enabled, it attempts to > iterate over the rebaseset to prevent the nodes being rebased from being > marked obsolete. This happens at the same time as rebase's > _filterobsoleterevs function trying to iterate over the rebaseset to figure > out which ones are obsolete. The two of these iterating over the same > revset generatorset cause a 'generator already executing' exception. This is > probably a flaw in the revset implementation, since iterating over the same > set twice should be supported. Perhaps, revset, repoview or obsolete? The fix is simple enough for stable. Queued, thanks.
Patch
diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -281,7 +281,7 @@ " unrebased descendants"), hint=_('use --keep to keep original changesets')) - obsrevs = _filterobsoleterevs(self.repo, rebaseset) + obsrevs = _filterobsoleterevs(self.repo, set(rebaseset)) self._handleskippingobsolete(rebaseset, obsrevs, dest) result = buildstate(self.repo, dest, rebaseset, self.collapsef,
# HG changeset patch # User Simon Farnsworth <simonfar@fb.com> # Date 1468924193 25200 # Tue Jul 19 03:29:53 2016 -0700 # Branch stable # Node ID 39ca6ff7183e4a42fea832e0fedfe65de9b70ffa # Parent 02a8fea4289b51992b2495a06d4b12cbda876cf1 rebase: turn rebase revs into set before filtering obsolete When the inhibit extension from mutable-history is enabled, it attempts to iterate over the rebaseset to prevent the nodes being rebased from being marked obsolete. This happens at the same time as rebase's _filterobsoleterevs function trying to iterate over the rebaseset to figure out which ones are obsolete. The two of these iterating over the same revset generatorset cause a 'generator already executing' exception. This is probably a flaw in the revset implementation, since iterating over the same set twice should be supported. This regression was introduced in 5d16ebe7b14, since it changed _filterobsoleterevs to be called before the rebaseset was turned into a set(). For now let’s just make the rebaseset an actual set again before calling that function. This was caught by the inhibit tests. The relevant call stack from test-inhibit.t: File "/tmp/hgtests.jgjrN5/install/lib/python/hgext/rebase.py", line 285, in _preparenewrebase obsrevs = _filterobsoleterevs(self.repo, rebaseset) File "/data/hgbuild/facebook-hg-rpms/mutable-history/hgext/inhibit.py", line 197, in _filterobsoleterevswrap r = orig(repo, rebasesetrevs, *args, **kwargs) File "/tmp/hgtests.jgjrN5/install/lib/python/hgext/rebase.py", line 1380, in _filterobsoleterevs return set(r for r in revs if repo[r].obsolete()) File "/tmp/hgtests.jgjrN5/install/lib/python/hgext/rebase.py", line 1380, in <genexpr> return set(r for r in revs if repo[r].obsolete()) File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3079, in _iterordered val2 = next(iter2) File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3417, in gen yield nextrev() File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3424, in _consumegen for item in self._gen: File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 71, in iterate cl = repo.changelog File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 319, in changelog revs = filterrevs(unfi, self.filtername) File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 261, in filterrevs repo.filteredrevcache[filtername] = func(repo.unfiltered()) File "/data/hgbuild/facebook-hg-rpms/mutable-history/hgext/directaccess.py", line 65, in _computehidden hidden = repoview.filterrevs(repo, 'visible') File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 261, in filterrevs repo.filteredrevcache[filtername] = func(repo.unfiltered()) File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 175, in computehidden hideable = hideablerevs(repo) File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/repoview.py", line 33, in hideablerevs return obsolete.getrevs(repo, 'obsolete') File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/obsolete.py", line 1097, in getrevs repo.obsstore.caches[name] = cachefuncs[name](repo) File "/data/hgbuild/facebook-hg-rpms/mutable-history/hgext/inhibit.py", line 255, in _computeobsoleteset if getrev(n) not in blacklist: File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3264, in __contains__ return x in self._r1 or x in self._r2 File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3348, in __contains__ for l in self._consumegen(): File "/tmp/hgtests.jgjrN5/install/lib/python/mercurial/revset.py", line 3424, in _consumegen for item in self._gen: ValueError: generator already executing