[0/5] Handle conflicting pull options

Message

Philippe Blain via GitGitGadget July 15, 2021, 2:40 a.m. UTC

We've recently discussed how to handle conflicting configuration and command
line options in git pull, including at least a few different
proposals[1][2][3] to handle different subsets of the possibilities. We also
have a user report from someone who had conflicting configuration and got
surprised when one of the options overruled the other -- with no warning
from the command or the documentation that such would happen. Here's my
attempt to impose clear and simple rules, which can be seen in the commit
message of the third patch.

(The first two patches are just preparatory changes to make patch 3 easier
to read.)

Since the handling of conflicting options was holding up two of Alex's
patches[4][5], I also include those two patches at the end of my series,
though I've made quite a few changes and additions to the latter of those.

Possible areas of concern:

 * Documentation/git-pull.txt includes merge-options.txt. While git-rebase
   supports many of those "merge" options, I suspect there are others that
   it does not support. We are probably silently ignoring those unsupported
   options whenever one of those is specified at the same time a rebase is
   requested; we should instead likely error out and report the
   incompatibility. I have not yet addressed that, as I was focused on the
   main rebase vs. merge incompatibility and default warning/error
   associated with it.

[1]
https://lore.kernel.org/git/[e-mail]/
[2] https://lore.kernel.org/git/xmqq8s2b489p.fsf@gitster.g/ [3]
https://lore.kernel.org/git/[e-mail]/
[4]
https://lore.kernel.org/git/[e-mail]/
[5]
https://lore.kernel.org/git/[e-mail]/

Alex Henrie (1):
  pull: abort if --ff-only is given and fast-forwarding is impossible

Elijah Newren (4):
  pull: move definitions of parse_config_rebase and parse_opt_rebase
  pull: convert OPT_PASSTHRU for fast-forward options to OPT_CALLBACK
  pull: handle conflicting rebase/merge options via last option wins
  pull: abort by default when fast-forwarding is not possible

 Documentation/config/pull.txt |   3 +-
 Documentation/git-pull.txt    |  19 +++--
 advice.c                      |   5 ++
 advice.h                      |   1 +
 builtin/merge.c               |   2 +-
 builtin/pull.c                | 144 ++++++++++++++++++++--------------
 t/t4013-diff-various.sh       |   2 +-
 t/t5520-pull.sh               |  20 ++---
 t/t5521-pull-options.sh       |   4 +-
 t/t5524-pull-msg.sh           |   4 +-
 t/t5553-set-upstream.sh       |  14 ++--
 t/t5604-clone-reference.sh    |   4 +-
 t/t6402-merge-rename.sh       |  18 ++---
 t/t6409-merge-subtree.sh      |   6 +-
 t/t6417-merge-ours-theirs.sh  |  10 +--
 t/t7601-merge-pull-config.sh  |  97 ++++++++++++++++++++++-
 t/t7603-merge-reduce-heads.sh |   2 +-
 17 files changed, 242 insertions(+), 113 deletions(-)


base-commit: 75ae10bc75336db031ee58d13c5037b929235912
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1047%2Fnewren%2Fhandle-conflicting-pull-options-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1047/newren/handle-conflicting-pull-options-v1
Pull-Request: https://github.com/git/git/pull/1047

Comments

Felipe Contreras July 15, 2021, 9:37 a.m. UTC | #1

Elijah Newren via GitGitGadget wrote:
> We've recently discussed how to handle conflicting configuration and command
> line options in git pull, including at least a few different
> proposals[1][2][3] to handle different subsets of the possibilities. We also
> have a user report from someone who had conflicting configuration and got
> surprised when one of the options overruled the other -- with no warning
> from the command or the documentation that such would happen.

No, that's not what happened, you are assuming wrongly.

It's perfectly fine to have pull.ff and pull.rebase, because the former
will be honored when running `git pull --merge`, as I already explained
[1].

[1] http://lore.kernel.org/git/[e-mail]