Re: [PATCH RFC] rebase: add --revisions flag

On 2009.12.08 18:44:49 +0200, Michael S. Tsirkin wrote:

On Tue, Dec 08, 2009 at 05:37:37PM +0100, Björn Steinbrink wrote:

quoted

On 2009.12.08 18:14:07 +0200, Michael S. Tsirkin wrote:

quoted

On Tue, Dec 08, 2009 at 05:08:22PM +0100, Björn Steinbrink wrote:

quoted

On 2009.12.08 16:47:42 +0200, Michael S. Tsirkin wrote:

quoted

Add --revisions flag to rebase, so that it can be used
to apply an arbitrary range of commits on top
of a current branch.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

I've been wishing for this functionality for a while now,
so here goes. This isn't yet properly documented and I didn't
write a test, but the patch seems to work fine for me.
Any early flames/feedback?

This pretty much reverses what rebase normally does. Instead of "rebase
this onto that" it's "'rebase' that onto this". And instead of updating
the branch head that got rebased, the, uhm, "upstream" gets updated.

The last sentence is wrong I think - it is still the branch head that
is updated.

But you don't rebase the branch head. Before the rebase, the branch head
doesn't reference the commits that get rebased. For example:

git checkout bar
git rebase --revisions foo bar

You "rebase" the commits in foo's history, but you update bar.

Yes, that's the who point of the patch.

Yes, and it's "backwards" compared to the existing "rebase" modes, but
more like "cherry-pick".

The above applies a single commit, foo, on top of current branch bar.

Hm, no. I expected it to turn all commits reachable from foo into
patches and applying them to bar. But actually, that should hit the
special <since> mode of format-patch. So
git rebase --revisions foo bar
is (with your patch) actually the same as
git rebase foo bar

So actually the example should have been:
git rebase --root --revisions foo bar

Both invocations probably mess up the diff-stat as that becomes:
git diff --stat --summary foo
So it creates a diffstat of the diff from the working tree to "foo",
which can't be right.

quoted

WRT the result, the above command should be equivalent to:
git checkout bar
git reset --hard foo
git rebase --root --onto ORIG_HEAD;

And here, the commits currently reachable through "bar" are rebased, and
"bar" also gets updated.

So this 
1. won't be very useful, as you show it is easy
   to achieve with existing commands.

One can "almost" achieve it.
git rebase --revision A..B foo

is about the same as:
git checkout foo
git reset --hard B
git rebase --onto ORIG_HEAD A

But:
a) The "reset --hard" obviously lacks the safety checks for clean
index/working tree.
b) "git rebase --abort" won't take you back to your initial state, but
to B.
c) It's not really obvious that you can do it and how to do it.

Another possibility would be:

git checkout B^0 # detach HEAD at B
git rebase foo # rebase onto foo
git checkout foo 
git merge HEAD@{1} # Fast-forwards foo to the rebased stuff

That fixes a), avoid b) [because you don't mess up any branch head
early] but is still subject to c).

And for both methods, the ORIG_HEAD and HEAD@{1} arguments are somewhat
"unstable", e.g. checking out the wrong branch head first, and only then
the correct one, you'd have to use HEAD@{2} instead of HEAD@{1} (because
the reflog for HEAD got a new entry).

So you can already do what you want to do, but wrapping it in a single
porcelain might still be useful because it's obviously a  lot easier and
safer that way. That said, I wonder what kind of workflow you're using
though, and why you require that feature. I've never needed something
like that.

2. interprets "foo" as branch name as opposed to
   revision range.

Well, a single committish is a "range" as far as the range-based
commands are concerned, e.g. "git log master" treats "master" to mean
all commits reachable it. If "rebase --revisions master" would do the
same, that's at least consistent (and for single commit picks, there's
already cherry-pick). The problem with your patch is that it passes the
revision argument to format-patch as is, and:
git format-patch foo
is the same as
git format-patch foo..HEAD

OTOH, rebase --revisions as I implemented is a "smarter cherry-pick"
which can't easily be achieved with existing commands, especially if
you add "-i".

And that "is a 'smarter cherry-pick'" is why I think that rebase is
actually the wrong command to get that feature. While rebase internally
does just mass-cherry-picking, it does that with commits in the current
branch onto a specified branch. The --revisions flag makes it do things
the other way around.

Björn