All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH] pull: avoid running both merge and rebase
Date: Fri, 27 Mar 2020 15:54:48 -0700	[thread overview]
Message-ID: <xmqqy2rlcssn.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <pull.739.git.git.1585345900763.gitgitgadget@gmail.com> (Elijah Newren via GitGitGadget's message of "Fri, 27 Mar 2020 21:51:40 +0000")

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> When opt_rebase is true, we still first check if we can fast-forward.
> If the branch is fast-forwardable, then we can avoid the rebase and just
> use merge to do the fast-forward logic.  However, when commit a6d7eb2c7a
> ("pull: optionally rebase submodules (remote submodule changes only)",
> 2017-06-23) added the ability to rebase submodules it accidentally
> caused us to run BOTH a merge and a rebase.  Add a flag to avoid doing
> both.

This is a fun one.  Thanks for digging to the bottom of the issue.

> diff --git a/builtin/pull.c b/builtin/pull.c
> index 3e624d1e008..19899b45c1d 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -976,6 +976,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>  
>  	if (opt_rebase) {
>  		int ret = 0;
> +		int ran_ff = 0;
>  		if ((recurse_submodules == RECURSE_SUBMODULES_ON ||
>  		     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&
>  		    submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))
> @@ -992,10 +993,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>  			if (is_descendant_of(merge_head, list)) {
>  				/* we can fast-forward this without invoking rebase */
>  				opt_ff = "--ff-only";
> +				ran_ff = 1;
>  				ret = run_merge();
>  			}
>  		}
> -		ret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);
> +		if (!ran_ff)
> +			ret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);
>  
>  		if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
>  			     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))
>
> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925

  reply	other threads:[~2020-03-27 22:54 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-27 21:51 [PATCH] pull: avoid running both merge and rebase Elijah Newren via GitGitGadget
2020-03-27 22:54 ` Junio C Hamano [this message]
2020-03-28 15:56 ` Jeff King
2020-03-28 16:55   ` Derrick Stolee
2020-03-28 17:17     ` Elijah Newren
2020-03-28 17:18   ` Elijah Newren
2020-04-04 13:47     ` GGG Cc: bug, was " Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy2rlcssn.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.