diff mbox series

[5/5] pull: abort by default when fast-forwarding is not possible

Message ID 7e12c45fc9a94e7b56a6efdc085ebe081dd40afc.1626316849.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Handle conflicting pull options | expand

Commit Message

Elijah Newren July 15, 2021, 2:40 a.m. UTC 
From: Elijah Newren <newren@gmail.com>

We have for some time shown a long warning when the user does not
specify how to reconcile divergent branches with git pull.  Make it an
error now.

Initial-patch-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-pull.txt    | 18 ++++++++++--------
 builtin/pull.c                | 31 +++++++++++++++----------------
 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  |  6 ++----
 t/t7603-merge-reduce-heads.sh |  2 +-
 13 files changed, 69 insertions(+), 70 deletions(-)

Comments

Eric Sunshine July 15, 2021, 5:18 a.m. UTC | #1 
On Wed, Jul 14, 2021 at 10:41 PM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> We have for some time shown a long warning when the user does not
> specify how to reconcile divergent branches with git pull.  Make it an
> error now.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>

Teeny tiny nits below...

> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> @@ -15,14 +15,16 @@ SYNOPSIS
> -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`.
> -
> -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'.

The original text has an odd mix of apostrophes and backticks...

> +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 or --rebase (or the
> +corresponding configuration options in pull.ff or pull.rebase).
> +
> +More precisely, 'git pull' runs 'git fetch' with the given parameters
> +and then depending on config options or command line flags, will call
> +either 'git merge' or 'git rebase' to reconcile diverging branches.

... and the revised text adds "no quotes" to the selection. These
days, we'd probably backtick all of these:

    --no-ff
    --rebase
    pull.ff
    pull.rebase
    git pull
    git fetch
    git merge
    git rebase

The rest of this document is actually pretty good about using
backticks, though there are some exceptions.

There's also an odd mix of "configuration options" and "config
options" in the revised text. Perhaps stick with "configuration
options" to be a bit more formal?

>  <repository> should be the name of a remote repository as
>  passed to linkgit:git-fetch[1].  <refspec> can name an

And, as an aside, we'd backtick <repository> and <refspec>, though
your patch isn't touching that, so outside the scope of this change.

> diff --git a/builtin/pull.c b/builtin/pull.c
> @@ -1074,9 +1074,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>                 if (opt_ff) {
>                         if (!strcmp(opt_ff, "--ff-only"))
>                                 die_ff_impossible();
> -               } else {
> -                       if (rebase_unspecified && opt_verbosity >= 0)
> -                               show_advice_pull_non_ff();
> +               } else if (rebase_unspecified) {
> +                       die_pull_non_ff();
>                 }

When reading the previous patch, I was wondering why an `if else`
wasn't used, and here I see that it does become an `if else`. I guess
you didn't want to alter Alex's original patch?
Felipe Contreras July 15, 2021, 9:48 a.m. UTC | #2 
Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> We have for some time shown a long warning when the user does not
> specify how to reconcile divergent branches with git pull.  Make it an
> error now.

A warning doesn't imply deprecation.

Users have been warnged *zero* days that the default is going to change,
to what it's going to change, how to configure the new behavior, or how
to keep the old behavior.

NAK.
Elijah Newren July 15, 2021, 4:56 p.m. UTC | #3 
On Wed, Jul 14, 2021 at 10:19 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Wed, Jul 14, 2021 at 10:41 PM Elijah Newren via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > We have for some time shown a long warning when the user does not
> > specify how to reconcile divergent branches with git pull.  Make it an
> > error now.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
>
> Teeny tiny nits below...
>
> > diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> > @@ -15,14 +15,16 @@ SYNOPSIS
> > -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`.
> > -
> > -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'.
>
> The original text has an odd mix of apostrophes and backticks...
>
> > +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 or --rebase (or the
> > +corresponding configuration options in pull.ff or pull.rebase).
> > +
> > +More precisely, 'git pull' runs 'git fetch' with the given parameters
> > +and then depending on config options or command line flags, will call
> > +either 'git merge' or 'git rebase' to reconcile diverging branches.
>
> ... and the revised text adds "no quotes" to the selection. These
> days, we'd probably backtick all of these:
>
>     --no-ff
>     --rebase
>     pull.ff
>     pull.rebase
>     git pull
>     git fetch
>     git merge
>     git rebase
>
> The rest of this document is actually pretty good about using
> backticks, though there are some exceptions.
>
> There's also an odd mix of "configuration options" and "config
> options" in the revised text. Perhaps stick with "configuration
> options" to be a bit more formal?
>
> >  <repository> should be the name of a remote repository as
> >  passed to linkgit:git-fetch[1].  <refspec> can name an
>
> And, as an aside, we'd backtick <repository> and <refspec>, though
> your patch isn't touching that, so outside the scope of this change.

I'll try to fix these up, at least those in the nearby area I'm
modifying, if others agree with the general approach towards --ff-only
& --no-ff vs. --rebase.

> > diff --git a/builtin/pull.c b/builtin/pull.c
> > @@ -1074,9 +1074,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
> >                 if (opt_ff) {
> >                         if (!strcmp(opt_ff, "--ff-only"))
> >                                 die_ff_impossible();
> > -               } else {
> > -                       if (rebase_unspecified && opt_verbosity >= 0)
> > -                               show_advice_pull_non_ff();
> > +               } else if (rebase_unspecified) {
> > +                       die_pull_non_ff();
> >                 }
>
> When reading the previous patch, I was wondering why an `if else`
> wasn't used, and here I see that it does become an `if else`. I guess
> you didn't want to alter Alex's original patch?

Yep.  :-)
Ævar Arnfjörð Bjarmason July 16, 2021, 9:32 a.m. UTC | #4 
On Thu, Jul 15 2021, Elijah Newren via GitGitGadget wrote:

> -static void show_advice_pull_non_ff(void)
> +static void NORETURN 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(_("You have divergent branches and need to specify how to reconcile them.\n"
> +	      "You can do so by running one of the following commands sometime before\n"
> +	      "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"));
>  }

Dying and advise() shouldn't be mutually exclusive, we should reword the
advise() message to idate for this being a die(), and then:

>  int cmd_pull(int argc, const char **argv, const char *prefix)
> @@ -1074,9 +1074,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>  		if (opt_ff) {
>  			if (!strcmp(opt_ff, "--ff-only"))
>  				die_ff_impossible();
> -		} else {
> -			if (rebase_unspecified && opt_verbosity >= 0)
> -				show_advice_pull_non_ff();
> +		} else if (rebase_unspecified) {
> +			die_pull_non_ff();
>  		}

Here we should:

    show_advice_pull_non_ff();
    die(_("some much briefer summary"))

I.e. we should not being showing giantic advice-y die() messages, the
die messages should always be brief, but we might also show advice just
before dying.
Felipe Contreras July 16, 2021, 6:13 p.m. UTC | #5 
Ævar Arnfjörð Bjarmason wrote:
> On Thu, Jul 15 2021, Elijah Newren via GitGitGadget wrote:

> >  int cmd_pull(int argc, const char **argv, const char *prefix)
> > @@ -1074,9 +1074,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
> >  		if (opt_ff) {
> >  			if (!strcmp(opt_ff, "--ff-only"))
> >  				die_ff_impossible();
> > -		} else {
> > -			if (rebase_unspecified && opt_verbosity >= 0)
> > -				show_advice_pull_non_ff();
> > +		} else if (rebase_unspecified) {
> > +			die_pull_non_ff();
> >  		}
> 
> Here we should:
> 
>     show_advice_pull_non_ff();
>     die(_("some much briefer summary"))
> 
> I.e. we should not being showing giantic advice-y die() messages, the
> die messages should always be brief, but we might also show advice just
> before dying.

Indeed, just like my proposal does:

		diverging_advice();
		die(_("The pull was not fast-forward, either merge or rebase.\n"));
diff mbox series

Patch

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 03e8694e146..cb65d33e15e 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -15,14 +15,16 @@  SYNOPSIS
 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`.
-
-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'.
+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 or --rebase (or the
+corresponding configuration options in pull.ff or pull.rebase).
+
+More precisely, 'git pull' runs 'git fetch' with the given parameters
+and then depending on config options or command line flags, will call
+either 'git merge' or 'git rebase' to reconcile diverging branches.
 
 <repository> should be the name of a remote repository as
 passed to linkgit:git-fetch[1].  <refspec> can name an
diff --git a/builtin/pull.c b/builtin/pull.c
index 2c90bbb1588..0f8fdb7d42b 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -946,20 +946,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 NORETURN 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(_("You have divergent branches and need to specify how to reconcile them.\n"
+	      "You can do so by running one of the following commands sometime before\n"
+	      "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"));
 }
 
 int cmd_pull(int argc, const char **argv, const char *prefix)
@@ -1074,9 +1074,8 @@  int cmd_pull(int argc, const char **argv, const char *prefix)
 		if (opt_ff) {
 			if (!strcmp(opt_ff, "--ff-only"))
 				die_ff_impossible();
-		} else {
-			if (rebase_unspecified && opt_verbosity >= 0)
-				show_advice_pull_non_ff();
+		} else if (rebase_unspecified) {
+			die_pull_non_ff();
 		}
 	}
 
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 7fadc985ccc..eb989f7f191 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -65,7 +65,7 @@  test_expect_success setup '
 	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&
 
 	git checkout master &&
-	git pull -s ours . side &&
+	git pull -s ours --no-rebase . side &&
 
 	GIT_AUTHOR_DATE="2006-06-26 00:05:00 +0000" &&
 	GIT_COMMITTER_DATE="2006-06-26 00:05:00 +0000" &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index e2c0c510222..672001a18bd 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -136,12 +136,12 @@  test_expect_success 'the default remote . should not break explicit pull' '
 	git reset --hard HEAD^ &&
 	echo file >expect &&
 	test_cmp expect file &&
-	git pull . second &&
+	git pull --no-rebase . second &&
 	echo modified >expect &&
 	test_cmp expect file &&
 	git reflog -1 >reflog.actual &&
 	sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy &&
-	echo "OBJID HEAD@{0}: pull . second: Fast-forward" >reflog.expected &&
+	echo "OBJID HEAD@{0}: pull --no-rebase . second: Fast-forward" >reflog.expected &&
 	test_cmp reflog.expected reflog.fuzzy
 '
 
@@ -226,7 +226,7 @@  test_expect_success 'fail if the index has unresolved entries' '
 	test_commit modified2 file &&
 	git ls-files -u >unmerged &&
 	test_must_be_empty unmerged &&
-	test_must_fail git pull . second &&
+	test_must_fail git pull --no-rebase . second &&
 	git ls-files -u >unmerged &&
 	test_file_not_empty unmerged &&
 	cp file expected &&
@@ -409,37 +409,37 @@  test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
 
 test_expect_success 'pull succeeds with dirty working directory and merge.autostash set' '
 	test_config merge.autostash true &&
-	test_pull_autostash 2
+	test_pull_autostash 2 --no-rebase
 '
 
 test_expect_success 'pull --autostash & merge.autostash=true' '
 	test_config merge.autostash true &&
-	test_pull_autostash 2 --autostash
+	test_pull_autostash 2 --autostash --no-rebase
 '
 
 test_expect_success 'pull --autostash & merge.autostash=false' '
 	test_config merge.autostash false &&
-	test_pull_autostash 2 --autostash
+	test_pull_autostash 2 --autostash --no-rebase
 '
 
 test_expect_success 'pull --autostash & merge.autostash unset' '
 	test_unconfig merge.autostash &&
-	test_pull_autostash 2 --autostash
+	test_pull_autostash 2 --autostash --no-rebase
 '
 
 test_expect_success 'pull --no-autostash & merge.autostash=true' '
 	test_config merge.autostash true &&
-	test_pull_autostash_fail --no-autostash
+	test_pull_autostash_fail --no-autostash --no-rebase
 '
 
 test_expect_success 'pull --no-autostash & merge.autostash=false' '
 	test_config merge.autostash false &&
-	test_pull_autostash_fail --no-autostash
+	test_pull_autostash_fail --no-autostash --no-rebase
 '
 
 test_expect_success 'pull --no-autostash & merge.autostash unset' '
 	test_unconfig merge.autostash &&
-	test_pull_autostash_fail --no-autostash
+	test_pull_autostash_fail --no-autostash --no-rebase
 '
 
 test_expect_success 'pull.rebase' '
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 63a688bdbf5..7601c919fdc 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -113,7 +113,7 @@  test_expect_success 'git pull --force' '
 	git pull two &&
 	test_commit A &&
 	git branch -f origin &&
-	git pull --all --force
+	git pull --no-rebase --all --force
 	)
 '
 
@@ -179,7 +179,7 @@  test_expect_success 'git pull --allow-unrelated-histories' '
 	(
 		cd dst &&
 		test_must_fail git pull ../src side &&
-		git pull --allow-unrelated-histories ../src side
+		git pull --no-rebase --allow-unrelated-histories ../src side
 	)
 '
 
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index c278adaa5a2..b2be3605f5a 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -28,7 +28,7 @@  test_expect_success setup '
 test_expect_success pull '
 (
 	cd cloned &&
-	git pull --log &&
+	git pull --no-rebase --log &&
 	git log -2 &&
 	git cat-file commit HEAD >result &&
 	grep Dollar result
@@ -41,7 +41,7 @@  test_expect_success '--log=1 limits shortlog length' '
 	git reset --hard HEAD^ &&
 	test "$(cat afile)" = original &&
 	test "$(cat bfile)" = added &&
-	git pull --log=1 &&
+	git pull --no-rebase --log=1 &&
 	git log -3 &&
 	git cat-file commit HEAD >result &&
 	grep Dollar result &&
diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh
index b1d614ce18c..9c12c0f8c32 100755
--- a/t/t5553-set-upstream.sh
+++ b/t/t5553-set-upstream.sh
@@ -108,27 +108,27 @@  test_expect_success 'setup commit on main and other pull' '
 
 test_expect_success 'pull --set-upstream upstream main sets branch main but not other' '
 	clear_config main other &&
-	git pull --set-upstream upstream main &&
+	git pull --no-rebase --set-upstream upstream main &&
 	check_config main upstream refs/heads/main &&
 	check_config_missing other
 '
 
 test_expect_success 'pull --set-upstream main:other2 does not set the branch other2' '
 	clear_config other2 &&
-	git pull --set-upstream upstream main:other2 &&
+	git pull --no-rebase --set-upstream upstream main:other2 &&
 	check_config_missing other2
 '
 
 test_expect_success 'pull --set-upstream upstream other sets branch main' '
 	clear_config main other &&
-	git pull --set-upstream upstream other &&
+	git pull --no-rebase --set-upstream upstream other &&
 	check_config main upstream refs/heads/other &&
 	check_config_missing other
 '
 
 test_expect_success 'pull --set-upstream upstream tag does not set the tag' '
 	clear_config three &&
-	git pull --tags --set-upstream upstream three &&
+	git pull --no-rebase --tags --set-upstream upstream three &&
 	check_config_missing three
 '
 
@@ -144,16 +144,16 @@  test_expect_success 'pull --set-upstream http://nosuchdomain.example.com fails w
 
 test_expect_success 'pull --set-upstream upstream HEAD sets branch HEAD' '
 	clear_config main other &&
-	git pull --set-upstream upstream HEAD &&
+	git pull --no-rebase --set-upstream upstream HEAD &&
 	check_config main upstream HEAD &&
 	git checkout other &&
-	git pull --set-upstream upstream HEAD &&
+	git pull --no-rebase --set-upstream upstream HEAD &&
 	check_config other upstream HEAD
 '
 
 test_expect_success 'pull --set-upstream upstream with more than one branch does nothing' '
 	clear_config main three &&
-	git pull --set-upstream upstream main three &&
+	git pull --no-rebase --set-upstream upstream main three &&
 	check_config_missing main &&
 	check_config_missing three
 '
diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
index e845d621f61..24340e6d56e 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -87,7 +87,7 @@  test_expect_success 'updating origin' '
 '
 
 test_expect_success 'pulling changes from origin' '
-	git -C C pull origin
+	git -C C pull --no-rebase origin
 '
 
 # the 2 local objects are commit and tree from the merge
@@ -96,7 +96,7 @@  test_expect_success 'that alternate to origin gets used' '
 '
 
 test_expect_success 'pulling changes from origin' '
-	git -C D pull origin
+	git -C D pull --no-rebase origin
 '
 
 # the 5 local objects are expected; file3 blob, commit in A to add it
diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 425dad97d54..02a842697b8 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -103,7 +103,7 @@  test_expect_success 'setup' '
 test_expect_success 'pull renaming branch into unrenaming one' \
 '
 	git show-branch &&
-	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 &&
@@ -121,7 +121,7 @@  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 &&
@@ -137,7 +137,7 @@  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 &&
@@ -153,7 +153,7 @@  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 &&
@@ -173,7 +173,7 @@  test_expect_success 'interference with untracked working tree file' '
 	git reset --hard &&
 	git show-branch &&
 	echo >A this file should not matter &&
-	test_expect_code 1 git pull . white &&
+	test_expect_code 1 git pull --no-rebase . white &&
 	test_path_is_file A
 '
 
@@ -183,7 +183,7 @@  test_expect_success 'interference with untracked working tree file' '
 	git show-branch &&
 	rm -f A &&
 	echo >A this file should not matter &&
-	test_expect_code 1 git pull . red &&
+	test_expect_code 1 git pull --no-rebase . red &&
 	test_path_is_file A
 '
 
@@ -193,7 +193,7 @@  test_expect_success 'interference with untracked working tree file' '
 	git checkout -f main &&
 	git tag -f anchor &&
 	git show-branch &&
-	git pull . yellow &&
+	git pull --no-rebase . yellow &&
 	test_path_is_missing M &&
 	git reset --hard anchor
 '
@@ -220,7 +220,7 @@  test_expect_success 'updated working tree file should prevent the merge' '
 	echo >>M one line addition &&
 	cat M >M.saved &&
 	git update-index M &&
-	test_expect_code 128 git pull . yellow &&
+	test_expect_code 128 git pull --no-rebase . yellow &&
 	test_cmp M M.saved &&
 	rm -f M.saved
 '
@@ -232,7 +232,7 @@  test_expect_success 'interference with untracked working tree file' '
 	git tag -f anchor &&
 	git show-branch &&
 	echo >M this file should not matter &&
-	git pull . main &&
+	git pull --no-rebase . main &&
 	test_path_is_file M &&
 	! {
 		git ls-files -s |
diff --git a/t/t6409-merge-subtree.sh b/t/t6409-merge-subtree.sh
index d406b2343cb..ba7890ec521 100755
--- a/t/t6409-merge-subtree.sh
+++ b/t/t6409-merge-subtree.sh
@@ -100,7 +100,7 @@  test_expect_success 'merge update' '
 	git checkout -b topic_2 &&
 	git commit -m "update git-gui" &&
 	cd ../git &&
-	git pull -s subtree gui topic_2 &&
+	git pull --no-rebase -s subtree gui topic_2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o3 0	git-gui/git-gui.sh" &&
@@ -129,7 +129,7 @@  test_expect_success 'initial ambiguous subtree' '
 test_expect_success 'merge using explicit' '
 	cd ../git &&
 	git reset --hard topic_2 &&
-	git pull -Xsubtree=git-gui gui topic_2 &&
+	git pull --no-rebase -Xsubtree=git-gui gui topic_2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o3 0	git-gui/git-gui.sh" &&
@@ -142,7 +142,7 @@  test_expect_success 'merge using explicit' '
 test_expect_success 'merge2 using explicit' '
 	cd ../git &&
 	git reset --hard topic_2 &&
-	git pull -Xsubtree=git-gui2 gui topic_2 &&
+	git pull --no-rebase -Xsubtree=git-gui2 gui topic_2 &&
 	git ls-files -s >actual &&
 	(
 		echo "100644 $o1 0	git-gui/git-gui.sh" &&
diff --git a/t/t6417-merge-ours-theirs.sh b/t/t6417-merge-ours-theirs.sh
index ac9aee9a662..ec065d6a658 100755
--- a/t/t6417-merge-ours-theirs.sh
+++ b/t/t6417-merge-ours-theirs.sh
@@ -69,11 +69,11 @@  test_expect_success 'binary file with -Xours/-Xtheirs' '
 '
 
 test_expect_success 'pull passes -X to underlying merge' '
-	git reset --hard main && git pull -s recursive -Xours . side &&
-	git reset --hard main && git pull -s recursive -X ours . side &&
-	git reset --hard main && git pull -s recursive -Xtheirs . side &&
-	git reset --hard main && git pull -s recursive -X theirs . side &&
-	git reset --hard main && test_must_fail git pull -s recursive -X bork . side
+	git reset --hard main && git pull --no-rebase -s recursive -Xours . side &&
+	git reset --hard main && git pull --no-rebase -s recursive -X ours . side &&
+	git reset --hard main && git pull --no-rebase -s recursive -Xtheirs . side &&
+	git reset --hard main && git pull --no-rebase -s recursive -X theirs . side &&
+	git reset --hard main && test_must_fail git pull --no-rebase -s recursive -X bork . side
 '
 
 test_expect_success SYMLINKS 'symlink with -Xours/-Xtheirs' '
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 6b9e80db97b..84404f4f0c3 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -86,10 +86,8 @@  test_expect_success 'pull.rebase not set and --ff-only given' '
 
 test_expect_success 'pull.rebase not set (not-fast-forward)' '
 	git reset --hard c2 &&
-	git -c color.advice=always pull . c1 2>err &&
-	test_decode_color <err >decoded &&
-	test_i18ngrep "<YELLOW>hint: " decoded &&
-	test_i18ngrep "Pulling without specifying how to reconcile" decoded
+	test_must_fail git pull . c1 2>err &&
+	test_i18ngrep "You have divergent branches" err
 '
 
 test_expect_success 'pull.rebase not set and pull.ff=true (not-fast-forward)' '
diff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh
index 98948955ae5..27cd94ad6f7 100755
--- a/t/t7603-merge-reduce-heads.sh
+++ b/t/t7603-merge-reduce-heads.sh
@@ -68,7 +68,7 @@  test_expect_success 'merge c1 with c2, c3, c4, c5' '
 
 test_expect_success 'pull c2, c3, c4, c5 into c1' '
 	git reset --hard c1 &&
-	git pull . c2 c3 c4 c5 &&
+	git pull --no-rebase . c2 c3 c4 c5 &&
 	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
 	test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
 	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&