Re: [PATCH v5 0/6] Reject non-ff pulls by default

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> These are the steps needed to achieve this:

The overall progression (this comment is only about the design, not
the implementation) looks almost sensible, but I may have missed
some issues because the presentation was done in reverse.

In the following comment, I'll flip the presentation order to better
show natural progression of what the users will see.

> 1) Rename pull.rename to pull.mode and
>    branch.<name>.rebase to branch.<name>.pullmode
>
> This way the configurations and options remain consistent:
>
>   git pull --merge
>   pull.mode = merge
>   branch.<name>.pullmode = merge
>
>   git pull --rebase
>   pull.mode = rebase
>   branch.<name>.pullmode = rebase
>
>   git pull --rebase=preserve
>   pull.mode = rebase-preserve
>   branch.<name>.pullmode = rebase-preserve
>
>   git pull
>   pull.mode = merge-ff-only
>   branch.<name>.pullmode = merge-ff-only

Until the "--merge" option is added, "pull.mode = merge" cannot be
the same as "git pull --merge".  I think you either need to squash
these two steps into one, or flip the order of them.

> 2) Add --merge option
>
> Since we have a message that says "If unsure, run 'git pull --merge'", which is
> more friendly than 'git pull --no-rebase', we should add this option, and
> deprecate --no-rebase.

Obviously s/have a/will have a/, but the intention is good.

> However, the documentation would become confusing if --merge is configured in
> pull.rebase, instead, we want something like this:
>
>   See `pull.mode`, `branch.<name>.pullmode` in linkgit:git-config[1] if you want
>   to make `git pull` always use `--merge`.

It gets unclear to me how the transition is planned around here.  Is
this a correct paraphrasing of these four steps?

    - Add pull.mode (and its branch-specific friend) and "pull
      --merge" so that people can set the former to "merge" or train
      their fingers to type the latter to keep doing the
      fetch-and-merge (your steps 1 and 2)

    - Add ff-only to pull.mode (your step 3)

    - With the endgame of "out of box Git without any configuration
      refuses 'git pull' (without --merge/--rebase) that does not
      fast forward" in mind, start warning "In the future you will
      have to either set pull.mode (and/or its friends) or type
      "pull --merge" (or "pull --rebase") when the endgame version
      of 'git pull' would fail with the error message, but still do
      as was asked to do as before.  At this step, existing users
      can set pull.mode to "merge" or "rebase" or whatever to
      squelch the warning.

    - Flip the default.  By the time this happens, thanks to the
      previous step to warn beforehand, nobody needs to see the
      warning. (your step 4)

If that is the rough outline, I think it is sensible.

> 3) Add merge-ff-only config
>
> This option would trigger a check inside `git pull` itself, and error out with
> the aforementioned message if it's not possible to do a fast-forward merge.
>
> However, this option conflicts with --rebase, and --no-rebase. Solution below.

Am I reading you correctly that setting "pull.mode = ff-only" will
require you to explicitly say "git pull --merge/--rebase"?  If that
is the case, I think the step makes sense.

> 4) Only allow fast-forward merges by default
>
> We could pass --ff-only to `git merge`, however, if we do that we'll get an error like this:
>
>   Not possible to fast-forward, aborting.
>
> This is not friendly; we want an error that is user-friendly:
>
>   The pull was not fast-forward, please either merge or rebase.
>   If unsure, run 'git pull --merge'.
>
> When we do this we want to give the users the option to go back to the previous
> behavior, so a new configuration is needed.