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
Feb 14, 2018 at 10:51 UTC
79d49b7d8c75b80d5aeaf9d84895361b85e9c748
4 files changed
+199
-9
merge-recursive.c
+166
-8
@@ -1510,6 +1510,91 @@ static void remove_hashmap_entries(struct hashmap *dir_renames,
1510
string_list_clear(items_to_remove, 0);
1511
}
1512
1513
+/*
1514
+ * See if there is a directory rename for path, and if there are any file
1515
+ * level conflicts for the renamed location. If there is a rename and
1516
+ * there are no conflicts, return the new name. Otherwise, return NULL.
1517
+ */
1518
+static char *handle_path_level_conflicts(struct merge_options *o,
1519
+ const char *path,
1520
+ struct dir_rename_entry *entry,
1521
+ struct hashmap *collisions,
1522
+ struct tree *tree)
1523
+{
1524
+ char *new_path = NULL;
1525
+ struct collision_entry *collision_ent;
1526
+ int clean = 1;
1527
+ struct strbuf collision_paths = STRBUF_INIT;
1528
+
1529
+ /*
1530
+ * entry has the mapping of old directory name to new directory name
1531
+ * that we want to apply to path.
1532
+ */
1533
+ new_path = apply_dir_rename(entry, path);
1534
+
1535
+ if (!new_path) {
1536
+ /* This should only happen when entry->non_unique_new_dir set */
1537
+ if (!entry->non_unique_new_dir)
1538
+ BUG("entry->non_unqiue_dir not set and !new_path");
1539
+ output(o, 1, _("CONFLICT (directory rename split): "
1540
+ "Unclear where to place %s because directory "
1541
+ "%s was renamed to multiple other directories, "
1542
+ "with no destination getting a majority of the "
1543
+ "files."),
1544
+ path, entry->dir);
1545
+ clean = 0;
1546
+ return NULL;
1547
+ }
1548
+
1549
+ /*
1550
+ * The caller needs to have ensured that it has pre-populated
1551
+ * collisions with all paths that map to new_path. Do a quick check
1552
+ * to ensure that's the case.
1553
+ */
1554
+ collision_ent = collision_find_entry(collisions, new_path);
1555
+ if (collision_ent == NULL)
1556
+ BUG("collision_ent is NULL");
1557
+
1558
+ /*
1559
+ * Check for one-sided add/add/.../add conflicts, i.e.
1560
+ * where implicit renames from the other side doing
1561
+ * directory rename(s) can affect this side of history
1562
+ * to put multiple paths into the same location. Warn
1563
+ * and bail on directory renames for such paths.
1564
+ */
1565
+ if (collision_ent->reported_already) {
1566
+ clean = 0;
1567
+ } else if (tree_has_path(tree, new_path)) {
1568
+ collision_ent->reported_already = 1;
1569
+ strbuf_add_separated_string_list(&collision_paths, ", ",
1570
+ &collision_ent->source_files);
1571
+ output(o, 1, _("CONFLICT (implicit dir rename): Existing "
1572
+ "file/dir at %s in the way of implicit "
1573
+ "directory rename(s) putting the following "
1574
+ "path(s) there: %s."),
1575
+ new_path, collision_paths.buf);
1576
+ clean = 0;
1577
+ } else if (collision_ent->source_files.nr > 1) {
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): Cannot map "
1582
+ "more than one path to %s; implicit directory "
1583
+ "renames tried to put these paths there: %s"),
1584
+ new_path, collision_paths.buf);
1585
+ clean = 0;
1586
+ }
1587
+
1588
+ /* Free memory we no longer need */
1589
+ strbuf_release(&collision_paths);
1590
+ if (!clean && new_path) {
1591
+ free(new_path);
1592
+ return NULL;
1593
+ }
1594
+
1595
+ return new_path;
1596
+}
1597
+
1598
/*
1599
* There are a couple things we want to do at the directory level:
1600
* 1. Check for both sides renaming to the same thing, in order to avoid
@@ -1789,6 +1874,59 @@ static void compute_collisions(struct hashmap *collisions,
1874
}
1875
}
1876
1877
+static char *check_for_directory_rename(struct merge_options *o,
1878
+ const char *path,
1879
+ struct tree *tree,
1880
+ struct hashmap *dir_renames,
1881
+ struct hashmap *dir_rename_exclusions,
1882
+ struct hashmap *collisions,
1883
+ int *clean_merge)
1884
+{
1885
+ char *new_path = NULL;
1886
+ struct dir_rename_entry *entry = check_dir_renamed(path, dir_renames);
1887
+ struct dir_rename_entry *oentry = NULL;
1888
+
1889
+ if (!entry)
1890
+ return new_path;
1891
+
1892
+ /*
1893
+ * This next part is a little weird. We do not want to do an
1894
+ * implicit rename into a directory we renamed on our side, because
1895
+ * that will result in a spurious rename/rename(1to2) conflict. An
1896
+ * example:
1897
+ * Base commit: dumbdir/afile, otherdir/bfile
1898
+ * Side 1: smrtdir/afile, otherdir/bfile
1899
+ * Side 2: dumbdir/afile, dumbdir/bfile
1900
+ * Here, while working on Side 1, we could notice that otherdir was
1901
+ * renamed/merged to dumbdir, and change the diff_filepair for
1902
+ * otherdir/bfile into a rename into dumbdir/bfile. However, Side
1903
+ * 2 will notice the rename from dumbdir to smrtdir, and do the
1904
+ * transitive rename to move it from dumbdir/bfile to
1905
+ * smrtdir/bfile. That gives us bfile in dumbdir vs being in
1906
+ * smrtdir, a rename/rename(1to2) conflict. We really just want
1907
+ * the file to end up in smrtdir. And the way to achieve that is
1908
+ * to not let Side1 do the rename to dumbdir, since we know that is
1909
+ * the source of one of our directory renames.
1910
+ *
1911
+ * That's why oentry and dir_rename_exclusions is here.
1912
+ *
1913
+ * As it turns out, this also prevents N-way transient rename
1914
+ * confusion; See testcases 9c and 9d of t6043.
1915
+ */
1916
+ oentry = dir_rename_find_entry(dir_rename_exclusions, entry->new_dir.buf);
1917
+ if (oentry) {
1918
+ output(o, 1, _("WARNING: Avoiding applying %s -> %s rename "
1919
+ "to %s, because %s itself was renamed."),
1920
+ entry->dir, entry->new_dir.buf, path, entry->new_dir.buf);
1921
+ } else {
1922
+ new_path = handle_path_level_conflicts(o, path, entry,
1923
+ collisions, tree);
1924
+ *clean_merge &= (new_path != NULL);
1925
+ }
1926
+
1927
+ return new_path;
1928
+}
1929
+
1930
/*
1931
* Get information of all renames which occurred in 'pairs', making use of
1932
* any implicit directory renames inferred from the other side of history.
@@ -1799,11 +1937,13 @@ static void compute_collisions(struct hashmap *collisions,
1937
static struct string_list *get_renames(struct merge_options *o,
1938
struct diff_queue_struct *pairs,
1939
struct hashmap *dir_renames,
1940
+ struct hashmap *dir_rename_exclusions,
1941
struct tree *tree,
1942
struct tree *o_tree,
1943
struct tree *a_tree,
1944
struct tree *b_tree,
1806
- struct string_list *entries)
1945
+ struct string_list *entries,
1946
+ int *clean_merge)
1947
{
1948
int i;
1949
struct hashmap collisions;
@@ -1818,11 +1958,22 @@ static struct string_list *get_renames(struct merge_options *o,
1958
struct string_list_item *item;
1959
struct rename *re;
1960
struct diff_filepair *pair = pairs->queue[i];
1961
+ char *new_path; /* non-NULL only with directory renames */
1962
1822
- if (pair->status != 'R') {
1963
+ if (pair->status == 'D') {
1964
+ diff_free_filepair(pair);
1965
+ continue;
1966
+ }
1967
+ new_path = check_for_directory_rename(o, pair->two->path, tree,
1968
+ dir_renames,
1969
+ dir_rename_exclusions,
1970
+ &collisions,
1971
+ clean_merge);
1972
+ if (pair->status != 'R' && !new_path) {
1973
diff_free_filepair(pair);
1974
continue;
1975
}
1976
+
1977
re = xmalloc(sizeof(*re));
1978
re->processed = 0;
1979
re->pair = pair;
@@ -2140,7 +2291,7 @@ static int handle_renames(struct merge_options *o,
2291
{
2292
struct diff_queue_struct *head_pairs, *merge_pairs;
2293
struct hashmap *dir_re_head, *dir_re_merge;
2143
- int clean;
2294
+ int clean = 1;
2295
2296
ri->head_renames = NULL;
2297
ri->merge_renames = NULL;
@@ -2159,13 +2310,20 @@ static int handle_renames(struct merge_options *o,
2310
dir_re_merge, merge);
2311
2312
ri->head_renames = get_renames(o, head_pairs,
2162
- dir_re_merge, head,
2163
- common, head, merge, entries);
2313
+ dir_re_merge, dir_re_head, head,
2314
+ common, head, merge, entries,
2315
+ &clean);
2316
+ if (clean < 0)
2317
+ goto cleanup;
2318
ri->merge_renames = get_renames(o, merge_pairs,
2165
- dir_re_head, merge,
2166
- common, head, merge, entries);
2167
- clean = process_renames(o, ri->head_renames, ri->merge_renames);
2319
+ dir_re_head, dir_re_merge, merge,
2320
+ common, head, merge, entries,
2321
+ &clean);
2322
+ if (clean < 0)
2323
+ goto cleanup;
2324
+ clean &= process_renames(o, ri->head_renames, ri->merge_renames);
2325
2326
+cleanup:
2327
/*
2328
* Some cleanup is deferred until cleanup_renames() because the
2329
* 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)
@@ -163,6 +164,21 @@ struct strbuf **strbuf_split_buf(const char *str, size_t slen,
164
return ret;
165
}
166
167
+void strbuf_add_separated_string_list(struct strbuf *str,
168
+ const char *sep,
169
+ struct string_list *slist)
170
+{
171
+ struct string_list_item *item;
172
+ int sep_needed = 0;
173
+
174
+ for_each_string_list_item(item, slist) {
175
+ if (sep_needed)
176
+ strbuf_addstr(str, sep);
177
+ strbuf_addstr(str, item->string);
178
+ sep_needed = 1;
179
+ }
180
+}
181
+
182
void strbuf_list_free(struct strbuf **sbs)
183
{
184
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
@@ -528,6 +530,20 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,
530
return strbuf_split_max(sb, terminator, 0);
531
}
532
533
+/*
534
+ * Adds all strings of a string list to the strbuf, separated by the given
535
+ * separator. For example, if sep is
536
+ * ', '
537
+ * and slist contains
538
+ * ['element1', 'element2', ..., 'elementN'],
539
+ * then write:
540
+ * 'element1, element2, ..., elementN'
541
+ * to str. If only one element, just write "element1" to str.
542
+ */
543
+extern void strbuf_add_separated_string_list(struct strbuf *str,
544
+ const char *sep,
545
+ struct string_list *slist);
546
+
547
/**
548
* Free a NULL-terminated list of strbufs (for example, the return
549
* 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