Bug 3111 - Collapsing changesets with rebase onto parent does not work
Summary: Collapsing changesets with rebase onto parent does not work
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: rebase (show other bugs)
Version: unspecified
Hardware: All All
: normal bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-18 08:09 UTC by Laurens Holst
Modified: 2012-12-29 12:28 UTC (History)
7 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 Laurens Holst 2011-11-18 08:09 UTC
Hi,

If I have the following tree:

  0 -- 1 -- 2

I expect that I can use rebase --collapse to collapse changesets 2 and 1 on
top of changeset 0. However, when I try this nothing happens:

  $ mkdir test
  $ cd test
  $ hg init
  $ echo a > a
  $ hg add a
  $ hg commit -m a
  $ echo b > b
  $ hg add b
  $ hg commit -m b
  $ echo c > c
  $ hg add c
  $ hg commit -m c
  $ hg rebase --collapse --source 1 --dest 0
  nothing to rebase
Comment 1 Patrick Mézard 2012-05-03 05:08 UTC
This is not hard to fix, it means removing a couple of checks in rebase and
adding a flag to allow fast-forward merge of changesets belonging to the
same branch in merge.update().

But making this change is silly, collapsing by repeated merge is inefficient
at best. We should do it the amend way: copies.pathcopies() + commitctx(),
update and strip. That would change rebase --collapse behaviour a bit, the
user will do a real rebase then the result would get collapsed. It could
introduce regressions with complicated copy records as copies.pathcopies()
has shortcomings. I do not know if this is acceptable.

Then we have a working rebase --collapse (which, excluding tags handling,
will work better than the collapse extension for it preserves (at least
some) copies). But do we want to promote "rebase --collapse" as the official
way to collapse changesets in Mercurial? I think not because rebase UI is
designed for rebasing with all kind of options like --base, --source, --rev
etc. and these are mostly useless for collapse, and confusing.

At this point, I am tempted to say we should first promote a better way for
collapsing revisions (including and fixing collapse extension maybe), then
enable it properly for rebase.
Comment 2 Laurens Holst 2012-05-03 05:19 UTC
So by that reasoning, why does rebase have a --collapse option at all.
Comment 3 Laurens Holst 2012-05-03 05:34 UTC
Anyway, I think the rebase command shown makes sense, should work but does not.

Perhaps not the most efficient or best way to collapse, but it’s already
there and built in, and performance characteristics won’t be worse than a
regular rebase I imagine.

Optimising collapse can always be done at a later time. The issue you
mention with copies.pathcopies() don’t sound very appealing, it is not
preserving copies correctly. That sounds like it should be fixed and it
complicated to fix. I don’t really see the point in holding off on this
issue until that happens.
Comment 4 Patrick Mézard 2012-05-03 05:35 UTC
My theory is rebasing is pretty deep in the revision rewriting business and
the feature was both easy and tempting to implement.

Honestly, I cannot think of a workflow where I would use it. I certainly
rebase revisions. I certainly see the need for collapsing. But doing both at
the same time, definitely not.
Comment 5 Laurens Holst 2012-05-03 05:37 UTC
I can agree with you there :).

I’m not a big collapse fan myself, but it’s a recurring question on stack
overflow and this bug report arose as a result of attempting to answer one
of those questions.
Comment 6 Pierre-Yves David 2012-05-03 05:44 UTC
Making rebase --collapse works when there is nothing to rebase should not be
too complicated. We should go allow it.

However it is quite awful UI-wise and we should provide a nicer UI for that.
The collapse extension seems the way to go as a lot of work have already
been done there.
Comment 7 Patrick Mézard 2012-05-03 07:41 UTC
Patch submitted here:

  http://selenic.com/pipermail/mercurial-devel/2012-May/039666.html
Comment 8 Patrick Mézard 2012-05-03 07:43 UTC
@marmoute, yes collapse extension comes to mind but looking at the code,
two-thirds of it seems to deal with deprecated options and unless I misread
it, it just does a revert + tag rewriting + commit. Perhaps the tag
rewriting part makes sense but everything else can be rewritten in a simpler
way.
Comment 9 Bugzilla 2012-05-12 09:25 UTC

--- Bug imported by bugzilla@serpentine.com 2012-05-12 09:25 EDT  ---

This bug was previously known as _bug_ 3111 at http://mercurial.selenic.com/bts/issue3111
Comment 10 Pierre-Yves David 2012-11-26 22:02 UTC
This was fixed by d1afbf03e69a long ago (2012-05-03)

http://www.selenic.com/hg/rev/d1afbf03e69a