threads / patch / 10535

patch, 3 partsgit-rebase--interactive.sh: Don't pass a strategy to git-cherry-pick.

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

## tl;dr

5 messages between Oct 31, 2007 and Oct 31, 2007. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Björn Steinbrink· Oct 31, 2007, 02:20 UTC · lore

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

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>
Show changes to git-rebase--interactive.sh +0 −1
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
Björn Steinbrink· Oct 31, 2007, 02:20 UTC · re: Björn Steinbrink · lore

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>
Show changes to git-rebase--interactive.sh +2 −2
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
Björn Steinbrink· Oct 31, 2007, 02:20 UTC · re: Björn Steinbrink · lore

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

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>
Show changes to git-rebase--interactive.sh +3 −0
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
Johannes Schindelin· Oct 31, 2007, 02:42 UTC · re: Björn Steinbrink · lore

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

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

Björn Steinbrink· Oct 31, 2007, 02:53 UTC · re: Johannes Schindelin · lore

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

Hi Dscho,
On 2007.10.31 02:42:03 +0000, Johannes Schindelin wrote:
Show 11 quoted lines
> 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

← back to recent threads