Merge pull request #929 from arrbee/diff-iter-fixes

Fix problems in diff iterator record chaining

src/diff_output.c

@@ -1204,13 +1204,17 @@ static int diffiter_hunk_cb(
 	if (info->last_hunk)
 		info->last_hunk->next = hunk;
 	info->last_hunk = hunk;
+	info->last_line = NULL;
 
 	memcpy(&hunk->range, range, sizeof(hunk->range));
 
 	iter->hunk_count++;
 
-	if (iter->hunk_head == NULL)
-		iter->hunk_curr = iter->hunk_head = hunk;
+	/* adding first hunk to list */
+	if (iter->hunk_head == NULL) {
+		iter->hunk_head = hunk;
+		iter->hunk_curr = NULL;
+	}
 
 	return 0;
 }
@@ -1281,8 +1285,9 @@ static void diffiter_do_unload_file(git_diff_iterator *iter)
 	}
 
 	iter->ctxt.delta = NULL;
-	iter->hunk_head = NULL;
+	iter->hunk_curr = iter->hunk_head = NULL;
 	iter->hunk_count = 0;
+	iter->line_curr = NULL;
 }
 
 int git_diff_iterator_new(
@@ -1345,9 +1350,14 @@ int git_diff_iterator_num_hunks_in_file(git_diff_iterator *iter)
 int git_diff_iterator_num_lines_in_hunk(git_diff_iterator *iter)
 {
 	int error = diffiter_do_diff_file(iter);
-	if (!error && iter->hunk_curr)
-		error = iter->hunk_curr->line_count;
-	return error;
+	if (error)
+		return error;
+
+	if (iter->hunk_curr)
+		return iter->hunk_curr->line_count;
+	if (iter->hunk_head)
+		return iter->hunk_head->line_count;
+	return 0;
 }
 
 int git_diff_iterator_next_file(
@@ -1386,7 +1396,7 @@ int git_diff_iterator_next_file(
 	}
 
 	if (iter->ctxt.delta == NULL) {
-		iter->hunk_curr = NULL;
+		iter->hunk_curr = iter->hunk_head = NULL;
 		iter->line_curr = NULL;
 	}
 
@@ -1409,11 +1419,13 @@ int git_diff_iterator_next_hunk(
 		return error;
 
 	if (iter->hunk_curr == NULL) {
-		if (range_ptr) *range_ptr = NULL;
-		if (header) *header = NULL;
-		if (header_len) *header_len = 0;
-		iter->line_curr = NULL;
-		return GIT_ITEROVER;
+		if (iter->hunk_head == NULL)
+			goto no_more_hunks;
+		iter->hunk_curr = iter->hunk_head;
+	} else {
+		if (iter->hunk_curr->next == NULL)
+			goto no_more_hunks;
+		iter->hunk_curr = iter->hunk_curr->next;
 	}
 
 	range = &iter->hunk_curr->range;
@@ -1436,9 +1448,16 @@ int git_diff_iterator_next_hunk(
 	}
 
 	iter->line_curr = iter->hunk_curr->line_head;
-	iter->hunk_curr = iter->hunk_curr->next;
 
 	return error;
+
+no_more_hunks:
+	if (range_ptr) *range_ptr = NULL;
+	if (header) *header = NULL;
+	if (header_len) *header_len = 0;
+	iter->line_curr = NULL;
+
+	return GIT_ITEROVER;
 }
 
 int git_diff_iterator_next_line(
@@ -1453,7 +1472,7 @@ int git_diff_iterator_next_line(
 		return error;
 
 	/* if the user has not called next_hunk yet, call it implicitly (OK?) */
-	if (iter->hunk_curr == iter->hunk_head) {
+	if (iter->hunk_curr == NULL) {
 		error = git_diff_iterator_next_hunk(NULL, NULL, NULL, iter);
 		if (error)
 			return error;

tests-clar/diff/tree.c

@@ -256,3 +256,85 @@ void test_diff_tree__merge(void)
 
 	git_diff_list_free(diff1);
 }
+
+void test_diff_tree__larger_hunks(void)
+{
+	const char *a_commit = "d70d245ed97ed2aa596dd1af6536e4bfdb047b69";
+	const char *b_commit = "7a9e0b02e63179929fed24f0a3e0f19168114d10";
+	git_tree *a, *b;
+	git_diff_options opts = {0};
+	git_diff_list *diff = NULL;
+	git_diff_iterator *iter = NULL;
+	git_diff_delta *delta;
+	diff_expects exp;
+	int error, num_files = 0;
+
+	g_repo = cl_git_sandbox_init("diff");
+
+	cl_assert((a = resolve_commit_oid_to_tree(g_repo, a_commit)) != NULL);
+	cl_assert((b = resolve_commit_oid_to_tree(g_repo, b_commit)) != NULL);
+
+	opts.context_lines = 1;
+	opts.interhunk_lines = 0;
+
+	memset(&exp, 0, sizeof(exp));
+
+	cl_git_pass(git_diff_tree_to_tree(g_repo, &opts, a, b, &diff));
+	cl_git_pass(git_diff_iterator_new(&iter, diff));
+
+	/* this should be exact */
+	cl_assert(git_diff_iterator_progress(iter) == 0.0f);
+
+	/* You wouldn't actually structure an iterator loop this way, but
+	 * I have here for testing purposes of the return value
+	 */
+	while (!(error = git_diff_iterator_next_file(&delta, iter))) {
+		git_diff_range *range;
+		const char *header;
+		size_t header_len;
+		int actual_hunks = 0, num_hunks;
+		float expected_progress;
+
+		num_files++;
+
+		expected_progress = (float)num_files / 2.0f;
+		cl_assert(expected_progress == git_diff_iterator_progress(iter));
+
+		num_hunks = git_diff_iterator_num_hunks_in_file(iter);
+
+		while (!(error = git_diff_iterator_next_hunk(
+					&range, &header, &header_len, iter)))
+		{
+			int actual_lines = 0;
+			int num_lines = git_diff_iterator_num_lines_in_hunk(iter);
+			char origin;
+			const char *line;
+			size_t line_len;
+
+			while (!(error = git_diff_iterator_next_line(
+						&origin, &line, &line_len, iter)))
+			{
+				actual_lines++;
+			}
+
+			cl_assert_equal_i(GIT_ITEROVER, error);
+			cl_assert_equal_i(actual_lines, num_lines);
+
+			actual_hunks++;
+		}
+
+		cl_assert_equal_i(GIT_ITEROVER, error);
+		cl_assert_equal_i(actual_hunks, num_hunks);
+	}
+
+	cl_assert_equal_i(GIT_ITEROVER, error);
+	cl_assert_equal_i(2, num_files);
+	cl_assert(git_diff_iterator_progress(iter) == 1.0f);
+
+	git_diff_iterator_free(iter);
+	git_diff_list_free(diff);
+	diff = NULL;
+
+	git_tree_free(a);
+	git_tree_free(b);
+}

tests-clar/diff/workdir.c

@@ -670,3 +670,94 @@ void test_diff_workdir__eof_newline_changes(void)
  *
  * Expect 13 files, 0 ADD, 4 DEL, 4 MOD, 1 IGN, 4 UNTR
  */
+
+
+void test_diff_workdir__larger_hunks(void)
+{
+	const char *a_commit = "d70d245ed97ed2aa596dd1af6536e4bfdb047b69";
+	const char *b_commit = "7a9e0b02e63179929fed24f0a3e0f19168114d10";
+	git_tree *a, *b;
+	git_diff_options opts = {0};
+	int i, error;
+
+	g_repo = cl_git_sandbox_init("diff");
+
+	cl_assert((a = resolve_commit_oid_to_tree(g_repo, a_commit)) != NULL);
+	cl_assert((b = resolve_commit_oid_to_tree(g_repo, b_commit)) != NULL);
+
+	opts.context_lines = 1;
+	opts.interhunk_lines = 0;
+
+	for (i = 0; i <= 2; ++i) {
+		git_diff_list *diff = NULL;
+		git_diff_iterator *iter = NULL;
+		git_diff_delta *delta;
+		int num_files = 0;
+
+		/* okay, this is a bit silly, but oh well */
+		switch (i) {
+		case 0:
+			cl_git_pass(git_diff_workdir_to_index(g_repo, &opts, &diff));
+			break;
+		case 1:
+			cl_git_pass(git_diff_workdir_to_tree(g_repo, &opts, a, &diff));
+			break;
+		case 2:
+			cl_git_pass(git_diff_workdir_to_tree(g_repo, &opts, b, &diff));
+			break;
+		}
+
+		cl_git_pass(git_diff_iterator_new(&iter, diff));
+
+		cl_assert(git_diff_iterator_progress(iter) == 0.0f);
+
+		while (!(error = git_diff_iterator_next_file(&delta, iter))) {
+			git_diff_range *range;
+			const char *header;
+			size_t header_len;
+			int actual_hunks = 0, num_hunks;
+			float expected_progress;
+
+			num_files++;
+
+			expected_progress = (float)num_files / 2.0f;
+			cl_assert(expected_progress == git_diff_iterator_progress(iter));
+
+			num_hunks = git_diff_iterator_num_hunks_in_file(iter);
+
+			while (!(error = git_diff_iterator_next_hunk(
+						 &range, &header, &header_len, iter)))
+			{
+				int actual_lines = 0;
+				int num_lines = git_diff_iterator_num_lines_in_hunk(iter);
+				char origin;
+				const char *line;
+				size_t line_len;
+
+				while (!(error = git_diff_iterator_next_line(
+							 &origin, &line, &line_len, iter)))
+				{
+					actual_lines++;
+				}
+
+				cl_assert_equal_i(GIT_ITEROVER, error);
+				cl_assert_equal_i(actual_lines, num_lines);
+
+				actual_hunks++;
+			}
+
+			cl_assert_equal_i(GIT_ITEROVER, error);
+			cl_assert_equal_i(actual_hunks, num_hunks);
+		}
+
+		cl_assert_equal_i(GIT_ITEROVER, error);
+		cl_assert_equal_i(2, num_files);
+		cl_assert(git_diff_iterator_progress(iter) == 1.0f);
+
+		git_diff_iterator_free(iter);
+		git_diff_list_free(diff);
+	}
+
+	git_tree_free(a);
+	git_tree_free(b);
+}

tests-clar/resources/diff/.gitted/HEAD

@@ -0,0 +1 @@
+ref: refs/heads/master

tests-clar/resources/diff/.gitted/config

@@ -0,0 +1,6 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = false
+	logallrefupdates = true
+	ignorecase = false

tests-clar/resources/diff/.gitted/description

@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.

tests-clar/resources/diff/.gitted/info/exclude

@@ -0,0 +1,6 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~

tests-clar/resources/diff/.gitted/logs/HEAD

@@ -0,0 +1,2 @@
+0000000000000000000000000000000000000000 d70d245ed97ed2aa596dd1af6536e4bfdb047b69 Russell Belfer <rb@github.com> 1347559804 -0700	commit (initial): initial commit
+d70d245ed97ed2aa596dd1af6536e4bfdb047b69 7a9e0b02e63179929fed24f0a3e0f19168114d10 Russell Belfer <rb@github.com> 1347560491 -0700	commit: some changes

tests-clar/resources/diff/.gitted/logs/refs/heads/master

@@ -0,0 +1,2 @@
+0000000000000000000000000000000000000000 d70d245ed97ed2aa596dd1af6536e4bfdb047b69 Russell Belfer <rb@github.com> 1347559804 -0700	commit (initial): initial commit
+d70d245ed97ed2aa596dd1af6536e4bfdb047b69 7a9e0b02e63179929fed24f0a3e0f19168114d10 Russell Belfer <rb@github.com> 1347560491 -0700	commit: some changes

tests-clar/resources/diff/.gitted/objects/88/789109439c1e1c3cd45224001edee5304ed53c

@@ -0,0 +1 @@
+x
+)JMU07g040031QHÌË/ÉH-Ò+©(aÉ)Ž[¼Åwz{Œïj­“û%;¡ÊŠRSrSÁª4Wïö½Ç4ã<34><C3A3>Žø¼NîÚ+©Ë
¶a

tests-clar/resources/diff/.gitted/objects/d7/0d245ed97ed2aa596dd1af6536e4bfdb047b69

@@ -0,0 +1 @@
+x
•<>Û	!óm·<>_ׄRB:XÝkVpWpµÿ© ¿‡™9±î{î ,^z#‚œôšŒ7JygÔš¬áA¦„« i1Y©Ù2úV¼ÇyR)𢒨Á½…ç'÷m„[¬û„ÒÑ;®áÊ-çl®ó¯Oô_“å#÷¼ø%Øœv8¤

tests-clar/resources/diff/.gitted/refs/heads/master

@@ -0,0 +1 @@
+7a9e0b02e63179929fed24f0a3e0f19168114d10

tests-clar/resources/diff/another.txt

@@ -0,0 +1,38 @@
+Git is fast. With Git, nearly all operations are performed locally, giving
+it an huge speed advantage on centralized systems that constantly have to
+communicate with a server somewh3r3.
+
+For testing, large AWS instances were set up in the same availability
+zone. Git and SVN were installed on both machines, the Ruby repository was
+copied to both Git and SVN servers, and common operations were performed on
+both.
+
+In some cases the commands don't match up exactly. Here, matching on the
+lowest common denominator was attempted. For example, the 'commit' tests
+also include the time to push for Git, though most of the time you would not
+actually be pushing to the server immediately after a commit where the two
+commands cannot be separated in SVN.
+
+Note that this is the best case scenario for SVN - a server with no load
+with an 80MB/s bandwidth connection to the client machine. Nearly all of
+these times would be even worse for SVN if that connection was slower, while
+many of the Git times would not be affected.
+
+Clearly, in many of these common version control operations, Git is one or
+two orders of magnitude faster than SVN, even under ideal conditions for
+SVN.
+
+Let's see how common operations stack up against Subversion, a common
+centralized version control system that is similar to CVS or
+Perforce. Smaller is faster.
+
+One place where Git is slower is in the initial clone operation. Here, Git
+One place where Git is slower is in the initial clone operation. Here, Git
+One place where Git is slower is in the initial clone operation. Here, Git
+seen in the above charts, it's not considerably slower for an operation that
+is only performed once.
+
+It's also interesting to note that the size of the data on the client side
+is very similar even though Git also has every version of every file for the
+entire history of the project. This illustrates how efficient it is at
+compressing and storing data on the client side.

tests-clar/resources/diff/readme.txt

@@ -0,0 +1,36 @@
+The Git feature that r3ally mak3s it stand apart from n3arly 3v3ry other SCM
+out there is its branching model.
+
+Git allows and encourages you to have multiple local branches that can be
+entirely independent of each other. The creation, merging, and deletion of
+those lines of development takes seconds.
+
+Git allows and encourages you to have multiple local branches that can be
+entirely independent of each other. The creation, merging, and deletion of
+those lines of development takes seconds.
+
+This means that you can do things like:
+
+Role-Bas3d Codelin3s. Have a branch that always contains only what goes to
+production, another that you merge work into for testing, and several
+smaller ones for day to day work.
+
+Feature Based Workflow. Create new branches for each new feature you're
+working on so you can seamlessly switch back and forth between them, then
+delete each branch when that feature gets merged into your main line.
+
+Disposable Experimentation. Create a branch to experiment in, realize it's
+not going to work, and just delete it - abandoning the work—with nobody else
+ever seeing it (even if you've pushed other branches in the meantime).
+
+Notably, when you push to a remote repository, you do not have to push all
+share it with others.
+
+Git allows and encourages you to have multiple local branches that can be
+entirely independent of each other. The creation, merging, and deletion of
+those lines of development takes seconds.
+
+There are ways to accomplish some of this with other systems, but the work
+involved is much more difficult and error-prone. Git makes this process
+incredibly easy and it changes the way most developers work when they learn
+it.!