Re: [PATCH] rebase -p: seed first commit in case it's before the merge bases.

[Stephen Haberman <stephen@exigencecorp.com>]

> > However, I have a strong feeling that just piling onto the current
> > code will not fix the underlying issues.
> 
> BTW just to clarify what I mean by "underlying issues": if you say
> "git rebase -i" in Sitaram's test case, you will see the two commits
> -- as expected.
> 
> However, if you add "-p", all of a sudden you will only see "noop".
> IMO there is no excuse that the code can hide them at all.  If the
> commits are reachable from HEAD but not from $UPSTREAM, they have to
> be in the list.  As simple as that.

Agreed--the rewritten-parent probing being rooted at the merge bases
was not good enough.

> Another thing that I find horribly wrong: there is a "touch
> $REWRITTEN/sha1".  There was a simple design in the beginning: the
> files in $REWRITTEN are actually a mapping from old SHA-1 (file name)
> to new SHA-1 (content).  This was broken, without any good
> explanation.

Perhaps it is not "good", but the explanation a blank REWRITTEN/sha1 is
used a marker during the probe phase that this commit will be rewritten.
So when looking at any of its children commits, they should be rewritten
if a REWRITTEN/parentSha1 exists. Then as the rewriting actually happens,
they get filled in with the new sha1. I cribbed this approach from
Stephan's sequencer rewrite of rebase-i-p.

If you want a different data structure, be it file based, or bash/list
based, or whatever, to track "this commit will eventually be rewritten
but we haven't gotten there yet" during the probe, then we could go back
to leaving REWRITTEN/sha1 alone until after the sha1 commit has been
rebased.

I'm open to suggestions.

Also, as you seem to realize, the current bug stems from not knowing how
to initialize the rewritten data structure. For Sitaram's case, the
first commit is behind any of the merge bases, so marking its parents
(if they exist) as rewritten to ONTO seems reasonable.

If there are no parents, as you point out, I added a "-o sha1 = FIRST"
that should also get the ball rolling. It's another hack, but does this
address your concern until a large refactoring happens?

-------------------------- git-rebase--interactive.sh --------------------------
index c8b0861..8740d9f 100755
@@ -604,11 +604,18 @@ first and then run 'git rebase --continue' again."
 				echo $ONTO > "$REWRITTEN"/$c ||
 					die "Could not init rewritten commits"
 			done
+			# Along with the merge bases, look at the first commit's
+			# parent (which may be before the merge base) and mark it
+			# as rewritten to ONTO
+			FIRST="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
+			for p in $(git rev-list --parents -1 $FIRST | cut -d' ' -f2)
+			do
+				echo $ONTO > "$REWRITTEN/$p"
+			done
 			# No cherry-pick because our first pass is to determine
 			# parents to rewrite and skipping dropped commits would
 			# prematurely end our probe
 			MERGES_OPTION=
-			first_after_upstream="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)"
 		else
 			MERGES_OPTION="--no-merges --cherry-pick"
 		fi
@@ -629,12 +636,12 @@ first and then run 'git rebase --continue' again."
 				preserve=t
 				for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)
 				do
-					if test -f "$REWRITTEN"/$p -a \( $p != $UPSTREAM -o $sha1 = $first_after_upstream \)
+					if test -f "$REWRITTEN"/$p -a $p != $UPSTREAM
 					then
 						preserve=f
 					fi
 				done
-				if test f = "$preserve"
+				if test f = "$preserve" -o $sha1 = $FIRST
 				then
 					touch "$REWRITTEN"/$sha1
 					echo "pick $shortsha1 $rest" >> "$TODO"

(I'm adding the other 3 cc's back after my failed patch attempt
stripped them out--sorry, guys.)

- Stephen