@matheusbsbs / matheus-bagliano / commits / 98a1d3d888

merge-recursive: exit early if index != head

We had a rule to enforce that the index matches head, but it was found at the beginning of merge_trees() and would only trigger when opt->call_depth was 0. Since merge_recursive() doesn't call merge_trees() until after returning from recursing, this meant that the check wasn't triggered by merge_recursive() until it had first finished all the intermediate merges to create virtual merge bases. That is a potentially huge amount of computation (and writing of intermediate merge results into the .git/objects directory) before it errors out and says, in effect, "Sorry, I can't do any merging because you have some local changes that would be overwritten." Further, not enforcing this requirement earlier allowed other bugs (such as an unintentional unconditional dropping and reloading of the index in merge_recursive() even when no recursion was necessary), to mask bugs in other callers (which were fixed in the commit prior to this one). 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. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Aug 17, 2019 at 11:41 UTC 98a1d3d88895a1cbc01b88627776bb4e2bfd6b84

1 file changed +72 -29

merge-recursive.c

+72 -29

@@ -3382,23 +3382,14 @@ static int process_entry(struct merge_options *opt,

3382 return clean_merge;

3383 }

3384

3385 -int merge_trees(struct merge_options *opt,

3386 - struct tree *head,

3387 - struct tree *merge,

3388 - struct tree *common,

3389 - struct tree **result)

3385 +static int merge_trees_internal(struct merge_options *opt,

3386 + struct tree *head,

3387 + struct tree *merge,

3388 + struct tree *common,

3389 + struct tree **result)

3390 {

3391 struct index_state *istate = opt->repo->index;

3392 int code, clean;

3393 - struct strbuf sb = STRBUF_INIT;

3394 -

3395 - assert(opt->ancestor != NULL);

3396 -

3397 - if (!opt->call_depth && repo_index_has_changes(opt->repo, head, &sb)) {

3398 - err(opt, _("Your local changes to the following files would be overwritten by merge:\n %s"),

3399 - sb.buf);

3400 - return -1;

3401 - }

3393

3394 if (opt->subtree_shift) {

3395 merge = shift_tree_object(opt->repo, head, merge, opt->subtree_shift);

@@ -3502,11 +3493,11 @@ static struct commit_list *reverse_commit_list(struct commit_list *list)

3493 * Merge the commits h1 and h2, return the resulting virtual

3494 * commit object and a flag indicating the cleanness of the merge.

3495 */

3505 -int merge_recursive(struct merge_options *opt,

3506 - struct commit *h1,

3507 - struct commit *h2,

3508 - struct commit_list *ca,

3509 - struct commit **result)

3496 +static int merge_recursive_internal(struct merge_options *opt,

3497 + struct commit *h1,

3498 + struct commit *h2,

3499 + struct commit_list *ca,

3500 + struct commit **result)

3501 {

3502 struct commit_list *iter;

3503 struct commit *merged_common_ancestors;

@@ -3515,9 +3506,6 @@ int merge_recursive(struct merge_options *opt,

3506 const char *ancestor_name;

3507 struct strbuf merge_base_abbrev = STRBUF_INIT;

3508

3518 - if (!opt->call_depth)

3519 - assert(opt->ancestor == NULL);

3520 -

3509 if (show(opt, 4)) {

3510 output(opt, 4, _("Merging:"));

3511 output_commit_title(opt, h1);

@@ -3571,7 +3559,7 @@ int merge_recursive(struct merge_options *opt,

3559 saved_b2 = opt->branch2;

3560 opt->branch1 = "Temporary merge branch 1";

3561 opt->branch2 = "Temporary merge branch 2";

3574 - if (merge_recursive(opt, merged_common_ancestors, iter->item,

3562 + if (merge_recursive_internal(opt, merged_common_ancestors, iter->item,

3563 NULL, &merged_common_ancestors) < 0)

3564 return -1;

3565 opt->branch1 = saved_b1;

@@ -3587,12 +3575,12 @@ int merge_recursive(struct merge_options *opt,

3575 repo_read_index(opt->repo);

3576

3577 opt->ancestor = ancestor_name;

3590 - clean = merge_trees(opt,

3591 - repo_get_commit_tree(opt->repo, h1),

3592 - repo_get_commit_tree(opt->repo, h2),

3593 - repo_get_commit_tree(opt->repo,

3594 - merged_common_ancestors),

3595 - &mrtree);

3578 + clean = merge_trees_internal(opt,

3579 + repo_get_commit_tree(opt->repo, h1),

3580 + repo_get_commit_tree(opt->repo, h2),

3581 + repo_get_commit_tree(opt->repo,

3582 + merged_common_ancestors),

3583 + &mrtree);

3584 strbuf_release(&merge_base_abbrev);

3585 if (clean < 0) {

3586 flush_output(opt);

@@ -3613,6 +3601,61 @@ int merge_recursive(struct merge_options *opt,

3601 return clean;

3602 }

3603

3604 +static int merge_start(struct merge_options *opt, struct tree *head)

3605 +{

3606 + struct strbuf sb = STRBUF_INIT;

3607 +

3608 + if (repo_index_has_changes(opt->repo, head, &sb)) {

3609 + err(opt, _("Your local changes to the following files would be overwritten by merge:\n %s"),

3610 + sb.buf);

3611 + strbuf_release(&sb);

3612 + return -1;

3613 + }

3614 +

3615 + return 0;

3616 +}

3617 +

3618 +static void merge_finalize(struct merge_options *opt)

3619 +{

3620 + /* Common code for wrapping up merges will be added here later */

3621 +}

3622 +

3623 +int merge_trees(struct merge_options *opt,

3624 + struct tree *head,

3625 + struct tree *merge,

3626 + struct tree *common,

3627 + struct tree **result)

3628 +{

3629 + int clean;

3630 +

3631 + assert(opt->ancestor != NULL);

3632 +

3633 + if (merge_start(opt, head))

3634 + return -1;

3635 + clean = merge_trees_internal(opt, head, merge, common, result);

3636 + merge_finalize(opt);

3637 +

3638 + return clean;

3639 +}

3640 +

3641 +int merge_recursive(struct merge_options *opt,

3642 + struct commit *h1,

3643 + struct commit *h2,

3644 + struct commit_list *ca,

3645 + struct commit **result)

3646 +{

3647 + int clean;

3648 +

3649 + assert(opt->ancestor == NULL);

3650 +

3651 + if (merge_start(opt, repo_get_commit_tree(opt->repo, h1)))

3652 + return -1;

3653 + clean = merge_recursive_internal(opt, h1, h2, ca, result);

3654 + merge_finalize(opt);

3655 +

3656 + return clean;

3657 +}

3658 +

3659 static struct commit *get_ref(struct repository *repo, const struct object_id *oid,

3660 const char *name)

3661 {