Bug 5422 - rebase shows "nothing to rebase" even in cases where the rebaseset can be partially rebased
Summary: rebase shows "nothing to rebase" even in cases where the rebaseset can be par...
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: rebase (show other bugs)
Version: default branch
Hardware: PC Windows
: wish feature
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-09 14:13 UTC by Jun Wu
Modified: 2018-03-27 00:00 UTC (History)
4 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jun Wu 2016-11-09 14:13 UTC
Given the following DAG:

	o  f
	|
	o  e
	|
	| o  d
	|/
	o  c
	|
	| o  b
	|/
	o  a

`hg rebase -s 'b+d+f' -d e` fails with:
nothing to rebase

The error message is not accurate because both b and d are valid sources to be rebased. With "--debug" the reason is shown:

$ hg rebase -s 'b+d+f' -d e --debug
rebase onto cd22cb85be7c starting from 27056920676d
rebase onto cd22cb85be7c starting from 7fde43e95481
source is a child of destination
nothing to rebase

However, we should probably improve the error message so it's more informative than just "nothing to rebase".
Comment 1 Pierre-Yves David 2016-11-21 20:30 UTC
note: Mercurial is eagerly aborting whenever something is strange in the inputs in multiple place. So this behavior might have been kept on purpose.
Comment 2 Jun Wu 2016-11-22 08:55 UTC
I think the "abort" behavior could be kept. But the message should change: something like "x is a child of destination" is more useful. Currently it is only printed in the debug message. We may want to use "hint" to tell the user how to exclude "unwanted" revisions using revset.
Comment 3 Pierre-Yves David 2016-11-23 09:54 UTC
I'm a bit fan on taking minimal step. I agree that the current message is too unhelpful and would be happy to take an improvement there. Then we can take some time to see if the message improvement is good enough or if more is needed here.
Comment 4 Martin von Zweigbergk 2016-11-28 09:33 UTC
One use case that sounds reasonable to me is "hg rebase -d @ -r 'draft()'". Would be good if that could work.
Comment 5 Bugzilla 2017-04-28 00:00 UTC
Bug was inactive for 150 days, archiving
Comment 6 Martin von Zweigbergk 2017-07-20 11:42 UTC
Fixed by 78496ac3
Comment 7 HG Bot 2017-10-04 16:53 UTC
Fixed by https://mercurial-scm.org/repo/hg/rev/51e7c83e05ee
Jun Wu <quark@fb.com>
rebase: calculate ancestors for --base separately (issue5420)

Previously, the --base option only works with a single "branch" - if there
is one changeset in the "--base" revset whose branching point(s) is/are
different from another changeset in the "--base" revset, "rebase" will error
out with:

  abort: source is ancestor of destination

This happens if the user has multiple draft branches, and uses "hg rebase -b
'draft()' -d master", for example. The error message looks cryptic to users
who don't know the implementation detail.

This patch changes the logic to calculate the common ancestor for every
"base" changeset separately so we won't (incorrectly) select "source" which
is an ancestor of the destination.

This patch should not change the behavior where all changesets specified by
"--base" have the same branching point(s).

A new situation is: some of the specified changesets could be rebased, while
some couldn't (because they are descendants of the destination, or they do
not share a common ancestor with the destination). The current behavior is
to show "nothing to rebase" and exits with 1.

This patch maintains the current behavior (show "nothing to rebase") even if
part of the "--base" revset could be rebased. A clearer error message may be
"cannot find branching point for X", or "X is a descendant of destination".
The error message issue is tracked by issue5422 separately.

A test is added with all kinds of tricky cases I could think of for now.

(please test the fix)
Comment 8 HG Bot 2017-10-04 17:32 UTC
Fixed by https://mercurial-scm.org/repo/hg/rev/78496ac30025
Martin von Zweigbergk <martinvonz@google.com>
rebase: allow rebase even if some revisions need no rebase (BC) (issue5422)

This allows you to do e.g. "hg rebase -d @ -r 'draft()'" even if some
drafts are already based off of @. You'd still need to exclude
obsolete and troubled revisions, though. We will deal with those cases
later.

Implemented by treating state[rev]==rev as "no need to rebase". I
considered adding another fake revision number like revdone=-6. That
would make the code clearer in a few places, but would add extra code
in other places.

I moved the existing test out of test-rebase-base.t and into a new
file and added more tests there, since not all are using --base.

(please test the fix)
Comment 9 Bugzilla 2017-10-14 00:00 UTC
Bug was set to TESTING for 9 days, resolving
Comment 10 HG Bot 2018-03-19 21:50 UTC
Fixed by https://mercurial-scm.org/repo/hg/rev/b9a6ee2066f9
Martin von Zweigbergk <martinvonz@google.com>
tests: demonstrate aborted rebase strips commits that didn't need rebasing

I haven't verified, but this has probably been broken ever since I
added the feature in 78496ac30025 (rebase: allow rebase even if some
revisions need no rebase (BC) (issue5422), 2017-05-11).

Differential Revision: https://phab.mercurial-scm.org/D2877

(please test the fix)
Comment 11 HG Bot 2018-03-19 21:50 UTC
Fixed by https://mercurial-scm.org/repo/hg/rev/177f3b90335f
Martin von Zweigbergk <martinvonz@google.com>
rebase: on abort, don't strip commits that didn't need rebased (issue5822)

I clearly missed adding this condition in 78496ac30025 (rebase: allow
rebase even if some revisions need no rebase (BC) (issue5422),
2017-05-11). Perhaps I should have opted for the "revdone" solution I
mentioned there...

Differential Revision: https://phab.mercurial-scm.org/D2879

(please test the fix)
Comment 12 Bugzilla 2018-03-27 00:00 UTC
Bug was set to TESTING for 7 days, resolving