Clean up iterator APIs

This removes the need to explicitly pass the repo into iterators
where the repo is implied by the other parameters.  This moves
the repo to be owned by the parent struct.  Also, this has some
iterator related updates to the internal diff API to lay the
groundwork for checkout improvements.

include/git2/tree.h

@@ -81,6 +81,14 @@ GIT_INLINE(void) git_tree_free(git_tree *tree)
 GIT_EXTERN(const git_oid *) git_tree_id(const git_tree *tree);
 
 /**
+ * Get the repository that contains the tree.
+ *
+ * @param tree A previously loaded tree.
+ * @return Repository that contains this tree.
+ */
+GIT_EXTERN(git_repository *) git_tree_owner(const git_tree *tree);
+
+/**
  * Get the number of entries listed in a tree
  *
  * @param tree a previously loaded tree.

src/checkout.c

@@ -448,8 +448,7 @@ static int checkout_get_actions(
 		!(error == GIT_ENOTFOUND || error == GIT_EORPHANEDHEAD))
 			return -1;
 
-	if ((error = git_iterator_for_tree_range(
-			 &hiter, data->repo, head, pfx, pfx)) < 0)
+	if ((error = git_iterator_for_tree_range(&hiter, head, pfx, pfx)) < 0)
 		goto fail;
 
 	if ((diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0 &&

src/diff.c

@@ -570,7 +570,7 @@ static int diff_list_init_from_iterators(
 	return 0;
 }
 
-static int diff_from_iterators(
+int git_diff__from_iterators(
 	git_diff_list **diff_ptr,
 	git_repository *repo,
 	git_iterator *old_iter,
@@ -610,9 +610,10 @@ static int diff_from_iterators(
 
 	/* run iterators building diffs */
 	while (oitem || nitem) {
+		int cmp = oitem ? (nitem ? diff->entrycomp(oitem, nitem) : -1) : 1;
 
 		/* create DELETED records for old items not matched in new */
-		if (oitem && (!nitem || diff->entrycomp(oitem, nitem) < 0)) {
+		if (cmp < 0) {
 			if (diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem) < 0)
 				goto fail;
 
@@ -637,7 +638,7 @@ static int diff_from_iterators(
 		/* create ADDED, TRACKED, or IGNORED records for new items not
 		 * matched in old (and/or descend into directories as needed)
 		 */
-		else if (nitem && (!oitem || diff->entrycomp(oitem, nitem) > 0)) {
+		else if (cmp > 0) {
 			git_delta_t delta_type = GIT_DELTA_UNTRACKED;
 
 			/* check if contained in ignored parent directory */
@@ -733,7 +734,7 @@ static int diff_from_iterators(
 		 * (or ADDED and DELETED pair if type changed)
 		 */
 		else {
-			assert(oitem && nitem && diff->entrycomp(oitem, nitem) == 0);
+			assert(oitem && nitem && cmp == 0);
 
 			if (maybe_modified(old_iter, oitem, new_iter, nitem, diff) < 0 ||
 				git_iterator_advance(old_iter, &oitem) < 0 ||
@@ -759,8 +760,8 @@ fail:
 	git_iterator *a = NULL, *b = NULL; \
 	char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; \
 	GITERR_CHECK_VERSION(opts, GIT_DIFF_OPTIONS_VERSION, "git_diff_options"); \
-	if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \
-		error = diff_from_iterators(diff, repo, a, b, opts); \
+    if (!(error = MAKE_FIRST) && !(error = MAKE_SECOND)) \
+		error = git_diff__from_iterators(diff, repo, a, b, opts); \
 	git__free(pfx); git_iterator_free(a); git_iterator_free(b); \
 } while (0)
 
@@ -776,8 +777,8 @@ int git_diff_tree_to_tree(
 	assert(diff && repo);
 
 	DIFF_FROM_ITERATORS(
-		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
-		git_iterator_for_tree_range(&b, repo, new_tree, pfx, pfx)
+		git_iterator_for_tree_range(&a, old_tree, pfx, pfx),
+		git_iterator_for_tree_range(&b, new_tree, pfx, pfx)
 	);
 
 	return error;
@@ -798,7 +799,7 @@ int git_diff_index_to_tree(
 		return error;
 
 	DIFF_FROM_ITERATORS(
-		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
+		git_iterator_for_tree_range(&a, old_tree, pfx, pfx),
 	    git_iterator_for_index_range(&b, index, pfx, pfx)
 	);
 
@@ -838,7 +839,7 @@ int git_diff_workdir_to_tree(
 	assert(diff && repo);
 
 	DIFF_FROM_ITERATORS(
-		git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx),
+		git_iterator_for_tree_range(&a, old_tree, pfx, pfx),
 	    git_iterator_for_workdir_range(&b, repo, pfx, pfx)
 	);
 

src/diff.h

@@ -61,6 +61,12 @@ extern bool git_diff_delta__should_skip(
 extern int git_diff__oid_for_file(
 	git_repository *, const char *, uint16_t, git_off_t, git_oid *);
 
+extern int git_diff__from_iterators(
+	git_diff_list **diff_ptr,
+	git_repository *repo,
+	git_iterator *old_iter,
+	git_iterator *new_iter,
+	const git_diff_options *opts);
 
 #endif
 

src/index.c

@@ -1641,8 +1641,7 @@ int git_index_read_tree_match(
 
 	pfx = git_pathspec_prefix(strspec);
 
-	if ((error = git_iterator_for_tree_range(
-			 &iter, INDEX_OWNER(index), tree, pfx, pfx)) < 0 ||
+	if ((error = git_iterator_for_tree_range(&iter, tree, pfx, pfx)) < 0 ||
 		(error = git_iterator_current(iter, &entry)) < 0)
 		goto cleanup;
 

src/iterator.c

@@ -18,7 +18,7 @@
 	(P)->base.type    = GIT_ITERATOR_ ## NAME_UC; \
 	(P)->base.start   = start ? git__strdup(start) : NULL; \
 	(P)->base.end     = end ? git__strdup(end) : NULL; \
-	(P)->base.ignore_case = 0; \
+	(P)->base.ignore_case = false; \
 	(P)->base.current = NAME_LC ## _iterator__current; \
 	(P)->base.at_end  = NAME_LC ## _iterator__at_end; \
 	(P)->base.advance = NAME_LC ## _iterator__advance; \
@@ -91,7 +91,6 @@ struct tree_iterator_frame {
 
 typedef struct {
 	git_iterator base;
-	git_repository *repo;
 	tree_iterator_frame *stack, *tail;
 	git_index_entry entry;
 	git_buf path;
@@ -205,7 +204,7 @@ static int tree_iterator__expand_tree(tree_iterator *ti)
 			git__prefixcmp(ti->path.ptr, ti->base.end) > 0)
 			return tree_iterator__to_end(ti);
 
-		if ((error = git_tree_lookup(&subtree, ti->repo, &te->oid)) < 0)
+		if ((error = git_tree_lookup(&subtree, ti->base.repo, &te->oid)) < 0)
 			return error;
 
 		relpath = NULL;
@@ -302,7 +301,6 @@ static int tree_iterator__reset(git_iterator *self)
 
 int git_iterator_for_tree_range(
 	git_iterator **iter,
-	git_repository *repo,
 	git_tree *tree,
 	const char *start,
 	const char *end)
@@ -315,7 +313,7 @@ int git_iterator_for_tree_range(
 
 	ITERATOR_BASE_INIT(ti, tree, TREE);
 
-	ti->repo  = repo;
+	ti->base.repo = git_tree_owner(tree);
 	ti->stack = ti->tail = tree_iterator__alloc_frame(tree, ti->base.start);
 
 	if ((error = tree_iterator__expand_tree(ti)) < 0)
@@ -424,6 +422,7 @@ int git_iterator_for_index_range(
 	ITERATOR_BASE_INIT(ii, index, INDEX);
 
 	ii->index = index;
+	ii->base.repo = git_index_owner(index);
 	ii->base.ignore_case = ii->index->ignore_case;
 
 	index_iterator__reset((git_iterator *)ii);
@@ -461,7 +460,6 @@ struct workdir_iterator_frame {
 
 typedef struct {
 	git_iterator base;
-	git_repository *repo;
 	size_t root_len;
 	workdir_iterator_frame *stack;
 	git_ignores ignores;
@@ -716,7 +714,7 @@ static int workdir_iterator__update_entry(workdir_iterator *wi)
 
 	/* detect submodules */
 	if (S_ISDIR(wi->entry.mode)) {
-		int res = git_submodule_lookup(NULL, wi->repo, wi->entry.path);
+		int res = git_submodule_lookup(NULL, wi->base.repo, wi->entry.path);
 		bool is_submodule = (res == 0);
 		if (res == GIT_ENOTFOUND)
 			giterr_clear();
@@ -750,7 +748,7 @@ int git_iterator_for_workdir_range(
 		return error;
 
 	ITERATOR_BASE_INIT(wi, workdir, WORKDIR);
-	wi->repo = repo;
+	wi->base.repo = repo;
 
 	if ((error = git_repository_index__weakptr(&index, repo)) < 0) {
 		git__free(wi);

src/iterator.h

@@ -28,32 +28,33 @@ typedef enum {
 
 struct git_iterator {
 	git_iterator_type_t type;
+	git_repository *repo;
 	char *start;
 	char *end;
+	bool ignore_case;
+
 	int (*current)(git_iterator *, const git_index_entry **);
 	int (*at_end)(git_iterator *);
 	int (*advance)(git_iterator *, const git_index_entry **);
 	int (*seek)(git_iterator *, const char *prefix);
 	int (*reset)(git_iterator *);
 	void (*free)(git_iterator *);
-	unsigned int ignore_case:1;
 };
 
 extern int git_iterator_for_nothing(git_iterator **iter);
 
 extern int git_iterator_for_tree_range(
-	git_iterator **iter, git_repository *repo, git_tree *tree,
+	git_iterator **iter, git_tree *tree,
 	const char *start, const char *end);
 
 GIT_INLINE(int) git_iterator_for_tree(
-	git_iterator **iter, git_repository *repo, git_tree *tree)
+	git_iterator **iter, git_tree *tree)
 {
-	return git_iterator_for_tree_range(iter, repo, tree, NULL, NULL);
+	return git_iterator_for_tree_range(iter, tree, NULL, NULL);
 }
 
 extern int git_iterator_for_index_range(
-	git_iterator **iter, git_index *index,
-	const char *start, const char *end);
+	git_iterator **iter, git_index *index, const char *start, const char *end);
 
 GIT_INLINE(int) git_iterator_for_index(
 	git_iterator **iter, git_index *index)
@@ -90,7 +91,8 @@ GIT_INLINE(int) git_iterator_spoolandsort(
 	git_iterator **iter, git_iterator *towrap,
 	git_vector_cmp comparer, bool ignore_case)
 {
-	return git_iterator_spoolandsort_range(iter, towrap, comparer, ignore_case, NULL, NULL);
+	return git_iterator_spoolandsort_range(
+		iter, towrap, comparer, ignore_case, NULL, NULL);
 }
 
 /* Entry is not guaranteed to be fully populated.  For a tree iterator,
@@ -149,6 +151,11 @@ GIT_INLINE(git_iterator_type_t) git_iterator_type(git_iterator *iter)
 	return iter->type;
 }
 
+GIT_INLINE(git_repository *) git_iterator_owner(git_iterator *iter)
+{
+	return iter->repo;
+}
+
 extern int git_iterator_current_tree_entry(
 	git_iterator *iter, const git_tree_entry **tree_entry);
 

src/notes.c

@@ -593,7 +593,7 @@ int git_note_foreach(
 
 	if (!(error = retrieve_note_tree_and_commit(
 			&tree, &commit, repo, &notes_ref)) &&
-		!(error = git_iterator_for_tree(&iter, repo, tree)))
+		!(error = git_iterator_for_tree(&iter, tree)))
 		error = git_iterator_current(iter, &item);
 
 	while (!error && item) {

src/submodule.c

@@ -1156,7 +1156,7 @@ static int load_submodule_config_from_head(
 	if ((error = git_repository_head_tree(&head, repo)) < 0)
 		return error;
 
-	if ((error = git_iterator_for_tree(&i, repo, head)) < 0) {
+	if ((error = git_iterator_for_tree(&i, head)) < 0) {
 		git_tree_free(head);
 		return error;
 	}

src/tree.c

@@ -207,9 +207,14 @@ void git_tree__free(git_tree *tree)
 	git__free(tree);
 }
 
-const git_oid *git_tree_id(const git_tree *c)
+const git_oid *git_tree_id(const git_tree *t)
 {
-	return git_object_id((const git_object *)c);
+	return git_object_id((const git_object *)t);
+}
+
+git_repository *git_tree_owner(const git_tree *t)
+{
+	return git_object_owner((const git_object *)t);
 }
 
 git_filemode_t git_tree_entry_filemode(const git_tree_entry *entry)

tests-clar/diff/iterator.c

@@ -35,7 +35,7 @@ static void tree_iterator_test(
 	git_repository *repo = cl_git_sandbox_init(sandbox);
 
 	cl_assert(t = resolve_commit_oid_to_tree(repo, treeish));
-	cl_git_pass(git_iterator_for_tree_range(&i, repo, t, start, end));
+	cl_git_pass(git_iterator_for_tree_range(&i, t, start, end));
 	cl_git_pass(git_iterator_current(i, &entry));
 
 	while (entry != NULL) {
@@ -294,7 +294,7 @@ void test_diff_iterator__tree_special_functions(void)
 		repo, "24fa9a9fc4e202313e24b648087495441dab432b");
 	cl_assert(t != NULL);
 
-	cl_git_pass(git_iterator_for_tree_range(&i, repo, t, NULL, NULL));
+	cl_git_pass(git_iterator_for_tree_range(&i, t, NULL, NULL));
 	cl_git_pass(git_iterator_current(i, &entry));
 
 	while (entry != NULL) {