Re: [PATCH 3/5] rebase: factor out merge_base calculation

From: Jonathan Tan <jonathantanmy@google.com>
Date: Aug 24, 2022, 22:02 UTC
Message-ID: <20220824220251.1682480-1-jonathantanmy@google.com>
In-Reply-To: <019158db9d2dbb371705ba79a96a907e4a17cdb1.1660576283.git.gitgitgadget@gmail.com>

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 13 quoted lines

> @@ -1668,7 +1678,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  			die(_("Does not point to a valid commit '%s'"),
>  				options.onto_name);
>  	}
> -
> +	if (keep_base) {
> +		oidcpy(&merge_base, &options.onto->object.oid);
> +	} else {
> +		fill_merge_base(&options, &merge_base);
> +	}
>  	if (options.fork_point > 0)
>  		options.restrict_revision =
>  			get_fork_point(options.upstream_name, options.orig_head);

This patch makes sense, except for this part: why is fill_merge_base() only called for non-keep_base, but it seemed to be unconditionally called before? For what it's worth, all tests pass even with this diff:

  -       if (keep_base) {
  -               oidcpy(&merge_base, &options.onto->object.oid);
  -       } else {
  -               fill_merge_base(&options, &merge_base);
  -       }
  +       fill_merge_base(&options, &merge_base);