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

[Jonathan Tan <jonathantanmy@google.com>]

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> @@ -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);