oid: use an oid array instead of shallowarray

Users should provide us an array of object ids; we don't need a separate
type. And especially, we should not be mutating user-providing values.
Instead, use `git_oid *` in the shallow code.

Author: ethomson

include/git2/sys/transport.h

@@ -25,12 +25,11 @@
 
 GIT_BEGIN_DECL
 
-typedef struct git_shallowarray git_shallowarray;
-
 typedef struct {
 	const git_remote_head * const *refs;
-	size_t count;
-	git_shallowarray *shallow_roots;
+	size_t refs_len;
+	git_oid *shallow_roots;
+	size_t shallow_roots_len;
 	int depth;
 } git_fetch_negotiation;
 
@@ -107,6 +106,16 @@ struct git_transport {
 		git_repository *repo,
 		const git_fetch_negotiation *fetch_data);
 
+	/**
+	 * Return the shallow roots of the remote.
+	 *
+	 * This function may be called after a successful call to
+	 * `negotiate_fetch`.
+	 */
+	int GIT_CALLBACK(shallow_roots)(
+		git_oidarray *out,
+		git_transport *transport);
+
 	/**
 	 * Start downloading the packfile from the remote repository.
 	 *
@@ -450,11 +459,6 @@ GIT_EXTERN(int) git_smart_subtransport_ssh(
 	git_transport *owner,
 	void *param);
 
-GIT_EXTERN(size_t) git_shallowarray_count(git_shallowarray *array);
-GIT_EXTERN(const git_oid *) git_shallowarray_get(git_shallowarray *array, size_t idx);
-GIT_EXTERN(int) git_shallowarray_add(git_shallowarray *array, git_oid *oid);
-GIT_EXTERN(int) git_shallowarray_remove(git_shallowarray *array, git_oid *oid);
-
 /** @} */
 GIT_END_DECL
 #endif

src/libgit2/fetch.c

@@ -61,7 +61,7 @@ static int mark_local(git_remote *remote)
 
 	git_vector_foreach(&remote->refs, i, head) {
 		/* If we have the object, mark it so we don't ask for it.
-		   However if we are unshallowing, we need to ask for it 
+		   However if we are unshallowing, we need to ask for it
 		   even though the head exists locally. */
 		if (remote->nego.depth != INT_MAX && git_odb_exists(odb, &head->oid))
 			head->local = 1;
@@ -169,6 +169,7 @@ static int filter_wants(git_remote *remote, const git_fetch_options *opts)
 int git_fetch_negotiate(git_remote *remote, const git_fetch_options *opts)
 {
 	git_transport *t = remote->transport;
+	int error;
 
 	remote->need_pack = 0;
 
@@ -191,33 +192,39 @@ int git_fetch_negotiate(git_remote *remote, const git_fetch_options *opts)
 	 * server what we want and what we have.
 	 */
 	remote->nego.refs = (const git_remote_head * const *)remote->refs.contents;
-	remote->nego.count = remote->refs.length;
-	remote->nego.shallow_roots = git__malloc(sizeof(*remote->nego.shallow_roots));
-
-	git_array_init(remote->nego.shallow_roots->array);
+	remote->nego.refs_len = remote->refs.length;
 
-	git_repository__shallow_roots(&remote->nego.shallow_roots->array, remote->repo);
+	if (git_repository__shallow_roots(&remote->nego.shallow_roots,
+	                                  &remote->nego.shallow_roots_len,
+	                                  remote->repo) < 0)
+		return -1;
 
-	return t->negotiate_fetch(t,
+	error = t->negotiate_fetch(t,
 		remote->repo,
 		&remote->nego);
+
+	git__free(remote->nego.shallow_roots);
+
+	return error;
 }
 
 int git_fetch_download_pack(git_remote *remote)
 {
+	git_oidarray shallow_roots = { NULL };
 	git_transport *t = remote->transport;
 	int error;
 
 	if (!remote->need_pack)
 		return 0;
 
-	if ((error = t->download_pack(t, remote->repo, &remote->stats)) != 0)
+	if ((error = t->download_pack(t, remote->repo, &remote->stats)) != 0 ||
+	    (error = t->shallow_roots(&shallow_roots, t)) != 0)
 		return error;
 
-	if ((error = git_repository__shallow_roots_write(remote->repo, remote->nego.shallow_roots->array)) != 0)
-		return error;
+	error = git_repository__shallow_roots_write(remote->repo, &shallow_roots);
 
-	return 0;
+	git_oidarray_dispose(&shallow_roots);
+	return error;
 }
 
 int git_fetch_options_init(git_fetch_options *opts, unsigned int version)

src/libgit2/grafts.c

@@ -243,20 +243,26 @@ int git_grafts_get(git_commit_graft **out, git_grafts *grafts, const git_oid *oi
 	return 0;
 }
 
-int git_grafts_get_oids(git_array_oid_t *out, git_grafts *grafts)
+int git_grafts_oids(git_oid **out, size_t *out_len, git_grafts *grafts)
 {
+	git_array_oid_t array = GIT_ARRAY_INIT;
 	const git_oid *oid;
-	size_t i = 0;
-	int error;
+	size_t existing, i = 0;
 
 	GIT_ASSERT_ARG(out && grafts);
 
-	while ((error = git_oidmap_iterate(NULL, grafts->commits, &i, &oid)) == 0) {
-		git_oid *cpy = git_array_alloc(*out);
+	if ((existing = git_oidmap_size(grafts->commits)) > 0)
+		git_array_init_to_size(array, existing);
+
+	while (git_oidmap_iterate(NULL, grafts->commits, &i, &oid) == 0) {
+		git_oid *cpy = git_array_alloc(array);
 		GIT_ERROR_CHECK_ALLOC(cpy);
 		git_oid_cpy(cpy, oid);
 	}
 
+	*out = array.ptr;
+	*out_len = array.size;
+
 	return 0;
 }
 

src/libgit2/grafts.h

@@ -30,7 +30,7 @@ int git_grafts_parse(git_grafts *grafts, const char *buf, size_t len);
 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_array_oid_t *out, git_grafts *grafts);
+int git_grafts_oids(git_oid **out, size_t *out_len, git_grafts *grafts);
 size_t git_grafts_size(git_grafts *grafts);
 
 #endif

src/libgit2/oidarray.c

@@ -15,10 +15,17 @@ void git_oidarray_dispose(git_oidarray *arr)
 	git__free(arr->ids);
 }
 
-void git_oidarray__from_array(git_oidarray *arr, git_array_oid_t *array)
+void git_oidarray__from_array(git_oidarray *out, const git_array_oid_t *array)
 {
-	arr->count = array->size;
-	arr->ids = array->ptr;
+	out->count = array->size;
+	out->ids = array->ptr;
+}
+
+void git_oidarray__to_array(git_array_oid_t *out, const git_oidarray *array)
+{
+	out->ptr = array->ids;
+	out->size = array->count;
+	out->asize = array->count;
 }
 
 void git_oidarray__reverse(git_oidarray *arr)
@@ -33,6 +40,45 @@ void git_oidarray__reverse(git_oidarray *arr)
 	}
 }
 
+int git_oidarray__add(git_array_oid_t *arr, git_oid *id)
+{
+	git_oid *add, *iter;
+	size_t i;
+
+	git_array_foreach(*arr, i, iter) {
+		if (git_oid_cmp(iter, id) == 0)
+			return 0;
+	}
+
+	if ((add = git_array_alloc(*arr)) == NULL)
+		return -1;
+
+	git_oid_cpy(add, id);
+	return 0;
+}
+
+bool git_oidarray__remove(git_array_oid_t *arr, git_oid *id)
+{
+	bool found = false;
+	size_t remain, i;
+	git_oid *iter;
+
+	git_array_foreach(*arr, i, iter) {
+		if (git_oid_cmp(iter, id) == 0) {
+			arr->size--;
+			remain = arr->size - i;
+
+			if (remain > 0)
+				memmove(&arr->ptr[i], &arr->ptr[i+1], remain * sizeof(git_oid));
+
+			found = true;
+			break;
+		}
+	}
+
+	return found;
+}
+
 #ifndef GIT_DEPRECATE_HARD
 
 void git_oidarray_free(git_oidarray *arr)

src/libgit2/oidarray.h

@@ -15,6 +15,10 @@
 typedef git_array_t(git_oid) git_array_oid_t;
 
 extern void git_oidarray__reverse(git_oidarray *arr);
-extern void git_oidarray__from_array(git_oidarray *arr, git_array_oid_t *array);
+extern void git_oidarray__from_array(git_oidarray *out, const git_array_oid_t *array);
+extern void git_oidarray__to_array(git_array_oid_t *out, const git_oidarray *array);
+
+int git_oidarray__add(git_array_oid_t *arr, git_oid *id);
+bool git_oidarray__remove(git_array_oid_t *arr, git_oid *id);
 
 #endif

src/libgit2/remote.c

@@ -2167,11 +2167,6 @@ void git_remote_free(git_remote *remote)
 		remote->transport = NULL;
 	}
 
-	if (remote->nego.shallow_roots) {
-		git_array_clear(remote->nego.shallow_roots->array);
-		git__free(remote->nego.shallow_roots);
-	}
-
 	git_vector_free(&remote->refs);
 
 	free_refspecs(&remote->refspecs);

src/libgit2/repository.c

@@ -3651,7 +3651,11 @@ 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_oid **out,
+	size_t *out_len,
+	git_repository *repo)
+{
 	int error = 0;
 
 	if (!repo->shallow_grafts && (error = load_grafts(repo)) < 0)
@@ -3660,19 +3664,18 @@ int git_repository__shallow_roots(git_array_oid_t *out, git_repository *repo) {
 	if ((error = git_grafts_refresh(repo->shallow_grafts)) < 0)
 		return error;
 
-	if ((error = git_grafts_get_oids(out, repo->shallow_grafts)) < 0)
+	if ((error = git_grafts_oids(out, out_len, repo->shallow_grafts)) < 0)
 		return error;
 
 	return 0;
 }
 
-int git_repository__shallow_roots_write(git_repository *repo, git_array_oid_t roots)
+int git_repository__shallow_roots_write(git_repository *repo, git_oidarray *roots)
 {
 	git_filebuf file = GIT_FILEBUF_INIT;
 	git_str path = GIT_STR_INIT;
 	char oid_str[GIT_OID_MAX_HEXSIZE + 1];
-	size_t idx;
-	git_oid *oid;
+	size_t i;
 	int filebuf_hash, error = 0;
 
 	GIT_ASSERT_ARG(repo);
@@ -3686,8 +3689,8 @@ int git_repository__shallow_roots_write(git_repository *repo, git_array_oid_t ro
 	if ((error = git_filebuf_open(&file, git_str_cstr(&path), filebuf_hash, 0666)) < 0)
 		goto on_error;
 
-	git_array_foreach(roots, idx, oid) {
-		git_oid_tostr(oid_str, sizeof(oid_str), oid);
+	for (i = 0; i < roots->count; i++) {
+		git_oid_tostr(oid_str, sizeof(oid_str), &roots->ids[i]);
 		git_filebuf_write(&file, oid_str, git_oid_hexsize(repo->oid_type));
 		git_filebuf_write(&file, "\n", 1);
 	}
@@ -3699,7 +3702,7 @@ int git_repository__shallow_roots_write(git_repository *repo, git_array_oid_t ro
 		goto on_error;
 	}
 
-	if (git_array_size(roots) == 0)
+	if (!roots->count)
 		remove(path.ptr);
 
 on_error:
@@ -3727,6 +3730,7 @@ int git_repository_is_shallow(git_repository *repo)
 
 	if (error < 0)
 		return error;
+
 	return st.st_size == 0 ? 0 : 1;
 }
 

src/libgit2/repository.h

@@ -246,8 +246,8 @@ extern size_t git_repository__reserved_names_posix_len;
 bool git_repository__reserved_names(
 	git_str **out, size_t *outlen, git_repository *repo, bool include_ntfs);
 
-int git_repository__shallow_roots(git_array_oid_t *out, git_repository *repo);
-int git_repository__shallow_roots_write(git_repository *repo, git_array_oid_t roots);
+int git_repository__shallow_roots(git_oid **out, size_t *out_len, git_repository *repo);
+int git_repository__shallow_roots_write(git_repository *repo, git_oidarray *roots);
 
 /*
  * The default branch for the repository; the `init.defaultBranch`

src/libgit2/transports/local.c

@@ -320,6 +320,16 @@ static int local_negotiate_fetch(
 	return 0;
 }
 
+static int local_shallow_roots(
+	git_oidarray *out,
+	git_transport *transport)
+{
+	GIT_UNUSED(out);
+	GIT_UNUSED(transport);
+
+	return 0;
+}
+
 static int local_push_update_remote_ref(
 	git_repository *remote_repo,
 	const char *lref,
@@ -745,6 +755,7 @@ int git_transport_local(git_transport **out, git_remote *owner, void *param)
 	t->parent.oid_type = local_oid_type;
 #endif
 	t->parent.negotiate_fetch = local_negotiate_fetch;
+	t->parent.shallow_roots = local_shallow_roots;
 	t->parent.download_pack = local_download_pack;
 	t->parent.push = local_push;
 	t->parent.close = local_close;