Re: [PATCH] rebase-i-p: only list commits that require rewriting in todo

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 20, 2008, 23:36 UTC
Message-ID: <7vej2a3kl5.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20081020115003.GA11309@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

> On Wed, Oct 15, 2008 at 02:44:38AM -0500, Stephen Haberman wrote:
>
>> +					cat "$TODO" | grep -v "${rev:0:7}" > "${TODO}2" ; mv "${TODO}2" "$TODO"
>
> Substring expansion (like ${rev:0:7}) is not portable. At least it
> doesn't work on FreeBSD /bin/sh, and "it's not even in POSIX", I
> believe.

True.

I do not remember the individual patches in the series, but I have to say that the script at the tip of the topic is, eh, less than ideal.

Here is a small untested patch to fix a few issues I spotted while reading it for two minutes.

 * Why filter output from "rev-list --left-right A...B" and look for the
   ones that begin with ">"?  Wouldn't "rev-list A..B" give that?

 * The abbreviated SHA-1 are made with "rev-list --abbrev=7" into $TODO in
   an earlier invocation, and it can be more than 7 letters to avoid
   ambiguity.  Not just that "${r:0:7} is not even in POSIX", but use of
   it here is actively wrong.

 * There is no point in catting a single file and piping it into grep.

 git-rebase--interactive.sh |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git i/git-rebase--interactive.sh w/git-rebase--interactive.sh
index 848fbe7..a563dea 100755
--- i/git-rebase--interactive.sh
+++ w/git-rebase--interactive.sh
@@ -635,8 +635,8 @@ first and then run 'git rebase --continue' again."
 				sed -n "s/^>//p" > "$DOTEST"/not-cherry-picks
 			# Now all commits and note which ones are missing in
 			# not-cherry-picks and hence being dropped
-			git rev-list $UPSTREAM...$HEAD --left-right | \
-				sed -n "s/^>//p" | while read rev
+			git rev-list $UPSTREAM..$HEAD |
+			while read rev
 			do
 				if test -f "$REWRITTEN"/$rev -a "$(grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
 				then
@@ -645,7 +645,8 @@ first and then run 'git rebase --continue' again."
 					# just the history of its first-parent for others that will
 					# be rebasing on top of it
 					git rev-list --parents -1 $rev | cut -d' ' -f2 > "$DROPPED"/$rev
-					cat "$TODO" | grep -v "${rev:0:7}" > "${TODO}2" ; mv "${TODO}2" "$TODO"
+					short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
+					grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
 					rm "$REWRITTEN"/$rev
 				fi
 			done