threads / patch / 24563

patchrebase: support -X to pass through strategy options

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

## tl;dr

4 messages between Jul 28, 2010 and Aug 4, 2010. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Thomas Rast· Jul 28, 2010, 22:04 UTC · lore
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(-)
Show changes to 3 files +38 −1

Documentation/git-rebase.txt, git-rebase.sh, t/t3402-rebase-merge.sh

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
Junio C Hamano· Aug 3, 2010, 23:07 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to git-rebase.sh +2 −2
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
Thomas Rast· Aug 4, 2010, 09:51 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 6 quoted lines
> 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

← back to recent threads