[PATCH] pull: introduce --merge option

[PATCH] pull: introduce --merge option

[Felipe Contreras]

Previously --no-rebase (which still works for backwards compatibility).

Now we can update the default warning, and the git-pull(1) man page to
use --merge instead of the non-intuitive --no-rebase.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

I've sent many versions of this patch over the years (since 2013) in
many different series and nobody has objected to it (to the best of my
recollection).

The idea came after a comment from Linus Torvalds regarding what should
be the default mode of "git pull" and why [1].

It conflicts with some of my previous patch series, but they aren't even
in "seen" yet, and the ones that are resolve easily.

[1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/

 Documentation/git-pull.txt   | 7 +++++--
 builtin/pull.c               | 4 +++-
 t/t7601-merge-pull-config.sh | 8 ++++----
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 5c3fb67c01..d23128fa72 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -131,8 +131,11 @@ It rewrites history, which does not bode well when you
 published that history already.  Do *not* use this option
 unless you have read linkgit:git-rebase[1] carefully.
 
---no-rebase::
-	Override earlier --rebase.
+-m::
+--merge::
+	Force a merge.
++
+Previously this was --no-rebase, but that usage has been deprecated.
 
 Options related to fetching
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/pull.c b/builtin/pull.c
index e8927fc2ff..186adbff71 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -129,6 +129,8 @@ static struct option pull_options[] = {
 	  "(false|true|merges|preserve|interactive)",
 	  N_("incorporate changes by rebasing rather than merging"),
 	  PARSE_OPT_OPTARG, parse_opt_rebase),
+	OPT_SET_INT('m', "merge", &opt_rebase,
+		N_("incorporate changes by merging"), REBASE_FALSE),
 	OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
 		N_("do not show a diffstat at the end of the merge"),
 		PARSE_OPT_NOARG | PARSE_OPT_NONEG),
@@ -936,7 +938,7 @@ static void show_advice_pull_non_ff(void)
 		 "  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"
+		 "preference for all repositories. You can also pass --rebase, --merge,\n"
 		 "or --ff-only on the command line to override the configured default per\n"
 		 "invocation.\n"));
 }
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 52e8ccc933..6d03e0b9fe 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -60,9 +60,9 @@ test_expect_success 'pull.rebase not set and --rebase given' '
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
-test_expect_success 'pull.rebase not set and --no-rebase given' '
+test_expect_success 'pull.rebase not set and --merge given' '
 	git reset --hard c0 &&
-	git pull --no-rebase . c1 2>err &&
+	git pull --merge . c1 2>err &&
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
@@ -119,9 +119,9 @@ test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)'
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
-test_expect_success 'pull.rebase not set and --no-rebase given (not-fast-forward)' '
+test_expect_success 'pull.rebase not set and --merge given (not-fast-forward)' '
 	git reset --hard c2 &&
-	git pull --no-rebase . c1 2>err &&
+	git pull --merge . c1 2>err &&
 	test_i18ngrep ! "Pulling without specifying how to reconcile" err
 '
 
-- 
2.32.0

Re: [PATCH] pull: introduce --merge option

[Alex Henrie]

On Sun, Jun 27, 2021 at 9:16 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> The idea came after a comment from Linus Torvalds regarding what should
> be the default mode of "git pull" and why [1].
>
> [1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/

Oh wow, I didn't realize that Linus gave his opinion on the subject
back in 2013, and he suggested doing nearly exactly what we are trying
to do now.

> ---no-rebase::
> -       Override earlier --rebase.
> +-m::
> +--merge::
> +       Force a merge.
> ++
> +Previously this was --no-rebase, but that usage has been deprecated.

I don't think --no-rebase should be "deprecated", at least not yet.
After all, the equivalent config option is still pull.rebase=false.
Also, --merge doesn't necessarily force a merge because it does not
imply --no-ff. I would prefer to list --no-rebase, -m, and --merge
together on the same line and leave the description alone.

But other than the documentation and the commit message, I like the
idea here, and I think it will make Git a lot more user-friendly.

-Alex

Re: [PATCH] pull: introduce --merge option

[Felipe Contreras]

Alex Henrie wrote:
> On Sun, Jun 27, 2021 at 9:16 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > The idea came after a comment from Linus Torvalds regarding what should
> > be the default mode of "git pull" and why [1].
> >
> > [1] https://lore.kernel.org/git/CA+55aFz2Uvq4vmyjJPao5tS-uuVvKm6mbP7Uz8sdq1VMxMGJCw@mail.gmail.com/
> 
> Oh wow, I didn't realize that Linus gave his opinion on the subject
> back in 2013, and he suggested doing nearly exactly what we are trying
> to do now.

Indeed. I incorporated his feedback--along with the feedback of many
others--in my proposal.

I have been re-reading the old threads to write a blog post, and the
whole story starts from 2008. At this point I have probably read more
than one thousand emails, and I'm still far from done.

Actually now that I'm re-reading the draft, the first suggestion of
--merge came from Thomas Rast in 2009 [1] (I forgot).

Long story short: *everyone* (and I do mean everyone) is in favor making
ff-only the default. The disagreements come from how we get there.

> > ---no-rebase::
> > -       Override earlier --rebase.
> > +-m::
> > +--merge::
> > +       Force a merge.
> > ++
> > +Previously this was --no-rebase, but that usage has been deprecated.
> 
> I don't think --no-rebase should be "deprecated", at least not yet.

Deprecated means that we discourage people from using it. It doesn't
mean it's unsupported.

It's not obsolete.

> After all, the equivalent config option is still pull.rebase=false.

Yes, but the equivalent from the command line is:

  git pull --rebase=false

And that's still properly documented:

  -r, --rebase[=false|true|merges|preserve|interactive]

> Also, --merge doesn't necessarily force a merge because it does not
> imply --no-ff.

This is a bit of semantics that I have discussed before with Elijah, and
somehow I managed to convince him [2] that fast-forward can be used as
an adverb, and in fact it already is in plenty of the documentation.

Basically this:

  git merge --ff-only

Does a fast-forward merge. So it's a merge of a special kind. What kind?
The fast-forward kind.

> I would prefer to list --no-rebase, -m, and --merge
> together on the same line and leave the description alone.

I prefer to deprecate --no-rebase. Both --rebase=false and --merge are
less cumbersome alternatives.

But I'm not married to this. If other people want to keep --no-rebase at
the same level as the other two options, I would not object to it.

> But other than the documentation and the commit message, I like the
> idea here, and I think it will make Git a lot more user-friendly.

Good. And for the record I think if this patch is not merged, eventually
somebody else will send something along these lines, since I'm not the
first person to think of it, nor will I be the last.

Cheers.

[1] https://lore.kernel.org/git/200910201947.50423.trast@student.ethz.ch/
[2] https://lore.kernel.org/git/CABPp-BESMs1tuVoLFMy-BahSChFz7oANqTaeJShFa_zDbEnvBA@mail.gmail.com/

-- 
Felipe Contreras