Merge pull request #5570 from libgit2/pks/refdb-refactorings

refdb: a set of preliminary refactorings for the reftable backend

src/refdb.c

@@ -17,6 +17,9 @@
 #include "reflog.h"
 #include "posix.h"
 
+#define DEFAULT_NESTING_LEVEL	5
+#define MAX_NESTING_LEVEL		10
+
 int git_refdb_new(git_refdb **out, git_repository *repo)
 {
 	git_refdb *db;
@@ -134,6 +137,59 @@ int git_refdb_lookup(git_reference **out, git_refdb *db, const char *ref_name)
 	return 0;
 }
 
+int git_refdb_resolve(
+	git_reference **out,
+	git_refdb *db,
+	const char *ref_name,
+	int max_nesting)
+{
+	git_reference *ref = NULL;
+	int error = 0, nesting;
+
+	*out = NULL;
+
+	if (max_nesting > MAX_NESTING_LEVEL)
+		max_nesting = MAX_NESTING_LEVEL;
+	else if (max_nesting < 0)
+		max_nesting = DEFAULT_NESTING_LEVEL;
+
+	if ((error = git_refdb_lookup(&ref, db, ref_name)) < 0)
+		goto out;
+
+	for (nesting = 0; nesting < max_nesting; nesting++) {
+		git_reference *resolved;
+
+		if (ref->type == GIT_REFERENCE_DIRECT)
+			break;
+
+		if ((error = git_refdb_lookup(&resolved, db, git_reference_symbolic_target(ref))) < 0) {
+			/* If we found a symbolic reference with a nonexistent target, return it. */
+			if (error == GIT_ENOTFOUND) {
+				error = 0;
+				*out = ref;
+				ref = NULL;
+			}
+			goto out;
+		}
+
+		git_reference_free(ref);
+		ref = resolved;
+	}
+
+	if (ref->type != GIT_REFERENCE_DIRECT && max_nesting != 0) {
+		git_error_set(GIT_ERROR_REFERENCE,
+			"cannot resolve reference (>%u levels deep)", max_nesting);
+		error = -1;
+		goto out;
+	}
+
+	*out = ref;
+	ref = NULL;
+out:
+	git_reference_free(ref);
+	return error;
+}
+
 int git_refdb_iterator(git_reference_iterator **out, git_refdb *db, const char *glob)
 {
 	int error;
@@ -231,6 +287,85 @@ int git_refdb_reflog_read(git_reflog **out, git_refdb *db,  const char *name)
 	return 0;
 }
 
+int git_refdb_should_write_reflog(int *out, git_refdb *db, const git_reference *ref)
+{
+	int error, logall;
+
+	error = git_repository__configmap_lookup(&logall, db->repo, GIT_CONFIGMAP_LOGALLREFUPDATES);
+	if (error < 0)
+		return error;
+
+	/* Defaults to the opposite of the repo being bare */
+	if (logall == GIT_LOGALLREFUPDATES_UNSET)
+		logall = !git_repository_is_bare(db->repo);
+
+	*out = 0;
+	switch (logall) {
+	case GIT_LOGALLREFUPDATES_FALSE:
+		*out = 0;
+		break;
+
+	case GIT_LOGALLREFUPDATES_TRUE:
+		/* Only write if it already has a log,
+		 * or if it's under heads/, remotes/ or notes/
+		 */
+		*out = git_refdb_has_log(db, ref->name) ||
+			!git__prefixcmp(ref->name, GIT_REFS_HEADS_DIR) ||
+			!git__strcmp(ref->name, GIT_HEAD_FILE) ||
+			!git__prefixcmp(ref->name, GIT_REFS_REMOTES_DIR) ||
+			!git__prefixcmp(ref->name, GIT_REFS_NOTES_DIR);
+		break;
+
+	case GIT_LOGALLREFUPDATES_ALWAYS:
+		*out = 1;
+		break;
+	}
+
+	return 0;
+}
+
+int git_refdb_should_write_head_reflog(int *out, git_refdb *db, const git_reference *ref)
+{
+	git_reference *head = NULL, *resolved = NULL;
+	const char *name;
+	int error;
+
+	*out = 0;
+
+	if (ref->type == GIT_REFERENCE_SYMBOLIC) {
+		error = 0;
+		goto out;
+	}
+
+	if ((error = git_refdb_lookup(&head, db, GIT_HEAD_FILE)) < 0)
+		goto out;
+
+	if (git_reference_type(head) == GIT_REFERENCE_DIRECT)
+		goto out;
+
+	/* Go down the symref chain until we find the branch */
+	if ((error = git_refdb_resolve(&resolved, db, git_reference_symbolic_target(head), -1)) < 0) {
+		if (error != GIT_ENOTFOUND)
+			goto out;
+		error = 0;
+		name = git_reference_symbolic_target(head);
+	} else if (git_reference_type(resolved) == GIT_REFERENCE_SYMBOLIC) {
+		name = git_reference_symbolic_target(resolved);
+	} else {
+		name = git_reference_name(resolved);
+	}
+
+	if (strcmp(name, ref->name))
+		goto out;
+
+	*out = 1;
+
+out:
+	git_reference_free(resolved);
+	git_reference_free(head);
+	return error;
+}
+
 int git_refdb_has_log(git_refdb *db, const char *refname)
 {
 	assert(db && refname);

src/refdb.h

@@ -30,6 +30,31 @@ int git_refdb_lookup(
 	git_refdb *refdb,
 	const char *ref_name);
 
+/**
+ * Resolve the reference by following symbolic references.
+ *
+ * Given a reference name, this function will follow any symbolic references up
+ * to `max_nesting` deep and return the resolved direct reference. If any of
+ * the intermediate symbolic references points to a non-existing reference,
+ * then that symbolic reference is returned instead with an error code of `0`.
+ * If the given reference is a direct reference already, it is returned
+ * directly.
+ *
+ * If `max_nesting` is `0`, the reference will not be resolved. If it's
+ * negative, it will be set to the default resolve depth which is `5`.
+ *
+ * @param out Pointer to store the result in.
+ * @param db The refdb to use for resolving the reference.
+ * @param ref_name The reference name to lookup and resolve.
+ * @param max_nesting The maximum nesting depth.
+ * @return `0` on success, a negative error code otherwise.
+ */
+int git_refdb_resolve(
+	git_reference **out,
+	git_refdb *db,
+	const char *ref_name,
+	int max_nesting);
+
 int git_refdb_rename(
 	git_reference **out,
 	git_refdb *db,
@@ -50,6 +75,50 @@ int git_refdb_delete(git_refdb *refdb, const char *ref_name, const git_oid *old_
 int git_refdb_reflog_read(git_reflog **out, git_refdb *db,  const char *name);
 int git_refdb_reflog_write(git_reflog *reflog);
 
+/**
+ * Determine whether a reflog entry should be created for the given reference.
+ *
+ * Whether or not writing to a reference should create a reflog entry is
+ * dependent on a number of things. Most importantly, there's the
+ * "core.logAllRefUpdates" setting that controls in which situations a
+ * reference should get a corresponding reflog entry. The following values for
+ * it are understood:
+ *
+ *     - "false": Do not log reference updates.
+ *
+ *     - "true": Log normal reference updates. This will write entries for
+ *               references in "refs/heads", "refs/remotes", "refs/notes" and
+ *               "HEAD" or if the reference already has a log entry.
+ *
+ *     - "always": Always create a reflog entry.
+ *
+ * If unset, the value will default to "true" for non-bare repositories and
+ * "false" for bare ones.
+ *
+ * @param out pointer to which the result will be written, `1` means a reflog
+ *            entry should be written, `0` means none should be written.
+ * @param db The refdb to decide this for.
+ * @param ref The reference one wants to check.
+ * @return `0` on success, a negative error code otherwise.
+ */
+int git_refdb_should_write_reflog(int *out, git_refdb *db, const git_reference *ref);
+
+/**
+ * Determine whether a reflog entry should be created for HEAD if creating one
+ * for the given reference
+ *
+ * In case the given reference is being pointed to by HEAD, then creating a
+ * reflog entry for this reference also requires us to create a corresponding
+ * reflog entry for HEAD. This function can be used to determine that scenario.
+ *
+ * @param out pointer to which the result will be written, `1` means a reflog
+ *            entry should be written, `0` means none should be written.
+ * @param db The refdb to decide this for.
+ * @param ref The reference one wants to check.
+ * @return `0` on success, a negative error code otherwise.
+ */
+int git_refdb_should_write_head_reflog(int *out, git_refdb *db, const git_reference *ref);
+
 int git_refdb_has_log(git_refdb *db, const char *refname);
 int git_refdb_ensure_log(git_refdb *refdb, const char *refname);
 

src/refdb_fs.c

@@ -1128,44 +1128,6 @@ cleanup:
 }
 
 static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, const git_oid *old, const git_oid *new, const git_signature *author, const char *message);
-static int has_reflog(git_repository *repo, const char *name);
-
-static int should_write_reflog(int *write, git_repository *repo, const char *name)
-{
-	int error, logall;
-
-	error = git_repository__configmap_lookup(&logall, repo, GIT_CONFIGMAP_LOGALLREFUPDATES);
-	if (error < 0)
-		return error;
-
-	/* Defaults to the opposite of the repo being bare */
-	if (logall == GIT_LOGALLREFUPDATES_UNSET)
-		logall = !git_repository_is_bare(repo);
-
-	*write = 0;
-	switch (logall) {
-	case GIT_LOGALLREFUPDATES_FALSE:
-		*write = 0;
-		break;
-
-	case GIT_LOGALLREFUPDATES_TRUE:
-		/* Only write if it already has a log,
-		 * or if it's under heads/, remotes/ or notes/
-		 */
-		*write = has_reflog(repo, name) ||
-			!git__prefixcmp(name, GIT_REFS_HEADS_DIR) ||
-			!git__strcmp(name, GIT_HEAD_FILE) ||
-			!git__prefixcmp(name, GIT_REFS_REMOTES_DIR) ||
-			!git__prefixcmp(name, GIT_REFS_NOTES_DIR);
-		break;
-
-	case GIT_LOGALLREFUPDATES_ALWAYS:
-		*write = 1;
-		break;
-	}
-
-	return 0;
-}
 
 static int cmp_old_ref(int *cmp, git_refdb_backend *backend, const char *name,
 	const git_oid *old_id, const char *old_target)
@@ -1219,54 +1181,28 @@ out:
  */
 static int maybe_append_head(refdb_fs_backend *backend, const git_reference *ref, const git_signature *who, const char *message)
 {
-	int error;
+	git_reference *head = NULL;
+	git_refdb *refdb = NULL;
+	int error, write_reflog;
 	git_oid old_id;
-	git_reference *tmp = NULL, *head = NULL, *peeled = NULL;
-	const char *name;
 
-	if (ref->type == GIT_REFERENCE_SYMBOLIC)
-		return 0;
+	if ((error = git_repository_refdb(&refdb, backend->repo)) < 0 ||
+	    (error = git_refdb_should_write_head_reflog(&write_reflog, refdb, ref)) < 0)
+		goto out;
+	if (!write_reflog)
+		goto out;
 
 	/* if we can't resolve, we use {0}*40 as old id */
 	if (git_reference_name_to_id(&old_id, backend->repo, ref->name) < 0)
 		memset(&old_id, 0, sizeof(old_id));
 
-	if ((error = git_reference_lookup(&head, backend->repo, GIT_HEAD_FILE)) < 0)
-		return error;
-
-	if (git_reference_type(head) == GIT_REFERENCE_DIRECT)
-		goto cleanup;
-
-	if ((error = git_reference_lookup(&tmp, backend->repo, GIT_HEAD_FILE)) < 0)
-		goto cleanup;
-
-	/* Go down the symref chain until we find the branch */
-	while (git_reference_type(tmp) == GIT_REFERENCE_SYMBOLIC) {
-		error = git_reference_lookup(&peeled, backend->repo, git_reference_symbolic_target(tmp));
-		if (error < 0)
-			break;
-
-		git_reference_free(tmp);
-		tmp = peeled;
-	}
-
-	if (error == GIT_ENOTFOUND) {
-		error = 0;
-		name = git_reference_symbolic_target(tmp);
-	} else if (error < 0) {
-		goto cleanup;
-	} else {
-		name = git_reference_name(tmp);
-	}
-
-	if (strcmp(name, ref->name))
-		goto cleanup;
-
-	error = reflog_append(backend, head, &old_id, git_reference_target(ref), who, message);
+	if ((error = git_reference_lookup(&head, backend->repo, GIT_HEAD_FILE)) < 0 ||
+	    (error = reflog_append(backend, head, &old_id, git_reference_target(ref), who, message)) < 0)
+		goto out;
 
-cleanup:
-	git_reference_free(tmp);
+out:
 	git_reference_free(head);
+	git_refdb_free(refdb);
 	return error;
 }
 
@@ -1335,7 +1271,10 @@ static int refdb_fs_backend__write_tail(
 	}
 
 	if (update_reflog) {
-		if ((error = should_write_reflog(&should_write, backend->repo, ref->name)) < 0)
+		git_refdb *refdb;
+
+		if ((error = git_repository_refdb__weakptr(&refdb, backend->repo)) < 0 ||
+		    (error = git_refdb_should_write_reflog(&should_write, refdb, ref)) < 0)
 			goto on_error;
 
 		if (should_write) {

src/refs.c

@@ -27,9 +27,6 @@
 
 bool git_reference__enable_symbolic_ref_target_validation = true;
 
-#define DEFAULT_NESTING_LEVEL	5
-#define MAX_NESTING_LEVEL		10
-
 enum {
 	GIT_PACKREF_HAS_PEEL = 1,
 	GIT_PACKREF_WAS_LOOSE = 2
@@ -214,52 +211,29 @@ int git_reference_lookup_resolved(
 	const char *name,
 	int max_nesting)
 {
-	git_refname_t scan_name;
-	git_reference_t scan_type;
-	int error = 0, nesting;
-	git_reference *ref = NULL;
+	git_refname_t normalized;
 	git_refdb *refdb;
+	int error = 0;
 
 	assert(ref_out && repo && name);
 
-	*ref_out = NULL;
-
-	if (max_nesting > MAX_NESTING_LEVEL)
-		max_nesting = MAX_NESTING_LEVEL;
-	else if (max_nesting < 0)
-		max_nesting = DEFAULT_NESTING_LEVEL;
-
-	scan_type = GIT_REFERENCE_SYMBOLIC;
-
-	if ((error = reference_normalize_for_repo(scan_name, repo, name, true)) < 0)
-		return error;
-
-	if ((error = git_repository_refdb__weakptr(&refdb, repo)) < 0)
-		return error;
-
-	for (nesting = max_nesting;
-		 nesting >= 0 && scan_type == GIT_REFERENCE_SYMBOLIC;
-		 nesting--)
-	{
-		if (nesting != max_nesting) {
-			strncpy(scan_name, ref->target.symbolic, sizeof(scan_name));
-			git_reference_free(ref);
-		}
-
-		if ((error = git_refdb_lookup(&ref, refdb, scan_name)) < 0)
+	if ((error = reference_normalize_for_repo(normalized, repo, name, true)) < 0 ||
+	    (error = git_repository_refdb__weakptr(&refdb, repo)) < 0 ||
+	    (error = git_refdb_resolve(ref_out, refdb, normalized, max_nesting)) < 0)
 		return error;
 
-		scan_type = ref->type;
-	}
-
-	if (scan_type != GIT_REFERENCE_DIRECT && max_nesting != 0) {
-		git_error_set(GIT_ERROR_REFERENCE,
-			"cannot resolve reference (>%u levels deep)", max_nesting);
-		git_reference_free(ref);
-		return -1;
+	/*
+	 * The resolved reference may be a symbolic reference in case its
+	 * target doesn't exist. If the user asked us to resolve (e.g.
+	 * `max_nesting != 0`), then we need to return an error in case we got
+	 * a symbolic reference back.
+	 */
+	if (max_nesting && git_reference_type(*ref_out) == GIT_REFERENCE_SYMBOLIC) {
+		git_reference_free(*ref_out);
+		*ref_out = NULL;
+		return GIT_ENOTFOUND;
 	}
 
-	*ref_out = ref;
 	return 0;
 }
 
@@ -1154,40 +1128,6 @@ int git_reference_cmp(
 	return git_oid__cmp(&ref1->target.oid, &ref2->target.oid);
 }
 
-/**
- * Get the end of a chain of references. If the final one is not
- * found, we return the reference just before that.
- */
-static int get_terminal(git_reference **out, git_repository *repo, const char *ref_name, int nesting)
-{
-	git_reference *ref;
-	int error = 0;
-
-	if (nesting > MAX_NESTING_LEVEL) {
-		git_error_set(GIT_ERROR_REFERENCE, "reference chain too deep (%d)", nesting);
-		return GIT_ENOTFOUND;
-	}
-
-	/* set to NULL to let the caller know that they're at the end of the chain */
-	if ((error = git_reference_lookup(&ref, repo, ref_name)) < 0) {
-		*out = NULL;
-		return error;
-	}
-
-	if (git_reference_type(ref) == GIT_REFERENCE_DIRECT) {
-		*out = ref;
-		error = 0;
-	} else {
-		error = get_terminal(out, repo, git_reference_symbolic_target(ref), nesting + 1);
-		if (error == GIT_ENOTFOUND && !*out)
-			*out = ref;
-		else
-			git_reference_free(ref);
-	}
-
-	return error;
-}
-
 /*
  * Starting with the reference given by `ref_name`, follows symbolic
  * references until a direct reference is found and updated the OID
@@ -1202,31 +1142,37 @@ int git_reference__update_terminal(
 {
 	git_reference *ref = NULL, *ref2 = NULL;
 	git_signature *who = NULL;
+	git_refdb *refdb = NULL;
 	const git_signature *to_use;
 	int error = 0;
 
 	if (!sig && (error = git_reference__log_signature(&who, repo)) < 0)
-		return error;
+		goto out;
 
 	to_use = sig ? sig : who;
-	error = get_terminal(&ref, repo, ref_name, 0);
 
-	/* found a dangling symref */
-	if (error == GIT_ENOTFOUND && ref) {
-		assert(git_reference_type(ref) == GIT_REFERENCE_SYMBOLIC);
-		git_error_clear();
-		error = reference__create(&ref2, repo, ref->target.symbolic, oid, NULL, 0, to_use,
-					  log_message, NULL, NULL);
-	} else if (error == GIT_ENOTFOUND) {
+	if ((error = git_repository_refdb__weakptr(&refdb, repo)) < 0)
+		goto out;
+
+	if ((error = git_refdb_resolve(&ref, refdb, ref_name, -1)) < 0) {
+		if (error == GIT_ENOTFOUND) {
 			git_error_clear();
 			error = reference__create(&ref2, repo, ref_name, oid, NULL, 0, to_use,
 						  log_message, NULL, NULL);
-	}  else if (error == 0) {
-		assert(git_reference_type(ref) == GIT_REFERENCE_DIRECT);
+		}
+		goto out;
+	}
+
+	/* In case the resolved reference is symbolic, then it's a dangling symref. */
+	if (git_reference_type(ref) == GIT_REFERENCE_SYMBOLIC) {
+		error = reference__create(&ref2, repo, ref->target.symbolic, oid, NULL, 0, to_use,
+					  log_message, NULL, NULL);
+	} else {
 		error = reference__create(&ref2, repo, ref->name, oid, NULL, 1, to_use,
 					  log_message, &ref->target.oid, NULL);
 	}
 
+out:
 	git_reference_free(ref2);
 	git_reference_free(ref);
 	git_signature_free(who);

tests/refs/reflog/messages.c

@@ -24,11 +24,8 @@ void test_refs_reflog_messages__cleanup(void)
 void test_refs_reflog_messages__setting_head_updates_reflog(void)
 {
 	git_object *tag;
-	git_signature *sig;
 	git_annotated_commit *annotated;
 
-	cl_git_pass(git_signature_now(&sig, "me", "foo@example.com"));
-
 	cl_git_pass(git_repository_set_head(g_repo, "refs/heads/haacked")); /* 4 */
 	cl_git_pass(git_repository_set_head(g_repo, "refs/heads/unborn"));
 	cl_git_pass(git_revparse_single(&tag, g_repo, "tags/test"));
@@ -68,7 +65,6 @@ void test_refs_reflog_messages__setting_head_updates_reflog(void)
 
 	git_annotated_commit_free(annotated);
 	git_object_free(tag);
-	git_signature_free(sig);
 }
 
 void test_refs_reflog_messages__setting_head_to_same_target_ignores_reflog(void)
@@ -87,12 +83,9 @@ void test_refs_reflog_messages__setting_head_to_same_target_ignores_reflog(void)
 
 void test_refs_reflog_messages__detaching_writes_reflog(void)
 {
-	git_signature *sig;
 	git_oid id;
 	const char *msg;
 
-	cl_git_pass(git_signature_now(&sig, "me", "foo@example.com"));
-
 	msg = "checkout: moving from master to e90810b8df3e80c413d903f631643c716887138d";
 	git_oid_fromstr(&id, "e90810b8df3e80c413d903f631643c716887138d");
 	cl_git_pass(git_repository_set_head_detached(g_repo, &id));
@@ -107,8 +100,6 @@ void test_refs_reflog_messages__detaching_writes_reflog(void)
 		"e90810b8df3e80c413d903f631643c716887138d",
 		"258f0e2a959a364e40ed6603d5d44fbb24765b10",
 		NULL, msg);
-
-	git_signature_free(sig);
 }
 
 void test_refs_reflog_messages__orphan_branch_does_not_count(void)