# [PATCH 2/3] git-rebase--interactive.sh: Don't pass a strategy to git-cherry-pick. 5 messages from 2007-10-31 to 2007-10-31. Participants: Björn Steinbrink, Johannes Schindelin. Thread: https://gitlist.dev/t/10535 ## Björn Steinbrink, 2007-10-31 02:20 Subject: [PATCH 1/3] Fix --strategy parsing in git-rebase--interactive.sh Message-ID: <1193797232-1062-1-git-send-email-B.Steinbrink@gmx.de> URL: https://gitlist.dev/e/1193797232-1062-1-git-send-email-B.Steinbrink%40gmx.de ``` For the --strategy/-s option, git-rebase--interactive.sh dropped the parameter which it was trying to parse. Signed-off-by: Björn Steinbrink 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, 2007-10-31 02:20 Subject: [PATCH 2/3] git-rebase--interactive.sh: Don't pass a strategy to git-cherry-pick. Message-ID: <1193797232-1062-2-git-send-email-B.Steinbrink@gmx.de> URL: https://gitlist.dev/e/1193797232-1062-2-git-send-email-B.Steinbrink%40gmx.de In-Reply-To: <1193797232-1062-1-git-send-email-B.Steinbrink@gmx.de> ``` 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 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, 2007-10-31 02:20 Subject: [PATCH 3/3] git-rebase--interactive.sh: Make 3-way merge strategies work for -p. Message-ID: <1193797232-1062-3-git-send-email-B.Steinbrink@gmx.de> URL: https://gitlist.dev/e/1193797232-1062-3-git-send-email-B.Steinbrink%40gmx.de In-Reply-To: <1193797232-1062-2-git-send-email-B.Steinbrink@gmx.de> ``` 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 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, 2007-10-31 02:42 Subject: Re: [PATCH 3/3] git-rebase--interactive.sh: Make 3-way merge strategies work for -p. Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0710310239100.4362%40racer.site In-Reply-To: <1193797232-1062-3-git-send-email-B.Steinbrink@gmx.de> ``` 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 Thanks, Dscho ``` ## Björn Steinbrink, 2007-10-31 02:53 Subject: Re: [PATCH 3/3] git-rebase--interactive.sh: Make 3-way merge strategies work for -p. Message-ID: <20071031025320.GA1375@atjola.homenet> URL: https://gitlist.dev/e/20071031025320.GA1375%40atjola.homenet In-Reply-To: ``` 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 ```