repository: reuse grafts for shallow roots

The shallow roots are in fact another user of the grafting mechanism,
and in essence they do use the same file format for grafted commits.
Thus, instead of hand-coding the parsing logic a second time, we can
just reuse the `git_grafts` structure for shallow commits, as well.

6 files changed, 68 insertions, 85 deletions

diff --git a/include/git2/repository.h b/include/git2/repository.h
index 5a27cf08e..59e938710 100644
--- a/include/git2/repository.h
+++ b/include/git2/repository.h
@@ -878,7 +878,8 @@ GIT_EXTERN(int) git_repository_is_shallow(git_repository *repo);
 /**
  * Determine the shallow roots of the repository
  *
- * This oidarray is owned by the library. Do not free it.
+ * The resulting OID array needs to be free'd by calling
+ * `git_oidarray_free`.
  *
  * @param out An array of shallow oids.
  * @param repo The repository
diff --git a/src/grafts.c b/src/grafts.c
index 8ae39f4ac..23b012ce8 100644
--- a/src/grafts.c
+++ b/src/grafts.c
@@ -7,6 +7,7 @@
 
 #include "grafts.h"
 
+#include "oidarray.h"
 #include "parse.h"
 
 struct git_grafts {
@@ -155,6 +156,26 @@ int git_grafts_get(git_commit_graft **out, git_grafts *grafts, const git_oid *oi
 	return 0;
 }
 
+int git_grafts_get_oids(git_oidarray *out, git_grafts *grafts)
+{
+	git_array_oid_t oids = GIT_ARRAY_INIT;
+	const git_oid *oid;
+	size_t i = 0;
+	int error;
+
+	assert(out && grafts);
+
+	while ((error = git_oidmap_iterate(NULL, grafts->commits, &i, &oid)) == 0) {
+		git_oid *cpy = git_array_alloc(oids);
+		GIT_ERROR_CHECK_ALLOC(cpy);
+		git_oid_cpy(cpy, oid);
+	}
+
+	git_oidarray__from_array(out, &oids);
+
+	return 0;
+}
+
 size_t git_grafts_size(git_grafts *grafts)
 {
 	return git_oidmap_size(grafts->commits);
diff --git a/src/grafts.h b/src/grafts.h
index 723792ded..305b0d61a 100644
--- a/src/grafts.h
+++ b/src/grafts.h
@@ -27,6 +27,7 @@ int git_grafts_parse(git_grafts *grafts, const char *content, size_t contentlen)
 int git_grafts_add(git_grafts *grafts, const git_oid *oid, git_array_oid_t parents);
 int git_grafts_remove(git_grafts *grafts, const git_oid *oid);
 int git_grafts_get(git_commit_graft **out, git_grafts *grafts, const git_oid *oid);
+int git_grafts_get_oids(git_oidarray *out, git_grafts *grafts);
 size_t git_grafts_size(git_grafts *grafts);
 
 #endif
diff --git a/src/repository.c b/src/repository.c
index 7721c3cd0..fc14ef4d6 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -148,7 +148,7 @@ int git_repository__cleanup(git_repository *repo)
 	git_cache_clear(&repo->objects);
 	git_attr_cache_flush(repo);
 	git_grafts_free(repo->grafts);
-	git_array_clear(repo->shallow_oids);
+	git_grafts_free(repo->shallow_grafts);
 
 	set_config(repo, NULL);
 	set_index(repo, NULL);
@@ -613,22 +613,22 @@ cleanup:
 
 static int load_shallow(git_repository *repo)
 {
-	int error = 0;
-	git_array_oid_t roots = GIT_ARRAY_INIT;
 	git_array_oid_t parents = GIT_ARRAY_INIT;
+	git_oidarray roots;
+	int error;
 	size_t i;
-	git_oid *graft_oid;
 
 	/* Graft shallow roots */
-	if ((error = git_repository__shallow_roots(&roots, repo)) < 0) {
-		return error;
-	}
+	if ((error = git_repository_shallow_roots(&roots, repo)) < 0)
+		goto out;
 
-	git_array_foreach(roots, i, graft_oid) {
-		if ((error = git_grafts_add(repo->grafts, graft_oid, parents)) < 0)
-			return error;
-	}
-	return 0;
+	for (i = 0; i < roots.count; i++)
+		if ((error = git_grafts_add(repo->grafts, &roots.ids[i], parents)) < 0)
+			goto out;
+
+out:
+	git_oidarray_free(&roots);
+	return error;
 }
 
 int git_repository_open_bare(
@@ -2960,79 +2960,38 @@ int git_repository_state_cleanup(git_repository *repo)
 	return git_repository__cleanup_files(repo, state_files, ARRAY_SIZE(state_files));
 }
 
-int git_repository__shallow_roots(git_array_oid_t *out, git_repository *repo)
+int git_repository_shallow_roots(git_oidarray *out, git_repository *repo)
 {
-	git_buf path = GIT_BUF_INIT;
-	git_buf contents = GIT_BUF_INIT;
-	int error, updated, line_num = 1;
-	char *line;
-	char *buffer;
-
-    assert(out && repo);
-
-	if ((error = git_buf_joinpath(&path, repo->gitdir, "shallow")) < 0)
-		return error;
-
-	error = git_futils_readbuffer_updated(&contents, git_buf_cstr(&path), &repo->shallow_checksum, &updated);
-	git_buf_dispose(&path);
+	git_buf path = GIT_BUF_INIT, contents = GIT_BUF_INIT;
+	int error, updated = 0;
 
-	if (error < 0 && error != GIT_ENOTFOUND)
-		return error;
-
-	/* cancel out GIT_ENOTFOUND */
-	git_error_clear();
-	error = 0;
-
-	if (!updated) {
-		*out = repo->shallow_oids;
-		goto cleanup;
-	}
+	assert(out && repo);
 
-	git_array_clear(repo->shallow_oids);
+	memset(out, 0, sizeof(*out));
 
-	buffer = contents.ptr;
-	while ((line = git__strsep(&buffer, "\n")) != NULL) {
-		git_oid *oid = git_array_alloc(repo->shallow_oids);
+	if (!repo->shallow_grafts && (error = git_grafts_new(&repo->shallow_grafts)) < 0)
+		goto error;
 
-		error = git_oid_fromstr(oid, line);
-		if (error < 0) {
-			git_error_set(GIT_ERROR_REPOSITORY, "Invalid OID at line %d", line_num);
-			git_array_clear(repo->shallow_oids);
-			error = -1;
-			goto cleanup;
-		}
-		++line_num;
+	if ((error = git_buf_joinpath(&path, repo->gitdir, "shallow")) < 0 ||
+	    (error = git_futils_readbuffer_updated(&contents, git_buf_cstr(&path),
+						   &repo->shallow_checksum, &updated)) < 0) {
+		if (error == GIT_ENOTFOUND)
+			error = 0;
+		goto error;
 	}
 
-	if (*buffer) {
-		git_error_set(GIT_ERROR_REPOSITORY, "No EOL at line %d", line_num);
-		git_array_clear(repo->shallow_oids);
-		error = -1;
-		goto cleanup;
-	}
+	if (updated && (error = git_grafts_parse(repo->shallow_grafts, contents.ptr, contents.size)) < 0)
+		goto error;
 
-	*out = repo->shallow_oids;
+	if ((error = git_grafts_get_oids(out, repo->shallow_grafts)) < 0)
+		goto error;
 
-cleanup:
+error:
+	git_buf_dispose(&path);
 	git_buf_dispose(&contents);
-
 	return error;
 }
 
-int git_repository_shallow_roots(git_oidarray *out, git_repository *repo)
-{
-	int ret;
-	git_array_oid_t array = GIT_ARRAY_INIT;
-
-	assert(out);
-
-	ret = git_repository__shallow_roots(&array, repo);
-
-	git_oidarray__from_array(out, &array);
-
-	return ret;
-}
-
 int git_repository_is_shallow(git_repository *repo)
 {
 	git_buf path = GIT_BUF_INIT;
diff --git a/src/repository.h b/src/repository.h
index fd7c274a8..14b266567 100644
--- a/src/repository.h
+++ b/src/repository.h
@@ -157,8 +157,8 @@ struct git_repository {
 	git_grafts *grafts;
 	git_oid graft_checksum;
 
+	git_grafts *shallow_grafts;
 	git_oid shallow_checksum;
-	git_array_oid_t shallow_oids;
 
 	git_atomic attr_session_key;
 
@@ -263,6 +263,4 @@ extern size_t git_repository__reserved_names_posix_len;
 bool git_repository__reserved_names(
 	git_buf **out, size_t *outlen, git_repository *repo, bool include_ntfs);
 
-int git_repository__shallow_roots(git_array_oid_t *out, git_repository *repo);
-
 #endif
diff --git a/tests/grafts/shallow.c b/tests/grafts/shallow.c
index e4a0f741f..fc74de438 100644
--- a/tests/grafts/shallow.c
+++ b/tests/grafts/shallow.c
@@ -42,38 +42,41 @@ void test_grafts_shallow__clears_errors(void)
 
 void test_grafts_shallow__shallow_oids(void)
 {
-	git_oidarray oids, oids2;
+	git_oidarray oids;
 	g_repo = cl_git_sandbox_init("shallow.git");
 
 	cl_git_pass(git_repository_shallow_roots(&oids, g_repo));
 	cl_assert_equal_i(1, oids.count);
 	cl_assert_equal_oid(&g_shallow_oid, &oids.ids[0]);
 
-	cl_git_pass(git_repository_shallow_roots(&oids2, g_repo));
-	cl_assert_equal_p(oids.ids, oids2.ids);
+	git_oidarray_free(&oids);
 }
 
 void test_grafts_shallow__cache_clearing(void)
 {
-	git_oidarray oids, oids2;
+	git_oidarray oids;
 	git_oid tmp_oid;
 
-	git_oid_fromstr(&tmp_oid, "0000000000000000000000000000000000000000");
+	cl_git_pass(git_oid_fromstr(&tmp_oid, "0000000000000000000000000000000000000000"));
 	g_repo = cl_git_sandbox_init("shallow.git");
 
 	cl_git_pass(git_repository_shallow_roots(&oids, g_repo));
 	cl_assert_equal_i(1, oids.count);
 	cl_assert_equal_oid(&g_shallow_oid, &oids.ids[0]);
+	git_oidarray_free(&oids);
 
 	cl_git_mkfile("shallow.git/shallow",
 		"be3563ae3f795b2b4353bcce3a527ad0a4f7f644\n"
 		"0000000000000000000000000000000000000000\n"
 	);
 
-	cl_git_pass(git_repository_shallow_roots(&oids2, g_repo));
-	cl_assert_equal_i(2, oids2.count);
-	cl_assert_equal_oid(&g_shallow_oid, &oids2.ids[0]);
-	cl_assert_equal_oid(&tmp_oid, &oids2.ids[1]);
+	cl_git_pass(git_repository_shallow_roots(&oids, g_repo));
+	cl_assert_equal_i(2, oids.count);
+	cl_assert((git_oid_equal(&g_shallow_oid, &oids.ids[0]) &&
+		   git_oid_equal(&tmp_oid, &oids.ids[1])) ||
+		  (git_oid_equal(&g_shallow_oid, &oids.ids[1]) &&
+		   git_oid_equal(&tmp_oid, &oids.ids[0])));
+	git_oidarray_free(&oids);
 
 	cl_git_pass(p_unlink("shallow.git/shallow"));
 	cl_git_pass(git_repository_shallow_roots(&oids, g_repo));