[PATCH] rebase: support -X to pass through strategy options

[PATCH] rebase: support -X to pass through strategy options

[Thomas Rast]

From: Mike Lundy <mike@fluffypenguin.org>

git-rebase calls out to merge strategies, but did not support merge
strategy options so far.  Add this, in the same style used in
git-merge.

Sadly we have to do the full quoting/eval dance here, since
merge-recursive supports the --subtree=<path> option which potentially
contains whitespace.

This patch does not cover git rebase -i, which does not call any merge
strategy directly except in --preserve-merges, and even then only for
merges.

Signed-off-by: Mike Lundy <mike@fluffypenguin.org>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Mike posted this back in February but never picked up again after
Junio pointed out that the options can contain spaces.  This is an
attempt to at least finish the job for normal rebase.  Apart from the
whitespace issues, I changed the test to verify that the option
actually takes effect and added docs.


 Documentation/git-rebase.txt |    7 +++++++
 git-rebase.sh                |   23 ++++++++++++++++++++++-
 t/t3402-rebase-merge.sh      |    9 +++++++++
 3 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 41e0604..dc8979e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -249,6 +249,13 @@ on top of the <upstream> branch using the given strategy, using
 the 'ours' strategy simply discards all patches from the <branch>,
 which makes little sense.
 
+-X <strategy-option>::
+--strategy-option=<strategy-option>::
+	Pass the <strategy-option> through to the merge strategy.
+	This implies `\--merge` and, if no strategy has been
+	specified, `-s recursive`.  Note the reversal of 'ours' and
+	'theirs' as noted in above for the `-m` option.
+
 -q::
 --quiet::
 	Be quiet. Implies --no-stat.
diff --git a/git-rebase.sh b/git-rebase.sh
index 694546f..70efbba 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -51,6 +51,7 @@ To restore the original branch and stop rebasing run \"git rebase --abort\".
 "
 unset newbase
 strategy=recursive
+strategy_opts=
 do_merge=
 dotest="$GIT_DIR"/rebase-merge
 prec=4
@@ -119,7 +120,7 @@ call_merge () {
 	then
 		export GIT_MERGE_VERBOSITY=1
 	fi
-	git-merge-$strategy "$cmt^" -- "$hd" "$cmt"
+	eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
 	rv=$?
 	case "$rv" in
 	0)
@@ -300,6 +301,26 @@ do
 	-M|-m|--m|--me|--mer|--merg|--merge)
 		do_merge=t
 		;;
+	-X*|--strategy-option*)
+		case "$#,$1" in
+		1,-X|1,--strategy-option)
+			usage ;;
+		*,-X|*,--strategy-option)
+			newopt="$2"
+			shift ;;
+		*,--strategy-option=*)
+			newopt="$(expr "$1" : '--strategy-option=\(.*\)')" ;;
+		*,-X*)
+			newopt="$(expr "$1" : '-X\(.*\)')" ;;
+		1,*)
+			usage ;;
+		esac
+		strategy_opts="$strategy_opts $(git rev-parse --sq-quote "--$newopt")"
+		do_merge=t
+		if test -n "$strategy"; then
+			strategy=recursive
+		fi
+		;;
 	-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
 		--strateg=*|--strategy=*|\
 	-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
index 7b7d072..2bea656 100755
--- a/t/t3402-rebase-merge.sh
+++ b/t/t3402-rebase-merge.sh
@@ -74,6 +74,15 @@ test_expect_success 'rebase the other way' '
 	git rebase --merge side
 '
 
+test_expect_success 'rebase -Xtheirs' '
+	git checkout -b conflicting master~2 &&
+	echo "AB $T" >> original &&
+	git commit -mconflicting original &&
+	git rebase -Xtheirs master &&
+	grep AB original &&
+	! grep 11 original
+'
+
 test_expect_success 'merge and rebase should match' '
 	git diff-tree -r test-rebase test-merge >difference &&
 	if test -s difference
-- 
1.7.2.350.ga619b6.dirty

Re: [PATCH] rebase: support -X to pass through strategy options

[Junio C Hamano]

Looks good; thanks both.

Re: [PATCH] rebase: support -X to pass through strategy options

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Looks good; thanks both.

I spoke too fast.  You would need something like this on top to avoid
confusing certain implementations of "expr" (e.g. FreeBSD 8.0).

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

diff --git a/git-rebase.sh b/git-rebase.sh
index b1743ea..7d1c5c3 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -302,9 +302,9 @@ do
 			newopt="$2"
 			shift ;;
 		*,--strategy-option=*)
-			newopt="$(expr "$1" : '--strategy-option=\(.*\)')" ;;
+			newopt="$(expr " $1" : ' --strategy-option=\(.*\)')" ;;
 		*,-X*)
-			newopt="$(expr "$1" : '-X\(.*\)')" ;;
+			newopt="$(expr " $1" : ' -X\(.*\)')" ;;
 		1,*)
 			usage ;;
 		esac

Re: [PATCH] rebase: support -X to pass through strategy options

[Thomas Rast]

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Looks good; thanks both.
> 
> I spoke too fast.  You would need something like this on top to avoid
> confusing certain implementations of "expr" (e.g. FreeBSD 8.0).

Annoying, but thanks...  and it turns out this is even in the expr
info manual, so I should have known.  Sigh.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch