Alex Henrie wrote:
> The behavior of warning but merging anyway is difficult to explain to
> users because it sends mixed signals. End the confusion by firmly
> requiring the user to specify whether they want a merge or a rebase.
When were users warned about this change? Generally before making such
big UI changes to core commands there needs to be a deprecation period
before pulling the trigger.
Second, supposing this was a proposal to add such warning, we would need
a configuration so the user can decide to retain the old behavior, or
move to the new one. What is that configuration?
How can a user choose to enable the new behavior in v2.32 (or any other
version)?
Also, a bunch of tests are broken after this change:
t4013-diff-various.sh
t5521-pull-options.sh
t5524-pull-msg.sh
t5520-pull.sh
t5553-set-upstream.sh
t5604-clone-reference.sh
t6409-merge-subtree.sh
t6402-merge-rename.sh
t6417-merge-ours-theirs.sh
t7601-merge-pull-config.sh
t7603-merge-reduce-heads.sh
If you didn't mean this patch to be applied then perhaps add the RFC
prefix.
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -16,13 +16,17 @@ DESCRIPTION
> -----------
>
> Incorporates changes from a remote repository into the current
> -branch. In its default mode, `git pull` is shorthand for
> -`git fetch` followed by `git merge FETCH_HEAD`.
> +branch. `git pull` is shorthand for `git fetch` followed by
> +`git merge FETCH_HEAD` or `git rebase FETCH_HEAD`.
Is it?
If I have a pull.rebase=merges is `git pull` the same as doing `git
rebase FETCH_HEAD`?
> More precisely, 'git pull' runs 'git fetch' with the given
> -parameters and calls 'git merge' to merge the retrieved branch
> -heads into the current branch.
> -With `--rebase`, it runs 'git rebase' instead of 'git merge'.
> +parameters and then, by default, attempts to fast-forward the
> +current branch to the remote branch head.
OK.
> +If fast-forwarding
> +is not possible because the local and remote branches have
> +diverged, the `--rebase` option causes 'git rebase' to be
> +called to rebase the local branch upon the remote branch,
Does --rebase only work when a fast-forward isn't possible.
> +and
> +the `--no-rebase` option causes 'git merge' to be called to
> +merge the retrieved branch heads into the current branch.
Isn't merge called when a fast-forward is possible?
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -925,20 +925,20 @@ static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_
> return ret;
> }
>
> -static void show_advice_pull_non_ff(void)
> +static void die_pull_non_ff(void)
> {
> - advise(_("Pulling without specifying how to reconcile divergent branches is\n"
> - "discouraged. You can squelch this message by running one of the following\n"
> - "commands sometime before your next pull:\n"
> - "\n"
> - " git config pull.rebase false # merge (the default strategy)\n"
> - " git config pull.rebase true # rebase\n"
> - " git config pull.ff only # fast-forward only\n"
> - "\n"
> - "You can replace \"git config\" with \"git config --global\" to set a default\n"
> - "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
> - "or --ff-only on the command line to override the configured default per\n"
> - "invocation.\n"));
> + die(_("Pulling without specifying how to reconcile divergent branches is not\n"
> + "possible. You can squelch this message by running one of the following\n"
> + "commands sometime before your next pull:\n"
Is squelching this message really what we are aming for?
> + " git config pull.rebase false # merge\n"
> + " git config pull.rebase true # rebase\n"
> + " git config pull.ff only # fast-forward only\n"
`git config pull.ff only` will get rid of this messge, but the pull
would still fail, only that it will be a different message:
fatal: Not possible to fast-forward, aborting.
Doesn't seem very user-friendly.
> + "You can replace \"git config\" with \"git config --global\" to set a default\n"
> + "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
> + "or --ff-only on the command line to override the configured default per\n"
> + "invocation.\n"));
Can I?
git -c pull.rebase=true pull --ff-only
`--ff-only` doesn't seem to be overriding the configuration.
In addition to all the issues I've already pointed out, the message
seems rather big for a die().
I think this is close to the end-goal we should be aiming for, but
there's a lot that is missing before we should even consider flipping
the switch.
Cheers.
--
Felipe Contreras
