Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 12, 2013, 06:46 UTC
Message-ID: <7vpptjz9r1.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1376288518-4875-1-git-send-email-stephen@exigencecorp.com>

Stephen Haberman <stephen@exigencecorp.com> writes:

> I assume I'm doing the right thing by just posting another version of
> this patch to the git list; let me know if I'm missing something.

Thanks. Writing the "story so far..." summary like you did after the three-dash line was very helpful.

Show 10 quoted lines

> diff --git a/git-pull.sh b/git-pull.sh
> index f0df41c..6ae6640 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -4,7 +4,7 @@
>  #
>  # Fetch one or more remote refs and merge it/them into the current HEAD.
>  
> -USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff] [-s strategy]... [<fetch-options>] <repo> <head>...'
> +USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff] [-r=[true|false|preserve]] [-s strategy]... [<fetch-options>] <repo> <head>...'

"-r", even though it happens to be accepted, is not a good idea here, as there are other --r* commands that are not --rebase.

	[--[no-]rebase|--rebase=preserve]

would be better.

Show 14 quoted lines

> @@ -40,13 +40,13 @@ test -f "$GIT_DIR/MERGE_HEAD" && die_merge
>  
>  strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
>  log_arg= verbosity= progress= recurse_submodules= verify_signatures=
> -merge_args= edit=
> +merge_args= edit= rebase_args=
>  curr_branch=$(git symbolic-ref -q HEAD)
>  curr_branch_short="${curr_branch#refs/heads/}"
> -rebase=$(git config --bool branch.$curr_branch_short.rebase)
> +rebase=$(git config branch.$curr_branch_short.rebase)
>  if test -z "$rebase"
>  then
> -	rebase=$(git config --bool pull.rebase)
> +	rebase=$(git config pull.rebase)

This is a grave regression (the same for the earlier one that reads the branch.*.rebase configuraiton). Those who did any of the following will be broken:

	[pull]
		;; any of the following are "true"
        	rebase
                rebase = yes
                rebase = 1
                ;; any of the following are "false"
                rebase = no
                rebase = 0

You would want "bool or string" helper function to parse it correctly, something like:

	bool_or_string_config () {
        	git config --bool "$1" 2>/dev/null || git config "$1"
	}

	rebase=$(boo_or_string_config pull.rebase)