git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] pull: avoid running both merge and rebase

From
Jeff King <peff@peff.net>
Date
Mar 28, 2020, 15:56 UTC
Message-ID
<20200328155627.GB1215566@coredump.intra.peff.net>
In-Reply-To
<pull.739.git.git.1585345900763.gitgitgadget@gmail.com>
On Fri, Mar 27, 2020 at 09:51:40PM +0000, Elijah Newren via GitGitGadget wrote:
Show 19 quoted lines
> 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 was found when a user had both pull.rebase and rebase.autosquash
> set to true.  In such a case, the running of both merge and rebase would
> cause ORIG_HEAD to be updated twice (and match HEAD at the end instead
> of the commit before the rebase started), against expectation.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>     pull: avoid running both merge and rebase
>     
>     Cc: Norbert Kiesel nkiesel@gmail.com [nkiesel@gmail.com], Jeff King 
>     peff@peff.net [peff@peff.net]

I'm not sure how cc is supposed to work with GGG, but it clearly didn't here. :)

Anyway, the patch looks good. Thanks for following through on this.
Show 11 quoted lines
> @@ -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);

It feels like there should be some arrangement of the conditionals that doesn't require setting an extra flag, but I actually don't think there is. And anyway, doing the most obvious and minimal fix here is the right place to start. We don't need more regressions. ;)

-Peff
Previous: Junio C HamanoNext: Derrick Stolee
Message 3 of 7 in “pull: avoid running both merge and rebase”
  1. pull: avoid running both merge and rebaseElijah Newren via GitGitGadget, Mar 27, 2020
  2. Junio C HamanoMar 27, 2020
  3. Jeff KingMar 28, 2020
  4. Derrick StoleeMar 28, 2020
  5. Elijah NewrenMar 28, 2020
  6. Elijah NewrenMar 28, 2020
  7. GGG Cc: bug, was Re: [PATCH] pull: avoid running both merge and rebaseJohannes Schindelin, Apr 4, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.