From: Junio C Hamano Date: Mon, 18 Jun 2018 17:18:44 +0000 (-0700) Subject: Merge branch 'en/rename-directory-detection' X-Git-Tag: v2.18.0~16 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/f72432d64ebc4fea80265114464ddb7b25ce9633?hp=-c Merge branch 'en/rename-directory-detection' Newly added codepath in merge-recursive had potential buffer overrun, which has been fixed. * en/rename-directory-detection: merge-recursive: use xstrdup() instead of fixed buffer --- f72432d64ebc4fea80265114464ddb7b25ce9633 diff --combined merge-recursive.c index 5eb907f46e,7ea70c4a9d..f110e1c5ec --- a/merge-recursive.c +++ b/merge-recursive.c @@@ -23,7 -23,6 +23,7 @@@ #include "merge-recursive.h" #include "dir.h" #include "submodule.h" +#include "revision.h" struct path_hashmap_entry { struct hashmap_entry e; @@@ -163,7 -162,7 +163,7 @@@ static struct commit *make_virtual_comm struct commit *commit = alloc_commit_node(); set_merge_remote_desc(commit, comment, (struct object *)commit); - commit->tree = tree; + commit->maybe_tree = tree; commit->object.parsed = 1; return commit; } @@@ -291,7 -290,7 +291,7 @@@ static void output_commit_title(struct strbuf_addf(&o->obuf, "virtual %s\n", merge_remote_util(commit)->name); else { - strbuf_add_unique_abbrev(&o->obuf, commit->object.oid.hash, + strbuf_add_unique_abbrev(&o->obuf, &commit->object.oid, DEFAULT_ABBREV); strbuf_addch(&o->obuf, ' '); if (parse_commit(commit) != 0) @@@ -317,7 -316,7 +317,7 @@@ static int add_cacheinfo(struct merge_o ce = make_cache_entry(mode, oid ? oid->hash : null_sha1, path, stage, 0); if (!ce) - return err(o, _("addinfo_cache failed for path '%s'"), path); + return err(o, _("add_cacheinfo failed for path '%s'; merge aborting."), path); ret = add_cache_entry(ce, options); if (refresh) { @@@ -325,7 -324,7 +325,7 @@@ nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING); if (!nce) - return err(o, _("addinfo_cache failed for path '%s'"), path); + return err(o, _("add_cacheinfo failed to refresh for path '%s'; merge aborting."), path); if (nce != ce) ret = add_cache_entry(nce, options); } @@@ -338,14 -337,13 +338,14 @@@ static void init_tree_desc_from_tree(st init_tree_desc(desc, tree->buffer, tree->size); } -static int git_merge_trees(struct merge_options *o, - struct tree *common, - struct tree *head, - struct tree *merge) +static int unpack_trees_start(struct merge_options *o, + struct tree *common, + struct tree *head, + struct tree *merge) { int rc; struct tree_desc t[3]; + struct index_state tmp_index = { NULL }; memset(&o->unpack_opts, 0, sizeof(o->unpack_opts)); if (o->call_depth) @@@ -356,8 -354,7 +356,8 @@@ o->unpack_opts.head_idx = 2; o->unpack_opts.fn = threeway_merge; o->unpack_opts.src_index = &the_index; - o->unpack_opts.dst_index = &the_index; + o->unpack_opts.dst_index = &tmp_index; + o->unpack_opts.aggressive = !merge_detect_rename(o); setup_unpack_trees_porcelain(&o->unpack_opts, "merge"); init_tree_desc_from_tree(t+0, common); @@@ -365,27 -362,16 +365,27 @@@ init_tree_desc_from_tree(t+2, merge); rc = unpack_trees(3, t, &o->unpack_opts); + cache_tree_free(&active_cache_tree); + /* - * unpack_trees NULLifies src_index, but it's used in verify_uptodate, - * so set to the new index which will usually have modification - * timestamp info copied over. + * Update the_index to match the new results, AFTER saving a copy + * in o->orig_index. Update src_index to point to the saved copy. + * (verify_uptodate() checks src_index, and the original index is + * the one that had the necessary modification timestamps.) */ - o->unpack_opts.src_index = &the_index; - cache_tree_free(&active_cache_tree); + o->orig_index = the_index; + the_index = tmp_index; + o->unpack_opts.src_index = &o->orig_index; + return rc; } +static void unpack_trees_finish(struct merge_options *o) +{ + discard_index(&o->orig_index); + clear_unpack_trees_porcelain(&o->unpack_opts); +} + struct tree *write_tree_from_memory(struct merge_options *o) { struct tree *result = NULL; @@@ -399,7 -385,7 +399,7 @@@ fprintf(stderr, "BUG: %d %.*s\n", ce_stage(ce), (int)ce_namelen(ce), ce->name); } - die("BUG: unmerged index entries in merge-recursive.c"); + BUG("unmerged index entries in merge-recursive.c"); } if (!active_cache_tree) @@@ -416,7 -402,7 +416,7 @@@ return result; } -static int save_files_dirs(const unsigned char *sha1, +static int save_files_dirs(const struct object_id *oid, struct strbuf *base, const char *path, unsigned int mode, int stage, void *context) { @@@ -441,16 -427,16 +441,16 @@@ static void get_files_dirs(struct merge read_tree_recursive(tree, "", 0, 0, &match_all, save_files_dirs, o); } -static int get_tree_entry_if_blob(const unsigned char *tree, +static int get_tree_entry_if_blob(const struct object_id *tree, const char *path, - unsigned char *hashy, + struct object_id *hashy, unsigned int *mode_o) { int ret; ret = get_tree_entry(tree, path, hashy, mode_o); if (S_ISDIR(*mode_o)) { - hashcpy(hashy, null_sha1); + oidcpy(hashy, &null_oid); *mode_o = 0; } return ret; @@@ -466,12 -452,12 +466,12 @@@ static struct stage_data *insert_stage_ { struct string_list_item *item; struct stage_data *e = xcalloc(1, sizeof(struct stage_data)); - get_tree_entry_if_blob(o->object.oid.hash, path, - e->stages[1].oid.hash, &e->stages[1].mode); - get_tree_entry_if_blob(a->object.oid.hash, path, - e->stages[2].oid.hash, &e->stages[2].mode); - get_tree_entry_if_blob(b->object.oid.hash, path, - e->stages[3].oid.hash, &e->stages[3].mode); + get_tree_entry_if_blob(&o->object.oid, path, + &e->stages[1].oid, &e->stages[1].mode); + get_tree_entry_if_blob(&a->object.oid, path, + &e->stages[2].oid, &e->stages[2].mode); + get_tree_entry_if_blob(&b->object.oid, path, + &e->stages[3].oid, &e->stages[3].mode); item = string_list_insert(entries, path); item->util = e; return e; @@@ -787,78 -773,31 +787,78 @@@ static int dir_in_way(const char *path !(empty_ok && is_empty_dir(path)); } -static int was_tracked(const char *path) +/* + * Returns whether path was tracked in the index before the merge started, + * and its oid and mode match the specified values + */ +static int was_tracked_and_matches(struct merge_options *o, const char *path, + const struct object_id *oid, unsigned mode) { - int pos = cache_name_pos(path, strlen(path)); + int pos = index_name_pos(&o->orig_index, path, strlen(path)); + struct cache_entry *ce; + + if (0 > pos) + /* we were not tracking this path before the merge */ + return 0; + + /* See if the file we were tracking before matches */ + ce = o->orig_index.cache[pos]; + return (oid_eq(&ce->oid, oid) && ce->ce_mode == mode); +} + +/* + * Returns whether path was tracked in the index before the merge started + */ +static int was_tracked(struct merge_options *o, const char *path) +{ + int pos = index_name_pos(&o->orig_index, path, strlen(path)); if (0 <= pos) - /* we have been tracking this path */ + /* we were tracking this path before the merge */ return 1; - /* - * Look for an unmerged entry for the path, - * specifically stage #2, which would indicate - * that "our" side before the merge started - * had the path tracked (and resulted in a conflict). - */ - for (pos = -1 - pos; - pos < active_nr && !strcmp(path, active_cache[pos]->name); - pos++) - if (ce_stage(active_cache[pos]) == 2) - return 1; return 0; } static int would_lose_untracked(const char *path) { - return !was_tracked(path) && file_exists(path); + /* + * This may look like it can be simplified to: + * return !was_tracked(o, path) && file_exists(path) + * but it can't. This function needs to know whether path was in + * the working tree due to EITHER having been tracked in the index + * before the merge OR having been put into the working copy and + * index by unpack_trees(). Due to that either-or requirement, we + * check the current index instead of the original one. + * + * Note that we do not need to worry about merge-recursive itself + * updating the index after unpack_trees() and before calling this + * function, because we strictly require all code paths in + * merge-recursive to update the working tree first and the index + * second. Doing otherwise would break + * update_file()/would_lose_untracked(); see every comment in this + * file which mentions "update_stages". + */ + int pos = cache_name_pos(path, strlen(path)); + + if (pos < 0) + pos = -1 - pos; + while (pos < active_nr && + !strcmp(path, active_cache[pos]->name)) { + /* + * If stage #0, it is definitely tracked. + * If it has stage #2 then it was tracked + * before this merge started. All other + * cases the path was not tracked. + */ + switch (ce_stage(active_cache[pos])) { + case 0: + case 2: + return 0; + } + pos++; + } + return file_exists(path); } static int was_dirty(struct merge_options *o, const char *path) @@@ -866,12 -805,12 +866,12 @@@ struct cache_entry *ce; int dirty = 1; - if (o->call_depth || !was_tracked(path)) + if (o->call_depth || !was_tracked(o, path)) return !dirty; - ce = cache_file_exists(path, strlen(path), ignore_case); - dirty = (ce->ce_stat_data.sd_mtime.sec > 0 && - verify_uptodate(ce, &o->unpack_opts) != 0); + ce = index_file_exists(o->unpack_opts.src_index, + path, strlen(path), ignore_case); + dirty = verify_uptodate(ce, &o->unpack_opts) != 0; return dirty; } @@@ -952,7 -891,7 +952,7 @@@ static int update_file_flags(struct mer goto update_index; } - buf = read_sha1_file(oid->hash, &type, &size); + buf = read_object_file(oid, &type, &size); if (!buf) return err(o, _("cannot read object %s '%s'"), oid_to_hex(oid), path); if (type != OBJ_BLOB) { @@@ -1003,9 -942,7 +1003,9 @@@ } update_index: if (!ret && update_cache) - add_cacheinfo(o, mode, oid, path, 0, update_wd, ADD_CACHE_OK_TO_ADD); + if (add_cacheinfo(o, mode, oid, path, 0, update_wd, + ADD_CACHE_OK_TO_ADD)) + return -1; return ret; } @@@ -1089,193 -1026,13 +1089,193 @@@ static int merge_3way(struct merge_opti return merge_status; } +static int find_first_merges(struct object_array *result, const char *path, + struct commit *a, struct commit *b) +{ + int i, j; + struct object_array merges = OBJECT_ARRAY_INIT; + struct commit *commit; + int contains_another; + + char merged_revision[42]; + const char *rev_args[] = { "rev-list", "--merges", "--ancestry-path", + "--all", merged_revision, NULL }; + struct rev_info revs; + struct setup_revision_opt rev_opts; + + memset(result, 0, sizeof(struct object_array)); + memset(&rev_opts, 0, sizeof(rev_opts)); + + /* get all revisions that merge commit a */ + xsnprintf(merged_revision, sizeof(merged_revision), "^%s", + oid_to_hex(&a->object.oid)); + init_revisions(&revs, NULL); + rev_opts.submodule = path; + /* FIXME: can't handle linked worktrees in submodules yet */ + revs.single_worktree = path != NULL; + setup_revisions(ARRAY_SIZE(rev_args)-1, rev_args, &revs, &rev_opts); + + /* save all revisions from the above list that contain b */ + if (prepare_revision_walk(&revs)) + die("revision walk setup failed"); + while ((commit = get_revision(&revs)) != NULL) { + struct object *o = &(commit->object); + if (in_merge_bases(b, commit)) + add_object_array(o, NULL, &merges); + } + reset_revision_walk(); + + /* Now we've got all merges that contain a and b. Prune all + * merges that contain another found merge and save them in + * result. + */ + for (i = 0; i < merges.nr; i++) { + struct commit *m1 = (struct commit *) merges.objects[i].item; + + contains_another = 0; + for (j = 0; j < merges.nr; j++) { + struct commit *m2 = (struct commit *) merges.objects[j].item; + if (i != j && in_merge_bases(m2, m1)) { + contains_another = 1; + break; + } + } + + if (!contains_another) + add_object_array(merges.objects[i].item, NULL, result); + } + + object_array_clear(&merges); + return result->nr; +} + +static void print_commit(struct commit *commit) +{ + struct strbuf sb = STRBUF_INIT; + struct pretty_print_context ctx = {0}; + ctx.date_mode.type = DATE_NORMAL; + format_commit_message(commit, " %h: %m %s", &sb, &ctx); + fprintf(stderr, "%s\n", sb.buf); + strbuf_release(&sb); +} + +static int merge_submodule(struct merge_options *o, + struct object_id *result, const char *path, + const struct object_id *base, const struct object_id *a, + const struct object_id *b) +{ + struct commit *commit_base, *commit_a, *commit_b; + int parent_count; + struct object_array merges; + + int i; + int search = !o->call_depth; + + /* store a in result in case we fail */ + oidcpy(result, a); + + /* we can not handle deletion conflicts */ + if (is_null_oid(base)) + return 0; + if (is_null_oid(a)) + return 0; + if (is_null_oid(b)) + return 0; + + if (add_submodule_odb(path)) { + output(o, 1, _("Failed to merge submodule %s (not checked out)"), path); + return 0; + } + + if (!(commit_base = lookup_commit_reference(base)) || + !(commit_a = lookup_commit_reference(a)) || + !(commit_b = lookup_commit_reference(b))) { + output(o, 1, _("Failed to merge submodule %s (commits not present)"), path); + return 0; + } + + /* check whether both changes are forward */ + if (!in_merge_bases(commit_base, commit_a) || + !in_merge_bases(commit_base, commit_b)) { + output(o, 1, _("Failed to merge submodule %s (commits don't follow merge-base)"), path); + return 0; + } + + /* Case #1: a is contained in b or vice versa */ + if (in_merge_bases(commit_a, commit_b)) { + oidcpy(result, b); + if (show(o, 3)) { + output(o, 3, _("Fast-forwarding submodule %s to the following commit:"), path); + output_commit_title(o, commit_b); + } else if (show(o, 2)) + output(o, 2, _("Fast-forwarding submodule %s"), path); + else + ; /* no output */ + + return 1; + } + if (in_merge_bases(commit_b, commit_a)) { + oidcpy(result, a); + if (show(o, 3)) { + output(o, 3, _("Fast-forwarding submodule %s to the following commit:"), path); + output_commit_title(o, commit_a); + } else if (show(o, 2)) + output(o, 2, _("Fast-forwarding submodule %s"), path); + else + ; /* no output */ + + return 1; + } + + /* + * Case #2: There are one or more merges that contain a and b in + * the submodule. If there is only one, then present it as a + * suggestion to the user, but leave it marked unmerged so the + * user needs to confirm the resolution. + */ + + /* Skip the search if makes no sense to the calling context. */ + if (!search) + return 0; + + /* find commit which merges them */ + parent_count = find_first_merges(&merges, path, commit_a, commit_b); + switch (parent_count) { + case 0: + output(o, 1, _("Failed to merge submodule %s (merge following commits not found)"), path); + break; + + case 1: + output(o, 1, _("Failed to merge submodule %s (not fast-forward)"), path); + output(o, 2, _("Found a possible merge resolution for the submodule:\n")); + print_commit((struct commit *) merges.objects[0].item); + output(o, 2, _( + "If this is correct simply add it to the index " + "for example\n" + "by using:\n\n" + " git update-index --cacheinfo 160000 %s \"%s\"\n\n" + "which will accept this suggestion.\n"), + oid_to_hex(&merges.objects[0].item->oid), path); + break; + + default: + output(o, 1, _("Failed to merge submodule %s (multiple merges found)"), path); + for (i = 0; i < merges.nr; i++) + print_commit((struct commit *) merges.objects[i].item); + } + + object_array_clear(&merges); + return 0; +} + static int merge_file_1(struct merge_options *o, - const struct diff_filespec *one, - const struct diff_filespec *a, - const struct diff_filespec *b, - const char *branch1, - const char *branch2, - struct merge_file_info *result) + const struct diff_filespec *one, + const struct diff_filespec *a, + const struct diff_filespec *b, + const char *filename, + const char *branch1, + const char *branch2, + struct merge_file_info *result) { result->merge = 0; result->clean = 1; @@@ -1320,9 -1077,8 +1320,9 @@@ if ((merge_status < 0) || !result_buf.ptr) ret = err(o, _("Failed to execute internal merge")); - if (!ret && write_sha1_file(result_buf.ptr, result_buf.size, - blob_type, result->oid.hash)) + if (!ret && + write_object_file(result_buf.ptr, result_buf.size, + blob_type, &result->oid)) ret = err(o, _("Unable to add %s to database"), a->path); @@@ -1331,45 -1087,33 +1331,45 @@@ return ret; result->clean = (merge_status == 0); } else if (S_ISGITLINK(a->mode)) { - result->clean = merge_submodule(&result->oid, + result->clean = merge_submodule(o, &result->oid, one->path, &one->oid, &a->oid, - &b->oid, - !o->call_depth); + &b->oid); } else if (S_ISLNK(a->mode)) { - oidcpy(&result->oid, &a->oid); - - if (!oid_eq(&a->oid, &b->oid)) - result->clean = 0; + switch (o->recursive_variant) { + case MERGE_RECURSIVE_NORMAL: + oidcpy(&result->oid, &a->oid); + if (!oid_eq(&a->oid, &b->oid)) + result->clean = 0; + break; + case MERGE_RECURSIVE_OURS: + oidcpy(&result->oid, &a->oid); + break; + case MERGE_RECURSIVE_THEIRS: + oidcpy(&result->oid, &b->oid); + break; + } } else - die("BUG: unsupported object type in the tree"); + BUG("unsupported object type in the tree"); } + if (result->merge) + output(o, 2, _("Auto-merging %s"), filename); + return 0; } static int merge_file_special_markers(struct merge_options *o, - const struct diff_filespec *one, - const struct diff_filespec *a, - const struct diff_filespec *b, - const char *branch1, - const char *filename1, - const char *branch2, - const char *filename2, - struct merge_file_info *mfi) + const struct diff_filespec *one, + const struct diff_filespec *a, + const struct diff_filespec *b, + const char *target_filename, + const char *branch1, + const char *filename1, + const char *branch2, + const char *filename2, + struct merge_file_info *mfi) { char *side1 = NULL; char *side2 = NULL; @@@ -1380,23 -1124,22 +1380,23 @@@ if (filename2) side2 = xstrfmt("%s:%s", branch2, filename2); - ret = merge_file_1(o, one, a, b, + ret = merge_file_1(o, one, a, b, target_filename, side1 ? side1 : branch1, side2 ? side2 : branch2, mfi); + free(side1); free(side2); return ret; } static int merge_file_one(struct merge_options *o, - const char *path, - const struct object_id *o_oid, int o_mode, - const struct object_id *a_oid, int a_mode, - const struct object_id *b_oid, int b_mode, - const char *branch1, - const char *branch2, - struct merge_file_info *mfi) + const char *path, + const struct object_id *o_oid, int o_mode, + const struct object_id *a_oid, int a_mode, + const struct object_id *b_oid, int b_mode, + const char *branch1, + const char *branch2, + struct merge_file_info *mfi) { struct diff_filespec one, a, b; @@@ -1407,7 -1150,7 +1407,7 @@@ a.mode = a_mode; oidcpy(&b.oid, b_oid); b.mode = b_mode; - return merge_file_1(o, &one, &a, &b, branch1, branch2, mfi); + return merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi); } static int conflict_rename_dir(struct merge_options *o, @@@ -1685,8 -1428,6 +1685,8 @@@ static int conflict_rename_rename_2to1( struct diff_filespec *c1 = ci->pair1->two; struct diff_filespec *c2 = ci->pair2->two; char *path = c1->path; /* == c2->path */ + char *path_side_1_desc; + char *path_side_2_desc; struct merge_file_info mfi_c1; struct merge_file_info mfi_c2; int ret; @@@ -1700,19 -1441,13 +1700,19 @@@ remove_file(o, 1, a->path, o->call_depth || would_lose_untracked(a->path)); remove_file(o, 1, b->path, o->call_depth || would_lose_untracked(b->path)); + path_side_1_desc = xstrfmt("%s (was %s)", path, a->path); + path_side_2_desc = xstrfmt("%s (was %s)", path, b->path); if (merge_file_special_markers(o, a, c1, &ci->ren1_other, + path_side_1_desc, o->branch1, c1->path, o->branch2, ci->ren1_other.path, &mfi_c1) || merge_file_special_markers(o, b, &ci->ren2_other, c2, + path_side_2_desc, o->branch1, ci->ren2_other.path, o->branch2, c2->path, &mfi_c2)) return -1; + free(path_side_1_desc); + free(path_side_2_desc); if (o->call_depth) { /* @@@ -1789,15 -1524,7 +1789,15 @@@ static struct diff_queue_struct *get_di diff_setup(&opts); opts.flags.recursive = 1; opts.flags.rename_empty = 0; - opts.detect_rename = DIFF_DETECT_RENAME; + opts.detect_rename = merge_detect_rename(o); + /* + * We do not have logic to handle the detection of copies. In + * fact, it may not even make sense to add such logic: would we + * really want a change to a base file to be propagated through + * multiple other files by a merge? + */ + if (opts.detect_rename > DIFF_DETECT_RENAME) + opts.detect_rename = DIFF_DETECT_RENAME; opts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit : o->diff_rename_limit >= 0 ? o->diff_rename_limit : 1000; @@@ -1822,11 -1549,11 +1822,11 @@@ static int tree_has_path(struct tree *tree, const char *path) { - unsigned char hashy[GIT_MAX_RAWSZ]; + struct object_id hashy; unsigned int mode_o; - return !get_tree_entry(tree->object.oid.hash, path, - hashy, &mode_o); + return !get_tree_entry(&tree->object.oid, path, + &hashy, &mode_o); } /* @@@ -2124,7 -1851,7 +2124,7 @@@ static struct hashmap *get_directory_re * renames, finding out how often each directory rename pair * possibility occurs. */ - dir_renames = xmalloc(sizeof(struct hashmap)); + dir_renames = xmalloc(sizeof(*dir_renames)); dir_rename_init(dir_renames); for (i = 0; i < pairs->nr; ++i) { struct string_list_item *item; @@@ -2144,7 -1871,7 +2144,7 @@@ entry = dir_rename_find_entry(dir_renames, old_dir); if (!entry) { - entry = xmalloc(sizeof(struct dir_rename_entry)); + entry = xmalloc(sizeof(*entry)); dir_rename_entry_init(entry, old_dir); hashmap_put(dir_renames, entry); } else { @@@ -2211,18 -1938,18 +2211,18 @@@ static struct dir_rename_entry *check_dir_renamed(const char *path, struct hashmap *dir_renames) { - char temp[PATH_MAX]; + char *temp = xstrdup(path); char *end; - struct dir_rename_entry *entry; + struct dir_rename_entry *entry = NULL;; - strcpy(temp, path); while ((end = strrchr(temp, '/'))) { *end = '\0'; entry = dir_rename_find_entry(dir_renames, temp); if (entry) - return entry; + break; } - return NULL; + free(temp); + return entry; } static void compute_collisions(struct hashmap *collisions, @@@ -2422,9 -2149,9 +2422,9 @@@ static void apply_directory_rename_modi * the various conflict_rename_*() functions update the index * explicitly rather than relying on unpack_trees() to have done it. */ - get_tree_entry(tree->object.oid.hash, + get_tree_entry(&tree->object.oid, pair->two->path, - re->dst_entry->stages[stage].oid.hash, + &re->dst_entry->stages[stage].oid, &re->dst_entry->stages[stage].mode); /* Update pair status */ @@@ -2604,7 -2331,7 +2604,7 @@@ static int process_renames(struct merge const char *ren2_dst = ren2->pair->two->path; enum rename_type rename_type; if (strcmp(ren1_src, ren2_src) != 0) - die("BUG: ren1_src != ren2_src"); + BUG("ren1_src != ren2_src"); ren2->dst_entry->processed = 1; ren2->processed = 1; if (strcmp(ren1_dst, ren2_dst) != 0) { @@@ -2638,7 -2365,7 +2638,7 @@@ ren2 = lookup->util; ren2_dst = ren2->pair->two->path; if (strcmp(ren1_dst, ren2_dst) != 0) - die("BUG: ren1_dst != ren2_dst"); + BUG("ren1_dst != ren2_dst"); clean_merge = 0; ren2->processed = 1; @@@ -2680,7 -2407,7 +2680,7 @@@ * add-source case). */ remove_file(o, 1, ren1_src, - renamed_stage == 2 || !was_tracked(ren1_src)); + renamed_stage == 2 || !was_tracked(o, ren1_src)); oidcpy(&src_other.oid, &ren1->src_entry->stages[other_stage].oid); @@@ -2837,7 -2564,7 +2837,7 @@@ static int handle_renames(struct merge_ ri->head_renames = NULL; ri->merge_renames = NULL; - if (!o->detect_rename) + if (!merge_detect_rename(o)) return 1; head_pairs = get_diffpairs(o, common, head); @@@ -2909,7 -2636,7 +2909,7 @@@ static int read_oid_strbuf(struct merge void *buf; enum object_type type; unsigned long size; - buf = read_sha1_file(oid->hash, &type, &size); + buf = read_object_file(oid, &type, &size); if (!buf) return err(o, _("cannot read object %s"), oid_to_hex(oid)); if (type != OBJ_BLOB) { @@@ -2988,7 -2715,7 +2988,7 @@@ static int handle_modify_delete(struct static int merge_content(struct merge_options *o, const char *path, - int file_in_way, + int is_dirty, struct object_id *o_oid, int o_mode, struct object_id *a_oid, int a_mode, struct object_id *b_oid, int b_mode, @@@ -3029,26 -2756,27 +3029,26 @@@ S_ISGITLINK(pair1->two->mode))) df_conflict_remains = 1; } - if (merge_file_special_markers(o, &one, &a, &b, + if (merge_file_special_markers(o, &one, &a, &b, path, o->branch1, path1, o->branch2, path2, &mfi)) return -1; - if (mfi.clean && !df_conflict_remains && - oid_eq(&mfi.oid, a_oid) && mfi.mode == a_mode) { + /* + * We can skip updating the working tree file iff: + * a) The merge is clean + * b) The merge matches what was in HEAD (content, mode, pathname) + * c) The target path is usable (i.e. not involved in D/F conflict) + */ + if (mfi.clean && + was_tracked_and_matches(o, path, &mfi.oid, mfi.mode) && + !df_conflict_remains) { output(o, 3, _("Skipped %s (merged same as existing)"), path); - /* - * The content merge resulted in the same file contents we - * already had. We can return early if those file contents - * are recorded at the correct path (which may not be true - * if the merge involves a rename). - */ - if (was_tracked(path)) { - add_cacheinfo(o, mfi.mode, &mfi.oid, path, - 0, (!o->call_depth), 0); - return mfi.clean; - } - } else - output(o, 2, _("Auto-merging %s"), path); + if (add_cacheinfo(o, mfi.mode, &mfi.oid, path, + 0, (!o->call_depth && !is_dirty), 0)) + return -1; + return mfi.clean; + } if (!mfi.clean) { if (S_ISGITLINK(mfi.mode)) @@@ -3060,7 -2788,7 +3060,7 @@@ return -1; } - if (df_conflict_remains || file_in_way) { + if (df_conflict_remains || is_dirty) { char *new_path; if (o->call_depth) { remove_file_from_cache(path); @@@ -3069,7 -2797,7 +3069,7 @@@ if (update_stages(o, path, &one, &a, &b)) return -1; } else { - int file_from_stage2 = was_tracked(path); + int file_from_stage2 = was_tracked(o, path); struct diff_filespec merged; oidcpy(&merged.oid, &mfi.oid); merged.mode = mfi.mode; @@@ -3082,10 -2810,6 +3082,10 @@@ } new_path = unique_path(o, path, rename_conflict_info->branch1); + if (is_dirty) { + output(o, 1, _("Refusing to lose dirty file at %s"), + path); + } output(o, 1, _("Adding as %s instead"), new_path); if (update_file(o, 0, &mfi.oid, mfi.mode, new_path)) { free(new_path); @@@ -3095,7 -2819,7 +3095,7 @@@ mfi.clean = 0; } else if (update_file(o, mfi.clean, &mfi.oid, mfi.mode, path)) return -1; - return mfi.clean; + return !is_dirty && mfi.clean; } static int conflict_rename_normal(struct merge_options *o, @@@ -3105,10 -2829,21 +3105,10 @@@ struct object_id *b_oid, unsigned int b_mode, struct rename_conflict_info *ci) { - int clean_merge; - int file_in_the_way = 0; - - if (was_dirty(o, path)) { - file_in_the_way = 1; - output(o, 1, _("Refusing to lose dirty file at %s"), path); - } - /* Merge the content and write it out */ - clean_merge = merge_content(o, path, file_in_the_way, - o_oid, o_mode, a_oid, a_mode, b_oid, b_mode, - ci); - if (clean_merge > 0 && file_in_the_way) - clean_merge = 0; - return clean_merge; + return merge_content(o, path, was_dirty(o, path), + o_oid, o_mode, a_oid, a_mode, b_oid, b_mode, + ci); } /* Per entry merge function */ @@@ -3231,8 -2966,7 +3231,8 @@@ static int process_entry(struct merge_o } else if (a_oid && b_oid) { /* Case C: Added in both (check for same permissions) and */ /* case D: Modified in both, but differently. */ - clean_merge = merge_content(o, path, 0 /* file_in_way */, + int is_dirty = 0; /* unpack_trees would have bailed if dirty */ + clean_merge = merge_content(o, path, is_dirty, o_oid, o_mode, a_oid, a_mode, b_oid, b_mode, NULL); } else if (!o_oid && !a_oid && !b_oid) { @@@ -3242,7 -2976,7 +3242,7 @@@ */ remove_file(o, 1, path, !a_mode); } else - die("BUG: fatal merge failure, shouldn't happen."); + BUG("fatal merge failure, shouldn't happen."); return clean_merge; } @@@ -3273,14 -3007,13 +3273,14 @@@ int merge_trees(struct merge_options *o return 1; } - code = git_merge_trees(o, common, head, merge); + code = unpack_trees_start(o, common, head, merge); if (code != 0) { if (show(o, 4) || o->call_depth) err(o, _("merging of trees %s and %s failed"), oid_to_hex(&head->object.oid), oid_to_hex(&merge->object.oid)); + unpack_trees_finish(o); return -1; } @@@ -3321,7 -3054,7 +3321,7 @@@ for (i = 0; i < entries->nr; i++) { struct stage_data *e = entries->items[i].util; if (!e->processed) - die("BUG: unprocessed path??? %s", + BUG("unprocessed path??? %s", entries->items[i].string); } @@@ -3333,16 -3066,12 +3333,16 @@@ cleanup hashmap_free(&o->current_file_dir_set, 1); - if (clean < 0) + if (clean < 0) { + unpack_trees_finish(o); return clean; + } } else clean = 1; + unpack_trees_finish(o); + if (o->call_depth && !(*result = write_tree_from_memory(o))) return -1; @@@ -3372,7 -3101,7 +3372,7 @@@ int merge_recursive(struct merge_option { struct commit_list *iter; struct commit *merged_common_ancestors; - struct tree *mrtree = mrtree; + struct tree *mrtree; int clean; if (show(o, 4)) { @@@ -3436,8 -3165,7 +3436,8 @@@ read_cache(); o->ancestor = "merged common ancestors"; - clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree, + clean = merge_trees(o, get_commit_tree(h1), get_commit_tree(h2), + get_commit_tree(merged_common_ancestors), &mrtree); if (clean < 0) { flush_output(o); @@@ -3501,13 -3229,11 +3501,13 @@@ int merge_recursive_generic(struct merg hold_locked_index(&lock, LOCK_DIE_ON_ERROR); clean = merge_recursive(o, head_commit, next_commit, ca, result); - if (clean < 0) + if (clean < 0) { + rollback_lock_file(&lock); return clean; + } - if (active_cache_changed && - write_locked_index(&the_index, &lock, COMMIT_LOCK)) + if (write_locked_index(&the_index, &lock, + COMMIT_LOCK | SKIP_IF_UNCHANGED)) return err(o, _("Unable to write index.")); return clean ? 0 : 1; @@@ -3515,18 -3241,9 +3515,18 @@@ static void merge_recursive_config(struct merge_options *o) { + char *value = NULL; git_config_get_int("merge.verbosity", &o->verbosity); git_config_get_int("diff.renamelimit", &o->diff_rename_limit); git_config_get_int("merge.renamelimit", &o->merge_rename_limit); + if (!git_config_get_string("diff.renames", &value)) { + o->diff_detect_rename = git_config_rename("diff.renames", value); + free(value); + } + if (!git_config_get_string("merge.renames", &value)) { + o->merge_detect_rename = git_config_rename("merge.renames", value); + free(value); + } git_config(git_xmerge_config, NULL); } @@@ -3539,8 -3256,7 +3539,8 @@@ void init_merge_options(struct merge_op o->diff_rename_limit = -1; o->merge_rename_limit = -1; o->renormalize = 0; - o->detect_rename = 1; + o->diff_detect_rename = -1; + o->merge_detect_rename = -1; merge_recursive_config(o); merge_verbosity = getenv("GIT_MERGE_VERBOSITY"); if (merge_verbosity) @@@ -3591,16 -3307,16 +3591,16 @@@ int parse_merge_opt(struct merge_option else if (!strcmp(s, "no-renormalize")) o->renormalize = 0; else if (!strcmp(s, "no-renames")) - o->detect_rename = 0; + o->merge_detect_rename = 0; else if (!strcmp(s, "find-renames")) { - o->detect_rename = 1; + o->merge_detect_rename = 1; o->rename_score = 0; } else if (skip_prefix(s, "find-renames=", &arg) || skip_prefix(s, "rename-threshold=", &arg)) { if ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0) return -1; - o->detect_rename = 1; + o->merge_detect_rename = 1; } else return -1;