merge-tree: add comments to clarify what these functions are doing
Rename the "branch1" parameter given to resolve() to "ours", to clarify what is going on. Also, annotate the unresolved_directory() function with some comments to show what decisions are made in each step, and highlight two bugs that need to be fixed. Add two tests to t4300 to illustrate these bugs. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Dec 6, 2012 at 16:15 UTC
8dd15c6a909c059c6fd4f4dd914dbf932617ea57
2 files changed
+66
-4
builtin/merge-tree.c
+22
-4
@@ -172,17 +172,17 @@ static char *traverse_path(const struct traverse_info *info, const struct name_e
172
return make_traverse_path(path, info, n);
173
}
174
175
-static void resolve(const struct traverse_info *info, struct name_entry *branch1, struct name_entry *result)
175
+static void resolve(const struct traverse_info *info, struct name_entry *ours, struct name_entry *result)
176
{
177
struct merge_list *orig, *final;
178
const char *path;
179
180
- /* If it's already branch1, don't bother showing it */
181
- if (!branch1)
180
+ /* If it's already ours, don't bother showing it */
181
+ if (!ours)
182
return;
183
184
path = traverse_path(info, result);
185
- orig = create_entry(2, branch1->mode, branch1->sha1, path);
185
+ orig = create_entry(2, ours->mode, ours->sha1, path);
186
final = create_entry(0, result->mode, result->sha1, path);
187
188
final->link = orig;
@@ -205,6 +205,15 @@ static int unresolved_directory(const struct traverse_info *info, struct name_en
205
}
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
+ */
216
+
217
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);
@@ -288,20 +297,29 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s
297
/* Same in both? */
298
if (same_entry(entry+1, entry+2)) {
299
if (entry[0].sha1) {
300
+ /* Modified identically */
301
resolve(info, NULL, entry+1);
302
return mask;
303
}
304
+ /* "Both added the same" is left unresolved */
305
}
306
307
if (same_entry(entry+0, entry+1)) {
308
if (entry[2].sha1 && !S_ISDIR(entry[2].mode)) {
309
+ /* We did not touch, they modified -- take theirs */
310
resolve(info, entry+1, entry+2);
311
return mask;
312
}
313
+ /*
314
+ * If we did not touch a directory but they made it
315
+ * into a file, we fall through and unresolved()
316
+ * recurses down. Likewise for the opposite case.
317
+ */
318
}
319
320
if (same_entry(entry+0, entry+2)) {
321
if (entry[1].sha1 && !S_ISDIR(entry[1].mode)) {
322
+ /* We modified, they did not touch -- take ours */
323
resolve(info, NULL, entry+1);
324
return mask;
325
}
t/t4300-merge-tree.sh
+44
@@ -254,4 +254,48 @@ EXPECTED
254
test_cmp expected actual
255
'
256
257
+test_expect_failure 'turn file to tree' '
258
+ git reset --hard initial &&
259
+ rm initial-file &&
260
+ mkdir initial-file &&
261
+ test_commit "turn-file-to-tree" "initial-file/ONE" "CCC" &&
262
+ git merge-tree initial initial turn-file-to-tree >actual &&
263
+ cat >expect <<-\EOF &&
264
+ added in remote
265
+ their 100644 43aa4fdec31eb92e1fdc2f0ce6ea9ddb7c32bcf7 initial-file/ONE
266
+ @@ -0,0 +1 @@
267
+ +CCC
268
+ removed in remote
269
+ base 100644 e79c5e8f964493290a409888d5413a737e8e5dd5 initial-file
270
+ our 100644 e79c5e8f964493290a409888d5413a737e8e5dd5 initial-file
271
+ @@ -1 +0,0 @@
272
+ -initial
273
+ EOF
274
+ test_cmp expect actual
275
+'
276
+
277
+test_expect_failure 'turn tree to file' '
278
+ git reset --hard initial &&
279
+ mkdir dir &&
280
+ test_commit "add-tree" "dir/path" "AAA" &&
281
+ test_commit "add-another-tree" "dir/another" "BBB" &&
282
+ rm -fr dir &&
283
+ test_commit "make-file" "dir" "CCC" &&
284
+ git merge-tree add-tree add-another-tree make-file >actual &&
285
+ cat >expect <<-\EOF &&
286
+ added in local
287
+ our 100644 ba629238ca89489f2b350e196ca445e09d8bb834 dir/another
288
+ removed in remote
289
+ base 100644 43d5a8ed6ef6c00ff775008633f95787d088285d dir/path
290
+ our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d dir/path
291
+ @@ -1 +0,0 @@
292
+ -AAA
293
+ added in remote
294
+ their 100644 43aa4fdec31eb92e1fdc2f0ce6ea9ddb7c32bcf7 dir
295
+ @@ -0,0 +1 @@
296
+ +CCC
297
+ EOF
298
+ test_cmp expect actual
299
+'
300
+
301
test_done