From a49b55d52e7dcfd628b6328c9098d734ebe7a97d Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 13 Mar 2021 22:22:02 +0000 Subject: merge-ort, diffcore-rename: tweak dirs_removed and relevant_source type As noted in the previous commit, we want to be able to take advantage of the "majority rules" portion of directory rename detection to avoid detecting more renames than necessary. However, for diffcore-rename to take advantage of that, it needs to know whether a rename source file was needed for just directory rename detection reasons, or if it is wanted for potential three-way content merging. Modify relevant_sources from a strset to a strintmap, so we can encode additional information. We also modify dirs_removed from a strset to a strintmap at the same time because trying to determine what files are needed for directory rename detection will require us tracking a bit more information for each directory. This commit only changes the types of the two variables from strset to strintmap; it does not actually store any special values yet and for now only checks for presence of entries in the strintmap. Thus, the code is functionally identical to how it behaved before. Future commits will start associating values with each key for these two maps. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- diffcore.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'diffcore.h') diff --git a/diffcore.h b/diffcore.h index 737c93a6cc..4f168b385f 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 @@ -166,8 +166,8 @@ 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 *relevant_sources, - 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 *); -- cgit 1.2.3-korg From fb52938eec1cf6ec3169152362fe774849f5ac9b Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 13 Mar 2021 22:22:03 +0000 Subject: merge-ort: record the reason that we want a rename for a directory When one side of history renames a directory, and the other side of history added files to the old directory, directory rename detection is used to warn about the location of the added files so the user can move them to the old directory or keep them with the new one. This sets up three different types of directories: * directories that had new files added to them * directories underneath a directory that had new files added to them * directories where no new files were added to it or any leading path Save this information in dirs_removed; the next several commits will make use of this information. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- diffcore-rename.c | 2 +- diffcore.h | 7 +++++++ merge-ort.c | 41 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 46 insertions(+), 4 deletions(-) (limited to 'diffcore.h') diff --git a/diffcore-rename.c b/diffcore-rename.c index 6487825317..fafec66b29 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -667,7 +667,7 @@ static void cleanup_dir_rename_info(struct dir_rename_info *info, const char *source_dir = entry->key; struct strintmap *counts = entry->value; - if (!strintmap_contains(dirs_removed, source_dir)) { + if (!strintmap_get(dirs_removed, source_dir)) { string_list_append(&to_remove, source_dir); strintmap_clear(counts); continue; diff --git a/diffcore.h b/diffcore.h index 4f168b385f..d5a497b7a1 100644 --- a/diffcore.h +++ b/diffcore.h @@ -161,6 +161,13 @@ 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 +}; + void partial_clear_dir_rename_count(struct strmap *dir_rename_count); void diffcore_break(struct repository *, int); diff --git a/merge-ort.c b/merge-ort.c index 6fa670396c..e2606c73ad 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -73,6 +73,10 @@ 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 strintmap dirs_removed[3]; @@ -729,10 +733,41 @@ static void collect_rename_info(struct merge_options *opt, 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) - strintmap_set(&renames->dirs_removed[1], fullname, 1); + strintmap_set(&renames->dirs_removed[1], fullname, + relevance); if (sides & 2) - strintmap_set(&renames->dirs_removed[2], fullname, 1); + 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) @@ -3446,7 +3481,7 @@ static void merge_start(struct merge_options *opt, struct merge_result *result) renames = &opt->priv->renames; for (i = MERGE_SIDE1; i <= MERGE_SIDE2; i++) { strintmap_init_with_options(&renames->dirs_removed[i], - 0, NULL, 0); + NOT_RELEVANT, NULL, 0); strmap_init_with_options(&renames->dir_rename_count[i], NULL, 1); strmap_init_with_options(&renames->dir_renames[i], -- cgit 1.2.3-korg From ec59da6015c457e84953a802fc6484ae2da2d774 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 13 Mar 2021 22:22:07 +0000 Subject: merge-ort: record the reason that we want a rename for a file There are two different reasons we might want a rename for a file -- for three-way content merging or as part of directory rename detection. Record the reason. diffcore-rename will potentially be able to filter some of the ones marked as needed only for directory rename detection, if it can determine those directory renames based solely on renames found via exact rename detection and basename-guided rename detection. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- diffcore.h | 6 ++++++ merge-ort.c | 15 ++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) (limited to 'diffcore.h') diff --git a/diffcore.h b/diffcore.h index d5a497b7a1..cf8d4cb861 100644 --- a/diffcore.h +++ b/diffcore.h @@ -167,6 +167,12 @@ enum dir_rename_relevance { 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); diff --git a/merge-ort.c b/merge-ort.c index f2b259986e..7f5750ce6a 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -99,16 +99,18 @@ struct rename_info { struct strmap dir_renames[3]; /* - * relevant_sources: deleted paths for which we need rename detection + * 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 - * * we need to detect renames for an ancestor directory * * 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. + * 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]; @@ -677,8 +679,11 @@ static void add_pair(struct merge_options *opt, unsigned content_relevant = (match_mask == 0); unsigned location_relevant = (dir_rename_mask == 0x07); - if (content_relevant || location_relevant) - strintmap_set(&renames->relevant_sources[side], pathname, 1); + 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); -- cgit 1.2.3-korg