Subject: Re: [PATCH v2 04/20] merge-recursive: exit early if index != head

From: Junio C Hamano <[email-masked]>
Date: Fri, 26 Jul 2019 12:32:12 -0700

Elijah Newren <[email-masked]> writes:

> Make sure we do the index == head check at the beginning of the merge,
> and error out immediately if it fails.  While we're at it, fix a small
> leak in the show-the-error codepath.

As the call to repo_index_has_changes() is moved to the very
beginning of merge_recursive() and merge_trees(), the workhorse of
the merge machinery, merge_trees_internal(), can lose it.

> +static int merge_start(struct merge_options *opt, struct tree *head)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +
> +	assert(opt->branch1 && opt->branch2);

This is a new assertion that did not exist in the original, isn't
it?  I do not object to new sensible assertions, and I think these
two fields must be non-null in a freshly initialized merge_options
structure, but shouldn't we be discussing if these two fields should
be non-NULL, and if there are other fields in the same structure
that we should be adding new assertions on, in a separate step on
its own?

> +	if (repo_index_has_changes(opt->repo, head, &sb)) {
> +		err(opt, _("Your local changes to the following files would be overwritten by merge:\n  %s"),
> +		    sb.buf);
> +		strbuf_release(&sb);
> +		return -1;
> +	}
> +
> +	return 0;
> +}