merge-tree: fix d/f conflicts

The previous commit documented two known breakages revolving around a case where one side flips a tree into a blob (or vice versa), where the original code simply gets confused and feeds a mixture of trees and blobs into either the recursive merge-tree (and recursing into the blob will fail) or three-way merge (and merging tree contents together with blobs will fail). Fix it by feeding trees (and only trees) into the recursive merge-tree machinery and blobs (and only blobs) into the three-way content level merge machinery separately; when this happens, the entire merge has to be marked as conflicting at the structure level. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Dec 13, 2012 at 15:51 UTC 35ffe7583108ab236dcf81226690388491d9962f
2 files changed +42 -34
builtin/merge-tree.c
+40 -32
@@ -25,7 +25,7 @@ static void add_merge_entry(struct merge_list *entry)
25 merge_result_end = &entry->next;
26 }
27
28 -static void merge_trees(struct tree_desc t[3], const char *base);
28 +static void merge_trees_recursive(struct tree_desc t[3], const char *base, int df_conflict);
29
30 static const char *explanation(struct merge_list *entry)
31 {
@@ -190,41 +190,35 @@ static void resolve(const struct traverse_info *info, struct name_entry *ours, s
190 add_merge_entry(final);
191 }
192
193 -static int unresolved_directory(const struct traverse_info *info, struct name_entry n[3])
193 +static void unresolved_directory(const struct traverse_info *info, struct name_entry n[3],
194 + int df_conflict)
195 {
196 char *newbase;
197 struct name_entry *p;
198 struct tree_desc t[3];
199 void *buf0, *buf1, *buf2;
200
200 - p = n;
201 - if (!p->mode) {
202 - p++;
203 - if (!p->mode)
204 - p++;
201 + for (p = n; p < n + 3; p++) {
202 + if (p->mode && S_ISDIR(p->mode))
203 + break;
204 }
206 - if (!S_ISDIR(p->mode))
207 - return 0;
208 - /*
209 - * NEEDSWORK: this is broken. The path can originally be a file
210 - * and then one side may have turned it into a directory, in which
211 - * case we return and let the three-way merge as if the tree were
212 - * a regular file. If the path that was originally a tree is
213 - * now a file in either branch, fill_tree_descriptor() below will
214 - * die when fed a blob sha1.
215 - */
205 + if (n + 3 <= p)
206 + return; /* there is no tree here */
207
208 newbase = traverse_path(info, p);
218 - buf0 = fill_tree_descriptor(t+0, n[0].sha1);
219 - buf1 = fill_tree_descriptor(t+1, n[1].sha1);
220 - buf2 = fill_tree_descriptor(t+2, n[2].sha1);
221 - merge_trees(t, newbase);
209 +
210 +#define ENTRY_SHA1(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->sha1 : NULL)
211 + buf0 = fill_tree_descriptor(t+0, ENTRY_SHA1(n + 0));
212 + buf1 = fill_tree_descriptor(t+1, ENTRY_SHA1(n + 1));
213 + buf2 = fill_tree_descriptor(t+2, ENTRY_SHA1(n + 2));
214 +#undef ENTRY_SHA1
215 +
216 + merge_trees_recursive(t, newbase, df_conflict);
217
218 free(buf0);
219 free(buf1);
220 free(buf2);
221 free(newbase);
227 - return 1;
222 }
223
224
@@ -247,18 +241,26 @@ static struct merge_list *link_entry(unsigned stage, const struct traverse_info
241 static void unresolved(const struct traverse_info *info, struct name_entry n[3])
242 {
243 struct merge_list *entry = NULL;
244 + int i;
245 + unsigned dirmask = 0, mask = 0;
246 +
247 + for (i = 0; i < 3; i++) {
248 + mask |= (1 << 1);
249 + if (n[i].mode && S_ISDIR(n[i].mode))
250 + dirmask |= (1 << i);
251 + }
252 +
253 + unresolved_directory(info, n, dirmask && (dirmask != mask));
254
251 - if (unresolved_directory(info, n))
255 + if (dirmask == mask)
256 return;
257
254 - /*
255 - * Do them in reverse order so that the resulting link
256 - * list has the stages in order - link_entry adds new
257 - * links at the front.
258 - */
259 - entry = link_entry(3, info, n + 2, entry);
260 - entry = link_entry(2, info, n + 1, entry);
261 - entry = link_entry(1, info, n + 0, entry);
258 + if (n[2].mode && !S_ISDIR(n[2].mode))
259 + entry = link_entry(3, info, n + 2, entry);
260 + if (n[1].mode && !S_ISDIR(n[1].mode))
261 + entry = link_entry(2, info, n + 1, entry);
262 + if (n[0].mode && !S_ISDIR(n[0].mode))
263 + entry = link_entry(1, info, n + 0, entry);
264
265 add_merge_entry(entry);
266 }
@@ -329,15 +331,21 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s
331 return mask;
332 }
333
332 -static void merge_trees(struct tree_desc t[3], const char *base)
334 +static void merge_trees_recursive(struct tree_desc t[3], const char *base, int df_conflict)
335 {
336 struct traverse_info info;
337
338 setup_traverse_info(&info, base);
339 + info.data = &df_conflict;
340 info.fn = threeway_callback;
341 traverse_trees(3, t, &info);
342 }
343
344 +static void merge_trees(struct tree_desc t[3], const char *base)
345 +{
346 + merge_trees_recursive(t, base, 0);
347 +}
348 +
349 static void *get_tree_descriptor(struct tree_desc *desc, const char *rev)
350 {
351 unsigned char sha1[20];
t/t4300-merge-tree.sh
+2 -2
@@ -254,7 +254,7 @@ EXPECTED
254 test_cmp expected actual
255 '
256
257 -test_expect_failure 'turn file to tree' '
257 +test_expect_success 'turn file to tree' '
258 git reset --hard initial &&
259 rm initial-file &&
260 mkdir initial-file &&
@@ -274,7 +274,7 @@ test_expect_failure 'turn file to tree' '
274 test_cmp expect actual
275 '
276
277 -test_expect_failure 'turn tree to file' '
277 +test_expect_success 'turn tree to file' '
278 git reset --hard initial &&
279 mkdir dir &&
280 test_commit "add-tree" "dir/path" "AAA" &&