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.
libgit2 · ethomson · May 9, 2023 · Aug 2, 2018 · Aug 2, 2018 · Oct 10, 2019
commit a11026ecbb3575b02638ae060b20e77227e2291c
diff --git 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
@@ -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
@@ -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
@@ -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 @@ static int load_grafts(git_repository *repo)
 
 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, g
67ED
raft_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
@@ -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
@@ -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));