Re: [PATCH 0/3] Reject non-ff pulls by default
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 5, 2013, 23:38 UTC
- Message-ID
- <xmqq7geug7oy.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <xmqqr4d4jird.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 24 quoted lines
> I can imagine users might want to say "when I am on these small > number of branches, I want to merge (or rebase), but when I am on > other, majority of my branches, because they are private, unfinished > and unpublished work, please stop me from accidentally messing their > histories with changes from upstream or anywhere else for that > matter". If that is the issue you are trying to raise, because > there is no > > [pull] > rebase = fail > [branch "master"] > rebase = yes > > to force "git pull" to fail by default on any branch while allowing > it to rebase (or merge, for that matter) only on a few selected > branches, we fall a bit short. > > Which can be solved by adding the above "fail" option, and then > renaming them to "pull.integrate" and "branch.<name>.integrate" to > clarify what these variables are about (it is no longer "do you > rebase or not---if you choose not to rebase, by definition you are > going to merge", as there is a third choice to "fail"), while > retaining "pull.rebase" and "branch.<name>.rebase" as a deprecated > synonym.
The first step of such an enhancement may look like this patch. It introduces "pull.integrate" and "branch.<name>.integrate" that will eventually deprecate "*.rebase", but at this step only supports values "rebase" and "merge" (i.e. no "fail" yet).
The steps after this change would be to
* Enhance addition to t5520 made by 949e0d8e (pull: require choice between rebase/merge on non-fast-forward pull, 2013-06-27) to make sure that setting pull.integrate and branch.<name>.integrate will squelch the safety added by that patch;
* Teach "branch.c" to set "branch.<name>.integrate" either instead of or in addition to "branch.<name>.rebase", and adjust tests that expect to see "branch.<name>.rebase" to expect to see that "branch.<name>.integrate" is set to "rebase";
* Add "fail" to the set of valid values for "*.integrate", and teach "git pull" honor it; and
* Update builtin/remote.c to show cases where branch.<name>.integrate is set to "fail" in some different way.
I do not plan to do these follow-up steps myself soonish (hint, hint).
builtin/remote.c | 12 ++++++++++-- git-pull.sh | 60 +++++++++++++++++++++++++++++++++++++------------------- 2 files changed, 50 insertions(+), 22 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c index 5e54d36..d3b6d0b5 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -274,7 +274,7 @@ static int config_read_branches(const char *key, const char *value, void *cb) char *name; struct string_list_item *item; struct branch_info *info; - enum { REMOTE, MERGE, REBASE } type; + enum { REMOTE, MERGE, REBASE, INTEGRATE } type; key += 7; if (!postfixcmp(key, ".remote")) { @@ -286,6 +286,9 @@ static int config_read_branches(const char *key, const char *value, void *cb) } else if (!postfixcmp(key, ".rebase")) { name = xstrndup(key, strlen(key) - 7); type = REBASE; + } else if (!postfixcmp(key, ".integrate")) { + name = xstrndup(key, strlen(key) - 10); + type = INTEGRATE; } else return 0; @@ -309,8 +312,13 @@ static int config_read_branches(const char *key, const char *value, void *cb) space = strchr(value, ' '); } string_list_append(&info->merge, xstrdup(value)); - } else + } else if (type == REBASE) { info->rebase = git_config_bool(orig_key, value); + } else if (type == INTEGRATE) { + if (!value) + return config_error_nonbool(orig_key); + info->rebase = !strcmp(value, "rebase"); + } } return 0; } diff --git a/git-pull.sh b/git-pull.sh index 88c198f..5c557b7 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -45,16 +45,34 @@ merge_args= edit= curr_branch=$(git symbolic-ref -q HEAD) curr_branch_short="${curr_branch#refs/heads/}" -# See if we are configured to rebase by default. -# The value $rebase is, throughout the main part of the code: +# See what we are configured to do by default. +# The value $integration is, throughout the main part of the code: # (empty) - the user did not have any preference -# true - the user told us to integrate by rebasing -# false - the user told us to integrate by merging -rebase=$(git config --bool branch.$curr_branch_short.rebase) -if test -z "$rebase" -then - rebase=$(git config --bool pull.rebase) -fi +# rebase - the user told us to integrate by rebasing +# merge - the user told us to integrate by merging + +integration= + +set_integration () { + integration=$(git config branch.$curr_branch_short.integrate) + test -n "$integration" && return + + case "$(git config --bool branch.$curr_branch_short.rebase)" in + true) integration=rebase ;; + false) integration=merge ;; + esac + test -n "$integration" && return + + integration=$(git config pull.integrate) + test -n "$integration" && return + + case "$(git config --bool pull.rebase)" in + true) integration=rebase ;; + false) integration=merge ;; + esac +} + +set_integration dry_run= while : @@ -119,11 +137,11 @@ do merge_args="$merge_args$xx " ;; -r|--r|--re|--reb|--reba|--rebas|--rebase) - rebase=true + integration=rebase ;; --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase|\ -m|--m|--me|--mer|--merg|--merge) - rebase=false + integration=merge ;; --recurse-submodules) recurse_submodules=--recurse-submodules @@ -166,7 +184,7 @@ error_on_no_merge_candidates () { esac done - if test true = "$rebase" + if test "$integration" = rebase then op_type=rebase op_prep=against @@ -180,7 +198,8 @@ error_on_no_merge_candidates () { remote=$(git config "branch.$curr_branch.remote") if [ $# -gt 1 ]; then - if [ "$rebase" = true ]; then + if test "$integration" = rebase + then printf "There is no candidate for rebasing against " else printf "There are no candidates for merging " @@ -203,7 +222,8 @@ error_on_no_merge_candidates () { exit 1 } -test true = "$rebase" && { +if test "$integration" = rebase +then if ! git rev-parse -q --verify HEAD >/dev/null then # On an unborn branch @@ -227,7 +247,7 @@ test true = "$rebase" && { break fi done -} +fi orig_head=$(git rev-parse -q --verify HEAD) git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1 @@ -269,7 +289,7 @@ case "$merge_head" in then die "$(gettext "Cannot merge multiple branches into empty head")" fi - if test true = "$rebase" + if test "$integration" = rebase then die "$(gettext "Cannot rebase onto multiple branches")" fi @@ -279,7 +299,7 @@ case "$merge_head" in # trigger this check when we will say "fast-forward" or "already # up-to-date". merge_head=${merge_head% } - if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" && + if test -z "$integration$no_ff$ff_only${squash#--no-squash}" && test -n "$orig_head" && test $# = 0 && ! git merge-base --is-ancestor "$orig_head" "$merge_head" && @@ -311,7 +331,7 @@ then exit fi -if test true = "$rebase" +if test "$integration" = rebase then o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref) if test "$oldremoteref" = "$o" @@ -321,8 +341,8 @@ then fi merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit -case "$rebase" in -true) +case "$integration" in +rebase) eval="git-rebase $diffstat $strategy_args $merge_args $verbosity" eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}" ;;