[PATCH v2] pull: introduce --merge option

[PATCH v2] pull: introduce --merge option

[Felipe Contreras]

Users need to specify if they want to either --merge or --rebase, but
unfortunately the former is missing.

In many discussions regarding `git pull` --merge is often mentioned, but
for one reason or another the patches have not been merged.

Let's just go ahead and do it.

Now we can update the default warning to say --merge instead of the
non-intuitive --no-rebase.

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

Since v1 I've removed any hint of deprecation (even though we obviously
want to do that), and added an actual test (I don't seem to find a
simialr one for --no-rebase).

Range-diff against v1:
1:  80d7866599 < -:  ---------- pull: introduce --merge option
-:  ---------- > 1:  5969fc4455 pull: introduce --merge option

 Documentation/git-pull.txt | 6 ++++++
 builtin/pull.c             | 4 +++-
 t/t5520-pull.sh            | 7 +++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 5c3fb67c01..c7a1f676cc 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -134,6 +134,12 @@ unless you have read linkgit:git-rebase[1] carefully.
 --no-rebase::
 	Override earlier --rebase.
 
+-m::
+--merge::
+	Do a merge.
++
+Alias for --no-rebase.
+
 Options related to fetching
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/builtin/pull.c b/builtin/pull.c
index 3e13f81084..0d76b54186 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/t5520-pull.sh b/t/t5520-pull.sh
index e2c0c51022..f299a82405 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -452,6 +452,13 @@ test_expect_success 'pull.rebase' '
 	test_cmp expect actual
 '
 
+test_expect_success 'pull.rebase with --merge' '
+	git reset --hard before-rebase &&
+	test_config pull.rebase true &&
+	git pull --merge . copy &&
+	test_cmp_rev HEAD^2 copy
+'
+
 test_expect_success 'pull --autostash & pull.rebase=true' '
 	test_config pull.rebase true &&
 	test_pull_autostash 1 --autostash
-- 
2.32.0.40.gb9b36f9b52

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

[Linus Torvalds]

On Wed, Jul 21, 2021 at 6:47 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Users need to specify if they want to either --merge or --rebase, but
> unfortunately the former is missing.

Ack. I think it's just historical, because long long ago it used to be
that 'git pull' always merged unless told otherwise with --rebase.

               Linus

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

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, Jul 21, 2021 at 6:47 AM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>>
>> Users need to specify if they want to either --merge or --rebase, but
>> unfortunately the former is missing.
>
> Ack. I think it's just historical, because long long ago it used to be
> that 'git pull' always merged unless told otherwise with --rebase.

The "--no-rebase" option, which is documented as a synonym for
"--rebase=false", has been there, but the implementation is buggy in
some corner cases, which has been worked on recently in a separate
thread.  I do not think it is too bad to add "--merge" as yet
another synonym for "--rebase=false".

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

[Alex Henrie]

On Wed, Jul 21, 2021 at 11:11 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> The "--no-rebase" option, which is documented as a synonym for
> "--rebase=false", has been there, but the implementation is buggy in
> some corner cases, which has been worked on recently in a separate
> thread.  I do not think it is too bad to add "--merge" as yet
> another synonym for "--rebase=false".

It's convenient to have the one-letter option `git pull -r` to
override the configuration and do a rebase. I'd really like to have a
similar one-letter option `git pull -m` to override the configuration
and do a merge. That would also alleviate a lot of the desire for a
separate `git update` (i.e. "fetch and rebase") command.

Junio, would you be willing to accept adding -m without adding --merge also?

-Alex

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

[Felipe Contreras]

Alex Henrie wrote:
> On Wed, Jul 21, 2021 at 11:11 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > The "--no-rebase" option, which is documented as a synonym for
> > "--rebase=false", has been there, but the implementation is buggy in
> > some corner cases, which has been worked on recently in a separate
> > thread.  I do not think it is too bad to add "--merge" as yet
> > another synonym for "--rebase=false".
> 
> It's convenient to have the one-letter option `git pull -r` to
> override the configuration and do a rebase. I'd really like to have a
> similar one-letter option `git pull -m` to override the configuration
> and do a merge. That would also alleviate a lot of the desire for a
> separate `git update` (i.e. "fetch and rebase") command.

My proposed `git update` is not "fetch and rebase", but fetch and
fast-forward.

Morevoer, `git pull -m` would still merge with the wrong order of the
parents. On the other hand `git update --merge` would merge them with
the correct order.

I'm not sure what -m would alleviate.

-- 
Felipe Contreras

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

[Felipe Contreras]

Junio C Hamano wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > On Wed, Jul 21, 2021 at 6:47 AM Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> >>
> >> Users need to specify if they want to either --merge or --rebase, but
> >> unfortunately the former is missing.
> >
> > Ack. I think it's just historical, because long long ago it used to be
> > that 'git pull' always merged unless told otherwise with --rebase.
> 
> The "--no-rebase" option, which is documented as a synonym for
> "--rebase=false", has been there, but the implementation is buggy in
> some corner cases, which has been worked on recently in a separate
> thread.  I do not think it is too bad to add "--merge" as yet
> another synonym for "--rebase=false".

Any particular reason why this is not a topic? [1]

[1] https://lore.kernel.org/git/xmqq35s0fj9o.fsf@gitster.g/

-- 
Felipe Contreras

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

[Junio C Hamano]

Alex Henrie <alexhenrie24@gmail.com> writes:

> Junio, would you be willing to accept adding -m without adding --merge also?

My gut feeling is that "-m" without "--merge" in the context of
"pull" is extremely unlikely to fly well.

As "git pull" is a "git fetch" followed by a "git merge" (or "git
rebase"), it takes the union of common command line options from
both phases, and "git merge" takes "-m 'message'" which is an option
fairly familiar to users (since it comes from "git commit").  Even
if we are never going to pass "-m message" from "git pull" down to
underlying "git merge", squatting on short and common "-m" would be
a bad idea.

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

[Alex Henrie]

On Tue, Jul 27, 2021 at 2:45 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > Junio, would you be willing to accept adding -m without adding --merge also?
>
> My gut feeling is that "-m" without "--merge" in the context of
> "pull" is extremely unlikely to fly well.
>
> As "git pull" is a "git fetch" followed by a "git merge" (or "git
> rebase"), it takes the union of common command line options from
> both phases, and "git merge" takes "-m 'message'" which is an option
> fairly familiar to users (since it comes from "git commit").  Even
> if we are never going to pass "-m message" from "git pull" down to
> underlying "git merge", squatting on short and common "-m" would be
> a bad idea.

Thanks for the explanation. I forgot that "-m" usually means
"message". That does seem like a good reason to not use "-m" for
"merge".

-Alex

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

[Felipe Contreras]

Alex Henrie wrote:
> On Tue, Jul 27, 2021 at 2:45 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Alex Henrie <alexhenrie24@gmail.com> writes:
> >
> > > Junio, would you be willing to accept adding -m without adding --merge also?
> >
> > My gut feeling is that "-m" without "--merge" in the context of
> > "pull" is extremely unlikely to fly well.
> >
> > As "git pull" is a "git fetch" followed by a "git merge" (or "git
> > rebase"), it takes the union of common command line options from
> > both phases, and "git merge" takes "-m 'message'" which is an option
> > fairly familiar to users (since it comes from "git commit").  Even
> > if we are never going to pass "-m message" from "git pull" down to
> > underlying "git merge", squatting on short and common "-m" would be
> > a bad idea.
> 
> Thanks for the explanation. I forgot that "-m" usually means
> "message". That does seem like a good reason to not use "-m" for
> "merge".

It means --merge plenty of times:

 * git restore -m
 * git checkout -m
 * git rebase -m
 * git diff -m
 * git read-tree -m
 * git diff-tree -m

-- 
Felipe Contreras

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

[Matthias Baumgarten]

On 7/27/21 6:48 PM, Felipe Contreras wrote:
> Alex Henrie wrote:
>> On Tue, Jul 27, 2021 at 2:45 AM Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> Alex Henrie <alexhenrie24@gmail.com> writes:
>>>
>>>> Junio, would you be willing to accept adding -m without adding --merge also?
>>>
>>> My gut feeling is that "-m" without "--merge" in the context of
>>> "pull" is extremely unlikely to fly well.
>>>
>>> As "git pull" is a "git fetch" followed by a "git merge" (or "git
>>> rebase"), it takes the union of common command line options from
>>> both phases, and "git merge" takes "-m 'message'" which is an option
>>> fairly familiar to users (since it comes from "git commit").  Even
>>> if we are never going to pass "-m message" from "git pull" down to
>>> underlying "git merge", squatting on short and common "-m" would be
>>> a bad idea.
>>
>> Thanks for the explanation. I forgot that "-m" usually means
>> "message". That does seem like a good reason to not use "-m" for
>> "merge".
> 
> It means --merge plenty of times:
> 
>   * git restore -m
>   * git checkout -m
>   * git rebase -m
>   * git diff -m
>   * git read-tree -m
>   * git diff-tree -m

Add to Felipes list:

  * git switch -m

and maybe git cherry-pick -m where -m does not mean "merge" itself but 
is used to determine the parent of the merge (when picking merge 
commits) to base on.

Other examples of where -m has different meaning than merge:

  * git am -m (message-id)
  * git branch -m (move branch)

I would rephrase the question as to what would I expect `git pull -m` to 
do, if I had never heard of it before. In the case of fast-forwarding 
and rebasing trying to add a merge commit message with -m would not even 
make sense. Only in the case of trying to create a merge commit by 
issuing git pull this would make sense. So if we could agree on that 
being not the most used scenario, I think -m would be a great short 
option for --merge.

-- 
aixigo AG
Karl-Friedrich-Str. 68, 52072 Aachen, Germany
phone: +49 (0)241 559709-390, fax: +49 (0)241 559709-99
email: matthias.baumgarten@aixigo.com
web: https://www.aixigo.com
District Court Aachen – HRB 8057
Board: Christian Friedrich, Tobias Haustein
Chairman of the Supervisory Board: Dr. Roland Schlager

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

[Junio C Hamano]

Matthias Baumgarten <matthias.baumgarten@aixigo.com> writes:

> Add to Felipes list:
>
>  * git switch -m
>
> and maybe git cherry-pick -m where -m does not mean "merge" itself but
> is used to determine the parent of the merge (when picking merge 
> commits) to base on.
>
> Other examples of where -m has different meaning than merge:
>
>  * git am -m (message-id)
>  * git branch -m (move branch)
>
> I would rephrase the question as to what would I expect `git pull -m`
> to do, if I had never heard of it before. In the case of
> fast-forwarding and rebasing trying to add a merge commit message with
> -m would not even make sense. Only in the case of trying to create a
> merge commit by issuing git pull this would make sense. So if we could
> agree on that being not the most used scenario, I think -m would be a
> great short option for --merge.

I am afraid that you are misinterpreting what I said, comparing
apples and oranges, and drawing a wrong conclusion.

When I said "-m" would not fly well as a short-hand for "--merge" in
the context of "pull", I didn't mean "nobody would think 'm' stands
for 'merge'", and I didn't mean "more people would think 'm' stands
for 'message' more than 'merge'".  The reason why I find it
problematic is because it can be ambiguous.

When we step back and think about your "switch -m" and its synonym
"checkout -m", we realize that these commands fundamentally never
take "--message", as there is no place to record such a message
(they do not create a commit after all), after they switch to a
different branch while carrying the local modification forward by
performing a (possibly conflicting) content-level merge.  That is
why we can give their "merge" operation a short-and-sweet "m"
without confusing our users.  So contrasting "switch" having "-m"
that means "merge" with "pull" that can conceivably take both
"merge" and "message" is not a comparison you can draw useful
conclusion from.

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

[Matthias Baumgarten]

On 7/28/21 7:18 PM, Junio C Hamano wrote:
> I am afraid that you are misinterpreting what I said, comparing
> apples and oranges, and drawing a wrong conclusion >
> When I said "-m" would not fly well as a short-hand for "--merge" in
> the context of "pull", I didn't mean "nobody would think 'm' stands
> for 'merge'", and I didn't mean "more people would think 'm' stands
> for 'message' more than 'merge'".  The reason why I find it
> problematic is because it can be ambiguous >
> When we step back and think about your "switch -m" and its synonym
> "checkout -m", we realize that these commands fundamentally never
> take "--message", as there is no place to record such a message
> (they do not create a commit after all), after they switch to a
> different branch while carrying the local modification forward by
> performing a (possibly conflicting) content-level merge.  That is
> why we can give their "merge" operation a short-and-sweet "m"
> without confusing our users.  So contrasting "switch" having "-m"
> that means "merge" with "pull" that can conceivably take both
> "merge" and "message" is not a comparison you can draw useful
> conclusion from.
I must confess that this comparison is indeed not a valid one. Maybe -m 
isn't as great as I thought it was.

-- 
aixigo AG
Karl-Friedrich-Str. 68, 52072 Aachen, Germany
phone: +49 (0)241 559709-390, fax: +49 (0)241 559709-99
email: matthias.baumgarten@aixigo.com
web: https://www.aixigo.com
District Court Aachen – HRB 8057
Board: Christian Friedrich, Tobias Haustein
Chairman of the Supervisory Board: Dr. Roland Schlager

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

[Felipe Contreras]

Junio C Hamano wrote:
> Matthias Baumgarten <matthias.baumgarten@aixigo.com> writes:
> 
> > Add to Felipes list:
> >
> >  * git switch -m
> >
> > and maybe git cherry-pick -m where -m does not mean "merge" itself but
> > is used to determine the parent of the merge (when picking merge 
> > commits) to base on.
> >
> > Other examples of where -m has different meaning than merge:
> >
> >  * git am -m (message-id)
> >  * git branch -m (move branch)
> >
> > I would rephrase the question as to what would I expect `git pull -m`
> > to do, if I had never heard of it before. In the case of
> > fast-forwarding and rebasing trying to add a merge commit message with
> > -m would not even make sense. Only in the case of trying to create a
> > merge commit by issuing git pull this would make sense. So if we could
> > agree on that being not the most used scenario, I think -m would be a
> > great short option for --merge.
> 
> I am afraid that you are misinterpreting what I said, comparing
> apples and oranges, and drawing a wrong conclusion.
> 
> When I said "-m" would not fly well as a short-hand for "--merge" in
> the context of "pull", I didn't mean "nobody would think 'm' stands
> for 'merge'", and I didn't mean "more people would think 'm' stands
> for 'message' more than 'merge'".  The reason why I find it
> problematic is because it can be ambiguous.

The question shouldn't be "can it be ambiguous?", the question should be
"is it ambiguous?".

The *main* purpose of `git pull` is to integrate remote changes, and the
first question asked is "how?".

  git pull --merge|-m
  git pull --rebase|-r

So I don't see why it is ambiguous.

The fact that a tiny minority of users might find a command (any command)
ambiguous is not valid reason for its inexistence. By that rationale
`git pull` shouldn't exist at all, because many find it ambiguous that
it's not the symmetric command opposed to `git push`.

The vast majority of users shouldn't suffer because of the confusion of
a tiny few. The tiny few can simply look at the documentation.

-- 
Felipe Contreras