merge-recursive: apply necessary modifications for directory renames

This commit hooks together all the directory rename logic by making the necessary changes to the rename struct, it's dst_entry, and the diff_filepair under consideration. Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Feb 14, 2018 at 10:52 UTC 3b9616f149e46e1a97bf6dd5bc727a663d92dded
2 files changed +211 -26
merge-recursive.c
+186 -1
@@ -180,6 +180,7 @@ static int oid_eq(const struct object_id *a, const struct object_id *b)
180
181 enum rename_type {
182 RENAME_NORMAL = 0,
183 + RENAME_DIR,
184 RENAME_DELETE,
185 RENAME_ONE_FILE_TO_ONE,
186 RENAME_ONE_FILE_TO_TWO,
@@ -610,6 +611,7 @@ struct rename {
611 */
612 struct stage_data *src_entry;
613 struct stage_data *dst_entry;
614 + unsigned add_turned_into_rename:1;
615 unsigned processed:1;
616 };
617
@@ -644,6 +646,27 @@ static int update_stages(struct merge_options *opt, const char *path,
646 return 0;
647 }
648
649 +static int update_stages_for_stage_data(struct merge_options *opt,
650 + const char *path,
651 + const struct stage_data *stage_data)
652 +{
653 + struct diff_filespec o, a, b;
654 +
655 + o.mode = stage_data->stages[1].mode;
656 + oidcpy(&o.oid, &stage_data->stages[1].oid);
657 +
658 + a.mode = stage_data->stages[2].mode;
659 + oidcpy(&a.oid, &stage_data->stages[2].oid);
660 +
661 + b.mode = stage_data->stages[3].mode;
662 + oidcpy(&b.oid, &stage_data->stages[3].oid);
663 +
664 + return update_stages(opt, path,
665 + is_null_oid(&o.oid) ? NULL : &o,
666 + is_null_oid(&a.oid) ? NULL : &a,
667 + is_null_oid(&b.oid) ? NULL : &b);
668 +}
669 +
670 static void update_entry(struct stage_data *entry,
671 struct diff_filespec *o,
672 struct diff_filespec *a,
@@ -1111,6 +1134,18 @@ static int merge_file_one(struct merge_options *o,
1134 return merge_file_1(o, &one, &a, &b, branch1, branch2, mfi);
1135 }
1136
1137 +static int conflict_rename_dir(struct merge_options *o,
1138 + struct diff_filepair *pair,
1139 + const char *rename_branch,
1140 + const char *other_branch)
1141 +{
1142 + const struct diff_filespec *dest = pair->two;
1143 +
1144 + if (update_file(o, 1, &dest->oid, dest->mode, dest->path))
1145 + return -1;
1146 + return 0;
1147 +}
1148 +
1149 static int handle_change_delete(struct merge_options *o,
1150 const char *path, const char *old_path,
1151 const struct object_id *o_oid, int o_mode,
@@ -1380,6 +1415,24 @@ static int conflict_rename_rename_2to1(struct merge_options *o,
1415 if (!ret)
1416 ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode,
1417 new_path2);
1418 + /*
1419 + * unpack_trees() actually populates the index for us for
1420 + * "normal" rename/rename(2to1) situtations so that the
1421 + * correct entries are at the higher stages, which would
1422 + * make the call below to update_stages_for_stage_data
1423 + * unnecessary. However, if either of the renames came
1424 + * from a directory rename, then unpack_trees() will not
1425 + * have gotten the right data loaded into the index, so we
1426 + * need to do so now. (While it'd be tempting to move this
1427 + * call to update_stages_for_stage_data() to
1428 + * apply_directory_rename_modifications(), that would break
1429 + * our intermediate calls to would_lose_untracked() since
1430 + * those rely on the current in-memory index. See also the
1431 + * big "NOTE" in update_stages()).
1432 + */
1433 + if (update_stages_for_stage_data(o, path, ci->dst_entry1))
1434 + ret = -1;
1435 +
1436 free(new_path2);
1437 free(new_path1);
1438 }
@@ -1942,6 +1995,111 @@ static char *check_for_directory_rename(struct merge_options *o,
1995 return new_path;
1996 }
1997
1998 +static void apply_directory_rename_modifications(struct merge_options *o,
1999 + struct diff_filepair *pair,
2000 + char *new_path,
2001 + struct rename *re,
2002 + struct tree *tree,
2003 + struct tree *o_tree,
2004 + struct tree *a_tree,
2005 + struct tree *b_tree,
2006 + struct string_list *entries,
2007 + int *clean)
2008 +{
2009 + struct string_list_item *item;
2010 + int stage = (tree == a_tree ? 2 : 3);
2011 +
2012 + /*
2013 + * In all cases where we can do directory rename detection,
2014 + * unpack_trees() will have read pair->two->path into the
2015 + * index and the working copy. We need to remove it so that
2016 + * we can instead place it at new_path. It is guaranteed to
2017 + * not be untracked (unpack_trees() would have errored out
2018 + * saying the file would have been overwritten), but it might
2019 + * be dirty, though.
2020 + */
2021 + remove_file(o, 1, pair->two->path, 0 /* no_wd */);
2022 +
2023 + /* Find or create a new re->dst_entry */
2024 + item = string_list_lookup(entries, new_path);
2025 + if (item) {
2026 + /*
2027 + * Since we're renaming on this side of history, and it's
2028 + * due to a directory rename on the other side of history
2029 + * (which we only allow when the directory in question no
2030 + * longer exists on the other side of history), the
2031 + * original entry for re->dst_entry is no longer
2032 + * necessary...
2033 + */
2034 + re->dst_entry->processed = 1;
2035 +
2036 + /*
2037 + * ...because we'll be using this new one.
2038 + */
2039 + re->dst_entry = item->util;
2040 + } else {
2041 + /*
2042 + * re->dst_entry is for the before-dir-rename path, and we
2043 + * need it to hold information for the after-dir-rename
2044 + * path. Before creating a new entry, we need to mark the
2045 + * old one as unnecessary (...unless it is shared by
2046 + * src_entry, i.e. this didn't use to be a rename, in which
2047 + * case we can just allow the normal processing to happen
2048 + * for it).
2049 + */
2050 + if (pair->status == 'R')
2051 + re->dst_entry->processed = 1;
2052 +
2053 + re->dst_entry = insert_stage_data(new_path,
2054 + o_tree, a_tree, b_tree,
2055 + entries);
2056 + item = string_list_insert(entries, new_path);
2057 + item->util = re->dst_entry;
2058 + }
2059 +
2060 + /*
2061 + * Update the stage_data with the information about the path we are
2062 + * moving into place. That slot will be empty and available for us
2063 + * to write to because of the collision checks in
2064 + * handle_path_level_conflicts(). In other words,
2065 + * re->dst_entry->stages[stage].oid will be the null_oid, so it's
2066 + * open for us to write to.
2067 + *
2068 + * It may be tempting to actually update the index at this point as
2069 + * well, using update_stages_for_stage_data(), but as per the big
2070 + * "NOTE" in update_stages(), doing so will modify the current
2071 + * in-memory index which will break calls to would_lose_untracked()
2072 + * that we need to make. Instead, we need to just make sure that
2073 + * the various conflict_rename_*() functions update the index
2074 + * explicitly rather than relying on unpack_trees() to have done it.
2075 + */
2076 + get_tree_entry(tree->object.oid.hash,
2077 + pair->two->path,
2078 + re->dst_entry->stages[stage].oid.hash,
2079 + &re->dst_entry->stages[stage].mode);
2080 +
2081 + /* Update pair status */
2082 + if (pair->status == 'A') {
2083 + /*
2084 + * Recording rename information for this add makes it look
2085 + * like a rename/delete conflict. Make sure we can
2086 + * correctly handle this as an add that was moved to a new
2087 + * directory instead of reporting a rename/delete conflict.
2088 + */
2089 + re->add_turned_into_rename = 1;
2090 + }
2091 + /*
2092 + * We don't actually look at pair->status again, but it seems
2093 + * pedagogically correct to adjust it.
2094 + */
2095 + pair->status = 'R';
2096 +
2097 + /*
2098 + * Finally, record the new location.
2099 + */
2100 + pair->two->path = new_path;
2101 +}
2102 +
2103 /*
2104 * Get information of all renames which occurred in 'pairs', making use of
2105 * any implicit directory renames inferred from the other side of history.
@@ -1991,6 +2149,7 @@ static struct string_list *get_renames(struct merge_options *o,
2149
2150 re = xmalloc(sizeof(*re));
2151 re->processed = 0;
2152 + re->add_turned_into_rename = 0;
2153 re->pair = pair;
2154 item = string_list_lookup(entries, re->pair->one->path);
2155 if (!item)
@@ -2007,6 +2166,12 @@ static struct string_list *get_renames(struct merge_options *o,
2166 re->dst_entry = item->util;
2167 item = string_list_insert(renames, pair->one->path);
2168 item->util = re;
2169 + if (new_path)
2170 + apply_directory_rename_modifications(o, pair, new_path,
2171 + re, tree, o_tree,
2172 + a_tree, b_tree,
2173 + entries,
2174 + clean_merge);
2175 }
2176
2177 hashmap_iter_init(&collisions, &iter);
@@ -2176,7 +2341,19 @@ static int process_renames(struct merge_options *o,
2341 dst_other.mode = ren1->dst_entry->stages[other_stage].mode;
2342 try_merge = 0;
2343
2179 - if (oid_eq(&src_other.oid, &null_oid)) {
2344 + if (oid_eq(&src_other.oid, &null_oid) &&
2345 + ren1->add_turned_into_rename) {
2346 + setup_rename_conflict_info(RENAME_DIR,
2347 + ren1->pair,
2348 + NULL,
2349 + branch1,
2350 + branch2,
2351 + ren1->dst_entry,
2352 + NULL,
2353 + o,
2354 + NULL,
2355 + NULL);
2356 + } else if (oid_eq(&src_other.oid, &null_oid)) {
2357 setup_rename_conflict_info(RENAME_DELETE,
2358 ren1->pair,
2359 NULL,
@@ -2593,6 +2770,14 @@ static int process_entry(struct merge_options *o,
2770 o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
2771 conflict_info);
2772 break;
2773 + case RENAME_DIR:
2774 + clean_merge = 1;
2775 + if (conflict_rename_dir(o,
2776 + conflict_info->pair1,
2777 + conflict_info->branch1,
2778 + conflict_info->branch2))
2779 + clean_merge = -1;
2780 + break;
2781 case RENAME_DELETE:
2782 clean_merge = 0;
2783 if (conflict_rename_delete(o,
t/t6043-merge-rename-directories.sh
+25 -25
@@ -69,7 +69,7 @@ test_expect_success '1a-setup: Simple directory rename detection' '
69 )
70 '
71
72 -test_expect_failure '1a-check: Simple directory rename detection' '
72 +test_expect_success '1a-check: Simple directory rename detection' '
73 (
74 cd 1a &&
75
@@ -136,7 +136,7 @@ test_expect_success '1b-setup: Merge a directory with another' '
136 )
137 '
138
139 -test_expect_failure '1b-check: Merge a directory with another' '
139 +test_expect_success '1b-check: Merge a directory with another' '
140 (
141 cd 1b &&
142
@@ -194,7 +194,7 @@ test_expect_success '1c-setup: Transitive renaming' '
194 )
195 '
196
197 -test_expect_failure '1c-check: Transitive renaming' '
197 +test_expect_success '1c-check: Transitive renaming' '
198 (
199 cd 1c &&
200
@@ -263,7 +263,7 @@ test_expect_success '1d-setup: Directory renames cause a rename/rename(2to1) con
263 )
264 '
265
266 -test_expect_failure '1d-check: Directory renames cause a rename/rename(2to1) conflict' '
266 +test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) conflict' '
267 (
268 cd 1d &&
269
@@ -342,7 +342,7 @@ test_expect_success '1e-setup: Renamed directory, with all files being renamed t
342 )
343 '
344
345 -test_expect_failure '1e-check: Renamed directory, with all files being renamed too' '
345 +test_expect_success '1e-check: Renamed directory, with all files being renamed too' '
346 (
347 cd 1e &&
348
@@ -408,7 +408,7 @@ test_expect_success '1f-setup: Split a directory into two other directories' '
408 )
409 '
410
411 -test_expect_failure '1f-check: Split a directory into two other directories' '
411 +test_expect_success '1f-check: Split a directory into two other directories' '
412 (
413 cd 1f &&
414
@@ -907,7 +907,7 @@ test_expect_success '5a-setup: Merge directories, other side adds files to origi
907 )
908 '
909
910 -test_expect_failure '5a-check: Merge directories, other side adds files to original and target' '
910 +test_expect_success '5a-check: Merge directories, other side adds files to original and target' '
911 (
912 cd 5a &&
913
@@ -981,7 +981,7 @@ test_expect_success '5b-setup: Rename/delete in order to get add/add/add conflic
981 )
982 '
983
984 -test_expect_failure '5b-check: Rename/delete in order to get add/add/add conflict' '
984 +test_expect_success '5b-check: Rename/delete in order to get add/add/add conflict' '
985 (
986 cd 5b &&
987
@@ -1061,7 +1061,7 @@ test_expect_success '5c-setup: Transitive rename would cause rename/rename/renam
1061 )
1062 '
1063
1064 -test_expect_failure '5c-check: Transitive rename would cause rename/rename/rename/add/add/add' '
1064 +test_expect_success '5c-check: Transitive rename would cause rename/rename/rename/add/add/add' '
1065 (
1066 cd 5c &&
1067
@@ -1145,7 +1145,7 @@ test_expect_success '5d-setup: Directory/file/file conflict due to directory ren
1145 )
1146 '
1147
1148 -test_expect_failure '5d-check: Directory/file/file conflict due to directory rename' '
1148 +test_expect_success '5d-check: Directory/file/file conflict due to directory rename' '
1149 (
1150 cd 5d &&
1151
@@ -1583,7 +1583,7 @@ test_expect_success '7a-setup: rename-dir vs. rename-dir (NOT split evenly) PLUS
1583 )
1584 '
1585
1586 -test_expect_failure '7a-check: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '
1586 +test_expect_success '7a-check: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '
1587 (
1588 cd 7a &&
1589
@@ -1655,7 +1655,7 @@ test_expect_success '7b-setup: rename/rename(2to1), but only due to transitive r
1655 )
1656 '
1657
1658 -test_expect_failure '7b-check: rename/rename(2to1), but only due to transitive rename' '
1658 +test_expect_success '7b-check: rename/rename(2to1), but only due to transitive rename' '
1659 (
1660 cd 7b &&
1661
@@ -1731,7 +1731,7 @@ test_expect_success '7c-setup: rename/rename(1to...2or3); transitive rename may
1731 )
1732 '
1733
1734 -test_expect_failure '7c-check: rename/rename(1to...2or3); transitive rename may add complexity' '
1734 +test_expect_success '7c-check: rename/rename(1to...2or3); transitive rename may add complexity' '
1735 (
1736 cd 7c &&
1737
@@ -1795,7 +1795,7 @@ test_expect_success '7d-setup: transitive rename involved in rename/delete; how
1795 )
1796 '
1797
1798 -test_expect_failure '7d-check: transitive rename involved in rename/delete; how is it reported?' '
1798 +test_expect_success '7d-check: transitive rename involved in rename/delete; how is it reported?' '
1799 (
1800 cd 7d &&
1801
@@ -1885,7 +1885,7 @@ test_expect_success '7e-setup: transitive rename in rename/delete AND dirs in th
1885 )
1886 '
1887
1888 -test_expect_failure '7e-check: transitive rename in rename/delete AND dirs in the way' '
1888 +test_expect_success '7e-check: transitive rename in rename/delete AND dirs in the way' '
1889 (
1890 cd 7e &&
1891
@@ -1976,7 +1976,7 @@ test_expect_success '8a-setup: Dual-directory rename, one into the others way' '
1976 )
1977 '
1978
1979 -test_expect_failure '8a-check: Dual-directory rename, one into the others way' '
1979 +test_expect_success '8a-check: Dual-directory rename, one into the others way' '
1980 (
1981 cd 8a &&
1982
@@ -2121,7 +2121,7 @@ test_expect_success '8c-setup: rename+modify/delete' '
2121 )
2122 '
2123
2124 -test_expect_failure '8c-check: rename+modify/delete' '
2124 +test_expect_success '8c-check: rename+modify/delete' '
2125 (
2126 cd 8c &&
2127
@@ -2208,7 +2208,7 @@ test_expect_success '8d-setup: rename/delete...or not?' '
2208 )
2209 '
2210
2211 -test_expect_failure '8d-check: rename/delete...or not?' '
2211 +test_expect_success '8d-check: rename/delete...or not?' '
2212 (
2213 cd 8d &&
2214
@@ -2283,7 +2283,7 @@ test_expect_success '8e-setup: Both sides rename, one side adds to original dire
2283 )
2284 '
2285
2286 -test_expect_failure '8e-check: Both sides rename, one side adds to original directory' '
2286 +test_expect_success '8e-check: Both sides rename, one side adds to original directory' '
2287 (
2288 cd 8e &&
2289
@@ -2370,7 +2370,7 @@ test_expect_success '9a-setup: Inner renamed directory within outer renamed dire
2370 )
2371 '
2372
2373 -test_expect_failure '9a-check: Inner renamed directory within outer renamed directory' '
2373 +test_expect_success '9a-check: Inner renamed directory within outer renamed directory' '
2374 (
2375 cd 9a &&
2376
@@ -2440,7 +2440,7 @@ test_expect_success '9b-setup: Transitive rename with content merge' '
2440 )
2441 '
2442
2443 -test_expect_failure '9b-check: Transitive rename with content merge' '
2443 +test_expect_success '9b-check: Transitive rename with content merge' '
2444 (
2445 cd 9b &&
2446
@@ -2530,7 +2530,7 @@ test_expect_success '9c-setup: Doubly transitive rename?' '
2530 )
2531 '
2532
2533 -test_expect_failure '9c-check: Doubly transitive rename?' '
2533 +test_expect_success '9c-check: Doubly transitive rename?' '
2534 (
2535 cd 9c &&
2536
@@ -2618,7 +2618,7 @@ test_expect_success '9d-setup: N-way transitive rename?' '
2618 )
2619 '
2620
2621 -test_expect_failure '9d-check: N-way transitive rename?' '
2621 +test_expect_success '9d-check: N-way transitive rename?' '
2622 (
2623 cd 9d &&
2624
@@ -2700,7 +2700,7 @@ test_expect_success '9e-setup: N-to-1 whammo' '
2700 )
2701 '
2702
2703 -test_expect_failure C_LOCALE_OUTPUT '9e-check: N-to-1 whammo' '
2703 +test_expect_success C_LOCALE_OUTPUT '9e-check: N-to-1 whammo' '
2704 (
2705 cd 9e &&
2706
@@ -2778,7 +2778,7 @@ test_expect_success '9f-setup: Renamed directory that only contained immediate s
2778 )
2779 '
2780
2781 -test_expect_failure '9f-check: Renamed directory that only contained immediate subdirs' '
2781 +test_expect_success '9f-check: Renamed directory that only contained immediate subdirs' '
2782 (
2783 cd 9f &&
2784