Initial implementation of new diff patch API

Replacing the `git_iterator` object, this creates a simple API
for accessing the "patch" for any file pair in a diff list and
then gives indexed access to the hunks in the patch and the lines
in the hunk.  This is the initial implementation of this revised
API - it is still broken, but at least builds cleanly.

include/git2/diff.h

@@ -98,6 +98,9 @@ enum {
 	GIT_DIFF_FILE_FREE_PATH  = (1 << 1),
 	GIT_DIFF_FILE_BINARY     = (1 << 2),
 	GIT_DIFF_FILE_NOT_BINARY = (1 << 3),
+	GIT_DIFF_FILE_FREE_DATA  = (1 << 4),
+	GIT_DIFF_FILE_UNMAP_DATA = (1 << 5),
+	GIT_DIFF_FILE_NO_DATA    = (1 << 6),
 };
 
 /**
@@ -425,7 +428,7 @@ GIT_EXTERN(int) git_diff_print_patch(
  * @param diff A git_diff_list generated by one of the above functions
  * @return Count of number of deltas in the list
  */
-GIT_EXTERN(size_t) git_diff_entrycount(git_diff_list *diff);
+GIT_EXTERN(size_t) git_diff_num_deltas(git_diff_list *diff);
 
 /**
  * Query how many diff deltas are there in a diff list filtered by type.
@@ -438,7 +441,7 @@ GIT_EXTERN(size_t) git_diff_entrycount(git_diff_list *diff);
  * @param type A git_delta_t value to filter the count
  * @return Count of number of deltas matching delta_t type
  */
-GIT_EXTERN(size_t) git_diff_entrycount_of_type(
+GIT_EXTERN(size_t) git_diff_num_deltas_of_type(
 	git_diff_list *diff,
 	git_delta_t type);
 
@@ -469,7 +472,7 @@ GIT_EXTERN(size_t) git_diff_entrycount_of_type(
  */
 GIT_EXTERN(int) git_diff_get_patch(
 	git_diff_patch **patch,
-	const git_diff_delta **delta,
+	git_diff_delta **delta,
 	git_diff_list *diff,
 	size_t idx);
 
@@ -482,43 +485,76 @@ GIT_EXTERN(void) git_diff_patch_free(
 /**
  * Get the delta associated with a patch
  */
-GIT_EXTERN(void) git_diff_patch_get_delta(
+GIT_EXTERN(const git_diff_delta *) git_diff_patch_delta(
-	const git_diff_delta **delta,
 	git_diff_patch *patch);
 
 /**
  * Get the number of hunks in a patch
  */
-GIT_EXTERN(size_t) git_diff_patch_hunks(
+GIT_EXTERN(size_t) git_diff_patch_num_hunks(
 	git_diff_patch *patch);
 
 /**
  * Get the information about a hunk in a patch
+ *
+ * Given a patch and a hunk index into the patch, this returns detailed
+ * information about that hunk.  Any of the output pointers can be passed
+ * as NULL if you don't care about that particular piece of information.
+ *
+ * @param range Output pointer to git_diff_range of hunk
+ * @param header Output pointer to header string for hunk.  Unlike the
+ *               content pointer for each line, this will be NUL-terminated
+ * @param header_len Output value of characters in header string
+ * @param lines_in_hunk Output count of total lines in this hunk
+ * @param patch Input pointer to patch object
+ * @param hunk_idx Input index of hunk to get information about
+ * @return 0 on success, GIT_ENOTFOUND if hunk_idx out of range, <0 on error
  */
 GIT_EXTERN(int) git_diff_patch_get_hunk(
-	const git_diff_range **range,
+	git_diff_range **range,
 	const char **header,
 	size_t *header_len,
 	size_t *lines_in_hunk,
 	git_diff_patch *patch,
-	size_t hunk);
+	size_t hunk_idx);
 
 /**
- * Get the number of lines in a hunk
+ * Get the number of lines in a hunk.
+ *
+ * @param patch The git_diff_patch object
+ * @param hunk_idx Index of the hunk
+ * @return Number of lines in hunk or -1 if invalid hunk index
  */
-GIT_EXTERN(size_t) git_diff_patch_lines_in_hunk(
+GIT_EXTERN(int) git_diff_patch_num_lines_in_hunk(
 	git_diff_patch *patch,
-	size_t hunk);
+	size_t hunk_idx);
 
 /**
- * Get a line in a hunk of a patch
+ * Get data about a line in a hunk of a patch.
+ *
+ * Given a patch, a hunk index, and a line index in the hunk, this
+ * will return a lot of details about that line.  If you pass a hunk
+ * index larger than the number of hunks or a line index larger than
+ * the number of lines in the hunk, this will return -1.
+ *
+ * @param line_origin A GIT_DIFF_LINE constant from above
+ * @param content Pointer to content of diff line, not NUL-terminated
+ * @param content_len Number of characters in content
+ * @param old_lineno Line number in old file or -1 if line is added
+ * @param new_lineno Line number in new file or -1 if line is deleted
+ * @param patch The patch to look in
+ * @param hunk_idx The index of the hunk
+ * @param line_of_index The index of the line in the hunk
+ * @return 0 on success, <0 on failure
  */
 GIT_EXTERN(int) git_diff_patch_get_line_in_hunk(
 	char *line_origin,
 	const char **content,
 	size_t *content_len,
+	int *old_lineno,
+	int *new_lineno,
 	git_diff_patch *patch,
-	size_t hunk,
+	size_t hunk_idx,
 	size_t line_of_hunk);
 
 /**@}*/

src/diff.c

@@ -5,8 +5,6 @@
  * a Linking Exception. For full terms see the included COPYING file.
  */
 #include "common.h"
-#include "git2/diff.h"
-#include "git2/oid.h"
 #include "diff.h"
 #include "fileops.h"
 #include "config.h"
@@ -268,9 +266,17 @@ static int diff_delta__from_two(
 	delta->old_file.mode = old_mode;
 	delta->old_file.flags |= GIT_DIFF_FILE_VALID_OID;
 
-	git_oid_cpy(&delta->new_file.oid, new_oid ? new_oid : &new_entry->oid);
+	git_oid_cpy(&delta->new_file.oid, &new_entry->oid);
 	delta->new_file.size = new_entry->file_size;
 	delta->new_file.mode = new_mode;
+
+	if (new_oid) {
+		if ((diff->opts.flags & GIT_DIFF_REVERSE) != 0)
+			git_oid_cpy(&delta->old_file.oid, new_oid);
+		else
+			git_oid_cpy(&delta->new_file.oid, new_oid);
+	}
+
 	if (new_oid || !git_oid_iszero(&new_entry->oid))
 		delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID;
 
@@ -425,6 +431,11 @@ void git_diff_list_free(git_diff_list *diff)
 	GIT_REFCOUNT_DEC(diff, diff_list_free);
 }
 
+void git_diff_list_addref(git_diff_list *diff)
+{
+	GIT_REFCOUNT_INC(diff);
+}
+
 static int oid_for_workdir_item(
 	git_repository *repo,
 	const git_index_entry *item,
@@ -519,17 +530,17 @@ static int maybe_modified(
 			 omode == nmode)
 		status = GIT_DELTA_UNMODIFIED;
 
-	/* if modes match and we have an unknown OID and a workdir iterator,
+	/* if we have an unknown OID and a workdir iterator, then check some
-	 * then check deeper for matching
+	 * circumstances that can accelerate things or need special handling
 	 */
-	else if (omode == nmode &&
+	else if (git_oid_iszero(&nitem->oid) &&
-		git_oid_iszero(&nitem->oid) &&
 			 new_iter->type == GIT_ITERATOR_WORKDIR)
 	{
 		/* TODO: add check against index file st_mtime to avoid racy-git */
 
-		/* if they files look exactly alike, then we'll assume the same */
+		/* if the stat data looks exactly alike, then assume the same */
-		if (oitem->file_size == nitem->file_size &&
+		if (omode == nmode &&
+			oitem->file_size == nitem->file_size &&
 			(!(diff->diffcaps & GIT_DIFFCAPS_TRUST_CTIME) ||
 			 (oitem->ctime.seconds == nitem->ctime.seconds)) &&
 			oitem->mtime.seconds == nitem->mtime.seconds &&
@@ -554,16 +565,15 @@ static int maybe_modified(
 				status = GIT_DELTA_UNMODIFIED;
 			}
 		}
+	}
 
-		/* TODO: check git attributes so we will not have to read the file
+	/* if we got here and decided that the files are modified, but we
-		 * in if it is marked binary.
+	 * haven't calculated the OID of the new item, then calculate it now
 	 */
-
+	if (status == GIT_DELTA_MODIFIED && git_oid_iszero(&nitem->oid)) {
-		else if (oid_for_workdir_item(diff->repo, nitem, &noid) < 0)
+		if (oid_for_workdir_item(diff->repo, nitem, &noid) < 0)
 			return -1;
-
+		else if (omode == nmode && git_oid_equal(&oitem->oid, &noid))
-		else if (git_oid_cmp(&oitem->oid, &noid) == 0 &&
-				 omode == nmode)
 			status = GIT_DELTA_UNMODIFIED;
 
 		/* store calculated oid so we don't have to recalc later */

src/diff.h

@@ -7,6 +7,9 @@
 #ifndef INCLUDE_diff_h__
 #define INCLUDE_diff_h__
 
+#include "git2/diff.h"
+#include "git2/oid.h"
+
 #include <stdio.h>
 #include "vector.h"
 #include "buffer.h"
@@ -25,14 +28,17 @@ enum {
 	GIT_DIFFCAPS_USE_DEV          = (1 << 4), /* use st_dev? */
 };
 
-#define MAX_DIFF_FILESIZE 0x20000000
+typedef struct {
+	git_refcount   rc;
+	git_diff_delta delta;
+} git_diff_delta_refcounted;
 
 struct git_diff_list {
 	git_refcount     rc;
 	git_repository   *repo;
 	git_diff_options opts;
 	git_vector       pathspec;
-	git_vector       deltas;    /* vector of git_diff_file_delta */
+	git_vector       deltas;    /* vector of git_diff_delta_refcounted */
 	git_pool pool;
 	git_iterator_type_t old_src;
 	git_iterator_type_t new_src;
@@ -42,27 +48,7 @@ struct git_diff_list {
 extern void git_diff__cleanup_modes(
 	uint32_t diffcaps, uint32_t *omode, uint32_t *nmode);
 
-/**
+extern void git_diff_list_addref(git_diff_list *diff);
- * Return the maximum possible number of files in the diff.
- *
- * NOTE: This number has to be treated as an upper bound on the number of
- * files that have changed if the diff is with the working directory.
- *
- * Why?! For efficiency, we defer loading the file contents as long as
- * possible, so if a file has been "touched" in the working directory and
- * then reverted to the original content, it may get stored in the diff list
- * as MODIFIED along with a flag that the status should be reconfirmed when
- * it is actually loaded into memory.  When that load happens, it could get
- * flipped to UNMODIFIED. If unmodified files are being skipped, then the
- * iterator will skip that file and this number may be too high.
- *
- * This behavior is true of `git_diff_foreach` as well, but the only
- * implication there is that the `progress` value would not advance evenly.
- *
- * @param iterator The iterator object
- * @return The maximum number of files to be iterated over
- */
-int git_diff_iterator__max_files(git_diff_iterator *iterator);
 
 #endif
 

src/diff_output.h

@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2009-2012 the libgit2 contributors
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+#ifndef INCLUDE_diff_output_h__
+#define INCLUDE_diff_output_h__
+
+#include "git2/blob.h"
+#include "diff.h"
+#include "map.h"
+#include "xdiff/xdiff.h"
+
+#define MAX_DIFF_FILESIZE 0x20000000
+
+enum {
+	GIT_DIFF_PATCH_ALLOCATED  = (1 << 0),
+	GIT_DIFF_PATCH_PREPPED    = (1 << 1),
+	GIT_DIFF_PATCH_LOADED     = (1 << 2),
+	GIT_DIFF_PATCH_DIFFABLE   = (1 << 3),
+	GIT_DIFF_PATCH_DIFFED     = (1 << 4),
+};
+
+/* context for performing diffs */
+typedef struct {
+	git_repository   *repo;
+	git_diff_list    *diff;
+	git_diff_options *opts;
+	git_diff_file_fn  file_cb;
+	git_diff_hunk_fn  hunk_cb;
+	git_diff_data_fn  data_cb;
+	void *cb_data;
+	int   cb_error;
+	git_diff_range cb_range;
+	xdemitconf_t xdiff_config;
+	xpparam_t    xdiff_params;
+} diff_context;
+
+/* cached information about a single span in a diff */
+typedef struct diff_patch_line diff_patch_line;
+struct diff_patch_line {
+	const char *ptr;
+	size_t len;
+	int lines, oldno, newno;
+	char origin;
+};
+
+/* cached information about a hunk in a diff */
+typedef struct diff_patch_hunk diff_patch_hunk;
+struct diff_patch_hunk {
+	git_diff_range range;
+	char   header[128];
+	size_t header_len;
+	size_t line_start;
+	size_t line_count;
+};
+
+struct git_diff_patch {
+	git_refcount rc;
+	git_diff_list *diff; /* for refcount purposes, maybe NULL for blob diffs */
+	git_diff_delta *delta;
+	diff_context *ctxt; /* only valid while generating patch */
+	git_iterator_type_t old_src;
+	git_iterator_type_t new_src;
+	git_blob *old_blob;
+	git_blob *new_blob;
+	git_map  old_data;
+	git_map  new_data;
+	uint32_t flags;
+	diff_patch_hunk *hunks;
+	size_t hunks_asize, hunks_size;
+	diff_patch_line *lines;
+	size_t lines_asize, lines_size;
+};
+
+/* context for performing diff on a single delta */
+typedef struct {
+	git_diff_patch *patch;
+	uint32_t prepped  : 1;
+	uint32_t loaded   : 1;
+	uint32_t diffable : 1;
+	uint32_t diffed   : 1;
+} diff_delta_context;
+
+#endif

src/submodule.c

@@ -1457,7 +1457,7 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm)
 		error = git_diff_index_to_tree(sm_repo, &opt, sm_head, &diff);
 
 		if (!error) {
-			if (git_diff_entrycount(diff, -1) > 0)
+			if (git_diff_num_deltas(diff) > 0)
 				*status |= GIT_SUBMODULE_STATUS_WD_INDEX_MODIFIED;
 
 			git_diff_list_free(diff);
@@ -1474,12 +1474,13 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm)
 		error = git_diff_workdir_to_index(sm_repo, &opt, &diff);
 
 		if (!error) {
-			int untracked = git_diff_entrycount(diff, GIT_DELTA_UNTRACKED);
+			int untracked =
+				git_diff_num_deltas_of_type(diff, GIT_DELTA_UNTRACKED);
 
 			if (untracked > 0)
 				*status |= GIT_SUBMODULE_STATUS_WD_UNTRACKED;
 
-			if (git_diff_entrycount(diff, -1) - untracked > 0)
+			if ((git_diff_num_deltas(diff) - untracked) > 0)
 				*status |= GIT_SUBMODULE_STATUS_WD_WD_MODIFIED;
 
 			git_diff_list_free(diff);

tests-clar/diff/diff_helpers.c

@@ -112,64 +112,65 @@ int diff_foreach_via_iterator(
 	git_diff_hunk_fn hunk_cb,
 	git_diff_data_fn line_cb)
 {
-	int error;
+	size_t d, num_d = git_diff_num_deltas(diff);
-	git_diff_iterator *iter;
+
+	for (d = 0; d < num_d; ++d) {
+		git_diff_patch *patch;
 		git_diff_delta *delta;
+		size_t h, num_h;
 
-	if ((error = git_diff_iterator_new(&iter, diff)) < 0)
+		cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d));
-		return error;
+		cl_assert(delta && patch);
-
-	while (!(error = git_diff_iterator_next_file(&delta, iter))) {
-		git_diff_range *range;
-		const char *hdr;
-		size_t hdr_len;
-		float progress = git_diff_iterator_progress(iter);
 
 		/* call file_cb for this file */
-		if (file_cb != NULL && file_cb(data, delta, progress) != 0)
+		if (file_cb != NULL && file_cb(data, delta, (float)d / num_d) != 0) {
+			git_diff_patch_free(patch);
 			goto abort;
+		}
 
-		if (!hunk_cb && !line_cb)
+		if (!hunk_cb && !line_cb) {
+			git_diff_patch_free(patch);
 			continue;
+		}
 
-		while (!(error = git_diff_iterator_next_hunk(
+		num_h = git_diff_patch_num_hunks(patch);
-				&range, &hdr, &hdr_len, iter))) {
+
+		for (h = 0; h < num_h; h++) {
+			git_diff_range *range;
+			const char *hdr;
+			size_t hdr_len, l, num_l;
+
+			cl_git_pass(git_diff_patch_get_hunk(
+				&range, &hdr, &hdr_len, &num_l, patch, h));
+
+			if (hunk_cb && hunk_cb(data, delta, range, hdr, hdr_len) != 0) {
+				git_diff_patch_free(patch);
+				goto abort;
+			}
+
+			for (l = 0; l < num_l; ++l) {
 				char origin;
 				const char *line;
 				size_t line_len;
+				int old_lineno, new_lineno;
 
-			if (hunk_cb && hunk_cb(data, delta, range, hdr, hdr_len) != 0)
+				cl_git_pass(git_diff_patch_get_line_in_hunk(
-				goto abort;
+					&origin, &line, &line_len, &old_lineno, &new_lineno,
+					patch, h, l));
 
-			if (!line_cb)
+				if (line_cb(data, delta, range, origin, line, line_len) != 0) {
-				continue;
+					git_diff_patch_free(patch);
-
-			while (!(error = git_diff_iterator_next_line(
-				&origin, &line, &line_len, iter))) {
-
-				if (line_cb(data, delta, range, origin, line, line_len) != 0)
 					goto abort;
 				}
-
+			}
-			if (error && error != GIT_ITEROVER)
-				goto done;
 		}
 
-		if (error && error != GIT_ITEROVER)
+		git_diff_patch_free(patch);
-			goto done;
 	}
 
-done:
+	return 0;
-	git_diff_iterator_free(iter);
-
-	if (error == GIT_ITEROVER)
-		error = 0;
-
-	return error;
 
 abort:
-	git_diff_iterator_free(iter);
 	giterr_clear();
-
 	return GIT_EUSER;
 }

tests-clar/diff/diffiter.c

@@ -14,11 +14,16 @@ void test_diff_diffiter__create(void)
 {
 	git_repository *repo = cl_git_sandbox_init("attr");
 	git_diff_list *diff;
-	git_diff_iterator *iter;
+	size_t d, num_d;
 
 	cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff));
-	cl_git_pass(git_diff_iterator_new(&iter, diff));
+
-	git_diff_iterator_free(iter);
+	num_d = git_diff_num_deltas(diff);
+	for (d = 0; d < num_d; ++d) {
+		git_diff_delta *delta;
+		cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d));
+	}
+
 	git_diff_list_free(diff);
 }
 
@@ -26,24 +31,22 @@ void test_diff_diffiter__iterate_files(void)
 {
 	git_repository *repo = cl_git_sandbox_init("attr");
 	git_diff_list *diff;
-	git_diff_iterator *iter;
+	size_t d, num_d;
-	git_diff_delta *delta;
+	int count = 0;
-	int error, count = 0;
 
 	cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff));
-	cl_git_pass(git_diff_iterator_new(&iter, diff));
 
-	while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) {
+	num_d = git_diff_num_deltas(diff);
-		cl_assert_equal_i(0, error);
+	cl_assert_equal_i(6, num_d);
+
+	for (d = 0; d < num_d; ++d) {
+		git_diff_delta *delta;
+		cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d));
 		cl_assert(delta != NULL);
 		count++;
 	}
-
-	cl_assert_equal_i(GIT_ITEROVER, error);
-	cl_assert(delta == NULL);
 	cl_assert_equal_i(6, count);
 
-	git_diff_iterator_free(iter);
 	git_diff_list_free(diff);
 }
 
@@ -51,24 +54,22 @@ void test_diff_diffiter__iterate_files_2(void)
 {
 	git_repository *repo = cl_git_sandbox_init("status");
 	git_diff_list *diff;
-	git_diff_iterator *iter;
+	size_t d, num_d;
-	git_diff_delta *delta;
+	int count = 0;
-	int error, count = 0;
 
 	cl_git_pass(git_diff_workdir_to_index(repo, NULL, &diff));
-	cl_git_pass(git_diff_iterator_new(&iter, diff));
 
-	while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) {
+	num_d = git_diff_num_deltas(diff);
-		cl_assert_equal_i(0, error);
+	cl_assert_equal_i(8, num_d);
+
+	for (d = 0; d < num_d; ++d) {
+		git_diff_delta *delta;
+		cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d));
 		cl_assert(delta != NULL);
 		count++;
 	}
-
-	cl_assert_equal_i(GIT_ITEROVER, error);
-	cl_assert(delta == NULL);
 	cl_assert_equal_i(8, count);
 
-	git_diff_iterator_free(iter);
 	git_diff_list_free(diff);
 }
 
@@ -77,12 +78,8 @@ void test_diff_diffiter__iterate_files_and_hunks(void)
 	git_repository *repo = cl_git_sandbox_init("status");
 	git_diff_options opts = {0};
 	git_diff_list *diff = NULL;
-	git_diff_iterator *iter;
+	size_t d, num_d;
-	git_diff_delta *delta;
+	int file_count = 0, hunk_count = 0;
-	git_diff_range *range;
-	const char *header;
-	size_t header_len;
-	int error, file_count = 0, hunk_count = 0;
 
 	opts.context_lines = 3;
 	opts.interhunk_lines = 1;
@@ -90,28 +87,42 @@ void test_diff_diffiter__iterate_files_and_hunks(void)
 
 	cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff));
 
-	cl_git_pass(git_diff_iterator_new(&iter, diff));
+	num_d = git_diff_num_deltas(diff);
+
+	for (d = 0; d < num_d; ++d) {
+		git_diff_patch *patch;
+		git_diff_delta *delta;
+		size_t h, num_h;
+
+		cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d));
 
-	while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) {
-		cl_assert_equal_i(0, error);
 		cl_assert(delta);
+		cl_assert(patch);
 
 		file_count++;
 
-		while ((error = git_diff_iterator_next_hunk(
+		num_h = git_diff_patch_num_hunks(patch);
-				&range, &header, &header_len, iter)) != GIT_ITEROVER) {
+
-			cl_assert_equal_i(0, error);
+		for (h = 0; h < num_h; h++) {
+			git_diff_range *range;
+			const char *header;
+			size_t header_len, num_l;
+
+			cl_git_pass(git_diff_patch_get_hunk(
+				&range, &header, &header_len, &num_l, patch, h));
+
 			cl_assert(range);
+			cl_assert(header);
+
 			hunk_count++;
 		}
+
+		git_diff_patch_free(patch);
 	}
 
-	cl_assert_equal_i(GIT_ITEROVER, error);
-	cl_assert(delta == NULL);
 	cl_assert_equal_i(13, file_count);
 	cl_assert_equal_i(8, hunk_count);
 
-	git_diff_iterator_free(iter);
 	git_diff_list_free(diff);
 }
 
@@ -120,45 +131,42 @@ void test_diff_diffiter__max_size_threshold(void)
 	git_repository *repo = cl_git_sandbox_init("status");
 	git_diff_options opts = {0};
 	git_diff_list *diff = NULL;
-	git_diff_iterator *iter;
+	int file_count = 0, binary_count = 0, hunk_count = 0;
-	git_diff_delta *delta;
+	size_t d, num_d;
-	int error, file_count = 0, binary_count = 0, hunk_count = 0;
 
 	opts.context_lines = 3;
 	opts.interhunk_lines = 1;
 	opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED;
 
 	cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff));
-	cl_git_pass(git_diff_iterator_new(&iter, diff));
+	num_d = git_diff_num_deltas(diff);
 
-	while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) {
+	for (d = 0; d < num_d; ++d) {
-		cl_assert_equal_i(0, error);
+		git_diff_patch *patch;
+		git_diff_delta *delta;
+
+		cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d));
 		cl_assert(delta);
+		cl_assert(patch);
 
 		file_count++;
-
+		hunk_count += git_diff_patch_num_hunks(patch);
-		hunk_count += git_diff_iterator_num_hunks_in_file(iter);
 
 		assert(delta->binary == 0 || delta->binary == 1);
-
 		binary_count += delta->binary;
-	}
 
-	cl_assert_equal_i(GIT_ITEROVER, error);
+		git_diff_patch_free(patch);
-	cl_assert(delta == NULL);
+	}
 
 	cl_assert_equal_i(13, file_count);
 	cl_assert_equal_i(0, binary_count);
 	cl_assert_equal_i(8, hunk_count);
 
-	git_diff_iterator_free(iter);
 	git_diff_list_free(diff);
 
 	/* try again with low file size threshold */
 
-	file_count = 0;
+	file_count = binary_count = hunk_count = 0;
-	binary_count = 0;
-	hunk_count = 0;
 
 	opts.context_lines = 3;
 	opts.interhunk_lines = 1;
@@ -166,36 +174,169 @@ void test_diff_diffiter__max_size_threshold(void)
 	opts.max_size = 50; /* treat anything over 50 bytes as binary! */
 
 	cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff));
-	cl_git_pass(git_diff_iterator_new(&iter, diff));
+	num_d = git_diff_num_deltas(diff);
 
-	while ((error = git_diff_iterator_next_file(&delta, iter)) != GIT_ITEROVER) {
+	for (d = 0; d < num_d; ++d) {
-		cl_assert_equal_i(0, error);
+		git_diff_patch *patch;
-		cl_assert(delta);
+		git_diff_delta *delta;
+
+		cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d));
 
 		file_count++;
-
+		hunk_count += git_diff_patch_num_hunks(patch);
-		hunk_count += git_diff_iterator_num_hunks_in_file(iter);
 
 		assert(delta->binary == 0 || delta->binary == 1);
-
 		binary_count += delta->binary;
+
+		git_diff_patch_free(patch);
 	}
 
-	cl_assert_equal_i(GIT_ITEROVER, error);
-	cl_assert(delta == NULL);
-
 	cl_assert_equal_i(13, file_count);
-
 	/* Three files are over the 50 byte threshold:
 	 * - staged_changes_file_deleted
 	 * - staged_changes_modified_file
 	 * - staged_new_file_modified_file
 	 */
 	cl_assert_equal_i(3, binary_count);
-
 	cl_assert_equal_i(5, hunk_count);
 
-	git_diff_iterator_free(iter);
+	git_diff_list_free(diff);
+}
+
+
+void test_diff_diffiter__iterate_all(void)
+{
+	git_repository *repo = cl_git_sandbox_init("status");
+	git_diff_options opts = {0};
+	git_diff_list *diff = NULL;
+	diff_expects exp = {0};
+	size_t d, num_d;
+
+	opts.context_lines = 3;
+	opts.interhunk_lines = 1;
+	opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED;
+
+	cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff));
+
+	num_d = git_diff_num_deltas(diff);
+	for (d = 0; d < num_d; ++d) {
+		git_diff_patch *patch;
+		git_diff_delta *delta;
+		size_t h, num_h;
+
+		cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d));
+		cl_assert(patch && delta);
+		exp.files++;
+
+		num_h = git_diff_patch_num_hunks(patch);
+		for (h = 0; h < num_h; h++) {
+			git_diff_range *range;
+			const char *header;
+			size_t header_len, l, num_l;
+
+			cl_git_pass(git_diff_patch_get_hunk(
+				&range, &header, &header_len, &num_l, patch, h));
+			cl_assert(range && header);
+			exp.hunks++;
+
+			for (l = 0; l < num_l; ++l) {
+				char origin;
+				const char *content;
+				size_t content_len;
+
+				cl_git_pass(git_diff_patch_get_line_in_hunk(
+					&origin, &content, &content_len, NULL, NULL, patch, h, l));
+				cl_assert(content);
+				exp.lines++;
+			}
+		}
+
+		git_diff_patch_free(patch);
+	}
+
+	cl_assert_equal_i(13, exp.files);
+	cl_assert_equal_i(8, exp.hunks);
+	cl_assert_equal_i(13, exp.lines);
+
+	git_diff_list_free(diff);
+}
+
+static void iterate_over_patch(git_diff_patch *patch, diff_expects *exp)
+{
+	size_t h, num_h = git_diff_patch_num_hunks(patch);
+
+	exp->files++;
+	exp->hunks += num_h;
+
+	/* let's iterate in reverse, just because we can! */
+	for (h = 1; h <= num_h; ++h)
+		exp->lines += git_diff_patch_num_lines_in_hunk(patch, num_h - h);
+}
+
+#define PATCH_CACHE 5
+
+void test_diff_diffiter__iterate_randomly_while_saving_state(void)
+{
+	git_repository *repo = cl_git_sandbox_init("status");
+	git_diff_options opts = {0};
+	git_diff_list *diff = NULL;
+	diff_expects exp = {0};
+	git_diff_patch *patches[PATCH_CACHE];
+	size_t p, d, num_d;
+
+	memset(patches, 0, sizeof(patches));
+
+	opts.context_lines = 3;
+	opts.interhunk_lines = 1;
+	opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED;
+
+	cl_git_pass(git_diff_workdir_to_index(repo, &opts, &diff));
+
+	num_d = git_diff_num_deltas(diff);
+
+	/* To make sure that references counts work for diff and patch objects,
+	 * this generates patches and randomly caches them.  Only when the patch
+	 * is removed from the cache are hunks and lines counted.  At the end,
+	 * there are still patches in the cache, so free the diff and try to
+	 * process remaining patches after the diff is freed.
+	 */
+
+	srand(121212);
+	p = rand() % PATCH_CACHE;
+
+	for (d = 0; d < num_d; ++d) {
+		/* take old patch */
+		git_diff_patch *patch = patches[p];
+		patches[p] = NULL;
+
+		/* cache new patch */
+		cl_git_pass(git_diff_get_patch(&patches[p], NULL, diff, d));
+		cl_assert(patches[p] != NULL);
+
+		/* process old patch if non-NULL */
+		if (patch != NULL) {
+			iterate_over_patch(patch, &exp);
+			git_diff_patch_free(patch);
+		}
+
+		p = rand() % PATCH_CACHE;
+	}
+
+	/* free diff list now - refcounts should keep things safe */
 	git_diff_list_free(diff);
 
+	/* process remaining unprocessed patches */
+	for (p = 0; p < PATCH_CACHE; p++) {
+		git_diff_patch *patch = patches[p];
+
+		if (patch != NULL) {
+			iterate_over_patch(patch, &exp);
+			git_diff_patch_free(patch);
+		}
+	}
+
+	/* hopefully it all still added up right */
+	cl_assert_equal_i(13, exp.files);
+	cl_assert_equal_i(8, exp.hunks);
+	cl_assert_equal_i(13, exp.lines);
 }

tests-clar/diff/tree.c

@@ -264,10 +264,12 @@ void test_diff_tree__larger_hunks(void)
 	git_tree *a, *b;
 	git_diff_options opts = {0};
 	git_diff_list *diff = NULL;
-	git_diff_iterator *iter = NULL;
+	size_t d, num_d, h, num_h, l, num_l, header_len, line_len;
 	git_diff_delta *delta;
-	diff_expects exp;
+	git_diff_patch *patch;
-	int error, num_files = 0;
+	git_diff_range *range;
+	const char *header, *line;
+	char origin;
 
 	g_repo = cl_git_sandbox_init("diff");
 
@@ -277,61 +279,38 @@ void test_diff_tree__larger_hunks(void)
 	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 */
+	num_d = git_diff_num_deltas(diff);
-	cl_assert(git_diff_iterator_progress(iter) == 0.0f);
+	for (d = 0; d < num_d; ++d) {
+		cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d));
+		cl_assert(patch && delta);
 
-	/* You wouldn't actually structure an iterator loop this way, but
+		num_h = git_diff_patch_num_hunks(patch);
-	 * I have here for testing purposes of the return value
+		for (h = 0; h < num_h; h++) {
-	 */
+			cl_git_pass(git_diff_patch_get_hunk(
-	while (!(error = git_diff_iterator_next_file(&delta, iter))) {
+				&range, &header, &header_len, &num_l, patch, h));
-		git_diff_range *range;
-		const char *header;
-		size_t header_len;
-		int actual_hunks = 0, num_hunks;
-		float expected_progress;
 
-		num_files++;
+			for (l = 0; l < num_l; ++l) {
-
+				cl_git_pass(git_diff_patch_get_line_in_hunk(
-		expected_progress = (float)num_files / 2.0f;
+					&origin, &line, &line_len, NULL, NULL, patch, h, l));
-		cl_assert(expected_progress == git_diff_iterator_progress(iter));
+				cl_assert(line);
-
-		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_git_fail(git_diff_patch_get_line_in_hunk(
-			cl_assert_equal_i(actual_lines, num_lines);
+				&origin, &line, &line_len, NULL, NULL, patch, h, num_l));
-
-			actual_hunks++;
 		}
 
-		cl_assert_equal_i(GIT_ITEROVER, error);
+		cl_git_fail(git_diff_patch_get_hunk(
-		cl_assert_equal_i(actual_hunks, num_hunks);
+			&range, &header, &header_len, &num_l, patch, num_h));
+
+		git_diff_patch_free(patch);
 	}
 
-	cl_assert_equal_i(GIT_ITEROVER, error);
+	cl_git_fail(git_diff_get_patch(&patch, &delta, diff, num_d));
-	cl_assert_equal_i(2, num_files);
+
-	cl_assert(git_diff_iterator_progress(iter) == 1.0f);
+	cl_assert_equal_i(2, num_d);
 
-	git_diff_iterator_free(iter);
 	git_diff_list_free(diff);
 	diff = NULL;
 

tests-clar/diff/workdir.c

@@ -678,7 +678,7 @@ void test_diff_workdir__larger_hunks(void)
 	const char *b_commit = "7a9e0b02e63179929fed24f0a3e0f19168114d10";
 	git_tree *a, *b;
 	git_diff_options opts = {0};
-	int i, error;
+	size_t i, d, num_d, h, num_h, l, num_l, header_len, line_len;
 
 	g_repo = cl_git_sandbox_init("diff");
 
@@ -690,9 +690,10 @@ void test_diff_workdir__larger_hunks(void)
 
 	for (i = 0; i <= 2; ++i) {
 		git_diff_list *diff = NULL;
-		git_diff_iterator *iter = NULL;
+		git_diff_patch *patch;
-		git_diff_delta *delta;
+		git_diff_range *range;
-		int num_files = 0;
+		const char *header, *line;
+		char origin;
 
 		/* okay, this is a bit silly, but oh well */
 		switch (i) {
@@ -707,54 +708,36 @@ void test_diff_workdir__larger_hunks(void)
 			break;
 		}
 
-		cl_git_pass(git_diff_iterator_new(&iter, diff));
+		num_d = git_diff_num_deltas(diff);
+		cl_assert_equal_i(2, (int)num_d);
 
-		cl_assert(git_diff_iterator_progress(iter) == 0.0f);
+		for (d = 0; d < num_d; ++d) {
+			cl_git_pass(git_diff_get_patch(&patch, NULL, diff, d));
+			cl_assert(patch);
 
-		while (!(error = git_diff_iterator_next_file(&delta, iter))) {
+			num_h = git_diff_patch_num_hunks(patch);
-			git_diff_range *range;
+			for (h = 0; h < num_h; h++) {
-			const char *header;
+				cl_git_pass(git_diff_patch_get_hunk(
-			size_t header_len;
+					&range, &header, &header_len, &num_l, patch, h));
-			int actual_hunks = 0, num_hunks;
-			float expected_progress;
 
-			num_files++;
+				for (l = 0; l < num_l; ++l) {
-
+					cl_git_pass(git_diff_patch_get_line_in_hunk(
-			expected_progress = (float)num_files / 2.0f;
+						&origin, &line, &line_len, NULL, NULL, patch, h, l));
-			cl_assert(expected_progress == git_diff_iterator_progress(iter));
+					cl_assert(line);
-
-			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);
+				/* confirm fail after the last item */
-				cl_assert_equal_i(actual_lines, num_lines);
+				cl_git_fail(git_diff_patch_get_line_in_hunk(
-
+					&origin, &line, &line_len, NULL, NULL, patch, h, num_l));
-				actual_hunks++;
 			}
 
-			cl_assert_equal_i(GIT_ITEROVER, error);
+			/* confirm fail after the last item */
-			cl_assert_equal_i(actual_hunks, num_hunks);
+			cl_git_fail(git_diff_patch_get_hunk(
+				&range, &header, &header_len, &num_l, patch, num_h));
+
+			git_diff_patch_free(patch);
 		}
 
-		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);
 	}