[PATCH v2 0/8] Handle pull option precedence
To
git@vger.kernel.org
Cc
Alex Henrie
Son Luong Ngoc
Matthias Baumgarten
Eric Sunshine
Ævar Arnfjörð Bjarmason
Elijah Newren
Elijah Newren
From
Elijah Newren via GitGitGadget
See Also
Prev
Date
2021-07-21 01:42:16 UTC
Based on a recent list of rules for flag/option precedence for git-pull[1]
from Junio (particularly focusing on rebase vs. merge vs. fast-forward),
here's an attempt to implement and document it. Given multiple recent
surprises from users about some of these behaviors[2][3] and a coworker just
yesterday expressing some puzzlement with git-pull and rebase vs. merge, it
seems like a good time to address some of these issues.

Since the handling of conflicting options was holding up two of Alex's
patches[4][5], and his patches fix some of the tests, I also include those
two patches in my series, with a few small changes to the first (so I've
kept him as author) and more substantial changes to the second (so I've
given him an Initial-patch-by attribution).

Changes since v1:

 * Rebased on latest master (resolved a simple conflict with
   dd/test-stdout-count-lines)
 * Patch 1: based on feedback from Junio, fixed some style issues, clarified
   function names, added a few new tests, and took a stab at fixing up the
   comments and test descriptions (but still unsure if I hit the mark on the
   last point)
 * Patch 2: changed the test expectations for one of the multiple head tests
   as per Junio's suggestion, and made one of the other tests expect a more
   specific error message
 * Patches 4 & 5 were squashed and fixed: these now address a submodule bug
   interaction with --ff-only
 * Old patch 6 (now 5): added a code comment explaining a subtle point
 * Old patch 8 (now 7): a few more documentation updates, especially making
   --ff-only not sound merge-specific
 * Old patch 9 (now 8): Updates for new test expectation from patch 2

Quick overview:

 * Patches 1-2: new testcases (see the commit messages for the rules)
 * Patch 3: Alex's recent patch (abort if --ff-only but can't do so)
 * Patches 4-6: fix the precedence parts Alex didn't cover
 * Patch 7: Alex's other patch, abort if rebase vs. merge not specified
 * Patch 8: Compatibility of git-pull with merge-options.txt (think
   rebasing)
 * Patch 9: Fix multiple heads handling too

[1] https://lore.kernel.org/git/xmqqwnpqot4m.fsf@gitster.g/ [2]
https://lore.kernel.org/git/CAL3xRKdOyVWvcLXK7zoXtFPiHBjgL24zi5hhg+3yjowwSUPgmg@mail.gmail.com/
[3]
https://lore.kernel.org/git/c62933fb-96b2-99f5-7169-372f486f6e39@aixigo.com/
[4]
https://lore.kernel.org/git/20210711012604.947321-1-alexhenrie24@gmail.com/
[5]
https://lore.kernel.org/git/20210627000855.530985-1-alexhenrie24@gmail.com/

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

Elijah Newren (7):
  t7601: test interaction of merge/rebase/fast-forward flags and options
  t7601: add tests of interactions with multiple merge heads and config
  pull: since --ff-only overrides, handle it first
  pull: make --rebase and --no-rebase override pull.ff=only
  pull: abort by default when fast-forwarding is not possible
  pull: update docs & code for option compatibility with rebasing
  pull: fix handling of multiple heads

 Documentation/git-merge.txt     |   2 +
 Documentation/git-pull.txt      |  21 +--
 Documentation/merge-options.txt |  40 ++++++
 advice.c                        |   5 +
 advice.h                        |   1 +
 builtin/merge.c                 |   2 +-
 builtin/pull.c                  |  63 ++++++---
 t/t4013-diff-various.sh         |   2 +-
 t/t5520-pull.sh                 |  26 ++--
 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    | 244 +++++++++++++++++++++++++++++---
 t/t7603-merge-reduce-heads.sh   |   2 +-
 18 files changed, 375 insertions(+), 93 deletions(-)


base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1049%2Fnewren%2Fhandle-pull-option-precedence-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1049/newren/handle-pull-option-precedence-v2
Pull-Request: https://github.com/git/git/pull/1049

Range-diff vs v1:

  1:  6cb771297f5 !  1:  17560927211 t7601: add relative precedence tests for merge and rebase flags/options
     @@ Metadata
      Author: Elijah Newren <newren@gmail.com>
      
       ## Commit message ##
     -    t7601: add relative precedence tests for merge and rebase flags/options
     +    t7601: test interaction of merge/rebase/fast-forward flags and options
      
          The interaction of rebase and merge flags and options was not well
          tested.  Add several tests to check for correct behavior from the
          following rules:
     -        * --ff-only takes precedence over --[no-]rebase
     -          * Corollary: pull.ff=only overrides pull.rebase
     -        * --rebase[=!false] takes precedence over --no-ff and --ff
     -          * Corollary: pull.rebase=!false overrides pull.ff=!only
     +        * --ff-only vs. --[no-]rebase
     +          (and the related pull.ff=only vs. pull.rebase)
     +        * --rebase[=!false] vs. --no-ff and --ff
     +          (and the related pull.rebase=!false overrides pull.ff=!only)
              * command line flags take precedence over config, except:
                * --no-rebase heeds pull.ff=!only
     -          * pull.rebase=!false takes precedence over --no-ff and --ff
     +          * pull.rebase=!false vs --no-ff and --ff
      
          For more details behind these rules and a larger table of individual
          cases, refer to https://lore.kernel.org/git/xmqqwnpqot4m.fsf@gitster.g/
     @@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase not set and --ff-
       	test_i18ngrep ! "Pulling without specifying how to reconcile" err
       '
       
     -+test_does_rebase() {
     ++test_does_rebase () {
      +	git reset --hard c2 &&
      +	git "$@" . c1 &&
      +	# Check that we actually did a rebase
     @@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase not set and --ff-
      +	rm actual expect
      +}
      +
     -+test_does_merge_noff() {
     ++# Prefers merge over fast-forward
     ++test_does_merge_when_ff_possible () {
      +	git reset --hard c0 &&
      +	git "$@" . c1 &&
      +	# Check that we actually did a merge
     @@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase not set and --ff-
      +	rm actual expect
      +}
      +
     -+test_does_merge_ff() {
     ++# Prefers fast-forward over merge or rebase
     ++test_does_fast_forward () {
      +	git reset --hard c0 &&
      +	git "$@" . c1 &&
     -+	# Check that we actually did a merge
     ++
     ++	# Check that we did not get any merges
      +	git rev-list --count HEAD >actual &&
      +	git rev-list --merges --count HEAD >>actual &&
      +	test_write_lines 2 0 >expect &&
      +	test_cmp expect actual &&
     ++
     ++	# Check that we ended up at c1
     ++	git rev-parse HEAD >actual &&
     ++	git rev-parse c1^{commit} >expect &&
     ++	test_cmp actual expect &&
     ++
     ++	# Remove temporary files
      +	rm actual expect
      +}
      +
     -+test_does_need_full_merge() {
     ++# Doesn't fail when fast-forward not possible; does a merge
     ++test_falls_back_to_full_merge () {
      +	git reset --hard c2 &&
      +	git "$@" . c1 &&
      +	# Check that we actually did a merge
     @@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase not set and --ff-
      +	rm actual expect
      +}
      +
     -+test_attempts_fast_forward() {
     ++# Attempts fast forward, which is impossible, and bails
     ++test_attempts_fast_forward () {
      +	git reset --hard c2 &&
      +	test_must_fail git "$@" . c1 2>err &&
      +	test_i18ngrep "Not possible to fast-forward, aborting" err
      +}
      +
      +#
     -+# Rule 1: --ff-only takes precedence over --[no-]rebase
     -+# (Corollary: pull.ff=only overrides pull.rebase)
     ++# Group 1: Interaction of --ff-only with --[no-]rebase
     ++# (And related interaction of pull.ff=only with pull.rebase)
      +#
     -+test_expect_failure '--ff-only takes precedence over --rebase' '
     ++test_expect_failure '--ff-only overrides --rebase' '
      +	test_attempts_fast_forward pull --rebase --ff-only
      +'
      +
     -+test_expect_failure '--ff-only takes precedence over --rebase even if first' '
     ++test_expect_failure '--ff-only overrides --rebase even if first' '
      +	test_attempts_fast_forward pull --ff-only --rebase
      +'
      +
     -+test_expect_success '--ff-only takes precedence over --no-rebase' '
     ++test_expect_success '--ff-only overrides --no-rebase' '
      +	test_attempts_fast_forward pull --ff-only --no-rebase
      +'
      +
     @@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase not set and --ff-
      +	test_attempts_fast_forward -c pull.ff=only -c pull.rebase=false pull
      +'
      +
     -+# Rule 2: --rebase=[!false] takes precedence over --no-ff and --ff
     -+# (Corollary: pull.rebase=!false overrides pull.ff=!only)
     -+test_expect_success '--rebase takes precedence over --no-ff' '
     ++# Group 2: --rebase=[!false] overrides --no-ff and --ff
     ++# (And related interaction of pull.rebase=!false and pull.ff=!only)
     ++test_expect_success '--rebase overrides --no-ff' '
      +	test_does_rebase pull --rebase --no-ff
      +'
      +
     -+test_expect_success '--rebase takes precedence over --ff' '
     ++test_expect_success '--rebase overrides --ff' '
      +	test_does_rebase pull --rebase --ff
      +'
      +
     -+test_expect_success 'pull.rebase=true takes precedence over pull.ff=false' '
     ++test_expect_success '--rebase fast-forwards when possible' '
     ++	test_does_fast_forward pull --rebase --ff
     ++'
     ++
     ++test_expect_success 'pull.rebase=true overrides pull.ff=false' '
      +	test_does_rebase -c pull.rebase=true -c pull.ff=false pull
      +'
      +
     -+test_expect_success 'pull.rebase=true takes precedence over pull.ff=true' '
     ++test_expect_success 'pull.rebase=true overrides pull.ff=true' '
      +	test_does_rebase -c pull.rebase=true -c pull.ff=true pull
      +'
      +
     -+# Rule 3: command line flags take precedence over config
     ++# Group 3: command line flags take precedence over config
      +test_expect_failure '--ff-only takes precedence over pull.rebase=true' '
      +	test_attempts_fast_forward -c pull.rebase=true pull --ff-only
      +'
     @@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase not set and --ff-
      +	test_attempts_fast_forward -c pull.rebase=false pull --ff-only
      +'
      +
     -+test_expect_failure '--no-rebase overrides pull.ff=only' '
     -+	test_does_need_full_merge -c pull.ff=only pull --no-rebase
     ++test_expect_failure '--no-rebase takes precedence over pull.ff=only' '
     ++	test_falls_back_to_full_merge -c pull.ff=only pull --no-rebase
      +'
      +
      +test_expect_success '--rebase takes precedence over pull.ff=only' '
      +	test_does_rebase -c pull.ff=only pull --rebase
      +'
      +
     -+test_expect_success '--rebase takes precedence over pull.ff=true' '
     ++test_expect_success '--rebase overrides pull.ff=true' '
      +	test_does_rebase -c pull.ff=true pull --rebase
      +'
      +
     -+test_expect_success '--rebase takes precedence over pull.ff=false' '
     ++test_expect_success '--rebase overrides pull.ff=false' '
      +	test_does_rebase -c pull.ff=false pull --rebase
      +'
      +
     -+test_expect_success '--rebase takes precedence over pull.ff unset' '
     ++test_expect_success '--rebase overrides pull.ff unset' '
      +	test_does_rebase pull --rebase
      +'
      +
     -+# Rule 4: --no-rebase heeds pull.ff=!only or explict --ff or --no-ff
     ++# Group 4: --no-rebase heeds pull.ff=!only or explict --ff or --no-ff
      +
      +test_expect_success '--no-rebase works with --no-ff' '
     -+	test_does_merge_noff pull --no-rebase --no-ff
     ++	test_does_merge_when_ff_possible pull --no-rebase --no-ff
      +'
      +
      +test_expect_success '--no-rebase works with --ff' '
     -+	test_does_merge_ff pull --no-rebase --ff
     ++	test_does_fast_forward pull --no-rebase --ff
      +'
      +
      +test_expect_success '--no-rebase does ff if pull.ff unset' '
     -+	test_does_merge_ff pull --no-rebase
     ++	test_does_fast_forward pull --no-rebase
      +'
      +
      +test_expect_success '--no-rebase heeds pull.ff=true' '
     -+	test_does_merge_ff -c pull.ff=true pull --no-rebase
     ++	test_does_fast_forward -c pull.ff=true pull --no-rebase
      +'
      +
      +test_expect_success '--no-rebase heeds pull.ff=false' '
     -+	test_does_merge_noff -c pull.ff=false pull --no-rebase
     ++	test_does_merge_when_ff_possible -c pull.ff=false pull --no-rebase
      +'
      +
     -+# Rule 5: pull.rebase=!false takes precedence over --no-ff and --ff
     -+test_expect_success 'pull.rebase=true takes precedence over --no-ff' '
     ++# Group 5: pull.rebase=!false in combination with --no-ff or --ff
     ++test_expect_success 'pull.rebase=true and --no-ff' '
      +	test_does_rebase -c pull.rebase=true pull --no-ff
      +'
      +
     -+test_expect_success 'pull.rebase=true takes precedence over --ff' '
     ++test_expect_success 'pull.rebase=true and --ff' '
      +	test_does_rebase -c pull.rebase=true pull --ff
      +'
      +
     -+# End of precedence rules
     ++test_expect_success 'pull.rebase=false and --no-ff' '
     ++	test_does_merge_when_ff_possible -c pull.rebase=false pull --no-ff
     ++'
     ++
     ++test_expect_success 'pull.rebase=false and --ff, ff possible' '
     ++	test_does_fast_forward -c pull.rebase=false pull --ff
     ++'
     ++
     ++test_expect_success 'pull.rebase=false and --ff, ff not possible' '
     ++	test_falls_back_to_full_merge -c pull.rebase=false pull --ff
     ++'
     ++
     ++# End of groupings for conflicting merge vs. rebase flags/options
      +
       test_expect_success 'merge c1 with c2' '
       	git reset --hard c1 &&
  2:  329802382bf !  2:  66fe7f7f934 t7601: add tests of interactions with multiple merge heads and config
     @@ Commit message
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## t/t7601-merge-pull-config.sh ##
     -@@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase=true takes precedence over --ff' '
     +@@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase=false and --ff, ff not possible' '
       
     - # End of precedence rules
     + # End of groupings for conflicting merge vs. rebase flags/options
       
     -+test_expect_failure 'Multiple heads does not warn about fast forwarding' '
     ++test_expect_failure 'Multiple heads warns about inability to fast forward' '
      +	git reset --hard c1 &&
     -+	git pull . c2 c3 2>err &&
     -+	test_i18ngrep ! "Pulling without specifying how to reconcile" err
     ++	test_must_fail git pull . c2 c3 2>err &&
     ++	test_i18ngrep "Pulling without specifying how to reconcile" err
      +'
      +
     -+test_expect_success 'Cannot fast-forward with multiple heads' '
     ++test_expect_failure 'Multiple can never be fast forwarded' '
      +	git reset --hard c0 &&
      +	test_must_fail git -c pull.ff=only pull . c1 c2 c3 2>err &&
      +	test_i18ngrep ! "Pulling without specifying how to reconcile" err &&
     -+	test_i18ngrep "Not possible to fast-forward, aborting" err
     ++	# In addition to calling out "cannot fast-forward", we very much
     ++	# want the "multiple branches" piece to be called out to users.
     ++	test_i18ngrep "Cannot fast-forward to multiple branches" err
      +'
      +
      +test_expect_success 'Cannot rebase with multiple heads' '
  3:  ae54afd8b01 !  3:  c45cd239666 pull: abort if --ff-only is given and fast-forwarding is impossible
     @@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
       	if (opt_rebase) {
      
       ## t/t7601-merge-pull-config.sh ##
     -@@ t/t7601-merge-pull-config.sh: test_attempts_fast_forward() {
     - # Rule 1: --ff-only takes precedence over --[no-]rebase
     - # (Corollary: pull.ff=only overrides pull.rebase)
     +@@ t/t7601-merge-pull-config.sh: test_attempts_fast_forward () {
     + # Group 1: Interaction of --ff-only with --[no-]rebase
     + # (And related interaction of pull.ff=only with pull.rebase)
       #
     --test_expect_failure '--ff-only takes precedence over --rebase' '
     -+test_expect_success '--ff-only takes precedence over --rebase' '
     +-test_expect_failure '--ff-only overrides --rebase' '
     ++test_expect_success '--ff-only overrides --rebase' '
       	test_attempts_fast_forward pull --rebase --ff-only
       '
       
     --test_expect_failure '--ff-only takes precedence over --rebase even if first' '
     -+test_expect_success '--ff-only takes precedence over --rebase even if first' '
     +-test_expect_failure '--ff-only overrides --rebase even if first' '
     ++test_expect_success '--ff-only overrides --rebase even if first' '
       	test_attempts_fast_forward pull --ff-only --rebase
       '
       
     -@@ t/t7601-merge-pull-config.sh: test_expect_success '--ff-only takes precedence over --no-rebase' '
     +@@ t/t7601-merge-pull-config.sh: test_expect_success '--ff-only overrides --no-rebase' '
       	test_attempts_fast_forward pull --ff-only --no-rebase
       '
       
     @@ t/t7601-merge-pull-config.sh: test_expect_success '--ff-only takes precedence ov
       	test_attempts_fast_forward -c pull.ff=only -c pull.rebase=true pull
       '
       
     -@@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase=true takes precedence over pull.ff=true' '
     +@@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase=true overrides pull.ff=true' '
       '
       
     - # Rule 3: command line flags take precedence over config
     + # Group 3: command line flags take precedence over config
      -test_expect_failure '--ff-only takes precedence over pull.rebase=true' '
      +test_expect_success '--ff-only takes precedence over pull.rebase=true' '
       	test_attempts_fast_forward -c pull.rebase=true pull --ff-only
       '
       
     -@@ t/t7601-merge-pull-config.sh: test_expect_failure '--no-rebase overrides pull.ff=only' '
     - 	test_does_need_full_merge -c pull.ff=only pull --no-rebase
     +@@ t/t7601-merge-pull-config.sh: test_expect_failure '--no-rebase takes precedence over pull.ff=only' '
     + 	test_falls_back_to_full_merge -c pull.ff=only pull --no-rebase
       '
       
      -test_expect_success '--rebase takes precedence over pull.ff=only' '
  4:  de4b460b09d <  -:  ----------- pull: since --ff-only overrides, handle it first
  5:  3d9ff69198e !  4:  1a821d3b1dd pull: ensure --rebase overrides ability to ff
     @@ Metadata
      Author: Elijah Newren <newren@gmail.com>
      
       ## Commit message ##
     -    pull: ensure --rebase overrides ability to ff
     +    pull: since --ff-only overrides, handle it first
      
     -    Now that the handling of fast-forward-only in combination with rebases
     -    has been moved before the merge-vs-rebase logic, we have an unnecessary
     -    special fast-forward case left within the rebase logic.  Actually, more
     -    than unnecessary, it's actually a violation of the rules.  As per
     -    https://lore.kernel.org/git/xmqqwnpqot4m.fsf@gitster.g/, --rebase is
     -    supposed to override all ff flags other than an explicit --ff-only.
     -    Ensure that it does so by removing the fast-forward special case that
     -    exists within the rebase logic.
     +    There are both merge and rebase branches in the logic, and previously
     +    both had to handle fast-forwarding.  Merge handled that implicitly
     +    (because git merge handles it directly), while in rebase it was
     +    explicit.  Given that the --ff-only flag is meant to override any
     +    --rebase or --no-rebase, make the code reflect that by handling
     +    --ff-only before the merge-vs-rebase logic.
     +
     +    It turns out that this also fixes a bug for submodules.  Previously,
     +    when --ff-only was given, the code would run `merge --ff-only` on the
     +    main module, and then run `submodule update --recursive --rebase` on the
     +    submodules.  With this change, we still run `merge --ff-only` on the
     +    main module, but now run `submodule update --recursive --checkout` on
     +    the submodules.  I believe this better reflects the intent of --ff-only
     +    to have it apply to both the main module and the submodules.
     +
     +    (Sidenote: It is somewhat interesting that all merges pass `--checkout`
     +    to submodule update, even when `--no-ff` is specified, meaning that it
     +    will only do fast-forward merges for submodules.  This was discussed in
     +    commit a6d7eb2c7a ("pull: optionally rebase submodules (remote submodule
     +    changes only)", 2017-06-23).  The same limitations apply now as then, so
     +    we are not trying to fix this at this time.)
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/pull.c ##
      @@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
     + 
     + 	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
     + 
     +-	if (!can_ff) {
     +-		if (opt_ff) {
     +-			if (!strcmp(opt_ff, "--ff-only"))
     +-				die_ff_impossible();
     +-		} else {
     +-			if (rebase_unspecified && opt_verbosity >= 0)
     +-				show_advice_pull_non_ff();
     +-		}
     ++	/* ff-only takes precedence over rebase */
     ++	if (opt_ff && !strcmp(opt_ff, "--ff-only")) {
     ++		if (!can_ff)
     ++			die_ff_impossible();
     ++		opt_rebase = REBASE_FALSE;
     + 	}
     ++	/* If no action specified and we can't fast forward, then warn. */
     ++	if (!opt_ff && rebase_unspecified && !can_ff)
     ++		show_advice_pull_non_ff();
     + 
     + 	if (opt_rebase) {
     + 		int ret = 0;
     +@@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
       		    submodule_touches_in_range(the_repository, &upstream, &curr_head))
       			die(_("cannot rebase with locally recorded submodule modifications"));
       
  6:  b379fea097d !  5:  9b116f3d284 pull: make --rebase and --no-rebase override pull.ff=only
     @@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
      -	if (!opt_ff)
      +	if (!opt_ff) {
       		opt_ff = xstrdup_or_null(config_get_ff());
     ++		/*
     ++		 * A subtle point: opt_ff was set on the line above via
     ++		 * reading from config.  opt_rebase, in contrast, is set
     ++		 * before this point via command line options.  The setting
     ++		 * of opt_rebase via reading from config (using
     ++		 * config_get_rebase()) does not happen until later.  We
     ++		 * are relying on the next if-condition happening before
     ++		 * the config_get_rebase() call so that an explicit
     ++		 * "--rebase" can override a config setting of
     ++		 * pull.ff=only.
     ++		 */
      +		if (opt_rebase >= 0 && opt_ff && !strcmp(opt_ff, "--ff-only"))
      +			opt_ff = "--ff";
      +	}
     @@ t/t7601-merge-pull-config.sh: test_expect_success '--ff-only takes precedence ov
       	test_attempts_fast_forward -c pull.rebase=false pull --ff-only
       '
       
     --test_expect_failure '--no-rebase overrides pull.ff=only' '
     -+test_expect_success '--no-rebase overrides pull.ff=only' '
     - 	test_does_need_full_merge -c pull.ff=only pull --no-rebase
     +-test_expect_failure '--no-rebase takes precedence over pull.ff=only' '
     ++test_expect_success '--no-rebase takes precedence over pull.ff=only' '
     + 	test_falls_back_to_full_merge -c pull.ff=only pull --no-rebase
       '
       
      -test_expect_failure '--rebase takes precedence over pull.ff=only' '
  7:  dca0455898a !  6:  f061f8b4e75 pull: abort by default when fast-forwarding is not possible
     @@ builtin/pull.c: static int get_can_ff(struct object_id *orig_head, struct object
       		 "  git config pull.rebase false  # merge (the default strategy)\n"
       		 "  git config pull.rebase true   # rebase\n"
      @@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
     - 			return run_merge();
     + 		opt_rebase = REBASE_FALSE;
       	}
       	/* If no action specified and we can't fast forward, then warn. */
      -	if (!opt_ff && rebase_unspecified && !can_ff)
     @@ t/t6402-merge-rename.sh: test_expect_success 'setup' '
      -	test_expect_code 1 git pull . white &&
      +	test_expect_code 1 git pull --no-rebase . white &&
       	git ls-files -s &&
     - 	git ls-files -u B >b.stages &&
     - 	test_line_count = 3 b.stages &&
     + 	test_stdout_line_count = 3 git ls-files -u B &&
     + 	test_stdout_line_count = 1 git ls-files -s N &&
      @@ t/t6402-merge-rename.sh: test_expect_success 'pull renaming branch into another renaming one' \
       	rm -f B &&
       	git reset --hard &&
       	git checkout red &&
      -	test_expect_code 1 git pull . white &&
      +	test_expect_code 1 git pull --no-rebase . white &&
     - 	git ls-files -u B >b.stages &&
     - 	test_line_count = 3 b.stages &&
     - 	git ls-files -s N >n.stages &&
     + 	test_stdout_line_count = 3 git ls-files -u B &&
     + 	test_stdout_line_count = 1 git ls-files -s N &&
     + 	sed -ne "/^g/{
      @@ t/t6402-merge-rename.sh: test_expect_success 'pull unrenaming branch into renaming one' \
       '
       	git reset --hard &&
       	git show-branch &&
      -	test_expect_code 1 git pull . main &&
      +	test_expect_code 1 git pull --no-rebase . main &&
     - 	git ls-files -u B >b.stages &&
     - 	test_line_count = 3 b.stages &&
     - 	git ls-files -s N >n.stages &&
     + 	test_stdout_line_count = 3 git ls-files -u B &&
     + 	test_stdout_line_count = 1 git ls-files -s N &&
     + 	sed -ne "/^g/{
      @@ t/t6402-merge-rename.sh: test_expect_success 'pull conflicting renames' \
       '
       	git reset --hard &&
       	git show-branch &&
      -	test_expect_code 1 git pull . blue &&
      +	test_expect_code 1 git pull --no-rebase . blue &&
     - 	git ls-files -u A >a.stages &&
     - 	test_line_count = 1 a.stages &&
     - 	git ls-files -u B >b.stages &&
     + 	test_stdout_line_count = 1 git ls-files -u A &&
     + 	test_stdout_line_count = 1 git ls-files -u B &&
     + 	test_stdout_line_count = 1 git ls-files -u C &&
      @@ t/t6402-merge-rename.sh: test_expect_success 'interference with untracked working tree file' '
       	git reset --hard &&
       	git show-branch &&
     @@ t/t7601-merge-pull-config.sh: test_expect_success 'setup' '
      +	test_i18ngrep ! "You have divergent branches" err
       '
       
     - test_does_rebase() {
     + test_does_rebase () {
     +@@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase=false and --ff, ff not possible' '
     + 
     + # End of groupings for conflicting merge vs. rebase flags/options
     + 
     +-test_expect_failure 'Multiple heads warns about inability to fast forward' '
     ++test_expect_success 'Multiple heads warns about inability to fast forward' '
     + 	git reset --hard c1 &&
     + 	test_must_fail git pull . c2 c3 2>err &&
     +-	test_i18ngrep "Pulling without specifying how to reconcile" err
     ++	test_i18ngrep "You have divergent branches" err
     + '
     + 
     + test_expect_failure 'Multiple can never be fast forwarded' '
     + 	git reset --hard c0 &&
     + 	test_must_fail git -c pull.ff=only pull . c1 c2 c3 2>err &&
     +-	test_i18ngrep ! "Pulling without specifying how to reconcile" err &&
     ++	test_i18ngrep ! "You have divergent branches" err &&
     + 	# In addition to calling out "cannot fast-forward", we very much
     + 	# want the "multiple branches" piece to be called out to users.
     + 	test_i18ngrep "Cannot fast-forward to multiple branches" err
     +@@ t/t7601-merge-pull-config.sh: test_expect_failure 'Multiple can never be fast forwarded' '
     + test_expect_success 'Cannot rebase with multiple heads' '
     + 	git reset --hard c0 &&
     + 	test_must_fail git -c pull.rebase=true pull . c1 c2 c3 2>err &&
     +-	test_i18ngrep ! "Pulling without specifying how to reconcile" err &&
     ++	test_i18ngrep ! "You have divergent branches" err &&
     + 	test_i18ngrep "Cannot rebase onto multiple branches." err
     + '
     + 
      
       ## t/t7603-merge-reduce-heads.sh ##
      @@ t/t7603-merge-reduce-heads.sh: test_expect_success 'merge c1 with c2, c3, c4, c5' '
  8:  d1952f014f2 !  7:  90d49e0fb78 pull: update docs & code for option compatibility with rebasing
     @@ Commit message
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     + ## Documentation/git-merge.txt ##
     +@@ Documentation/git-merge.txt: merge has resulted in conflicts.
     + 
     + OPTIONS
     + -------
     ++:git-merge: 1
     ++
     + include::merge-options.txt[]
     + 
     + -m <msg>::
     +
       ## Documentation/git-pull.txt ##
     -@@ Documentation/git-pull.txt: When false, merge the current branch into the upstream branch.
     - +
     - When `interactive`, enable the interactive mode of rebase.
     - +
     -+Note that `--ff-only` takes precedence over any `--rebase` flag.
     -++
     - See `pull.rebase`, `branch.<name>.rebase` and `branch.autoSetupRebase` in
     - linkgit:git-config[1] if you want to make `git pull` always use
     - `--rebase` instead of merging.
     +@@ Documentation/git-pull.txt: Incorporates changes from a remote repository into the current branch.
     + If the current branch is behind the remote, then by default it will
     + fast-forward the current branch to match the remote.  If the current
     + branch and the remote have diverged, the user needs to specify how to
     +-reconcile the divergent branches with `--no-ff`, `--ff`, or `--rebase`
     +-(or the corresponding configuration options in `pull.ff` or
     +-`pull.rebase`).
     ++reconcile the divergent branches with `--rebase` or `--no-rebase` (or
     ++the corresponding configuration option in `pull.rebase`).
     + 
     + More precisely, `git pull` runs `git fetch` with the given parameters
     + and then depending on configuration options or command line flags,
     +-will call either `git merge` or `git rebase` to reconcile diverging
     ++will call either `git rebase` or `git merge` to reconcile diverging
     + branches.
     + 
     + <repository> should be the name of a remote repository as
     +@@ Documentation/git-pull.txt: published that history already.  Do *not* use this option
     + unless you have read linkgit:git-rebase[1] carefully.
     + 
     + --no-rebase::
     +-	Override earlier --rebase.
     ++	This is shorthand for --rebase=false.
     + 
     + Options related to fetching
     + ~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
       ## Documentation/merge-options.txt ##
      @@
     @@ Documentation/merge-options.txt
       +
       With --no-commit perform the merge and stop just before creating
       a merge commit, to give the user a chance to inspect and further
     -@@ Documentation/merge-options.txt: could instead be resolved as a fast-forward.
     +@@ Documentation/merge-options.txt: set to `no` at the beginning of them.
     + 	to `MERGE_MSG` before being passed on to the commit machinery in the
     + 	case of a merge conflict.
     + 
     ++ifdef::git-merge[]
     + --ff::
     + --no-ff::
     + --ff-only::
     +@@ Documentation/merge-options.txt: set to `no` at the beginning of them.
     + 	default unless merging an annotated (and possibly signed) tag
     + 	that is not stored in its natural place in the `refs/tags/`
     + 	hierarchy, in which case `--no-ff` is assumed.
     ++endif::git-merge[]
     ++ifdef::git-pull[]
     ++--ff-only::
     ++	Only update to the new history if there is no divergent local
     ++	history.  This is the default when no method for reconciling
     ++	divergent histories is provided (via the --rebase=* flags).
     ++
     ++--ff::
     ++--no-ff::
     ++	When merging rather than rebasing, specifies how a merge is
     ++	handled when the merged-in history is already a descendant of
     ++	the current history.  If merging is requested, `--ff` is the
     ++	default unless merging an annotated (and possibly signed) tag
     ++	that is not stored in its natural place in the `refs/tags/`
     ++	hierarchy, in which case `--no-ff` is assumed.
     ++endif::git-pull[]
     + +
     + With `--ff`, when possible resolve the merge as a fast-forward (only
     + update the branch pointer to match the merged branch; do not create a
     +@@ Documentation/merge-options.txt: descendant of the current history), create a merge commit.
     + +
     + With `--no-ff`, create a merge commit in all cases, even when the merge
     + could instead be resolved as a fast-forward.
     ++ifdef::git-merge[]
       +
       With `--ff-only`, resolve the merge as a fast-forward when possible.
       When not possible, refuse to merge and exit with a non-zero status.
     -+ifdef::git-pull[]
     -++
     -+Note that `--no-ff` and `--ff` are ignored when rebasing is requested.
     -+endif::git-pull[]
     ++endif::git-merge[]
       
       -S[<keyid>]::
       --gpg-sign[=<keyid>]::
  9:  3d8df246772 !  8:  f03b15b7eb0 pull: fix handling of multiple heads
     @@ Commit message
          pull: fix handling of multiple heads
      
          With multiple heads, we should not allow rebasing or fast-forwarding.
     -    Also, it seems wrong to have our can_ff computation return true, so fix
     -    that while we are at it too (we won't actually use the can_ff flag due
     -    to setting opt_ff to "--no-ff", but it's confusing to leave it as
     -    computed to be true).
     +    Make sure any fast-forward request calls out specifically the fact that
     +    multiple branches are in play.  Also, since we cannot fast-forward to
     +    multiple branches, fix our computation of can_ff.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     @@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
      +			die(_("Cannot rebase onto multiple branches."));
      +		if (opt_ff && !strcmp(opt_ff, "--ff-only"))
      +			die(_("Cannot fast-forward to multiple branches."));
     -+		if (!opt_ff)
     -+			opt_ff = "--no-ff";
      +	}
       
      -	can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
     @@ builtin/pull.c: int cmd_pull(int argc, const char **argv, const char *prefix)
       	if (opt_ff && !strcmp(opt_ff, "--ff-only")) {
      
       ## t/t7601-merge-pull-config.sh ##
     -@@ t/t7601-merge-pull-config.sh: test_expect_success 'pull.rebase=true takes precedence over --ff' '
     - 
     - # End of precedence rules
     +@@ t/t7601-merge-pull-config.sh: test_expect_success 'Multiple heads warns about inability to fast forward' '
     + 	test_i18ngrep "You have divergent branches" err
     + '
       
     --test_expect_failure 'Multiple heads does not warn about fast forwarding' '
     -+test_expect_success 'Multiple heads does not warn about fast forwarding' '
     - 	git reset --hard c1 &&
     - 	git pull . c2 c3 2>err &&
     - 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
     -@@ t/t7601-merge-pull-config.sh: test_expect_success 'Cannot fast-forward with multiple heads' '
     +-test_expect_failure 'Multiple can never be fast forwarded' '
     ++test_expect_success 'Multiple can never be fast forwarded' '
       	git reset --hard c0 &&
       	test_must_fail git -c pull.ff=only pull . c1 c2 c3 2>err &&
     - 	test_i18ngrep ! "Pulling without specifying how to reconcile" err &&
     --	test_i18ngrep "Not possible to fast-forward, aborting" err
     -+	test_i18ngrep "Cannot fast-forward to multiple branches" err
     - '
     - 
     - test_expect_success 'Cannot rebase with multiple heads' '
     + 	test_i18ngrep ! "You have divergent branches" err &&

-- 
gitgitgadget