Re: [PATCH] rebase -i -p: doesn't pick certain merge commits that are children of "upstream"

From: Andrew Wong <andrew.w@sohovfx.com>
Date: Jun 18, 2011, 06:40 UTC
Message-ID: <4DFC4863.2090803@sohovfx.com>
In-Reply-To: <20110616172454.13ff1a18@sh9>

Here's a list of those commits in "git log"-order for easy reference:
   80fe82e rebase-i-p: if todo was reordered use HEAD as the rewritten 
parent
   d80d6bc rebase-i-p: do not include non-first-parent commits touching 
UPSTREAM
   acc8559 rebase-i-p: only list commits that require rewriting in todo
   a4f25e3 rebase-i-p: fix 'no squashing merges' tripping up non-merges
   bb64507 rebase-i-p: delay saving current-commit to REWRITTEN if squashing
   72583e6 rebase-i-p: use HEAD for updating the ref instead of mapping 
OLDHEAD
   42f939e rebase-i-p: test to exclude commits from todo based on its 
parents

On 11-06-16 6:24 PM, Stephen Haberman wrote:

Show 6 quoted lines

> Perhaps that is unreasonable with whatever you guys are looking at now,
> but, IIRC, the use case was that B1=some old commit, like a 2.0
> release, and a bunch of work happened on the C1 branch, it was merged
> in E1, but now when you want to rebase D1/E1/F1 on top of B1, you don't
> want all of the noise of the C1 commit(s), since when rewriting E1 into
> E2, you can just reuse the un-rewritten C1 as its 2nd parent.

In commit a4f25e3, we could already rebase B1 and squash F1 onto D1, while reusing C1 and recreating the merge. That means we could already pass t3411.2if we adjusted the todo-list to account for the extra "pick C1" line.

Show 6 quoted lines

> Well, and not just the noise--since the todo is still flat, if C1
> was listed in the todo, there's no way to recreate E2 as a merge and
> maintain the C1 commit(s) as a separate branch. I think C1 would get
> flattened between D2/E2, depending on where it was in the todo. You'd
> lose a merge, contrary to the -p flag. That sounds like the core issue
> that was being fixed

The merge shouldn't get flattened when the "-p" is used. As long as the merge commit appears in the todo-list, git will trace the parents of the merge commit, find the original or rewritten parents, and perform the merge. Slightly off-topic, but I believe the branches will remain intact as long as the branch commits remain in the same topo-order relative to each other in the todo-list. i.e. git will be confused if we try to move a commit from one branch into the other.

The "noise" is filtered out by by commit d80d6bc. However, I think we should keep the commits from branch C1, since there could be a scenario where we actually want to squash F1 onto C1 instead. That commit also introduced a bug that Jeff King was running into: if we do "git rebase -i -p C1", the todo-list becomes a "noop", which means HEAD is reset to C1 and we lose the merge commit and F1.

So what I did in my patch is essentially revert the changes from d80d6bc, and adjust t3411.2 to account for the extra "pick C1" line.