diff --git a/branch.c b/branch.c index 9c9dae1eae321c..b71a2de29dbed9 100644 --- a/branch.c +++ b/branch.c @@ -344,6 +344,7 @@ void remove_merge_branch_state(struct repository *r) unlink(git_path_merge_rr(r)); unlink(git_path_merge_msg(r)); unlink(git_path_merge_mode(r)); + unlink(git_path_auto_merge(r)); save_autostash(git_path_merge_autostash(r)); } diff --git a/builtin/rebase.c b/builtin/rebase.c index 783b526f6e758a..ed1da1760e4cd8 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -738,6 +738,7 @@ static int finish_rebase(struct rebase_options *opts) int ret = 0; delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF); + unlink(git_path_auto_merge(the_repository)); apply_autostash(state_dir_path("autostash", opts)); close_object_store(the_repository->objects); /* diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index a66b5e8c75aec2..d19be40544c8fd 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -16,6 +16,7 @@ linux-gcc) export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main make test export GIT_TEST_SPLIT_INDEX=yes + export GIT_TEST_MERGE_ALGORITHM=recursive export GIT_TEST_FULL_IN_PACK_ARRAY=true export GIT_TEST_OE_SIZE=10 export GIT_TEST_OE_DELTA_SIZE=5 diff --git a/diffcore-rename.c b/diffcore-rename.c index e2ed64817654ec..963ca582216ba5 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -371,7 +371,7 @@ struct dir_rename_info { struct strintmap idx_map; struct strmap dir_rename_guess; struct strmap *dir_rename_count; - struct strset *relevant_source_dirs; + struct strintmap *relevant_source_dirs; unsigned setup; }; @@ -407,6 +407,28 @@ static const char *get_highest_rename_path(struct strintmap *counts) return highest_destination_dir; } +static char *UNKNOWN_DIR = "/"; /* placeholder -- short, illegal directory */ + +static int dir_rename_already_determinable(struct strintmap *counts) +{ + struct hashmap_iter iter; + struct strmap_entry *entry; + int first = 0, second = 0, unknown = 0; + strintmap_for_each_entry(counts, &iter, entry) { + const char *destination_dir = entry->key; + intptr_t count = (intptr_t)entry->value; + if (!strcmp(destination_dir, UNKNOWN_DIR)) { + unknown = count; + } else if (count >= first) { + second = first; + first = count; + } else if (count >= second) { + second = count; + } + } + return first > second + unknown; +} + static void increment_count(struct dir_rename_info *info, char *old_dir, char *new_dir) @@ -429,7 +451,7 @@ static void increment_count(struct dir_rename_info *info, } static void update_dir_rename_counts(struct dir_rename_info *info, - struct strset *dirs_removed, + struct strintmap *dirs_removed, const char *oldname, const char *newname) { @@ -461,10 +483,12 @@ static void update_dir_rename_counts(struct dir_rename_info *info, return; while (1) { + int drd_flag = NOT_RELEVANT; + /* Get old_dir, skip if its directory isn't relevant. */ dirname_munge(old_dir); if (info->relevant_source_dirs && - !strset_contains(info->relevant_source_dirs, old_dir)) + !strintmap_contains(info->relevant_source_dirs, old_dir)) break; /* Get new_dir */ @@ -509,16 +533,31 @@ static void update_dir_rename_counts(struct dir_rename_info *info, } } - if (strset_contains(dirs_removed, old_dir)) + /* + * Above we suggested that we'd keep recording renames for + * all ancestor directories where the trailing directories + * matched, i.e. for + * "a/b/c/d/e/foo.c" -> "a/b/some/thing/else/e/foo.c" + * we'd increment rename counts for each of + * a/b/c/d/e/ => a/b/some/thing/else/e/ + * a/b/c/d/ => a/b/some/thing/else/ + * However, we only need the rename counts for directories + * in dirs_removed whose value is RELEVANT_FOR_SELF. + * However, we add one special case of also recording it for + * first_time_in_loop because find_basename_matches() can + * use that as a hint to find a good pairing. + */ + if (dirs_removed) + drd_flag = strintmap_get(dirs_removed, old_dir); + if (drd_flag == RELEVANT_FOR_SELF || first_time_in_loop) increment_count(info, old_dir, new_dir); - else - break; + first_time_in_loop = 0; + if (drd_flag == NOT_RELEVANT) + break; /* If we hit toplevel directory ("") for old or new dir, quit */ if (!*old_dir || !*new_dir) break; - - first_time_in_loop = 0; } /* Free resources we don't need anymore */ @@ -527,14 +566,15 @@ static void update_dir_rename_counts(struct dir_rename_info *info, } static void initialize_dir_rename_info(struct dir_rename_info *info, - struct strset *dirs_removed, + struct strintmap *relevant_sources, + struct strintmap *dirs_removed, struct strmap *dir_rename_count) { struct hashmap_iter iter; struct strmap_entry *entry; int i; - if (!dirs_removed) { + if (!dirs_removed && !relevant_sources) { info->setup = 0; return; } @@ -549,7 +589,21 @@ static void initialize_dir_rename_info(struct dir_rename_info *info, strmap_init_with_options(&info->dir_rename_guess, NULL, 0); /* Setup info->relevant_source_dirs */ - info->relevant_source_dirs = dirs_removed; + info->relevant_source_dirs = NULL; + if (dirs_removed || !relevant_sources) { + info->relevant_source_dirs = dirs_removed; /* might be NULL */ + } else { + info->relevant_source_dirs = xmalloc(sizeof(struct strintmap)); + strintmap_init(info->relevant_source_dirs, 0 /* unused */); + strintmap_for_each_entry(relevant_sources, &iter, entry) { + char *dirname = get_dirname(entry->key); + if (!dirs_removed || + strintmap_contains(dirs_removed, dirname)) + strintmap_set(info->relevant_source_dirs, + dirname, 0 /* value irrelevant */); + free(dirname); + } + } /* * Loop setting up both info->idx_map, and doing setup of @@ -610,7 +664,7 @@ void partial_clear_dir_rename_count(struct strmap *dir_rename_count) } static void cleanup_dir_rename_info(struct dir_rename_info *info, - struct strset *dirs_removed, + struct strintmap *dirs_removed, int keep_dir_rename_count) { struct hashmap_iter iter; @@ -627,6 +681,13 @@ static void cleanup_dir_rename_info(struct dir_rename_info *info, /* dir_rename_guess */ strmap_clear(&info->dir_rename_guess, 1); + /* relevant_source_dirs */ + if (info->relevant_source_dirs && + info->relevant_source_dirs != dirs_removed) { + strintmap_clear(info->relevant_source_dirs); + FREE_AND_NULL(info->relevant_source_dirs); + } + /* dir_rename_count */ if (!keep_dir_rename_count) { partial_clear_dir_rename_count(info->dir_rename_count); @@ -638,18 +699,22 @@ static void cleanup_dir_rename_info(struct dir_rename_info *info, /* * Although dir_rename_count was passed in * diffcore_rename_extended() and we want to keep it around and - * return it to that caller, we first want to remove any data + * return it to that caller, we first want to remove any counts in + * the maps associated with UNKNOWN_DIR entries and any data * associated with directories that weren't renamed. */ strmap_for_each_entry(info->dir_rename_count, &iter, entry) { const char *source_dir = entry->key; struct strintmap *counts = entry->value; - if (!strset_contains(dirs_removed, source_dir)) { + if (!strintmap_get(dirs_removed, source_dir)) { string_list_append(&to_remove, source_dir); strintmap_clear(counts); continue; } + + if (strintmap_contains(counts, UNKNOWN_DIR)) + strintmap_remove(counts, UNKNOWN_DIR); } for (i = 0; i < to_remove.nr; ++i) strmap_remove(info->dir_rename_count, @@ -749,7 +814,8 @@ static int idx_possible_rename(char *filename, struct dir_rename_info *info) static int find_basename_matches(struct diff_options *options, int minimum_score, struct dir_rename_info *info, - struct strset *dirs_removed) + struct strintmap *relevant_sources, + struct strintmap *dirs_removed) { /* * When I checked in early 2020, over 76% of file renames in linux @@ -839,6 +905,11 @@ static int find_basename_matches(struct diff_options *options, intptr_t src_index; intptr_t dst_index; + /* Skip irrelevant sources */ + if (relevant_sources && + !strintmap_contains(relevant_sources, filename)) + continue; + /* * If the basename is unique among remaining sources, then * src_index will equal 'i' and we can attempt to match it @@ -967,7 +1038,7 @@ static int find_renames(struct diff_score *mx, int minimum_score, int copies, struct dir_rename_info *info, - struct strset *dirs_removed) + struct strintmap *dirs_removed) { int count = 0, i; @@ -991,11 +1062,12 @@ static int find_renames(struct diff_score *mx, return count; } -static void remove_unneeded_paths_from_src(int detecting_copies) +static void remove_unneeded_paths_from_src(int detecting_copies, + struct strintmap *interesting) { int i, new_num_src; - if (detecting_copies) + if (detecting_copies && !interesting) return; /* nothing to remove */ if (break_idx) return; /* culling incompatible with break detection */ @@ -1022,12 +1094,18 @@ static void remove_unneeded_paths_from_src(int detecting_copies) * from rename_src here. */ for (i = 0, new_num_src = 0; i < rename_src_nr; i++) { + struct diff_filespec *one = rename_src[i].p->one; + /* * renames are stored in rename_dst, so if a rename has * already been detected using this source, we can just * remove the source knowing rename_dst has its info. */ - if (rename_src[i].p->one->rename_used) + if (!detecting_copies && one->rename_used) + continue; + + /* If we don't care about the source path, skip it */ + if (interesting && !strintmap_contains(interesting, one->path)) continue; if (new_num_src < i) @@ -1039,8 +1117,136 @@ static void remove_unneeded_paths_from_src(int detecting_copies) rename_src_nr = new_num_src; } +static void handle_early_known_dir_renames(struct dir_rename_info *info, + struct strintmap *relevant_sources, + struct strintmap *dirs_removed) +{ + /* + * Directory renames are determined via an aggregate of all renames + * under them and using a "majority wins" rule. The fact that + * "majority wins", though, means we don't need all the renames + * under the given directory, we only need enough to ensure we have + * a majority. + */ + + int i, new_num_src; + struct hashmap_iter iter; + struct strmap_entry *entry; + + if (!dirs_removed || !relevant_sources) + return; /* nothing to cull */ + if (break_idx) + return; /* culling incompatbile with break detection */ + + /* + * Supplement dir_rename_count with number of potential renames, + * marking all potential rename sources as mapping to UNKNOWN_DIR. + */ + for (i = 0; i < rename_src_nr; i++) { + char *old_dir; + struct diff_filespec *one = rename_src[i].p->one; + + /* + * sources that are part of a rename will have already been + * removed by a prior call to remove_unneeded_paths_from_src() + */ + assert(!one->rename_used); + + old_dir = get_dirname(one->path); + while (*old_dir != '\0' && + NOT_RELEVANT != strintmap_get(dirs_removed, old_dir)) { + char *freeme = old_dir; + + increment_count(info, old_dir, UNKNOWN_DIR); + old_dir = get_dirname(old_dir); + + /* Free resources we don't need anymore */ + free(freeme); + } + /* + * old_dir and new_dir free'd in increment_count, but + * get_dirname() gives us a new pointer we need to free for + * old_dir. Also, if the loop runs 0 times we need old_dir + * to be freed. + */ + free(old_dir); + } + + /* + * For any directory which we need a potential rename detected for + * (i.e. those marked as RELEVANT_FOR_SELF in dirs_removed), check + * whether we have enough renames to satisfy the "majority rules" + * requirement such that detecting any more renames of files under + * it won't change the result. For any such directory, mark that + * we no longer need to detect a rename for it. However, since we + * might need to still detect renames for an ancestor of that + * directory, use RELEVANT_FOR_ANCESTOR. + */ + strmap_for_each_entry(info->dir_rename_count, &iter, entry) { + /* entry->key is source_dir */ + struct strintmap *counts = entry->value; + + if (strintmap_get(dirs_removed, entry->key) == + RELEVANT_FOR_SELF && + dir_rename_already_determinable(counts)) { + strintmap_set(dirs_removed, entry->key, + RELEVANT_FOR_ANCESTOR); + } + } + + for (i = 0, new_num_src = 0; i < rename_src_nr; i++) { + struct diff_filespec *one = rename_src[i].p->one; + int val; + + val = strintmap_get(relevant_sources, one->path); + + /* + * sources that were not found in relevant_sources should + * have already been removed by a prior call to + * remove_unneeded_paths_from_src() + */ + assert(val != -1); + + if (val == RELEVANT_LOCATION) { + int removable = 1; + char *dir = get_dirname(one->path); + while (1) { + char *freeme = dir; + int res = strintmap_get(dirs_removed, dir); + + /* Quit if not found or irrelevant */ + if (res == NOT_RELEVANT) + break; + /* If RELEVANT_FOR_SELF, can't remove */ + if (res == RELEVANT_FOR_SELF) { + removable = 0; + break; + } + /* Else continue searching upwards */ + assert(res == RELEVANT_FOR_ANCESTOR); + dir = get_dirname(dir); + free(freeme); + } + free(dir); + if (removable) { + strintmap_set(relevant_sources, one->path, + RELEVANT_NO_MORE); + continue; + } + } + + if (new_num_src < i) + memcpy(&rename_src[new_num_src], &rename_src[i], + sizeof(struct diff_rename_src)); + new_num_src++; + } + + rename_src_nr = new_num_src; +} + void diffcore_rename_extended(struct diff_options *options, - struct strset *dirs_removed, + struct strintmap *relevant_sources, + struct strintmap *dirs_removed, struct strmap *dir_rename_count) { int detect_rename = options->detect_rename; @@ -1060,6 +1266,8 @@ void diffcore_rename_extended(struct diff_options *options, want_copies = (detect_rename == DIFF_DETECT_COPY); if (dirs_removed && (break_idx || want_copies)) BUG("dirs_removed incompatible with break/copy detection"); + if (break_idx && relevant_sources) + BUG("break detection incompatible with source specification"); if (!minimum_score) minimum_score = DEFAULT_RENAME_SCORE; @@ -1127,9 +1335,10 @@ void diffcore_rename_extended(struct diff_options *options, /* * Cull sources: * - remove ones corresponding to exact renames + * - remove ones not found in relevant_sources */ trace2_region_enter("diff", "cull after exact", options->repo); - remove_unneeded_paths_from_src(want_copies); + remove_unneeded_paths_from_src(want_copies, relevant_sources); trace2_region_leave("diff", "cull after exact", options->repo); } else { /* Determine minimum score to match basenames */ @@ -1148,12 +1357,12 @@ void diffcore_rename_extended(struct diff_options *options, * - remove ones involved in renames (found via exact match) */ trace2_region_enter("diff", "cull after exact", options->repo); - remove_unneeded_paths_from_src(want_copies); + remove_unneeded_paths_from_src(want_copies, NULL); trace2_region_leave("diff", "cull after exact", options->repo); /* Preparation for basename-driven matching. */ trace2_region_enter("diff", "dir rename setup", options->repo); - initialize_dir_rename_info(&info, + initialize_dir_rename_info(&info, relevant_sources, dirs_removed, dir_rename_count); trace2_region_leave("diff", "dir rename setup", options->repo); @@ -1161,15 +1370,25 @@ void diffcore_rename_extended(struct diff_options *options, trace2_region_enter("diff", "basename matches", options->repo); rename_count += find_basename_matches(options, min_basename_score, - &info, dirs_removed); + &info, + relevant_sources, + dirs_removed); trace2_region_leave("diff", "basename matches", options->repo); /* * Cull sources, again: * - remove ones involved in renames (found via basenames) + * - remove ones not found in relevant_sources + * and + * - remove ones in relevant_sources which are needed only + * for directory renames IF no ancestory directory + * actually needs to know any more individual path + * renames under them */ trace2_region_enter("diff", "cull basename", options->repo); - remove_unneeded_paths_from_src(want_copies); + remove_unneeded_paths_from_src(want_copies, relevant_sources); + handle_early_known_dir_renames(&info, relevant_sources, + dirs_removed); trace2_region_leave("diff", "cull basename", options->repo); } @@ -1341,5 +1560,5 @@ void diffcore_rename_extended(struct diff_options *options, void diffcore_rename(struct diff_options *options) { - diffcore_rename_extended(options, NULL, NULL); + diffcore_rename_extended(options, NULL, NULL, NULL); } diff --git a/diffcore.h b/diffcore.h index b9a230ab7fe6d0..f5c6de4841ed83 100644 --- a/diffcore.h +++ b/diffcore.h @@ -8,8 +8,8 @@ struct diff_options; struct repository; +struct strintmap; struct strmap; -struct strset; struct userdiff_driver; /* This header file is internal between diff.c and its diff transformers @@ -161,12 +161,26 @@ struct diff_filepair *diff_queue(struct diff_queue_struct *, struct diff_filespec *); void diff_q(struct diff_queue_struct *, struct diff_filepair *); +/* dir_rename_relevance: the reason we want rename information for a dir */ +enum dir_rename_relevance { + NOT_RELEVANT = 0, + RELEVANT_FOR_ANCESTOR = 1, + RELEVANT_FOR_SELF = 2 +}; +/* file_rename_relevance: the reason(s) we want rename information for a file */ +enum file_rename_relevance { + RELEVANT_NO_MORE = 0, /* i.e. NOT relevant */ + RELEVANT_CONTENT = 1, + RELEVANT_LOCATION = 2 +}; + void partial_clear_dir_rename_count(struct strmap *dir_rename_count); void diffcore_break(struct repository *, int); void diffcore_rename(struct diff_options *); void diffcore_rename_extended(struct diff_options *options, - struct strset *dirs_removed, + struct strintmap *relevant_sources, + struct strintmap *dirs_removed, struct strmap *dir_rename_count); void diffcore_merge_broken(void); void diffcore_pickaxe(struct diff_options *); diff --git a/merge-ort.c b/merge-ort.c index ba35600d4e0d93..d1f4cf8a1f2ed4 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -18,6 +18,7 @@ #include "merge-ort.h" #include "alloc.h" +#include "attr.h" #include "blob.h" #include "cache-tree.h" #include "commit.h" @@ -25,6 +26,7 @@ #include "diff.h" #include "diffcore.h" #include "dir.h" +#include "entry.h" #include "ll-merge.h" #include "object-store.h" #include "revision.h" @@ -51,6 +53,12 @@ enum merge_side { MERGE_SIDE2 = 2 }; +struct traversal_callback_data { + unsigned long mask; + unsigned long dirmask; + struct name_entry names[3]; +}; + struct rename_info { /* * All variables that are arrays of size 3 correspond to data tracked @@ -67,8 +75,12 @@ struct rename_info { /* * dirs_removed: directories removed on a given side of history. + * + * The keys of dirs_removed[side] are the directories that were removed + * on the given side of history. The value of the strintmap for each + * directory is a value from enum dir_rename_relevance. */ - struct strset dirs_removed[3]; + struct strintmap dirs_removed[3]; /* * dir_rename_count: tracking where parts of a directory were renamed to @@ -88,6 +100,46 @@ struct rename_info { */ struct strmap dir_renames[3]; + /* + * relevant_sources: deleted paths wanted in rename detection, and why + * + * relevant_sources is a set of deleted paths on each side of + * history for which we need rename detection. If a path is deleted + * on one side of history, we need to detect if it is part of a + * rename if either + * * the file is modified/deleted on the other side of history + * * we need to detect renames for an ancestor directory + * If neither of those are true, we can skip rename detection for + * that path. The reason is stored as a value from enum + * file_rename_relevance, as the reason can inform the algorithm in + * diffcore_rename_extended(). + */ + struct strintmap relevant_sources[3]; + + /* + * dir_rename_mask: + * 0: optimization removing unmodified potential rename source okay + * 2 or 4: optimization okay, but must check for files added to dir + * 7: optimization forbidden; need rename source in case of dir rename + */ + unsigned dir_rename_mask:3; + + /* + * callback_data_*: supporting data structures for alternate traversal + * + * We sometimes need to be able to traverse through all the files + * in a given tree before all immediate subdirectories within that + * tree. Since traverse_trees() doesn't do that naturally, we have + * a traverse_trees_wrapper() that stores any immediate + * subdirectories while traversing files, then traverses the + * immediate subdirectories later. These callback_data* variables + * store the information for the subdirectories so that we can do + * that traversal order. + */ + struct traversal_callback_data *callback_data; + int callback_data_nr, callback_data_alloc; + char *callback_data_traverse_path; + /* * needed_limit: value needed for inexact rename detection to run * @@ -170,6 +222,16 @@ struct merge_options_internal { */ struct rename_info renames; + /* + * attr_index: hacky minimal index used for renormalization + * + * renormalization code _requires_ an index, though it only needs to + * find a .gitattributes file within the index. So, when + * renormalization is important, we create a special index with just + * that one file. + */ + struct index_state attr_index; + /* * current_dir_name, toplevel_dir: temporary vars * @@ -318,8 +380,8 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti, int i; void (*strmap_func)(struct strmap *, int) = reinitialize ? strmap_partial_clear : strmap_clear; - void (*strset_func)(struct strset *) = - reinitialize ? strset_partial_clear : strset_clear; + void (*strintmap_func)(struct strintmap *) = + reinitialize ? strintmap_partial_clear : strintmap_clear; /* * We marked opti->paths with strdup_strings = 0, so that we @@ -349,15 +411,20 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti, string_list_clear(&opti->paths_to_free, 0); opti->paths_to_free.strdup_strings = 0; + if (opti->attr_index.cache_nr) /* true iff opt->renormalize */ + discard_index(&opti->attr_index); + /* Free memory used by various renames maps */ for (i = MERGE_SIDE1; i <= MERGE_SIDE2; ++i) { - strset_func(&renames->dirs_removed[i]); + strintmap_func(&renames->dirs_removed[i]); partial_clear_dir_rename_count(&renames->dir_rename_count[i]); if (!reinitialize) strmap_clear(&renames->dir_rename_count[i], 1); strmap_func(&renames->dir_renames[i], 0); + + strintmap_func(&renames->relevant_sources[i]); } if (!reinitialize) { @@ -380,6 +447,12 @@ static void clear_or_reinit_internal_opts(struct merge_options_internal *opti, } strmap_clear(&opti->output, 0); } + + renames->dir_rename_mask = 0; + + /* Clean out callback_data as well. */ + FREE_AND_NULL(renames->callback_data); + renames->callback_data_nr = renames->callback_data_alloc = 0; } static int err(struct merge_options *opt, const char *err, ...) @@ -470,6 +543,82 @@ static char *unique_path(struct strmap *existing_paths, /*** Function Grouping: functions related to collect_merge_info() ***/ +static int traverse_trees_wrapper_callback(int n, + unsigned long mask, + unsigned long dirmask, + struct name_entry *names, + struct traverse_info *info) +{ + struct merge_options *opt = info->data; + struct rename_info *renames = &opt->priv->renames; + unsigned filemask = mask & ~dirmask; + + assert(n==3); + + if (!renames->callback_data_traverse_path) + renames->callback_data_traverse_path = xstrdup(info->traverse_path); + + if (filemask && filemask == renames->dir_rename_mask) + renames->dir_rename_mask = 0x07; + + ALLOC_GROW(renames->callback_data, renames->callback_data_nr + 1, + renames->callback_data_alloc); + renames->callback_data[renames->callback_data_nr].mask = mask; + renames->callback_data[renames->callback_data_nr].dirmask = dirmask; + COPY_ARRAY(renames->callback_data[renames->callback_data_nr].names, + names, 3); + renames->callback_data_nr++; + + return mask; +} + +/* + * Much like traverse_trees(), BUT: + * - read all the tree entries FIRST, saving them + * - note that the above step provides an opportunity to compute necessary + * additional details before the "real" traversal + * - loop through the saved entries and call the original callback on them + */ +static int traverse_trees_wrapper(struct index_state *istate, + int n, + struct tree_desc *t, + struct traverse_info *info) +{ + int ret, i, old_offset; + traverse_callback_t old_fn; + char *old_callback_data_traverse_path; + struct merge_options *opt = info->data; + struct rename_info *renames = &opt->priv->renames; + + assert(renames->dir_rename_mask == 2 || renames->dir_rename_mask == 4); + + old_callback_data_traverse_path = renames->callback_data_traverse_path; + old_fn = info->fn; + old_offset = renames->callback_data_nr; + + renames->callback_data_traverse_path = NULL; + info->fn = traverse_trees_wrapper_callback; + ret = traverse_trees(istate, n, t, info); + if (ret < 0) + return ret; + + info->traverse_path = renames->callback_data_traverse_path; + info->fn = old_fn; + for (i = old_offset; i < renames->callback_data_nr; ++i) { + info->fn(n, + renames->callback_data[i].mask, + renames->callback_data[i].dirmask, + renames->callback_data[i].names, + info); + } + + renames->callback_data_nr = old_offset; + free(renames->callback_data_traverse_path); + renames->callback_data_traverse_path = old_callback_data_traverse_path; + info->traverse_path = NULL; + return 0; +} + static void setup_path_info(struct merge_options *opt, struct string_list_item *result, const char *current_dir_name, @@ -533,12 +682,25 @@ static void add_pair(struct merge_options *opt, struct name_entry *names, const char *pathname, unsigned side, - unsigned is_add /* if false, is_delete */) + unsigned is_add /* if false, is_delete */, + unsigned match_mask, + unsigned dir_rename_mask) { struct diff_filespec *one, *two; struct rename_info *renames = &opt->priv->renames; int names_idx = is_add ? side : 0; + if (!is_add) { + unsigned content_relevant = (match_mask == 0); + unsigned location_relevant = (dir_rename_mask == 0x07); + + if (content_relevant || location_relevant) { + /* content_relevant trumps location_relevant */ + strintmap_set(&renames->relevant_sources[side], pathname, + content_relevant ? RELEVANT_CONTENT : RELEVANT_LOCATION); + } + } + one = alloc_filespec(pathname); two = alloc_filespec(pathname); fill_filespec(is_add ? two : one, @@ -557,14 +719,75 @@ static void collect_rename_info(struct merge_options *opt, struct rename_info *renames = &opt->priv->renames; unsigned side; + /* + * Update dir_rename_mask (determines ignore-rename-source validity) + * + * dir_rename_mask helps us keep track of when directory rename + * detection may be relevant. Basically, whenver a directory is + * removed on one side of history, and a file is added to that + * directory on the other side of history, directory rename + * detection is relevant (meaning we have to detect renames for all + * files within that directory to deduce where the directory + * moved). Also, whenever a directory needs directory rename + * detection, due to the "majority rules" choice for where to move + * it (see t6423 testcase 1f), we also need to detect renames for + * all files within subdirectories of that directory as well. + * + * Here we haven't looked at files within the directory yet, we are + * just looking at the directory itself. So, if we aren't yet in + * a case where a parent directory needed directory rename detection + * (i.e. dir_rename_mask != 0x07), and if the directory was removed + * on one side of history, record the mask of the other side of + * history in dir_rename_mask. + */ + if (renames->dir_rename_mask != 0x07 && + (dirmask == 3 || dirmask == 5)) { + /* simple sanity check */ + assert(renames->dir_rename_mask == 0 || + renames->dir_rename_mask == (dirmask & ~1)); + /* update dir_rename_mask; have it record mask of new side */ + renames->dir_rename_mask = (dirmask & ~1); + } + /* Update dirs_removed, as needed */ if (dirmask == 1 || dirmask == 3 || dirmask == 5) { /* absent_mask = 0x07 - dirmask; sides = absent_mask/2 */ unsigned sides = (0x07 - dirmask)/2; + unsigned relevance = (renames->dir_rename_mask == 0x07) ? + RELEVANT_FOR_ANCESTOR : NOT_RELEVANT; + /* + * Record relevance of this directory. However, note that + * when collect_merge_info_callback() recurses into this + * directory and calls collect_rename_info() on paths + * within that directory, if we find a path that was added + * to this directory on the other side of history, we will + * upgrade this value to RELEVANT_FOR_SELF; see below. + */ if (sides & 1) - strset_add(&renames->dirs_removed[1], fullname); + strintmap_set(&renames->dirs_removed[1], fullname, + relevance); if (sides & 2) - strset_add(&renames->dirs_removed[2], fullname); + strintmap_set(&renames->dirs_removed[2], fullname, + relevance); + } + + /* + * Here's the block that potentially upgrades to RELEVANT_FOR_SELF. + * When we run across a file added to a directory. In such a case, + * find the directory of the file and upgrade its relevance. + */ + if (renames->dir_rename_mask == 0x07 && + (filemask == 2 || filemask == 4)) { + /* + * Need directory rename for parent directory on other side + * of history from added file. Thus + * side = (~filemask & 0x06) >> 1 + * or + * side = 3 - (filemask/2). + */ + unsigned side = 3 - (filemask >> 1); + strintmap_set(&renames->dirs_removed[side], dirname, + RELEVANT_FOR_SELF); } if (filemask == 0 || filemask == 7) @@ -575,11 +798,15 @@ static void collect_rename_info(struct merge_options *opt, /* Check for deletion on side */ if ((filemask & 1) && !(filemask & side_mask)) - add_pair(opt, names, fullname, side, 0 /* delete */); + add_pair(opt, names, fullname, side, 0 /* delete */, + match_mask & filemask, + renames->dir_rename_mask); /* Check for addition on side */ if (!(filemask & 1) && (filemask & side_mask)) - add_pair(opt, names, fullname, side, 1 /* add */); + add_pair(opt, names, fullname, side, 1 /* add */, + match_mask & filemask, + renames->dir_rename_mask); } } @@ -597,12 +824,14 @@ static int collect_merge_info_callback(int n, */ struct merge_options *opt = info->data; struct merge_options_internal *opti = opt->priv; + struct rename_info *renames = &opt->priv->renames; struct string_list_item pi; /* Path Info */ struct conflict_info *ci; /* typed alias to pi.util (which is void*) */ struct name_entry *p; size_t len; char *fullpath; const char *dirname = opti->current_dir_name; + unsigned prev_dir_rename_mask = renames->dir_rename_mask; unsigned filemask = mask & ~dirmask; unsigned match_mask = 0; /* will be updated below */ unsigned mbase_null = !(mask & 1); @@ -743,8 +972,13 @@ static int collect_merge_info_callback(int n, original_dir_name = opti->current_dir_name; opti->current_dir_name = pi.string; - ret = traverse_trees(NULL, 3, t, &newinfo); + if (renames->dir_rename_mask == 0 || + renames->dir_rename_mask == 0x07) + ret = traverse_trees(NULL, 3, t, &newinfo); + else + ret = traverse_trees_wrapper(NULL, 3, t, &newinfo); opti->current_dir_name = original_dir_name; + renames->dir_rename_mask = prev_dir_rename_mask; for (i = MERGE_BASE; i <= MERGE_SIDE2; i++) free(buf[i]); @@ -968,6 +1202,63 @@ static int merge_submodule(struct merge_options *opt, return 0; } +static void initialize_attr_index(struct merge_options *opt) +{ + /* + * The renormalize_buffer() functions require attributes, and + * annoyingly those can only be read from the working tree or from + * an index_state. merge-ort doesn't have an index_state, so we + * generate a fake one containing only attribute information. + */ + struct merged_info *mi; + struct index_state *attr_index = &opt->priv->attr_index; + struct cache_entry *ce; + + attr_index->initialized = 1; + + if (!opt->renormalize) + return; + + mi = strmap_get(&opt->priv->paths, GITATTRIBUTES_FILE); + if (!mi) + return; + + if (mi->clean) { + int len = strlen(GITATTRIBUTES_FILE); + ce = make_empty_cache_entry(attr_index, len); + ce->ce_mode = create_ce_mode(mi->result.mode); + ce->ce_flags = create_ce_flags(0); + ce->ce_namelen = len; + oidcpy(&ce->oid, &mi->result.oid); + memcpy(ce->name, GITATTRIBUTES_FILE, len); + add_index_entry(attr_index, ce, + ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE); + get_stream_filter(attr_index, GITATTRIBUTES_FILE, &ce->oid); + } else { + int stage, len; + struct conflict_info *ci; + + ASSIGN_AND_VERIFY_CI(ci, mi); + for (stage = 0; stage < 3; stage++) { + unsigned stage_mask = (1 << stage); + + if (!(ci->filemask & stage_mask)) + continue; + len = strlen(GITATTRIBUTES_FILE); + ce = make_empty_cache_entry(attr_index, len); + ce->ce_mode = create_ce_mode(ci->stages[stage].mode); + ce->ce_flags = create_ce_flags(stage); + ce->ce_namelen = len; + oidcpy(&ce->oid, &ci->stages[stage].oid); + memcpy(ce->name, GITATTRIBUTES_FILE, len); + add_index_entry(attr_index, ce, + ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE); + get_stream_filter(attr_index, GITATTRIBUTES_FILE, + &ce->oid); + } + } +} + static int merge_3way(struct merge_options *opt, const char *path, const struct object_id *o, @@ -982,6 +1273,9 @@ static int merge_3way(struct merge_options *opt, char *base, *name1, *name2; int merge_status; + if (!opt->priv->attr_index.initialized) + initialize_attr_index(opt); + ll_opts.renormalize = opt->renormalize; ll_opts.extra_marker_size = extra_marker_size; ll_opts.xdl_opts = opt->xdl_opts; @@ -1020,7 +1314,7 @@ static int merge_3way(struct merge_options *opt, merge_status = ll_merge(result_buf, path, &orig, base, &src1, name1, &src2, name2, - opt->repo->index, &ll_opts); + &opt->priv->attr_index, &ll_opts); free(base); free(name1); @@ -1332,6 +1626,9 @@ static void get_provisional_directory_renames(struct merge_options *opt, } } + if (max == 0) + continue; + if (bad_max == max) { path_msg(opt, source_dir, 0, _("CONFLICT (directory rename split): " @@ -1340,18 +1637,7 @@ static void get_provisional_directory_renames(struct merge_options *opt, "no destination getting a majority of the " "files."), source_dir); - /* - * We should mark this as unclean IF something attempts - * to use this rename. We do not yet have the logic - * in place to detect if this directory rename is being - * used, and optimizations that reduce the number of - * renames cause this to falsely trigger. For now, - * just disable it, causing t6423 testcase 2a to break. - * We'll later fix the detection, and when we do we - * will re-enable setting *clean to 0 (and thereby fix - * t6423 testcase 2a). - */ - /* *clean = 0; */ + *clean = 0; } else { strmap_put(&renames->dir_renames[side], source_dir, (void*)best); @@ -1977,6 +2263,19 @@ static int process_renames(struct merge_options *opt, return clean_merge; } +static inline int possible_side_renames(struct rename_info *renames, + unsigned side_index) +{ + return renames->pairs[side_index].nr > 0 && + !strintmap_empty(&renames->relevant_sources[side_index]); +} + +static inline int possible_renames(struct rename_info *renames) +{ + return possible_side_renames(renames, 1) || + possible_side_renames(renames, 2); +} + static void resolve_diffpair_statuses(struct diff_queue_struct *q) { /* @@ -2013,6 +2312,16 @@ static void detect_regular_renames(struct merge_options *opt, struct diff_options diff_opts; struct rename_info *renames = &opt->priv->renames; + if (!possible_side_renames(renames, side_index)) { + /* + * No rename detection needed for this side, but we still need + * to make sure 'adds' are marked correctly in case the other + * side had directory renames. + */ + resolve_diffpair_statuses(&renames->pairs[side_index]); + return; + } + repo_diff_setup(opt->repo, &diff_opts); diff_opts.flags.recursive = 1; diff_opts.flags.rename_empty = 0; @@ -2028,6 +2337,7 @@ static void detect_regular_renames(struct merge_options *opt, diff_queued_diff = renames->pairs[side_index]; trace2_region_enter("diff", "diffcore_rename", opt->repo); diffcore_rename_extended(&diff_opts, + &renames->relevant_sources[side_index], &renames->dirs_removed[side_index], &renames->dir_rename_count[side_index]); trace2_region_leave("diff", "diffcore_rename", opt->repo); @@ -2129,6 +2439,8 @@ static int detect_and_process_renames(struct merge_options *opt, int need_dir_renames, s, clean = 1; memset(&combined, 0, sizeof(combined)); + if (!possible_renames(renames)) + goto cleanup; trace2_region_enter("merge", "regular renames", opt->repo); detect_regular_renames(opt, MERGE_SIDE1); @@ -2156,13 +2468,32 @@ static int detect_and_process_renames(struct merge_options *opt, clean &= collect_renames(opt, &combined, MERGE_SIDE2, &renames->dir_renames[1], &renames->dir_renames[2]); - QSORT(combined.queue, combined.nr, compare_pairs); + STABLE_QSORT(combined.queue, combined.nr, compare_pairs); trace2_region_leave("merge", "directory renames", opt->repo); trace2_region_enter("merge", "process renames", opt->repo); clean &= process_renames(opt, &combined); trace2_region_leave("merge", "process renames", opt->repo); + goto simple_cleanup; /* collect_renames() handles some of cleanup */ + +cleanup: + /* + * Free now unneeded filepairs, which would have been handled + * in collect_renames() normally but we skipped that code. + */ + for (s = MERGE_SIDE1; s <= MERGE_SIDE2; s++) { + struct diff_queue_struct *side_pairs; + int i; + + side_pairs = &renames->pairs[s]; + for (i = 0; i < side_pairs->nr; ++i) { + struct diff_filepair *p = side_pairs->queue[i]; + diff_free_filepair(p); + } + } + +simple_cleanup: /* Free memory for renames->pairs[] and combined */ for (s = MERGE_SIDE1; s <= MERGE_SIDE2; s++) { free(renames->pairs[s].queue); @@ -2207,6 +2538,61 @@ static int string_list_df_name_compare(const char *one, const char *two) return onelen - twolen; } +static int read_oid_strbuf(struct merge_options *opt, + const struct object_id *oid, + struct strbuf *dst) +{ + void *buf; + enum object_type type; + unsigned long size; + buf = read_object_file(oid, &type, &size); + if (!buf) + return err(opt, _("cannot read object %s"), oid_to_hex(oid)); + if (type != OBJ_BLOB) { + free(buf); + return err(opt, _("object %s is not a blob"), oid_to_hex(oid)); + } + strbuf_attach(dst, buf, size, size + 1); + return 0; +} + +static int blob_unchanged(struct merge_options *opt, + const struct version_info *base, + const struct version_info *side, + const char *path) +{ + struct strbuf basebuf = STRBUF_INIT; + struct strbuf sidebuf = STRBUF_INIT; + int ret = 0; /* assume changed for safety */ + const struct index_state *idx = &opt->priv->attr_index; + + if (!idx->initialized) + initialize_attr_index(opt); + + if (base->mode != side->mode) + return 0; + if (oideq(&base->oid, &side->oid)) + return 1; + + if (read_oid_strbuf(opt, &base->oid, &basebuf) || + read_oid_strbuf(opt, &side->oid, &sidebuf)) + goto error_return; + /* + * Note: binary | is used so that both renormalizations are + * performed. Comparison can be skipped if both files are + * unchanged since their sha1s have already been compared. + */ + if (renormalize_buffer(idx, path, basebuf.buf, basebuf.len, &basebuf) | + renormalize_buffer(idx, path, sidebuf.buf, sidebuf.len, &sidebuf)) + ret = (basebuf.len == sidebuf.len && + !memcmp(basebuf.buf, sidebuf.buf, basebuf.len)); + +error_return: + strbuf_release(&basebuf); + strbuf_release(&sidebuf); + return ret; +} + struct directory_versions { /* * versions: list of (basename -> version_info) @@ -2282,6 +2668,7 @@ static void write_tree(struct object_id *result_oid, */ relevant_entries.items = versions->items + offset; relevant_entries.nr = versions->nr - offset; + /* No need for STABLE_QSORT -- filenames must be unique */ QSORT(relevant_entries.items, relevant_entries.nr, tree_entry_order); /* Pre-allocate some space in buf */ @@ -2793,8 +3180,13 @@ static void process_entry(struct merge_options *opt, modify_branch = (side == 1) ? opt->branch1 : opt->branch2; delete_branch = (side == 1) ? opt->branch2 : opt->branch1; - if (ci->path_conflict && - oideq(&ci->stages[0].oid, &ci->stages[side].oid)) { + if (opt->renormalize && + blob_unchanged(opt, &ci->stages[0], &ci->stages[side], + path)) { + ci->merged.is_null = 1; + ci->merged.clean = 1; + } else if (ci->path_conflict && + oideq(&ci->stages[0].oid, &ci->stages[side].oid)) { /* * This came from a rename/delete; no action to take, * but avoid printing "modify/delete" conflict notice @@ -2966,23 +3358,27 @@ static int checkout(struct merge_options *opt, return ret; } -static int record_conflicted_index_entries(struct merge_options *opt, - struct index_state *index, - struct strmap *paths, - struct strmap *conflicted) +static int record_conflicted_index_entries(struct merge_options *opt) { struct hashmap_iter iter; struct strmap_entry *e; + struct index_state *index = opt->repo->index; + struct checkout state = CHECKOUT_INIT; int errs = 0; int original_cache_nr; - if (strmap_empty(conflicted)) + if (strmap_empty(&opt->priv->conflicted)) return 0; + /* If any entries have skip_worktree set, we'll have to check 'em out */ + state.force = 1; + state.quiet = 1; + state.refresh_cache = 1; + state.istate = index; original_cache_nr = index->cache_nr; /* Put every entry from paths into plist, then sort */ - strmap_for_each_entry(conflicted, &iter, e) { + strmap_for_each_entry(&opt->priv->conflicted, &iter, e) { const char *path = e->key; struct conflict_info *ci = e->value; int pos; @@ -3023,9 +3419,23 @@ static int record_conflicted_index_entries(struct merge_options *opt, * the higher order stages. Thus, we need override * the CE_SKIP_WORKTREE bit and manually write those * files to the working disk here. - * - * TODO: Implement this CE_SKIP_WORKTREE fixup. */ + if (ce_skip_worktree(ce)) { + struct stat st; + + if (!lstat(path, &st)) { + char *new_name = unique_path(&opt->priv->paths, + path, + "cruft"); + + path_msg(opt, path, 1, + _("Note: %s not up to date and in way of checking out conflicted version; old copy renamed to %s"), + path, new_name); + errs |= rename(path, new_name); + free(new_name); + } + errs |= checkout_entry(ce, &state, NULL, NULL); + } /* * Mark this cache entry for removal and instead add @@ -3057,6 +3467,11 @@ static int record_conflicted_index_entries(struct merge_options *opt, * entries we added to the end into their right locations. */ remove_marked_cache_entries(index, 1); + /* + * No need for STABLE_QSORT -- cmp_cache_name_compare sorts primarily + * on filename and secondarily on stage, and (name, stage #) are a + * unique tuple. + */ QSORT(index->cache, index->cache_nr, cmp_cache_name_compare); return errs; @@ -3070,7 +3485,8 @@ void merge_switch_to_result(struct merge_options *opt, { assert(opt->priv == NULL); if (result->clean >= 0 && update_worktree_and_index) { - struct merge_options_internal *opti = result->priv; + const char *filename; + FILE *fp; trace2_region_enter("merge", "checkout", opt->repo); if (checkout(opt, head, result->tree)) { @@ -3081,14 +3497,22 @@ void merge_switch_to_result(struct merge_options *opt, trace2_region_leave("merge", "checkout", opt->repo); trace2_region_enter("merge", "record_conflicted", opt->repo); - if (record_conflicted_index_entries(opt, opt->repo->index, - &opti->paths, - &opti->conflicted)) { + opt->priv = result->priv; + if (record_conflicted_index_entries(opt)) { /* failure to function */ + opt->priv = NULL; result->clean = -1; return; } + opt->priv = NULL; trace2_region_leave("merge", "record_conflicted", opt->repo); + + trace2_region_enter("merge", "write_auto_merge", opt->repo); + filename = git_path_auto_merge(opt->repo); + fp = xfopen(filename, "w"); + fprintf(fp, "%s\n", oid_to_hex(&result->tree->object.oid)); + fclose(fp); + trace2_region_leave("merge", "write_auto_merge", opt->repo); } if (display_update_msgs) { @@ -3133,6 +3557,8 @@ void merge_finalize(struct merge_options *opt, { struct merge_options_internal *opti = result->priv; + if (opt->renormalize) + git_attr_set_direction(GIT_ATTR_CHECKIN); assert(opt->priv == NULL); clear_or_reinit_internal_opts(opti, 0); @@ -3141,6 +3567,23 @@ void merge_finalize(struct merge_options *opt, /*** Function Grouping: helper functions for merge_incore_*() ***/ +static struct tree *shift_tree_object(struct repository *repo, + struct tree *one, struct tree *two, + const char *subtree_shift) +{ + struct object_id shifted; + + if (!*subtree_shift) { + shift_tree(repo, &one->object.oid, &two->object.oid, &shifted, 0); + } else { + shift_tree_by(repo, &one->object.oid, &two->object.oid, &shifted, + subtree_shift); + } + if (oideq(&two->object.oid, &shifted)) + return two; + return lookup_tree(repo, &shifted); +} + static inline void set_commit_tree(struct commit *c, struct tree *t) { c->maybe_tree = t; @@ -3208,6 +3651,10 @@ static void merge_start(struct merge_options *opt, struct merge_result *result) /* Default to histogram diff. Actually, just hardcode it...for now. */ opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF); + /* Handle attr direction stuff for renormalization */ + if (opt->renormalize) + git_attr_set_direction(GIT_ATTR_CHECKOUT); + /* Initialization of opt->priv, our internal merge data */ trace2_region_enter("merge", "allocate/init", opt->repo); if (opt->priv) { @@ -3220,12 +3667,14 @@ static void merge_start(struct merge_options *opt, struct merge_result *result) /* Initialization of various renames fields */ renames = &opt->priv->renames; for (i = MERGE_SIDE1; i <= MERGE_SIDE2; i++) { - strset_init_with_options(&renames->dirs_removed[i], - NULL, 0); + strintmap_init_with_options(&renames->dirs_removed[i], + NOT_RELEVANT, NULL, 0); strmap_init_with_options(&renames->dir_rename_count[i], NULL, 1); strmap_init_with_options(&renames->dir_renames[i], NULL, 0); + strintmap_init_with_options(&renames->relevant_sources[i], + 0, NULL, 0); } /* @@ -3264,6 +3713,13 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt, { struct object_id working_tree_oid; + if (opt->subtree_shift) { + side2 = shift_tree_object(opt->repo, side1, side2, + opt->subtree_shift); + merge_base = shift_tree_object(opt->repo, side1, merge_base, + opt->subtree_shift); + } + trace2_region_enter("merge", "collect_merge_info", opt->repo); if (collect_merge_info(opt, merge_base, side1, side2) != 0) { /* diff --git a/merge-recursive.c b/merge-recursive.c index ed31f9496cbcb2..7618303f7b425d 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1075,6 +1075,11 @@ static int merge_3way(struct merge_options *opt, read_mmblob(&src1, &a->oid); read_mmblob(&src2, &b->oid); + /* + * FIXME: Using a->path for normalization rules in ll_merge could be + * wrong if we renamed from a->path to b->path. We should use the + * target path for where the file will be written. + */ merge_status = ll_merge(result_buf, a->path, &orig, base, &src1, name1, &src2, name2, opt->repo->index, &ll_opts); @@ -1154,6 +1159,8 @@ static void print_commit(struct commit *commit) struct strbuf sb = STRBUF_INIT; struct pretty_print_context ctx = {0}; ctx.date_mode.type = DATE_NORMAL; + /* FIXME: Merge this with output_commit_title() */ + assert(!merge_remote_util(commit)); format_commit_message(commit, " %h: %m %s", &sb, &ctx); fprintf(stderr, "%s\n", sb.buf); strbuf_release(&sb); @@ -1177,6 +1184,11 @@ static int merge_submodule(struct merge_options *opt, int search = !opt->priv->call_depth; /* store a in result in case we fail */ + /* FIXME: This is the WRONG resolution for the recursive case when + * we need to be careful to avoid accidentally matching either side. + * Should probably use o instead there, much like we do for merging + * binaries. + */ oidcpy(result, a); /* we can not handle deletion conflicts */ @@ -1301,6 +1313,13 @@ static int merge_mode_and_contents(struct merge_options *opt, if ((S_IFMT & a->mode) != (S_IFMT & b->mode)) { result->clean = 0; + /* + * FIXME: This is a bad resolution for recursive case; for + * the recursive case we want something that is unlikely to + * accidentally match either side. Also, while it makes + * sense to prefer regular files over symlinks, it doesn't + * make sense to prefer regular files over submodules. + */ if (S_ISREG(a->mode)) { result->blob.mode = a->mode; oidcpy(&result->blob.oid, &a->oid); @@ -1349,6 +1368,7 @@ static int merge_mode_and_contents(struct merge_options *opt, free(result_buf.ptr); if (ret) return ret; + /* FIXME: bug, what if modes didn't match? */ result->clean = (merge_status == 0); } else if (S_ISGITLINK(a->mode)) { result->clean = merge_submodule(opt, &result->blob.oid, @@ -2663,6 +2683,14 @@ static int process_renames(struct merge_options *opt, struct string_list b_by_dst = STRING_LIST_INIT_NODUP; const struct rename *sre; + /* + * FIXME: As string-list.h notes, it's O(n^2) to build a sorted + * string_list one-by-one, but O(n log n) to build it unsorted and + * then sort it. Note that as we build the list, we do not need to + * check if the existing destination path is already in the list, + * because the structure of diffcore_rename guarantees we won't + * have duplicates. + */ for (i = 0; i < a_renames->nr; i++) { sre = a_renames->items[i].util; string_list_insert(&a_by_dst, sre->pair->two->path)->util @@ -3601,6 +3629,15 @@ static int merge_recursive_internal(struct merge_options *opt, return err(opt, _("merge returned no commit")); } + /* + * FIXME: Since merge_recursive_internal() is only ever called by + * places that ensure the index is loaded first + * (e.g. builtin/merge.c, rebase/sequencer, etc.), in the common + * case where the merge base was unique that means when we get here + * we immediately discard the index and re-read it, which is a + * complete waste of time. We should only be discarding and + * re-reading if we were forced to recurse. + */ discard_index(opt->repo->index); if (!opt->priv->call_depth) repo_read_index(opt->repo); diff --git a/path.c b/path.c index 7b385e5eb28227..9e883eb52446d3 100644 --- a/path.c +++ b/path.c @@ -1534,5 +1534,6 @@ REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR") REPO_GIT_PATH_FUNC(merge_mode, "MERGE_MODE") REPO_GIT_PATH_FUNC(merge_head, "MERGE_HEAD") REPO_GIT_PATH_FUNC(merge_autostash, "MERGE_AUTOSTASH") +REPO_GIT_PATH_FUNC(auto_merge, "AUTO_MERGE") REPO_GIT_PATH_FUNC(fetch_head, "FETCH_HEAD") REPO_GIT_PATH_FUNC(shallow, "shallow") diff --git a/path.h b/path.h index e7e77da6aaa5cf..251c78d980000a 100644 --- a/path.h +++ b/path.h @@ -176,6 +176,7 @@ struct path_cache { const char *merge_mode; const char *merge_head; const char *merge_autostash; + const char *auto_merge; const char *fetch_head; const char *shallow; }; @@ -191,6 +192,7 @@ const char *git_path_merge_rr(struct repository *r); const char *git_path_merge_mode(struct repository *r); const char *git_path_merge_head(struct repository *r); const char *git_path_merge_autostash(struct repository *r); +const char *git_path_auto_merge(struct repository *r); const char *git_path_fetch_head(struct repository *r); const char *git_path_shallow(struct repository *r); diff --git a/sequencer.c b/sequencer.c index b4c63ae207e32c..4fee4aa3ea9eb2 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2270,6 +2270,7 @@ static int do_pick_commit(struct repository *r, refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD", NULL, 0); unlink(git_path_merge_msg(r)); + unlink(git_path_auto_merge(r)); fprintf(stderr, _("dropping %s %s -- patch contents already upstream\n"), oid_to_hex(&commit->object.oid), msg.subject); @@ -2633,6 +2634,8 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose) need_cleanup = 1; } + unlink(git_path_auto_merge(r)); + if (!need_cleanup) return; @@ -4293,6 +4296,7 @@ static int pick_commits(struct repository *r, unlink(rebase_path_stopped_sha()); unlink(rebase_path_amend()); unlink(git_path_merge_head(r)); + unlink(git_path_auto_merge(r)); delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF); if (item->command == TODO_BREAK) { @@ -4687,6 +4691,7 @@ static int commit_staged_changes(struct repository *r, return error(_("could not commit staged changes.")); unlink(rebase_path_amend()); unlink(git_path_merge_head(r)); + unlink(git_path_auto_merge(r)); if (final_fixup) { unlink(rebase_path_fixup_msg()); unlink(rebase_path_squash_msg()); diff --git a/t/t3512-cherry-pick-submodule.sh b/t/t3512-cherry-pick-submodule.sh index 822f2d4bfbd5ad..c657840db33b6e 100755 --- a/t/t3512-cherry-pick-submodule.sh +++ b/t/t3512-cherry-pick-submodule.sh @@ -8,8 +8,11 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh . "$TEST_DIRECTORY"/lib-submodule-update.sh -KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 -KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 +if test "$GIT_TEST_MERGE_ALGORITHM" != ort +then + KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 + KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 +fi test_submodule_switch "cherry-pick" test_expect_success 'unrelated submodule/file conflict is ignored' ' diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh index a759f12cbb1d6b..74cd96e582234f 100755 --- a/t/t3513-revert-submodule.sh +++ b/t/t3513-revert-submodule.sh @@ -30,7 +30,10 @@ git_revert () { git revert HEAD } -KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 +if test "$GIT_TEST_MERGE_ALGORITHM" != ort +then + KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 +fi test_submodule_switch_func "git_revert" test_done diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh index 29537f4798ef85..4f92a116e1f0ec 100755 --- a/t/t5572-pull-submodule.sh +++ b/t/t5572-pull-submodule.sh @@ -42,8 +42,11 @@ git_pull_noff () { $2 git pull --no-ff } -KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 -KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 +if test "$GIT_TEST_MERGE_ALGORITHM" != ort +then + KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 + KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 +fi test_submodule_switch_func "git_pull_noff" test_expect_success 'pull --recurse-submodule setup' ' diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 5d3b711fe682d8..7134769149fc53 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -4797,7 +4797,7 @@ test_setup_12f () { ) } -test_expect_merge_algorithm failure success '12f: Trivial directory resolve, caching, all kinds of fun' ' +test_expect_merge_algorithm failure failure '12f: Trivial directory resolve, caching, all kinds of fun' ' test_setup_12f && ( cd 12f && @@ -4895,6 +4895,77 @@ test_expect_merge_algorithm failure success '12f: Trivial directory resolve, cac ) ' +# Testcase 12g, Testcase with two kinds of "relevant" renames +# Commit O: somefile_O, subdir/{a_O,b_O} +# Commit A: somefile_A, subdir/{a_O,b_O,c_A} +# Commit B: newfile_B, newdir/{a_B,b_B} +# Expected: newfile_{merged}, newdir/{a_B,b_B,c_A} + +test_setup_12g () { + test_create_repo 12g && + ( + cd 12g && + + mkdir -p subdir && + test_write_lines upon a time there was a >somefile && + test_write_lines 1 2 3 4 5 6 7 8 9 10 >subdir/a && + test_write_lines one two three four five six >subdir/b && + git add . && + test_tick && + git commit -m "O" && + + git branch O && + git branch A && + git branch B && + + git switch A && + test_write_lines once upon a time there was a >somefile && + > subdir/c && + git add somefile subdir/c && + test_tick && + git commit -m "A" && + + git checkout B && + git mv somefile newfile && + git mv subdir newdir && + echo repo >>newfile && + test_write_lines 1 2 3 4 5 6 7 8 9 10 11 >newdir/a && + test_write_lines one two three four five six seven >newdir/b && + git add newfile newdir && + test_tick && + git commit -m "B" + ) +} + +test_expect_success '12g: Testcase with two kinds of "relevant" renames' ' + test_setup_12g && + ( + cd 12g && + + git checkout A^0 && + + git -c merge.directoryRenames=true merge -s recursive B^0 && + + test_write_lines once upon a time there was a repo >expect && + test_cmp expect newfile && + + git ls-files -s >out && + test_line_count = 4 out && + + git rev-parse >actual \ + HEAD:newdir/a HEAD:newdir/b HEAD:newdir/c && + git rev-parse >expect \ + B:newdir/a B:newdir/b A:subdir/c && + test_cmp expect actual && + + test_must_fail git rev-parse HEAD:subdir/a && + test_must_fail git rev-parse HEAD:subdir/b && + test_must_fail git rev-parse HEAD:subdir/c && + test_path_is_missing subdir/ && + test_path_is_file newdir/c + ) +' + ########################################################################### # SECTION 13: Checking informational and conflict messages # diff --git a/t/t6428-merge-conflicts-sparse.sh b/t/t6428-merge-conflicts-sparse.sh new file mode 100755 index 00000000000000..7e8bf497f821da --- /dev/null +++ b/t/t6428-merge-conflicts-sparse.sh @@ -0,0 +1,158 @@ +#!/bin/sh + +test_description="merge cases" + +# The setup for all of them, pictorially, is: +# +# A +# o +# / \ +# O o ? +# \ / +# o +# B +# +# To help make it easier to follow the flow of tests, they have been +# divided into sections and each test will start with a quick explanation +# of what commits O, A, and B contain. +# +# Notation: +# z/{b,c} means files z/b and z/c both exist +# x/d_1 means file x/d exists with content d1. (Purpose of the +# underscore notation is to differentiate different +# files that might be renamed into each other's paths.) + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh + + +# Testcase basic, conflicting changes in 'numerals' + +test_setup_numerals () { + test_create_repo numerals_$1 && + ( + cd numerals_$1 && + + >README && + test_write_lines I II III >numerals && + git add README numerals && + test_tick && + git commit -m "O" && + + git branch O && + git branch A && + git branch B && + + git checkout A && + test_write_lines I II III IIII >numerals && + git add numerals && + test_tick && + git commit -m "A" && + + git checkout B && + test_write_lines I II III IV >numerals && + git add numerals && + test_tick && + git commit -m "B" && + + cat <<-EOF >expected-index && + H README + M numerals + M numerals + M numerals + EOF + + cat <<-EOF >expected-merge + I + II + III + <<<<<<< HEAD + IIII + ======= + IV + >>>>>>> B^0 + EOF + + ) +} + +test_expect_success 'conflicting entries written to worktree even if sparse' ' + test_setup_numerals plain && + ( + cd numerals_plain && + + git checkout A^0 && + + test_path_is_file README && + test_path_is_file numerals && + + git sparse-checkout init && + git sparse-checkout set README && + + test_path_is_file README && + test_path_is_missing numerals && + + test_must_fail git merge -s recursive B^0 && + + git ls-files -t >index_files && + test_cmp expected-index index_files && + + test_path_is_file README && + test_path_is_file numerals && + + test_cmp expected-merge numerals && + + # 4 other files: + # * expected-merge + # * expected-index + # * index_files + # * others + git ls-files -o >others && + test_line_count = 4 others + ) +' + +test_expect_merge_algorithm failure success 'present-despite-SKIP_WORKTREE handled reasonably' ' + test_setup_numerals in_the_way && + ( + cd numerals_in_the_way && + + git checkout A^0 && + + test_path_is_file README && + test_path_is_file numerals && + + git sparse-checkout init && + git sparse-checkout set README && + + test_path_is_file README && + test_path_is_missing numerals && + + echo foobar >numerals && + + test_must_fail git merge -s recursive B^0 && + + git ls-files -t >index_files && + test_cmp expected-index index_files && + + test_path_is_file README && + test_path_is_file numerals && + + test_cmp expected-merge numerals && + + # There should still be a file with "foobar" in it + grep foobar * && + + # 5 other files: + # * expected-merge + # * expected-index + # * index_files + # * others + # * whatever name was given to the numerals file that had + # "foobar" in it + git ls-files -o >others && + test_line_count = 5 others + ) +' + +test_done diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh index 0f92bcf326c848..e5e89c2045e714 100755 --- a/t/t6437-submodule-merge.sh +++ b/t/t6437-submodule-merge.sh @@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-merge.sh # # history @@ -328,7 +329,7 @@ test_expect_success 'setup file/submodule conflict' ' ) ' -test_expect_failure 'file/submodule conflict' ' +test_expect_merge_algorithm failure success 'file/submodule conflict' ' test_when_finished "git -C file-submodule reset --hard" && ( cd file-submodule && @@ -437,7 +438,7 @@ test_expect_failure 'directory/submodule conflict; keep submodule clean' ' ) ' -test_expect_failure !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' ' +test_expect_merge_algorithm failure success !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' ' test_when_finished "git -C directory-submodule/path reset --hard" && test_when_finished "git -C directory-submodule reset --hard" && ( diff --git a/t/t6438-submodule-directory-file-conflicts.sh b/t/t6438-submodule-directory-file-conflicts.sh index 04bf4be7d79224..8df67a0ef99d26 100755 --- a/t/t6438-submodule-directory-file-conflicts.sh +++ b/t/t6438-submodule-directory-file-conflicts.sh @@ -12,8 +12,11 @@ test_submodule_switch "merge --ff" test_submodule_switch "merge --ff-only" -KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 -KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 +if test "$GIT_TEST_MERGE_ALGORITHM" != ort +then + KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 + KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 +fi test_submodule_switch "merge --no-ff" test_done diff --git a/t/test-lib.sh b/t/test-lib.sh index d3f6af6a65451c..3dec266221cd07 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -448,6 +448,8 @@ export EDITOR GIT_DEFAULT_HASH="${GIT_TEST_DEFAULT_HASH:-sha1}" export GIT_DEFAULT_HASH +GIT_TEST_MERGE_ALGORITHM="${GIT_TEST_MERGE_ALGORITHM:-ort}" +export GIT_TEST_MERGE_ALGORITHM # Tests using GIT_TRACE typically don't want : output GIT_TRACE_BARE=1