merge-recursive: check for file level conflicts then get new name
Before trying to apply directory renames to paths within the given directories, we want to make sure that there aren't conflicts at the file level either. If there aren't any, then get the new name from any directory renames. 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
Apr 19, 2018 at 10:58 UTC
f6f775591881036a3c4bfcf6737c9119c251537d
4 files changed
+199
-9
merge-recursive.c
+166
-8
@@ -1520,6 +1520,91 @@ static void remove_hashmap_entries(struct hashmap *dir_renames,
1520
string_list_clear(items_to_remove, 0);
1521
}
1522
1523
+/*
1524
+ * See if there is a directory rename for path, and if there are any file
1525
+ * level conflicts for the renamed location. If there is a rename and
1526
+ * there are no conflicts, return the new name. Otherwise, return NULL.
1527
+ */
1528
+static char *handle_path_level_conflicts(struct merge_options *o,
1529
+ const char *path,
1530
+ struct dir_rename_entry *entry,
1531
+ struct hashmap *collisions,
1532
+ struct tree *tree)
1533
+{
1534
+ char *new_path = NULL;
1535
+ struct collision_entry *collision_ent;
1536
+ int clean = 1;
1537
+ struct strbuf collision_paths = STRBUF_INIT;
1538
+
1539
+ /*
1540
+ * entry has the mapping of old directory name to new directory name
1541
+ * that we want to apply to path.
1542
+ */
1543
+ new_path = apply_dir_rename(entry, path);
1544
+
1545
+ if (!new_path) {
1546
+ /* This should only happen when entry->non_unique_new_dir set */
1547
+ if (!entry->non_unique_new_dir)
1548
+ BUG("entry->non_unqiue_dir not set and !new_path");
1549
+ output(o, 1, _("CONFLICT (directory rename split): "
1550
+ "Unclear where to place %s because directory "
1551
+ "%s was renamed to multiple other directories, "
1552
+ "with no destination getting a majority of the "
1553
+ "files."),
1554
+ path, entry->dir);
1555
+ clean = 0;
1556
+ return NULL;
1557
+ }
1558
+
1559
+ /*
1560
+ * The caller needs to have ensured that it has pre-populated
1561
+ * collisions with all paths that map to new_path. Do a quick check
1562
+ * to ensure that's the case.
1563
+ */
1564
+ collision_ent = collision_find_entry(collisions, new_path);
1565
+ if (collision_ent == NULL)
1566
+ BUG("collision_ent is NULL");
1567
+
1568
+ /*
1569
+ * Check for one-sided add/add/.../add conflicts, i.e.
1570
+ * where implicit renames from the other side doing
1571
+ * directory rename(s) can affect this side of history
1572
+ * to put multiple paths into the same location. Warn
1573
+ * and bail on directory renames for such paths.
1574
+ */
1575
+ if (collision_ent->reported_already) {
1576
+ clean = 0;
1577
+ } else if (tree_has_path(tree, new_path)) {
1578
+ collision_ent->reported_already = 1;
1579
+ strbuf_add_separated_string_list(&collision_paths, ", ",
1580
+ &collision_ent->source_files);
1581
+ output(o, 1, _("CONFLICT (implicit dir rename): Existing "
1582
+ "file/dir at %s in the way of implicit "
1583
+ "directory rename(s) putting the following "
1584
+ "path(s) there: %s."),
1585
+ new_path, collision_paths.buf);
1586
+ clean = 0;
1587
+ } else if (collision_ent->source_files.nr > 1) {
1588
+ collision_ent->reported_already = 1;
1589
+ strbuf_add_separated_string_list(&collision_paths, ", ",
1590
+ &collision_ent->source_files);
1591
+ output(o, 1, _("CONFLICT (implicit dir rename): Cannot map "
1592
+ "more than one path to %s; implicit directory "
1593
+ "renames tried to put these paths there: %s"),
1594
+ new_path, collision_paths.buf);
1595
+ clean = 0;
1596
+ }
1597
+
1598
+ /* Free memory we no longer need */
1599
+ strbuf_release(&collision_paths);
1600
+ if (!clean && new_path) {
1601
+ free(new_path);
1602
+ return NULL;
1603
+ }
1604
+
1605
+ return new_path;
1606
+}
1607
+
1608
/*
1609
* There are a couple things we want to do at the directory level:
1610
* 1. Check for both sides renaming to the same thing, in order to avoid
@@ -1799,6 +1884,59 @@ static void compute_collisions(struct hashmap *collisions,
1884
}
1885
}
1886
1887
+static char *check_for_directory_rename(struct merge_options *o,
1888
+ const char *path,
1889
+ struct tree *tree,
1890
+ struct hashmap *dir_renames,
1891
+ struct hashmap *dir_rename_exclusions,
1892
+ struct hashmap *collisions,
1893
+ int *clean_merge)
1894
+{
1895
+ char *new_path = NULL;
1896
+ struct dir_rename_entry *entry = check_dir_renamed(path, dir_renames);
1897
+ struct dir_rename_entry *oentry = NULL;
1898
+
1899
+ if (!entry)
1900
+ return new_path;
1901
+
1902
+ /*
1903
+ * This next part is a little weird. We do not want to do an
1904
+ * implicit rename into a directory we renamed on our side, because
1905
+ * that will result in a spurious rename/rename(1to2) conflict. An
1906
+ * example:
1907
+ * Base commit: dumbdir/afile, otherdir/bfile
1908
+ * Side 1: smrtdir/afile, otherdir/bfile
1909
+ * Side 2: dumbdir/afile, dumbdir/bfile
1910
+ * Here, while working on Side 1, we could notice that otherdir was
1911
+ * renamed/merged to dumbdir, and change the diff_filepair for
1912
+ * otherdir/bfile into a rename into dumbdir/bfile. However, Side
1913
+ * 2 will notice the rename from dumbdir to smrtdir, and do the
1914
+ * transitive rename to move it from dumbdir/bfile to
1915
+ * smrtdir/bfile. That gives us bfile in dumbdir vs being in
1916
+ * smrtdir, a rename/rename(1to2) conflict. We really just want
1917
+ * the file to end up in smrtdir. And the way to achieve that is
1918
+ * to not let Side1 do the rename to dumbdir, since we know that is
1919
+ * the source of one of our directory renames.
1920
+ *
1921
+ * That's why oentry and dir_rename_exclusions is here.
1922
+ *
1923
+ * As it turns out, this also prevents N-way transient rename
1924
+ * confusion; See testcases 9c and 9d of t6043.
1925
+ */
1926
+ oentry = dir_rename_find_entry(dir_rename_exclusions, entry->new_dir.buf);
1927
+ if (oentry) {
1928
+ output(o, 1, _("WARNING: Avoiding applying %s -> %s rename "
1929
+ "to %s, because %s itself was renamed."),
1930
+ entry->dir, entry->new_dir.buf, path, entry->new_dir.buf);
1931
+ } else {
1932
+ new_path = handle_path_level_conflicts(o, path, entry,
1933
+ collisions, tree);
1934
+ *clean_merge &= (new_path != NULL);
1935
+ }
1936
+
1937
+ return new_path;
1938
+}
1939
+
1940
/*
1941
* Get information of all renames which occurred in 'pairs', making use of
1942
* any implicit directory renames inferred from the other side of history.
@@ -1809,11 +1947,13 @@ static void compute_collisions(struct hashmap *collisions,
1947
static struct string_list *get_renames(struct merge_options *o,
1948
struct diff_queue_struct *pairs,
1949
struct hashmap *dir_renames,
1950
+ struct hashmap *dir_rename_exclusions,
1951
struct tree *tree,
1952
struct tree *o_tree,
1953
struct tree *a_tree,
1954
struct tree *b_tree,
1816
- struct string_list *entries)
1955
+ struct string_list *entries,
1956
+ int *clean_merge)
1957
{
1958
int i;
1959
struct hashmap collisions;
@@ -1828,11 +1968,22 @@ static struct string_list *get_renames(struct merge_options *o,
1968
struct string_list_item *item;
1969
struct rename *re;
1970
struct diff_filepair *pair = pairs->queue[i];
1971
+ char *new_path; /* non-NULL only with directory renames */
1972
1832
- if (pair->status != 'R') {
1973
+ if (pair->status == 'D') {
1974
diff_free_filepair(pair);
1975
continue;
1976
}
1977
+ new_path = check_for_directory_rename(o, pair->two->path, tree,
1978
+ dir_renames,
1979
+ dir_rename_exclusions,
1980
+ &collisions,
1981
+ clean_merge);
1982
+ if (pair->status != 'R' && !new_path) {
1983
+ diff_free_filepair(pair);
1984
+ continue;
1985
+ }
1986
+
1987
re = xmalloc(sizeof(*re));
1988
re->processed = 0;
1989
re->pair = pair;
@@ -2150,7 +2301,7 @@ static int handle_renames(struct merge_options *o,
2301
{
2302
struct diff_queue_struct *head_pairs, *merge_pairs;
2303
struct hashmap *dir_re_head, *dir_re_merge;
2153
- int clean;
2304
+ int clean = 1;
2305
2306
ri->head_renames = NULL;
2307
ri->merge_renames = NULL;
@@ -2169,13 +2320,20 @@ static int handle_renames(struct merge_options *o,
2320
dir_re_merge, merge);
2321
2322
ri->head_renames = get_renames(o, head_pairs,
2172
- dir_re_merge, head,
2173
- common, head, merge, entries);
2323
+ dir_re_merge, dir_re_head, head,
2324
+ common, head, merge, entries,
2325
+ &clean);
2326
+ if (clean < 0)
2327
+ goto cleanup;
2328
ri->merge_renames = get_renames(o, merge_pairs,
2175
- dir_re_head, merge,
2176
- common, head, merge, entries);
2177
- clean = process_renames(o, ri->head_renames, ri->merge_renames);
2329
+ dir_re_head, dir_re_merge, merge,
2330
+ common, head, merge, entries,
2331
+ &clean);
2332
+ if (clean < 0)
2333
+ goto cleanup;
2334
+ clean &= process_renames(o, ri->head_renames, ri->merge_renames);
2335
2336
+cleanup:
2337
/*
2338
* Some cleanup is deferred until cleanup_renames() because the
2339
* data structures are still needed and referenced in
strbuf.c
+16
@@ -1,5 +1,6 @@
1
#include "cache.h"
2
#include "refs.h"
3
+#include "string-list.h"
4
#include "utf8.h"
5
6
int starts_with(const char *str, const char *prefix)
@@ -171,6 +172,21 @@ struct strbuf **strbuf_split_buf(const char *str, size_t slen,
172
return ret;
173
}
174
175
+void strbuf_add_separated_string_list(struct strbuf *str,
176
+ const char *sep,
177
+ struct string_list *slist)
178
+{
179
+ struct string_list_item *item;
180
+ int sep_needed = 0;
181
+
182
+ for_each_string_list_item(item, slist) {
183
+ if (sep_needed)
184
+ strbuf_addstr(str, sep);
185
+ strbuf_addstr(str, item->string);
186
+ sep_needed = 1;
187
+ }
188
+}
189
+
190
void strbuf_list_free(struct strbuf **sbs)
191
{
192
struct strbuf **s = sbs;
strbuf.h
+16
@@ -1,6 +1,8 @@
1
#ifndef STRBUF_H
2
#define STRBUF_H
3
4
+struct string_list;
5
+
6
/**
7
* strbuf's are meant to be used with all the usual C string and memory
8
* APIs. Given that the length of the buffer is known, it's often better to
@@ -537,6 +539,20 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,
539
return strbuf_split_max(sb, terminator, 0);
540
}
541
542
+/*
543
+ * Adds all strings of a string list to the strbuf, separated by the given
544
+ * separator. For example, if sep is
545
+ * ', '
546
+ * and slist contains
547
+ * ['element1', 'element2', ..., 'elementN'],
548
+ * then write:
549
+ * 'element1, element2, ..., elementN'
550
+ * to str. If only one element, just write "element1" to str.
551
+ */
552
+extern void strbuf_add_separated_string_list(struct strbuf *str,
553
+ const char *sep,
554
+ struct string_list *slist);
555
+
556
/**
557
* Free a NULL-terminated list of strbufs (for example, the return
558
* values of the strbuf_split*() functions).
t/t6043-merge-rename-directories.sh
+1
-1
@@ -489,7 +489,7 @@ test_expect_success '2a-setup: Directory split into two on one side, with equal
489
)
490
'
491
492
-test_expect_failure '2a-check: Directory split into two on one side, with equal numbers of paths' '
492
+test_expect_success '2a-check: Directory split into two on one side, with equal numbers of paths' '
493
(
494
cd 2a &&
495