Commit 21e0d297af95e49b933c2a8d09994a32011354b1

Vicent Martí 2012-10-09T11:45:50

Merge pull request #967 from arrbee/diff-submodule-tests-and-fixes Diff submodule tests and fixes

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
diff --git a/include/git2/diff.h b/include/git2/diff.h
index 1f7f8ab..121c403 100644
--- a/include/git2/diff.h
+++ b/include/git2/diff.h
@@ -47,7 +47,8 @@ enum {
 	GIT_DIFF_INCLUDE_UNMODIFIED = (1 << 9),
 	GIT_DIFF_RECURSE_UNTRACKED_DIRS = (1 << 10),
 	GIT_DIFF_DISABLE_PATHSPEC_MATCH = (1 << 11),
-	GIT_DIFF_DELTAS_ARE_ICASE = (1 << 12)
+	GIT_DIFF_DELTAS_ARE_ICASE = (1 << 12),
+	GIT_DIFF_INCLUDE_UNTRACKED_CONTENT = (1 << 13),
 };
 
 /**
diff --git a/include/git2/submodule.h b/include/git2/submodule.h
index 28057d2..b00fad2 100644
--- a/include/git2/submodule.h
+++ b/include/git2/submodule.h
@@ -120,8 +120,15 @@ typedef enum {
 } git_submodule_status_t;
 
 #define GIT_SUBMODULE_STATUS_IS_UNMODIFIED(S) \
-	(((S) & ~(GIT_SUBMODULE_STATUS_IN_HEAD | GIT_SUBMODULE_STATUS_IN_INDEX | \
-	GIT_SUBMODULE_STATUS_IN_CONFIG | GIT_SUBMODULE_STATUS_IN_WD)) == 0)
+	(((S) & ~(GIT_SUBMODULE_STATUS_IN_HEAD | \
+	GIT_SUBMODULE_STATUS_IN_INDEX | \
+	GIT_SUBMODULE_STATUS_IN_CONFIG | \
+	GIT_SUBMODULE_STATUS_IN_WD)) == 0)
+
+#define GIT_SUBMODULE_STATUS_IS_WD_DIRTY(S) \
+	(((S) & (GIT_SUBMODULE_STATUS_WD_INDEX_MODIFIED | \
+	GIT_SUBMODULE_STATUS_WD_WD_MODIFIED | \
+	GIT_SUBMODULE_STATUS_WD_UNTRACKED)) != 0)
 
 /**
  * Lookup submodule information by name or path.
diff --git a/src/diff.c b/src/diff.c
index 7a0051a..8ab8af3 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -224,7 +224,10 @@ static int diff_delta__from_one(
 	}
 
 	delta->old_file.flags |= GIT_DIFF_FILE_VALID_OID;
-	delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID;
+
+	if (delta->status == GIT_DELTA_DELETED ||
+		!git_oid_iszero(&delta->new_file.oid))
+		delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID;
 
 	if (git_vector_insert(&diff->deltas, delta) < 0) {
 		git__free(delta);
@@ -441,17 +444,28 @@ static int oid_for_workdir_item(
 	const git_index_entry *item,
 	git_oid *oid)
 {
-	int result;
+	int result = 0;
 	git_buf full_path = GIT_BUF_INIT;
 
-	if (git_buf_joinpath(&full_path, git_repository_workdir(repo), item->path) < 0)
+	if (git_buf_joinpath(
+		&full_path, git_repository_workdir(repo), item->path) < 0)
 		return -1;
 
-	/* calculate OID for file if possible*/
+	/* calculate OID for file if possible */
 	if (S_ISGITLINK(item->mode)) {
-		/* Don't bother to figure out an oid for a submodule. We won't use it anyway. */
-		memset(oid, 0, sizeof(*oid));
-		result = 0;
+		git_submodule *sm;
+		const git_oid *sm_oid;
+
+		if (!git_submodule_lookup(&sm, repo, item->path) &&
+			(sm_oid = git_submodule_wd_oid(sm)) != NULL)
+			git_oid_cpy(oid, sm_oid);
+		else {
+			/* if submodule lookup failed probably just in an intermediate
+			 * state where some init hasn't happened, so ignore the error
+			 */
+			giterr_clear();
+			memset(oid, 0, sizeof(*oid));
+		}
 	} else if (S_ISLNK(item->mode))
 		result = git_odb__hashlink(oid, full_path.ptr);
 	else if (!git__is_sizet(item->file_size)) {
@@ -570,6 +584,15 @@ static int maybe_modified(
 					return -1;
 				status = GIT_SUBMODULE_STATUS_IS_UNMODIFIED(sm_status)
 						 ? GIT_DELTA_UNMODIFIED : GIT_DELTA_MODIFIED;
+
+				/* grab OID while we are here */
+				if (git_oid_iszero(&nitem->oid)) {
+					const git_oid *sm_oid = git_submodule_wd_oid(sub);
+					if (sub != NULL) {
+						git_oid_cpy(&noid, sm_oid);
+						use_noid = &noid;
+					}
+				}
 			}
 		}
 	}
@@ -669,7 +692,8 @@ static int diff_from_iterators(
 
 			/* check if contained in ignored parent directory */
 			if (git_buf_len(&ignore_prefix) &&
-				ITERATOR_PREFIXCMP(*old_iter, nitem->path, git_buf_cstr(&ignore_prefix)) == 0)
+				ITERATOR_PREFIXCMP(*old_iter, nitem->path,
+					git_buf_cstr(&ignore_prefix)) == 0)
 				delta_type = GIT_DELTA_IGNORED;
 
 			if (S_ISDIR(nitem->mode)) {
@@ -677,10 +701,23 @@ static int diff_from_iterators(
 				 * it or if the user requested the contents of untracked
 				 * directories and it is not under an ignored directory.
 				 */
-				if ((oitem && ITERATOR_PREFIXCMP(*old_iter, oitem->path, nitem->path) == 0) ||
+				bool contains_tracked =
+					(oitem &&
+					 !ITERATOR_PREFIXCMP(*old_iter, oitem->path, nitem->path));
+				bool recurse_untracked =
 					(delta_type == GIT_DELTA_UNTRACKED &&
-					 (diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) != 0))
-				{
+					 (diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) != 0);
+
+				/* do not advance into directories that contain a .git file */
+				if (!contains_tracked && recurse_untracked) {
+					git_buf *full = NULL;
+					if (git_iterator_current_workdir_path(new_iter, &full) < 0)
+						goto fail;
+					if (git_path_contains_dir(full, DOT_GIT))
+						recurse_untracked = false;
+				}
+
+				if (contains_tracked || recurse_untracked) {
 					/* if this directory is ignored, remember it as the
 					 * "ignore_prefix" for processing contained items
 					 */
diff --git a/src/diff_output.c b/src/diff_output.c
index f5f6c38..10fbd39 100644
--- a/src/diff_output.c
+++ b/src/diff_output.c
@@ -275,30 +275,34 @@ static int get_workdir_sm_content(
 	int error = 0;
 	git_buf content = GIT_BUF_INIT;
 	git_submodule* sm = NULL;
-	const git_oid* sm_head = NULL;
 	unsigned int sm_status = 0;
 	const char* sm_status_text = "";
 	char oidstr[GIT_OID_HEXSZ+1];
 
-	if ((error = git_submodule_lookup(&sm, ctxt->repo, file->path)) < 0) {
+	if ((error = git_submodule_lookup(&sm, ctxt->repo, file->path)) < 0 ||
+		(error = git_submodule_status(&sm_status, sm)) < 0)
 		return error;
-	}
 
-	if ((sm_head = git_submodule_head_oid(sm)) == NULL) {
-		giterr_set(GITERR_SUBMODULE, "Cannot find head of submodule '%s'", file->path);
-		return -1;
-	}
+	/* update OID if we didn't have it previously */
+	if ((file->flags & GIT_DIFF_FILE_VALID_OID) == 0) {
+		const git_oid* sm_head;
 
-	if ((error = git_submodule_status(&sm_status, sm)) < 0) {
-		return -1;
+		if ((sm_head = git_submodule_wd_oid(sm)) != NULL ||
+			(sm_head = git_submodule_head_oid(sm)) != NULL)
+		{
+			git_oid_cpy(&file->oid, sm_head);
+			file->flags |= GIT_DIFF_FILE_VALID_OID;
+		}
 	}
-	if (!GIT_SUBMODULE_STATUS_IS_UNMODIFIED(sm_status)) {
+
+	git_oid_fmt(oidstr, &file->oid);
+	oidstr[GIT_OID_HEXSZ] = '\0';
+
+	if (GIT_SUBMODULE_STATUS_IS_WD_DIRTY(sm_status))
 		sm_status_text = "-dirty";
-	}
 
-	git_oid_fmt(oidstr, sm_head);
-	oidstr[GIT_OID_HEXSZ] = 0;
-	git_buf_printf(&content, "Subproject commit %s%s\n", oidstr, sm_status_text );
+	git_buf_printf(&content, "Subproject commit %s%s\n",
+				   oidstr, sm_status_text);
 
 	map->data = git_buf_detach(&content);
 	map->len = strlen(map->data);
@@ -318,9 +322,12 @@ static int get_workdir_content(
 	git_buf path = GIT_BUF_INIT;
 	const char *wd = git_repository_workdir(ctxt->repo);
 
-	if (file->mode == GIT_FILEMODE_COMMIT)
+	if (S_ISGITLINK(file->mode))
 		return get_workdir_sm_content(ctxt, file, map);
 
+	if (S_ISDIR(file->mode))
+		return 0;
+
 	if (git_buf_joinpath(&path, wd, file->path) < 0)
 		return -1;
 
@@ -535,6 +542,11 @@ static int diff_patch_load(
 		break;
 	case GIT_DELTA_MODIFIED:
 		break;
+	case GIT_DELTA_UNTRACKED:
+		delta->old_file.flags |= GIT_DIFF_FILE_NO_DATA;
+		if ((ctxt->opts->flags & GIT_DIFF_INCLUDE_UNTRACKED_CONTENT) == 0)
+			delta->new_file.flags |= GIT_DIFF_FILE_NO_DATA;
+		break;
 	default:
 		delta->new_file.flags |= GIT_DIFF_FILE_NO_DATA;
 		delta->old_file.flags |= GIT_DIFF_FILE_NO_DATA;
@@ -1070,6 +1082,9 @@ static int print_patch_file(
 
 	GIT_UNUSED(progress);
 
+	if (S_ISDIR(delta->new_file.mode))
+		return 0;
+
 	if (!oldpfx)
 		oldpfx = DIFF_OLD_PREFIX_DEFAULT;
 
@@ -1134,6 +1149,9 @@ static int print_patch_hunk(
 {
 	diff_print_info *pi = data;
 
+	if (S_ISDIR(d->new_file.mode))
+		return 0;
+
 	git_buf_clear(pi->buf);
 	if (git_buf_printf(pi->buf, "%.*s", (int)header_len, header) < 0)
 		return -1;
@@ -1158,6 +1176,9 @@ static int print_patch_line(
 {
 	diff_print_info *pi = data;
 
+	if (S_ISDIR(delta->new_file.mode))
+		return 0;
+
 	git_buf_clear(pi->buf);
 
 	if (line_origin == GIT_DIFF_LINE_ADDITION ||
diff --git a/src/iterator.c b/src/iterator.c
index e52554d..267687e 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -905,3 +905,15 @@ int git_iterator_cmp(
 	return ITERATOR_PREFIXCMP(*iter, entry->path, path_prefix);
 }
 
+int git_iterator_current_workdir_path(git_iterator *iter, git_buf **path)
+{
+	workdir_iterator *wi = (workdir_iterator *)iter;
+
+	if (iter->type != GIT_ITERATOR_WORKDIR || !wi->entry.path)
+		*path = NULL;
+	else
+		*path = &wi->path;
+
+	return 0;
+}
+
diff --git a/src/iterator.h b/src/iterator.h
index 11cd218..29c8985 100644
--- a/src/iterator.h
+++ b/src/iterator.h
@@ -10,6 +10,7 @@
 #include "common.h"
 #include "git2/index.h"
 #include "vector.h"
+#include "buffer.h"
 
 #define ITERATOR_PREFIXCMP(ITER, STR, PREFIX)	(((ITER).ignore_case) ? \
 	git__prefixcmp_icase((STR), (PREFIX)) : \
@@ -166,4 +167,11 @@ extern int git_iterator_advance_into_directory(
 extern int git_iterator_cmp(
 	git_iterator *iter, const char *path_prefix);
 
+/**
+ * Get the full path of the current item from a workdir iterator.
+ * This will return NULL for a non-workdir iterator.
+ */
+extern int git_iterator_current_workdir_path(
+	git_iterator *iter, git_buf **path);
+
 #endif
diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c
index b4c6876..de0e7e0 100644
--- a/tests-clar/diff/diff_helpers.c
+++ b/tests-clar/diff/diff_helpers.c
@@ -30,10 +30,13 @@ int diff_file_fn(
 
 	GIT_UNUSED(progress);
 
-	if (delta->binary)
+	e->files++;
+
+	if (delta->binary) {
 		e->at_least_one_of_them_is_binary = true;
+		e->files_binary++;
+	}
 
-	e->files++;
 	switch (delta->status) {
 	case GIT_DELTA_ADDED: e->file_adds++; break;
 	case GIT_DELTA_DELETED: e->file_dels++; break;
@@ -180,3 +183,25 @@ abort:
 	giterr_clear();
 	return GIT_EUSER;
 }
+
+static int diff_print_cb(
+	void *cb_data,
+	const git_diff_delta *delta,
+	const git_diff_range *range,
+	char line_origin, /**< GIT_DIFF_LINE_... value from above */
+	const char *content,
+	size_t content_len)
+{
+	GIT_UNUSED(cb_data);
+	GIT_UNUSED(delta);
+	GIT_UNUSED(range);
+	GIT_UNUSED(line_origin);
+	GIT_UNUSED(content_len);
+	fputs(content, (FILE *)cb_data);
+	return 0;
+}
+
+void diff_print(FILE *fp, git_diff_list *diff)
+{
+	cl_git_pass(git_diff_print_patch(diff, fp ? fp : stderr, diff_print_cb));
+}
diff --git a/tests-clar/diff/diff_helpers.h b/tests-clar/diff/diff_helpers.h
index 7984294..6291309 100644
--- a/tests-clar/diff/diff_helpers.h
+++ b/tests-clar/diff/diff_helpers.h
@@ -6,6 +6,8 @@ extern git_tree *resolve_commit_oid_to_tree(
 
 typedef struct {
 	int files;
+	int files_binary;
+
 	int file_adds;
 	int file_dels;
 	int file_mods;
@@ -51,3 +53,5 @@ extern int diff_foreach_via_iterator(
 	git_diff_file_fn file_cb,
 	git_diff_hunk_fn hunk_cb,
 	git_diff_data_fn line_cb);
+
+extern void diff_print(FILE *fp, git_diff_list *diff);
diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c
index d8af3ac..c5a0e62 100644
--- a/tests-clar/diff/tree.c
+++ b/tests-clar/diff/tree.c
@@ -113,16 +113,16 @@ void test_diff_tree__options(void)
 	 */
 	diff_expects test_expects[] = {
 		/* a vs b tests */
-		{ 5, 3, 0, 2, 0, 0, 0, 4, 0, 0, 51, 2, 46, 3 },
-		{ 5, 3, 0, 2, 0, 0, 0, 4, 0, 0, 53, 4, 46, 3 },
-		{ 5, 0, 3, 2, 0, 0, 0, 4, 0, 0, 52, 3, 3, 46 },
-		{ 5, 3, 0, 2, 0, 0, 0, 5, 0, 0, 54, 3, 47, 4 },
+		{ 5, 0, 3, 0, 2, 0, 0, 0, 4, 0, 0, 51, 2, 46, 3 },
+		{ 5, 0, 3, 0, 2, 0, 0, 0, 4, 0, 0, 53, 4, 46, 3 },
+		{ 5, 0, 0, 3, 2, 0, 0, 0, 4, 0, 0, 52, 3, 3, 46 },
+		{ 5, 0, 3, 0, 2, 0, 0, 0, 5, 0, 0, 54, 3, 47, 4 },
 		/* c vs d tests */
-		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 22, 9, 10, 3 },
-		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 19, 12, 7, 0 },
-		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 20, 11, 8, 1 },
-		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 20, 11, 8, 1 },
-		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 18, 11, 0, 7 },
+		{ 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 22, 9, 10, 3 },
+		{ 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 19, 12, 7, 0 },
+		{ 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 20, 11, 8, 1 },
+		{ 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 20, 11, 8, 1 },
+		{ 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 18, 11, 0, 7 },
 		{ 0 },
 	};
 	diff_expects *expected;
diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c
index 9161131..e184c28 100644
--- a/tests-clar/diff/workdir.c
+++ b/tests-clar/diff/workdir.c
@@ -744,3 +744,77 @@ void test_diff_workdir__larger_hunks(void)
 	git_tree_free(a);
 	git_tree_free(b);
 }
+
+/* Set up a test that exercises this code. The easiest test using existing
+ * test data is probably to create a sandbox of submod2 and then run a
+ * git_diff_workdir_to_tree against tree
+ * 873585b94bdeabccea991ea5e3ec1a277895b698. As for what you should actually
+ * test, you can start by just checking that the number of lines of diff
+ * content matches the actual output of git diff. That will at least
+ * demonstrate that the submodule content is being used to generate somewhat
+ * comparable outputs. It is a test that would fail without this code and
+ * will succeed with it.
+ */
+
+#include "../submodule/submodule_helpers.h"
+
+void test_diff_workdir__submodules(void)
+{
+	const char *a_commit = "873585b94bdeabccea991ea5e3ec1a277895b698";
+	git_tree *a;
+	git_diff_options opts = {0};
+	git_diff_list *diff = NULL;
+	diff_expects exp;
+
+	g_repo = cl_git_sandbox_init("submod2");
+
+	cl_fixture_sandbox("submod2_target");
+	p_rename("submod2_target/.gitted", "submod2_target/.git");
+
+	rewrite_gitmodules(git_repository_workdir(g_repo));
+	p_rename("submod2/not_submodule/.gitted", "submod2/not_submodule/.git");
+
+	cl_fixture_cleanup("submod2_target");
+
+	a = resolve_commit_oid_to_tree(g_repo, a_commit);
+
+	opts.flags =
+		GIT_DIFF_INCLUDE_UNTRACKED |
+		GIT_DIFF_RECURSE_UNTRACKED_DIRS |
+		GIT_DIFF_INCLUDE_UNTRACKED_CONTENT;
+
+	cl_git_pass(git_diff_workdir_to_tree(g_repo, &opts, a, &diff));
+
+	/* diff_print(stderr, diff); */
+
+	/* essentially doing: git diff 873585b94bdeabccea991ea5e3ec1a277895b698 */
+
+	memset(&exp, 0, sizeof(exp));
+	cl_git_pass(git_diff_foreach(
+		diff, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn));
+
+	/* the following differs from "git diff 873585" by one "untracked" file
+	 * because the diff list includes the "not_submodule/" directory which
+	 * is not displayed in the text diff.
+	 */
+
+	cl_assert_equal_i(10, exp.files);
+
+	cl_assert_equal_i(0, exp.file_adds);
+	cl_assert_equal_i(0, exp.file_dels);
+	cl_assert_equal_i(1, exp.file_mods);
+	cl_assert_equal_i(0, exp.file_ignored);
+	cl_assert_equal_i(9, exp.file_untracked);
+
+	/* the following numbers match "git diff 873585" exactly */
+
+	cl_assert_equal_i(9, exp.hunks);
+
+	cl_assert_equal_i(33, exp.lines);
+	cl_assert_equal_i(2, exp.line_ctxt);
+	cl_assert_equal_i(30, exp.line_adds);
+	cl_assert_equal_i(1, exp.line_dels);
+
+	git_diff_list_free(diff);
+	git_tree_free(a);
+}