[PATCH 1/3] Fix --strategy parsing in git-rebase--interactive.sh

[PATCH 1/3] Fix --strategy parsing in git-rebase--interactive.sh

[Björn Steinbrink]

For the --strategy/-s option, git-rebase--interactive.sh dropped the
parameter which it was trying to parse.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0dd77b4..f667ae8 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -387,7 +387,6 @@ do
 		output git reset --hard && do_rest
 		;;
 	-s|--strategy)
-		shift
 		case "$#,$1" in
 		*,*=*)
 			STRATEGY="-s `expr "z$1" : 'z-[^=]*=\(.*\)'`" ;;
-- 
1.5.3.4.456.g072a

[PATCH 2/3] git-rebase--interactive.sh: Don't pass a strategy to git-cherry-pick.

[Björn Steinbrink]

git-cherry-pick doesn't support a strategy paramter, so don't pass one.
This means that --strategy for interactive rebases is a no-op for
anything but merge commits, but that's still better than being broken. A
correct fix would probably need to port the --merge behaviour from plain
git-rebase.sh, but I have no clue how to integrate that cleanly.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f667ae8..cc949db 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -116,7 +116,7 @@ pick_one () {
 		sha1=$(git rev-parse --short $sha1)
 		output warn Fast forward to $sha1
 	else
-		output git cherry-pick $STRATEGY "$@"
+		output git cherry-pick "$@"
 	fi
 }
 
@@ -184,7 +184,7 @@ pick_one_preserving_merges () {
 			fi
 			;;
 		*)
-			output git cherry-pick $STRATEGY "$@" ||
+			output git cherry-pick "$@" ||
 				die_with_patch $sha1 "Could not pick $sha1"
 			;;
 		esac
-- 
1.5.3.4.456.g072a

[PATCH 3/3] git-rebase--interactive.sh: Make 3-way merge strategies work for -p.

[Björn Steinbrink]

git-rebase--interactive.sh used to pass all parents of a merge commit to
git-merge, which means that we have at least 3 heads to merge: HEAD,
first parent and second parent. So 3-way merge strategies like recursive
wouldn't work.

Fortunately, we have checked out the first parent right before the merge
anyway, so that is HEAD. Therefore we can drop simply it from the list
of parents, making 3-way strategies work for merge commits with only
two parents.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index cc949db..e63e1c9 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -172,6 +172,9 @@ pick_one_preserving_merges () {
 			author_script=$(get_author_ident_from_commit $sha1)
 			eval "$author_script"
 			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
+
+			# No point in merging the first parent, that's HEAD
+			new_parents=${new_parents# $first_parent}
 			# NEEDSWORK: give rerere a chance
 			if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
 				GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
-- 
1.5.3.4.456.g072a

Re: [PATCH 3/3] git-rebase--interactive.sh: Make 3-way merge strategies work for -p.

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 496 bytes --]

Hi Björn,

On Wed, 31 Oct 2007, Björn Steinbrink wrote:

> +			new_parents=${new_parents# $first_parent}

I wanted to comment that this might not be present in shells other than 
bash, but I see that we rely on that construct already in am, clone, 
commit, filter-branch, merge, pull, rebase, stash and submodule.

So I think it's okay.

The whole series is _very_ nicely done, and appears to me as obviously 
correct.

Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

Thanks,
Dscho

Re: [PATCH 3/3] git-rebase--interactive.sh: Make 3-way merge strategies work for -p.

[Björn Steinbrink]

Hi Dscho,

On 2007.10.31 02:42:03 +0000, Johannes Schindelin wrote:
> Hi Björn,
> 
> On Wed, 31 Oct 2007, Björn Steinbrink wrote:
> 
> > +			new_parents=${new_parents# $first_parent}
> 
> I wanted to comment that this might not be present in shells other than 
> bash, but I see that we rely on that construct already in am, clone, 
> commit, filter-branch, merge, pull, rebase, stash and submodule.
>
> So I think it's okay.

Actually, I checked first that it's not bash-only (I saw the flames
about that on lkml some time ago, so I've been scared ;-)). Works fine
with dash here and it's in the posix sh specs. See 2.6.2 in
http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html

So I hope that there's a bit of a broader support for it :-)

> The whole series is _very_ nicely done, and appears to me as obviously 
> correct.

Thanks :-)

Björn