[PATCH v2] rebase -X: do not clobber strategy

If any strategy options are passed to -X, the strategy will always be
set to 'recursive'. According to the documentation, it should default to
'recursive' if it is not set, but it should be possible to set it to
other values.

Signed-off-by: Martin von Zweigbergk <redacted>
---
 git-rebase.sh |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 42c0628..ec08f9c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh

@@ -312,10 +312,6 @@ do
 		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=*|\

-- 
1.7.3.2.167.ga361b

Heya,

On Wed, Nov 10, 2010 at 08:14, Martin von Zweigbergk
[off-list ref] wrote:

If any strategy options are passed to -X, the strategy will always be
set to 'recursive'. According to the documentation, it should default to
'recursive' if it is not set, but it should be possible to set it to
other values.

Repeat of other threat since this is a new patch: can we have a test for this?

-- 
Cheers,

Sverre Rabbelier

On Wed, Nov 10, 2010 at 1:56 PM, Sverre Rabbelier [off-list ref] wrote:

On Wed, Nov 10, 2010 at 08:14, Martin von Zweigbergk
[off-list ref] wrote:

quoted

If any strategy options are passed to -X, the strategy will always be
set to 'recursive'. According to the documentation, it should default to
'recursive' if it is not set, but it should be possible to set it to
other values.

Repeat of other threat since this is a new patch: can we have a test for this?

I don't think there are any merge strategies other than recursive that
accept options, so what I could add a test case for is that e.g.
'-s ours -X foo' uses the 'ours' strategy, even though 'foo' will be
ignored. I have very little experience with merge strategies, but I will
give it a try. Hopefully there is some existing test case I can copy and
modify.

Btw, why is the default (if no strategy is specifed) for 'git rebase' to
use 'recursive', while for 'git merge' "a built-in list of strategies is
used instead (git merge-recursive when merging a single head, git
merge-octopus otherwise)"?

Martin von Zweigbergk wrote:

Btw, why is the default (if no strategy is specifed) for 'git rebase' to
use 'recursive', while for 'git merge' "a built-in list of strategies is
used instead (git merge-recursive when merging a single head, git
merge-octopus otherwise)"?

Because rebase does a tree-level merge, so it never attempts to merge
than one branch, so octopus never enters the picture.

Sorry for the original breakage; while it has Mike Lundy assigned as
author, I resurrected and resubmitted his patch and should have
noticed.

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

On Thu, Nov 11, 2010 at 5:41 AM, Thomas Rast [off-list ref] wrote:

Martin von Zweigbergk wrote:

quoted

Btw, why is the default (if no strategy is specifed) for 'git rebase' to
use 'recursive', while for 'git merge' "a built-in list of strategies is
used instead (git merge-recursive when merging a single head, git
merge-octopus otherwise)"?

Because rebase does a tree-level merge, so it never attempts to merge
than one branch, so octopus never enters the picture.

I can see why octopus doesn't make sense when doing a linearizing
rebase, but what if it's merge-preserving rebase? The call to
'git merge' in git-rebase--interactive.sh looks like this:
    new_parents=${new_parents# $first_parent}
    [...]
    git merge $strategy -m "$msg" $new_parents

Also, it sounds like "a built-in list" is something that could one day
be expanded. So my question is then whether it would make sense to call
'git merge' without specifying a strategy when 'git rebase' is called
without a strategy?

Sorry for the original breakage; while it has Mike Lundy assigned as
author, I resurrected and resubmitted his patch and should have
noticed.

Well, since there are no other strategies than recursive that support
strategy options, it was not really a breakage in reality, at least as
far as I can see.

Speaking of that and about my earlier comment about writing a test case,
what should really happen if the user calls 'git rebase -s ours -X foo'?
Should it really be allowed? (I tried it and it does work, though.)