From: Junio C Hamano Date: Wed, 18 Jul 2018 19:20:27 +0000 (-0700) Subject: Merge branch 'en/merge-recursive-cleanup' X-Git-Tag: v2.19.0-rc0~167 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/473b8bb3aa01fb214c9905456f1f33c9c7def6b3?hp=-c Merge branch 'en/merge-recursive-cleanup' Code cleanup. * en/merge-recursive-cleanup: merge-recursive: add pointer about unduly complex looking code merge-recursive: rename conflict_rename_*() family of functions merge-recursive: clarify the rename_dir/RENAME_DIR meaning merge-recursive: align labels with their respective code blocks merge-recursive: fix numerous argument alignment issues merge-recursive: fix miscellaneous grammar error in comment --- 473b8bb3aa01fb214c9905456f1f33c9c7def6b3 diff --combined merge-recursive.c index bed4a5be02,284b27d895..8f42a999d9 --- a/merge-recursive.c +++ b/merge-recursive.c @@@ -15,7 -15,6 +15,7 @@@ #include "diff.h" #include "diffcore.h" #include "tag.h" +#include "alloc.h" #include "unpack-trees.h" #include "string-list.h" #include "xdiff-interface.h" @@@ -161,7 -160,7 +161,7 @@@ static struct tree *shift_tree_object(s static struct commit *make_virtual_commit(struct tree *tree, const char *comment) { - struct commit *commit = alloc_commit_node(); + struct commit *commit = alloc_commit_node(the_repository); set_merge_remote_desc(commit, comment, (struct object *)commit); commit->maybe_tree = tree; @@@ -182,7 -181,7 +182,7 @@@ static int oid_eq(const struct object_i enum rename_type { RENAME_NORMAL = 0, - RENAME_DIR, + RENAME_VIA_DIR, RENAME_DELETE, RENAME_ONE_FILE_TO_ONE, RENAME_ONE_FILE_TO_TWO, @@@ -287,12 -286,10 +287,12 @@@ static void output(struct merge_option static void output_commit_title(struct merge_options *o, struct commit *commit) { + struct merge_remote_desc *desc; + strbuf_addchars(&o->obuf, ' ', o->call_depth * 2); - if (commit->util) - strbuf_addf(&o->obuf, "virtual %s\n", - merge_remote_util(commit)->name); + desc = merge_remote_util(commit); + if (desc) + strbuf_addf(&o->obuf, "virtual %s\n", desc->name); else { strbuf_add_unique_abbrev(&o->obuf, &commit->object.oid, DEFAULT_ABBREV); @@@ -312,8 -309,8 +312,8 @@@ } static int add_cacheinfo(struct merge_options *o, - unsigned int mode, const struct object_id *oid, - const char *path, int stage, int refresh, int options) + unsigned int mode, const struct object_id *oid, + const char *path, int stage, int refresh, int options) { struct cache_entry *ce; int ret; @@@ -420,8 -417,8 +420,8 @@@ struct tree *write_tree_from_memory(str } static int save_files_dirs(const struct object_id *oid, - struct strbuf *base, const char *path, - unsigned int mode, int stage, void *context) + struct strbuf *base, const char *path, + unsigned int mode, int stage, void *context) { struct path_hashmap_entry *entry; int baselen = base->len; @@@ -542,7 -539,7 +542,7 @@@ static void record_df_conflict_files(st struct string_list *entries) { /* If there is a D/F conflict and the file for such a conflict - * currently exist in the working tree, we want to allow it to be + * currently exists in the working tree, we want to allow it to be * removed to make room for the corresponding directory if needed. * The files underneath the directories of such D/F conflicts will * be processed before the corresponding file involved in the D/F @@@ -916,7 -913,7 +916,7 @@@ static int make_room_for_path(struct me */ if (would_lose_untracked(path)) return err(o, _("refusing to lose untracked file at '%s'"), - path); + path); /* Successful unlink is good.. */ if (!unlink(path)) @@@ -995,16 -992,16 +995,16 @@@ static int update_file_flags(struct mer unlink(path); if (symlink(lnk, path)) ret = err(o, _("failed to symlink '%s': %s"), - path, strerror(errno)); + path, strerror(errno)); free(lnk); } else ret = err(o, _("do not know what to do with %06o %s '%s'"), mode, oid_to_hex(oid), path); - free_buf: + free_buf: free(buf); } - update_index: + update_index: if (!ret && update_cache) if (add_cacheinfo(o, mode, oid, path, 0, update_wd, ADD_CACHE_OK_TO_ADD)) @@@ -1093,7 -1090,7 +1093,7 @@@ static int merge_3way(struct merge_opti } static int find_first_merges(struct object_array *result, const char *path, - struct commit *a, struct commit *b) + struct commit *a, struct commit *b) { int i, j; struct object_array merges = OBJECT_ARRAY_INIT; @@@ -1111,7 -1108,7 +1111,7 @@@ /* get all revisions that merge commit a */ xsnprintf(merged_revision, sizeof(merged_revision), "^%s", - oid_to_hex(&a->object.oid)); + oid_to_hex(&a->object.oid)); init_revisions(&revs, NULL); rev_opts.submodule = path; /* FIXME: can't handle linked worktrees in submodules yet */ @@@ -1211,7 -1208,7 +1211,7 @@@ static int merge_submodule(struct merge 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 to %s"), path, oid_to_hex(b)); + output(o, 2, _("Fast-forwarding submodule %s"), path); else ; /* no output */ @@@ -1223,7 -1220,7 +1223,7 @@@ 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 to %s"), path, oid_to_hex(a)); + output(o, 2, _("Fast-forwarding submodule %s"), path); else ; /* no output */ @@@ -1253,12 -1250,12 +1253,12 @@@ 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); + "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: @@@ -1335,10 -1332,10 +1335,10 @@@ static int merge_file_1(struct merge_op result->clean = (merge_status == 0); } else if (S_ISGITLINK(a->mode)) { result->clean = merge_submodule(o, &result->oid, - one->path, - &one->oid, - &a->oid, - &b->oid); + one->path, + &one->oid, + &a->oid, + &b->oid); } else if (S_ISLNK(a->mode)) { switch (o->recursive_variant) { case MERGE_RECURSIVE_NORMAL: @@@ -1413,11 -1410,17 +1413,17 @@@ static int merge_file_one(struct merge_ return merge_file_1(o, &one, &a, &b, path, branch1, branch2, mfi); } - static int conflict_rename_dir(struct merge_options *o, - struct diff_filepair *pair, - const char *rename_branch, - const char *other_branch) + static int handle_rename_via_dir(struct merge_options *o, + struct diff_filepair *pair, + const char *rename_branch, + const char *other_branch) { + /* + * Handle file adds that need to be renamed due to directory rename + * detection. This differs from handle_rename_normal, because + * there is no content merge to do; just move the file into the + * desired final location. + */ const struct diff_filespec *dest = pair->two; if (!o->call_depth && would_lose_untracked(dest->path)) { @@@ -1446,13 -1449,13 +1452,13 @@@ } static int handle_change_delete(struct merge_options *o, - const char *path, const char *old_path, - const struct object_id *o_oid, int o_mode, - const struct object_id *changed_oid, - int changed_mode, - const char *change_branch, - const char *delete_branch, - const char *change, const char *change_past) + const char *path, const char *old_path, + const struct object_id *o_oid, int o_mode, + const struct object_id *changed_oid, + int changed_mode, + const char *change_branch, + const char *delete_branch, + const char *change, const char *change_past) { char *alt_path = NULL; const char *update_path = path; @@@ -1473,6 -1476,21 +1479,21 @@@ if (!ret) ret = update_file(o, 0, o_oid, o_mode, update_path); } else { + /* + * Despite the four nearly duplicate messages and argument + * lists below and the ugliness of the nested if-statements, + * having complete messages makes the job easier for + * translators. + * + * The slight variance among the cases is due to the fact + * that: + * 1) directory/file conflicts (in effect if + * !alt_path) could cause us to need to write the + * file to a different path. + * 2) renames (in effect if !old_path) could mean that + * there are two names for the path that the user + * may know the file by. + */ if (!alt_path) { if (!old_path) { output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s " @@@ -1512,10 -1530,10 +1533,10 @@@ return ret; } - static int conflict_rename_delete(struct merge_options *o, - struct diff_filepair *pair, - const char *rename_branch, - const char *delete_branch) + static int handle_rename_delete(struct merge_options *o, + struct diff_filepair *pair, + const char *rename_branch, + const char *delete_branch) { const struct diff_filespec *orig = pair->one; const struct diff_filespec *dest = pair->two; @@@ -1617,8 -1635,8 +1638,8 @@@ static int handle_file(struct merge_opt return ret; } - static int conflict_rename_rename_1to2(struct merge_options *o, - struct rename_conflict_info *ci) + static int handle_rename_rename_1to2(struct merge_options *o, + struct rename_conflict_info *ci) { /* One file was renamed in both branches, but to different names. */ struct diff_filespec *one = ci->pair1->one; @@@ -1679,8 -1697,8 +1700,8 @@@ return 0; } - static int conflict_rename_rename_2to1(struct merge_options *o, - struct rename_conflict_info *ci) + static int handle_rename_rename_2to1(struct merge_options *o, + struct rename_conflict_info *ci) { /* Two files, a & b, were renamed to the same thing, c. */ struct diff_filespec *a = ci->pair1->one; @@@ -2214,18 -2232,18 +2235,18 @@@ static struct hashmap *get_directory_re 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,7 -2440,7 +2443,7 @@@ static void apply_directory_rename_modi * "NOTE" in update_stages(), doing so will modify the current * in-memory index which will break calls to would_lose_untracked() * that we need to make. Instead, we need to just make sure that - * the various conflict_rename_*() functions update the index + * the various handle_rename_*() functions update the index * explicitly rather than relying on unpack_trees() to have done it. */ get_tree_entry(&tree->object.oid, @@@ -2695,7 -2713,7 +2716,7 @@@ static int process_renames(struct merge if (oid_eq(&src_other.oid, &null_oid) && ren1->add_turned_into_rename) { - setup_rename_conflict_info(RENAME_DIR, + setup_rename_conflict_info(RENAME_VIA_DIR, ren1->pair, NULL, branch1, @@@ -2826,12 -2844,12 +2847,12 @@@ static void initial_cleanup_rename(stru free(pairs); } - static int handle_renames(struct merge_options *o, - struct tree *common, - struct tree *head, - struct tree *merge, - struct string_list *entries, - struct rename_info *ri) + static int detect_and_process_renames(struct merge_options *o, + struct tree *common, + struct tree *head, + struct tree *merge, + struct string_list *entries, + struct rename_info *ri) { struct diff_queue_struct *head_pairs, *merge_pairs; struct hashmap *dir_re_head, *dir_re_merge; @@@ -2907,7 -2925,8 +2928,8 @@@ static struct object_id *stage_oid(cons } static int read_oid_strbuf(struct merge_options *o, - const struct object_id *oid, struct strbuf *dst) + const struct object_id *oid, + struct strbuf *dst) { void *buf; enum object_type type; @@@ -2960,10 -2979,10 +2982,10 @@@ error_return } static int handle_modify_delete(struct merge_options *o, - const char *path, - struct object_id *o_oid, int o_mode, - struct object_id *a_oid, int a_mode, - struct object_id *b_oid, int b_mode) + const char *path, + struct object_id *o_oid, int o_mode, + struct object_id *a_oid, int a_mode, + struct object_id *b_oid, int b_mode) { const char *modify_branch, *delete_branch; struct object_id *changed_oid; @@@ -3101,12 -3120,12 +3123,12 @@@ static int merge_content(struct merge_o return !is_dirty && mfi.clean; } - static int conflict_rename_normal(struct merge_options *o, - const char *path, - struct object_id *o_oid, unsigned int o_mode, - struct object_id *a_oid, unsigned int a_mode, - struct object_id *b_oid, unsigned int b_mode, - struct rename_conflict_info *ci) + static int handle_rename_normal(struct merge_options *o, + const char *path, + struct object_id *o_oid, unsigned int o_mode, + struct object_id *a_oid, unsigned int a_mode, + struct object_id *b_oid, unsigned int b_mode, + struct rename_conflict_info *ci) { /* Merge the content and write it out */ return merge_content(o, path, was_dirty(o, path), @@@ -3133,37 -3152,37 +3155,37 @@@ static int process_entry(struct merge_o switch (conflict_info->rename_type) { case RENAME_NORMAL: case RENAME_ONE_FILE_TO_ONE: - clean_merge = conflict_rename_normal(o, - path, - o_oid, o_mode, - a_oid, a_mode, - b_oid, b_mode, - conflict_info); + clean_merge = handle_rename_normal(o, + path, + o_oid, o_mode, + a_oid, a_mode, + b_oid, b_mode, + conflict_info); break; - case RENAME_DIR: + case RENAME_VIA_DIR: clean_merge = 1; - if (conflict_rename_dir(o, - conflict_info->pair1, - conflict_info->branch1, - conflict_info->branch2)) + if (handle_rename_via_dir(o, + conflict_info->pair1, + conflict_info->branch1, + conflict_info->branch2)) clean_merge = -1; break; case RENAME_DELETE: clean_merge = 0; - if (conflict_rename_delete(o, - conflict_info->pair1, - conflict_info->branch1, - conflict_info->branch2)) + if (handle_rename_delete(o, + conflict_info->pair1, + conflict_info->branch1, + conflict_info->branch2)) clean_merge = -1; break; case RENAME_ONE_FILE_TO_TWO: clean_merge = 0; - if (conflict_rename_rename_1to2(o, conflict_info)) + if (handle_rename_rename_1to2(o, conflict_info)) clean_merge = -1; break; case RENAME_TWO_FILES_TO_ONE: clean_merge = 0; - if (conflict_rename_rename_2to1(o, conflict_info)) + if (handle_rename_rename_2to1(o, conflict_info)) clean_merge = -1; break; default: @@@ -3303,8 -3322,8 +3325,8 @@@ int merge_trees(struct merge_options *o get_files_dirs(o, merge); entries = get_unmerged(); - clean = handle_renames(o, common, head, merge, entries, - &re_info); + clean = detect_and_process_renames(o, common, head, merge, + entries, &re_info); record_df_conflict_files(o, entries); if (clean < 0) goto cleanup; @@@ -3328,7 -3347,7 +3350,7 @@@ entries->items[i].string); } - cleanup: + cleanup: final_cleanup_renames(&re_info); string_list_clear(entries, 1); @@@ -3496,14 -3515,14 +3518,14 @@@ int merge_recursive_generic(struct merg struct commit *base; if (!(base = get_ref(base_list[i], oid_to_hex(base_list[i])))) return err(o, _("Could not parse object '%s'"), - oid_to_hex(base_list[i])); + oid_to_hex(base_list[i])); commit_list_insert(base, &ca); } } hold_locked_index(&lock, LOCK_DIE_ON_ERROR); clean = merge_recursive(o, head_commit, next_commit, ca, - result); + result); if (clean < 0) { rollback_lock_file(&lock); return clean;