I've caught multiple extensions in the wild lying about being
'internal', so it's time to move the goalposts on people. Goalpost
moving will continue until third party extensions stop trying to
defeat the system.
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
The following rebase implementation details are frustrating:
- storing a list of sorted revision numbers in a field named sortedstate
- having sortedstate be a field of the rebaseruntime class
- using sortedstate[-1] as opposed to a more intuitive max(self.state) to
compute the latest revision in the state
This commit fixes those imperfections.
As per Yuya's advice, we would like to slightly reduce the amount of state
which is stored in rebaseruntime class. In this case, we don't need to store
extrafn field, as we can produce the necessary value by calling _makeextrafn
and the perf overhead is negligible.
Rebase finish logic includes collapsing working directorystate into
a single commit, moving bookmarks, clearing status and collapsemsg files,
reporting skipped commits to the user and obsoleting precursors of the
newly created commits.
This code:
for rev in sortedstate:
...
...
newnode = concludenode(repo, rev, p1, rbsrt.external,
commitmsg=commitmsg,
extrafn=extrafn, editor=editor,
keepbranches=rbsrt.keepbranchesf,
date=rbsrt.date)
uses 'rev' variable in 'concludenode' function invocation. It is not
explicitly assigned before, but its value comes as last value or 'rev' in
a for loop, e.g. last element in a 'sortedstate'. IMO this a bad style and it
also makes it hard to refactor the function, so it is better to explicitly
define the value passed to 'concludenode'.
rebaseruntime is a class that will in future contain all of the state
necessary to perform rebase operation and have pieces of rebase logic as
its methods.
This commit introduces the class and moves the following local variables to
be its fields:
- originalwd
- external
- state
- activebookmark
Since (b) is banned, we should do the same for (a) for consistency.
a) from mercurial import hg
from mercurial.i18n import _
b) from . import hg
from .i18n import _
Before this patch, `hg pull --rebase` would be a strict sequence of `hg pull`
followed by `hg rebase` if anything was pulled.
Now that rebase pick his default destination the same way than merge, than
`hg rebase` step would abort in the case the repo already had multiple anonymous
heads (because of the ambiguity). (changed in 8822059a608a)
The intend of the user with `hg pull --rebase` is clearly to rebase on pulled
content. This used to be (mostly) enforced by the former default destination for
rebase, "tipmost changeset of the branch" as the tipmost would likely a
changeset that just got pulled. But this intended was no longer enforced with
the new defaul destination (unified with merge).
This changeset makes use of the '_destspace' mechanism introduced in the previous
changeset to enforce this.
This partially fixes issue5214 as no change at all have been made to the new
handling of the case with bookmark (unified with merge).
In the 'hg pull --rebase', we don't want to pick a rebase destination unrelated
to the pull, we lay down basic infrastructure to allow such restriction on
stable (before 3.8 release) in this case. See issue 5214 for details.
Actual usage and test will be in the next patch.
Before this patch, rebase --continue would crash when trying to resume a rebase
of obsolete revisions whose successors were in the destination.
This patch adds logic to recompute the mapping when rebase is resumed. This
patch also adds a test that showcased the crash before the code change.
This patch extracts the error handling code path to go in a separate function.
In the next patch we will able to reuse this logic and avoid duplicated code.
Consider the following use case. User has a set of commits he wants to rebase
onto some destination. Some of the commits in the set are already rebased
and their new versions are now among the ancestors of destination. Traditional
rebase behavior would make the rebase and effectively try to apply older
versions of these commits on top of newer versions, like this:
a` --> b --> a`
(where both 'a`' and 'a``' are rebased versions of 'a')
This is not desired since 'b' might have made changes to 'a`' which can now
result in merge conflicts. We can avoid these merge conflicts since we know
that 'a``' is an older version of 'a`', so we don't even need to put it on top
of 'b'. Rebaseskipobsolete allows us to do exactly that.
Another undesired effect of a pure rebase is that now 'a`' and 'a``' are both
successors to 'a' which is a divergence. We don't want that and not rebasing
'a' the second time allows to avoid it.
This was not enabled by default initially because we wanted to have some more
experience with it. After months of painless usages in multiple places, we are
confident enough to turn it on my default.
This patch consists of changes below (these can't be applied
separately).
- replace revset.extpredicate by registrar.revsetpredicate in
extensions
- remove setup() on an instance named as revsetpredicate in
uisetup()/extsetup() of each extensions
registrar.revsetpredicate doesn't have setup() API.
- put new entry for revsetpredicate into extraloaders in dispatch
This causes implicit loading predicate functions at loading
extension.
This loading mechanism requires that an extension has an instance
named as revsetpredicate, and this is reason why
largefiles/__init__.py is also changed in this patch.
Before this patch, test-revset.t tests that all decorated revset
predicates are loaded by explicit setup() at once ("all or nothing").
Now, test-revset.t tests that any revset predicate isn't loaded at
failure of loading extension, because loading itself is executed by
dispatch and it can't be controlled on extension side.
Changeset 9a4b77db854b introduced a guard against case where obsolete changesets
are included in the rebase in a way this will result in divergence (because
rebase create new successors for changeset which already have successors). In
the same go a 'rebase.allowdivergence' option was introduced to control that
behavior.
We rename this config option to 'experimental.allowdivergence' for multiple
reasons:
* First this behavior is attached to changeset evolution, a feature still
experimental.
* Second, there was no 'rebase' section in config before we introduced this
option. I would like to avoid proliferation of micro config section and
therefore would like to avoid the creation of this new section just for an
experimental feature.
* Third, this guard (warning the user about a history rewriting operation that
will create divergence) will very likely be generalised to all history
rewriting operations, making this not rebase specific.
* Finally, because this will likely be a general guard present a bit everywhere
in the UI we'll likely end up with something better than a config option to
control this behavior, so having the current config option living in
experimental will allow us make it disappear in the future.
So we banish this config option back to the experimental section where it
belongs, killing the newly born 'rebase' config section in the process.
This changeset finally make 'hg rebase' choose its default destination using the
same logic as 'hg merge'. The previous default was "tipmost changeset on the
current branch", the new default is "the other head if there is only one". This
change has multiple consequences:
- Multiple tests which were not rebasing anything (rebasing from tipmost head)
are now rebasing on the other "lower" branch. This is the expected new
behavior.
- A test is now explicitly aborting when there is too many heads on the branch.
This is the expected behavior.
- We gained a better detection of the "nothing to rebase" case while performing
'hg pull --rebase' so the message have been updated. Making clearer than an
update was performed and why. This is beneficial side-effect.
- Rebasing from an active bookmark will behave the same as 'hg merge' from a
bookmark.
Before this patch collapse message wasn't stored so when
you ran into the merge conflict while rebasing, running
rebase --continue didn't remember the message and
always opened editor to fill commit message.
This patch adds saving collapse message in
.hg/last-message.txt and restoring it later
when needed.
The whole rebase function is gargantuan and this computation is almost 100 lines
long. We extract it in a dedicated function as it is independent from the rest
of the rebase code.
Having it in its own function will make it easier to rework the default
destination logic.
The update logic have grow more and more complicated over time (eg bookmark
movement, new destination logic, warning on other head, etc). The rebase
extension was reimplementing its own basic version of update to be used by 'hg
pull --rebase'. We remove the custom code and use a combination of higher level
functions.
A test is added to check that the update is properly warning about other branch
heads.
I recently discovered that 'hg pull --rebase' was also running an update. And it
was running it in all cases as long as the update would move the working copy
somewhere else...
This felt wrong and it actually is. This 'update' call is introduced in
8bea699a182c. In that commit the intent is very clear. The update should happen
only when there was nothing to rebase. The implementation did not check if a
rebase was performed because, at that time, rebase would always leave you on the
top most changeset. Being on that top most changeset result in a no-op update
and the step was skipped.
However 642dc7838453d changed rebase behavior to preserve the working copy
parent, so if we are not on a head at pull time, the code performs both a rebase
and an update.
This changeset introduce a test for this case and restore the intended behavior.
There are other issues with this custom update code but they will be addressed
in later changeset (eg: own destination logic, lack of heads warning).
I'm not super happy with the explicitly comparison 'rebase(...) == 1' but a
later series will have a cleaner way to handle it anyway (while making 'rebase'
pick its default destination like 'merge').
Rather than look for the lowest revision, see if the rebase state is tracking
the parents of this revision. Otherwise we can't handle multiple revisions in
one rebase that includes a merge revision.
Fixes issue5044.
