Merge pull request #5563 from pks-t/pks/worktree-heads

Access HEAD via the refdb backends

8 files changed, 119 insertions, 245 deletions

diff --git a/src/branch.c b/src/branch.c
index 770c9a1ee..715f6cf99 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -134,39 +134,37 @@ int git_branch_create_from_annotated(
 		repository, branch_name, commit->commit, commit->description, force);
 }
 
-static int branch_equals(git_repository *repo, const char *path, void *payload)
+static int branch_is_checked_out(git_repository *worktree, void *payload)
 {
 	git_reference *branch = (git_reference *) payload;
 	git_reference *head = NULL;
-	int equal = 0;
+	int error;
 
-	if (git_reference__read_head(&head, repo, path) < 0 ||
-		git_reference_type(head) != GIT_REFERENCE_SYMBOLIC)
-		goto done;
+	if (git_repository_is_bare(worktree))
+		return 0;
 
-	equal = !git__strcmp(head->target.symbolic, branch->name);
+	if ((error = git_reference_lookup(&head, worktree, GIT_HEAD_FILE)) < 0) {
+		if (error == GIT_ENOTFOUND)
+			error = 0;
+		goto out;
+	}
 
-done:
+	if (git_reference_type(head) != GIT_REFERENCE_SYMBOLIC)
+		goto out;
+
+	error = !git__strcmp(head->target.symbolic, branch->name);
+
+out:
 	git_reference_free(head);
-	return equal;
+	return error;
 }
 
 int git_branch_is_checked_out(const git_reference *branch)
 {
-	git_repository *repo;
-	int flags = 0;
-
-	assert(branch);
-
 	if (!git_reference_is_branch(branch))
 		return 0;
-
-	repo = git_reference_owner(branch);
-
-	if (git_repository_is_bare(repo))
-		flags |= GIT_REPOSITORY_FOREACH_HEAD_SKIP_REPO;
-
-	return git_repository_foreach_head(repo, branch_equals, flags, (void *) branch) == 1;
+	return git_repository_foreach_worktree(git_reference_owner(branch),
+					       branch_is_checked_out, (void *)branch) == 1;
 }
 
 int git_branch_delete(git_reference *branch)
diff --git a/src/refs.c b/src/refs.c
index 9ef167739..ed900577d 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -237,40 +237,6 @@ int git_reference_lookup_resolved(
 	return 0;
 }
 
-int git_reference__read_head(
-	git_reference **out,
-	git_repository *repo,
-	const char *path)
-{
-	git_buf reference = GIT_BUF_INIT;
-	char *name = NULL;
-	int error;
-
-	if ((error = git_futils_readbuffer(&reference, path)) < 0)
-		goto out;
-	git_buf_rtrim(&reference);
-
-	if (git__strncmp(reference.ptr, GIT_SYMREF, strlen(GIT_SYMREF)) == 0) {
-		git_buf_consume(&reference, reference.ptr + strlen(GIT_SYMREF));
-
-		name = git_path_basename(path);
-
-		if ((*out = git_reference__alloc_symbolic(name, reference.ptr)) == NULL) {
-			error = -1;
-			goto out;
-		}
-	} else {
-		if ((error = git_reference_lookup(out, repo, reference.ptr)) < 0)
-			goto out;
-	}
-
-out:
-	git__free(name);
-	git_buf_dispose(&reference);
-
-	return error;
-}
-
 int git_reference_dwim(git_reference **out, git_repository *repo, const char *refname)
 {
 	int error = 0, i;
@@ -605,84 +571,33 @@ int git_reference_symbolic_set_target(
 typedef struct {
     const char *old_name;
     git_refname_t new_name;
-} rename_cb_data;
+} refs_update_head_payload;
 
-static int update_wt_heads(git_repository *repo, const char *path, void *payload)
+static int refs_update_head(git_repository *worktree, void *_payload)
 {
-	rename_cb_data *data = (rename_cb_data *) payload;
-	git_reference *head = NULL;
-	char *gitdir = NULL;
+	refs_update_head_payload *payload = (refs_update_head_payload *)_payload;
+	git_reference *head = NULL, *updated = NULL;
 	int error;
 
-	if ((error = git_reference__read_head(&head, repo, path)) < 0) {
-		git_error_set(GIT_ERROR_REFERENCE, "could not read HEAD when renaming references");
-		goto out;
-	}
-
-	if ((gitdir = git_path_dirname(path)) == NULL) {
-		error = -1;
+	if ((error = git_reference_lookup(&head, worktree, GIT_HEAD_FILE)) < 0)
 		goto out;
-	}
 
 	if (git_reference_type(head) != GIT_REFERENCE_SYMBOLIC ||
-	    git__strcmp(head->target.symbolic, data->old_name) != 0) {
-		error = 0;
+	    git__strcmp(git_reference_symbolic_target(head), payload->old_name) != 0)
 		goto out;
-	}
 
-	/* Update HEAD it was pointing to the reference being renamed */
-	if ((error = git_repository_create_head(gitdir, data->new_name)) < 0) {
+	/* Update HEAD if it was pointing to the reference being renamed */
+	if ((error = git_reference_symbolic_set_target(&updated, head, payload->new_name, NULL)) < 0) {
 		git_error_set(GIT_ERROR_REFERENCE, "failed to update HEAD after renaming reference");
 		goto out;
 	}
 
 out:
+	git_reference_free(updated);
 	git_reference_free(head);
-	git__free(gitdir);
-
-	return error;
-}
-
-static int reference__rename(git_reference **out, git_reference *ref, const char *new_name, int force,
-				 const git_signature *signature, const char *message)
-{
-	git_repository *repo;
-	git_refname_t normalized;
-	bool should_head_be_updated = false;
-	int error = 0;
-
-	assert(ref && new_name && signature);
-
-	repo = git_reference_owner(ref);
-
-	if ((error = reference_normalize_for_repo(
-		normalized, repo, new_name, true)) < 0)
-		return error;
-
-	/* Check if we have to update HEAD. */
-	if ((error = git_branch_is_head(ref)) < 0)
-		return error;
-
-	should_head_be_updated = (error > 0);
-
-	if ((error = git_refdb_rename(out, ref->db, ref->name, normalized, force, signature, message)) < 0)
-		return error;
-
-	/* Update HEAD if it was pointing to the reference being renamed */
-	if (should_head_be_updated) {
-		error = git_repository_set_head(ref->db->repo, normalized);
-	} else {
-		rename_cb_data payload;
-		payload.old_name = ref->name;
-		memcpy(&payload.new_name, &normalized, sizeof(normalized));
-
-		error = git_repository_foreach_head(repo, update_wt_heads, 0, &payload);
-	}
-
 	return error;
 }
 
-
 int git_reference_rename(
 	git_reference **out,
 	git_reference *ref,
@@ -690,17 +605,28 @@ int git_reference_rename(
 	int force,
 	const char *log_message)
 {
-	git_signature *who;
+	refs_update_head_payload payload;
+	git_signature *signature;
+	git_repository *repo;
 	int error;
 
 	assert(out && ref);
 
-	if ((error = git_reference__log_signature(&who, ref->db->repo)) < 0)
-		return error;
+	repo = git_reference_owner(ref);
 
-	error = reference__rename(out, ref, new_name, force, who, log_message);
-	git_signature_free(who);
+	if ((error = git_reference__log_signature(&signature, repo)) < 0 ||
+	    (error = reference_normalize_for_repo(payload.new_name, repo, new_name, true)) < 0 ||
+	    (error = git_refdb_rename(out, ref->db, ref->name, payload.new_name, force, signature, log_message)) < 0)
+		goto out;
+
+	payload.old_name = ref->name;
 
+	/* We may have to update any HEAD that was pointing to the renamed reference. */
+	if ((error = git_repository_foreach_worktree(repo, refs_update_head, &payload)) < 0)
+		goto out;
+
+out:
+	git_signature_free(signature);
 	return error;
 }
 
diff --git a/src/refs.h b/src/refs.h
index ece66468a..b8d6df7aa 100644
--- a/src/refs.h
+++ b/src/refs.h
@@ -116,24 +116,6 @@ int git_reference_lookup_resolved(
 	const char *name,
 	int max_deref);
 
-/**
- * Read reference from a file.
- *
- * This function will read in the file at `path`. If it is a
- * symref, it will return a new unresolved symbolic reference
- * with the given name pointing to the reference pointed to by
- * the file. If it is not a symbolic reference, it will return
- * the resolved reference.
- *
- * Note that because the refdb is not involved for symbolic references, they
- * won't be owned, hence you should either not make the returned reference
- * 'externally visible', or perform the lookup before returning it to the user.
- */
-int git_reference__read_head(
-	git_reference **out,
-	git_repository *repo,
-	const char *path);
-
 int git_reference__log_signature(git_signature **out, git_repository *repo);
 
 /** Update a reference after a commit. */
diff --git a/src/repository.c b/src/repository.c
index 5e818fb82..ebb9daa01 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -2177,12 +2177,6 @@ int git_repository_head_detached(git_repository *repo)
 	return exists;
 }
 
-static int get_worktree_file_path(git_buf *out, git_repository *repo, const char *worktree, const char *file)
-{
-	git_buf_clear(out);
-	return git_buf_printf(out, "%s/worktrees/%s/%s", repo->commondir, worktree, file);
-}
-
 int git_repository_head_detached_for_worktree(git_repository *repo, const char *name)
 {
 	git_reference *ref = NULL;
@@ -2223,7 +2217,8 @@ int git_repository_head(git_reference **head_out, git_repository *repo)
 
 int git_repository_head_for_worktree(git_reference **out, git_repository *repo, const char *name)
 {
-	git_buf path = GIT_BUF_INIT;
+	git_repository *worktree_repo = NULL;
+	git_worktree *worktree = NULL;
 	git_reference *head = NULL;
 	int error;
 
@@ -2231,65 +2226,68 @@ int git_repository_head_for_worktree(git_reference **out, git_repository *repo,
 
 	*out = NULL;
 
-	if ((error = get_worktree_file_path(&path, repo, name, GIT_HEAD_FILE)) < 0 ||
-	    (error = git_reference__read_head(&head, repo, path.ptr)) < 0)
+	if ((error = git_worktree_lookup(&worktree, repo, name)) < 0 ||
+	    (error = git_repository_open_from_worktree(&worktree_repo, worktree)) < 0 ||
+	    (error = git_reference_lookup(&head, worktree_repo, GIT_HEAD_FILE)) < 0)
 		goto out;
 
 	if (git_reference_type(head) != GIT_REFERENCE_DIRECT) {
-		git_reference *resolved;
-
-		error = git_reference_lookup_resolved(&resolved, repo, git_reference_symbolic_target(head), -1);
-		git_reference_free(head);
-		head = resolved;
+		if ((error = git_reference_lookup_resolved(out, worktree_repo, git_reference_symbolic_target(head), -1)) < 0)
+			goto out;
+	} else {
+		*out = head;
+		head = NULL;
 	}
 
-	*out = head;
-
 out:
-	if (error)
-		git_reference_free(head);
-
-	git_buf_dispose(&path);
-
+	git_reference_free(head);
+	git_worktree_free(worktree);
+	git_repository_free(worktree_repo);
 	return error;
 }
 
-int git_repository_foreach_head(git_repository *repo,
-				git_repository_foreach_head_cb cb,
-				int flags, void *payload)
+int git_repository_foreach_worktree(git_repository *repo,
+				    git_repository_foreach_worktree_cb cb,
+				    void *payload)
 {
-	git_strarray worktrees = GIT_VECTOR_INIT;
-	git_buf path = GIT_BUF_INIT;
-	int error = 0;
+	git_strarray worktrees = {0};
+	git_repository *worktree_repo = NULL;
+	git_worktree *worktree = NULL;
+	int error;
 	size_t i;
 
+	if ((error = git_repository_open(&worktree_repo, repo->commondir)) < 0 ||
+	    (error = cb(worktree_repo, payload) != 0))
+		goto out;
 
-	if (!(flags & GIT_REPOSITORY_FOREACH_HEAD_SKIP_REPO)) {
-		/* Gather HEAD of main repository */
-		if ((error = git_buf_joinpath(&path, repo->commondir, GIT_HEAD_FILE)) < 0 ||
-		    (error = cb(repo, path.ptr, payload) != 0))
-			goto out;
-	}
+	git_repository_free(worktree_repo);
+	worktree_repo = NULL;
 
-	if (!(flags & GIT_REPOSITORY_FOREACH_HEAD_SKIP_WORKTREES)) {
-		if ((error = git_worktree_list(&worktrees, repo)) < 0) {
-			error = 0;
-			goto out;
-		}
+	if ((error = git_worktree_list(&worktrees, repo)) < 0)
+		goto out;
 
-		/* Gather HEADs of all worktrees */
-		for (i = 0; i < worktrees.count; i++) {
-			if (get_worktree_file_path(&path, repo, worktrees.strings[i], GIT_HEAD_FILE) < 0)
-				continue;
+	for (i = 0; i < worktrees.count; i++) {
+		git_repository_free(worktree_repo);
+		worktree_repo = NULL;
+		git_worktree_free(worktree);
+		worktree = NULL;
 
-			if ((error = cb(repo, path.ptr, payload)) != 0)
+		if ((error = git_worktree_lookup(&worktree, repo, worktrees.strings[i]) < 0) ||
+		    (error = git_repository_open_from_worktree(&worktree_repo, worktree)) < 0) {
+			if (error != GIT_ENOTFOUND)
 				goto out;
+			error = 0;
+			continue;
 		}
+
+		if ((error = cb(worktree_repo, payload)) != 0)
+			goto out;
 	}
 
 out:
-	git_buf_dispose(&path);
 	git_strarray_dispose(&worktrees);
+	git_repository_free(worktree_repo);
+	git_worktree_free(worktree);
 	return error;
 }
 
diff --git a/src/repository.h b/src/repository.h
index bafdb5896..d73e77d70 100644
--- a/src/repository.h
+++ b/src/repository.h
@@ -166,34 +166,11 @@ GIT_INLINE(git_attr_cache *) git_repository_attr_cache(git_repository *repo)
 int git_repository_head_tree(git_tree **tree, git_repository *repo);
 int git_repository_create_head(const char *git_dir, const char *ref_name);
 
-/*
- * Called for each HEAD.
- *
- * Can return either 0, causing the iteration over HEADs to
- * continue, or a non-0 value causing the iteration to abort. The
- * return value is passed back to the caller of
- * `git_repository_foreach_head`
- */
-typedef int (*git_repository_foreach_head_cb)(git_repository *repo, const char *path, void *payload);
+typedef int (*git_repository_foreach_worktree_cb)(git_repository *, void *);
 
-enum {
-	/* Skip enumeration of the main repository HEAD */
-	GIT_REPOSITORY_FOREACH_HEAD_SKIP_REPO      = (1u << 0),
-	/* Skip enumeration of worktree HEADs */
-	GIT_REPOSITORY_FOREACH_HEAD_SKIP_WORKTREES = (1u << 1),
-};
-
-/*
- * Iterate over repository and all worktree HEADs.
- *
- * This function will be called for the repository HEAD and for
- * all HEADS of linked worktrees. For each HEAD, the callback is
- * executed with the given payload. The return value equals the
- * return value of the last executed callback function.
- */
-int git_repository_foreach_head(git_repository *repo,
-				git_repository_foreach_head_cb cb,
-				int flags, void *payload);
+int git_repository_foreach_worktree(git_repository *repo,
+				    git_repository_foreach_worktree_cb cb,
+				    void *payload);
 
 /*
  * Weak pointers to repository internals.
diff --git a/tests/worktree/refs.c b/tests/worktree/refs.c
index 501255274..27dc667ea 100644
--- a/tests/worktree/refs.c
+++ b/tests/worktree/refs.c
@@ -163,7 +163,10 @@ void test_worktree_refs__renaming_reference_updates_worktree_heads(void)
 	cl_git_pass(git_branch_lookup(&branch, fixture.repo,
 		    "testrepo-worktree", GIT_BRANCH_LOCAL));
 	cl_git_pass(git_reference_rename(&renamed, branch, "refs/heads/renamed", 0, NULL));
-	cl_git_pass(git_repository_head(&head, fixture.worktree));
+
+	cl_git_pass(git_reference_lookup(&head, fixture.worktree, GIT_HEAD_FILE));
+	cl_assert_equal_i(git_reference_type(head), GIT_REFERENCE_SYMBOLIC);
+	cl_assert_equal_s(git_reference_symbolic_target(head), "refs/heads/renamed");
 
 	git_reference_free(head);
 	git_reference_free(branch);
diff --git a/tests/worktree/repository.c b/tests/worktree/repository.c
index ca56413b7..c4eeadd35 100644
--- a/tests/worktree/repository.c
+++ b/tests/worktree/repository.c
@@ -50,9 +50,12 @@ void test_worktree_repository__head_detached(void)
 
 	cl_assert(git_repository_head_detached(fixture.worktree));
 	cl_assert(git_repository_head_detached_for_worktree(fixture.repo, "testrepo-worktree"));
-	cl_git_fail(git_repository_head_for_worktree(&head, fixture.repo, "testrepo-worktree"));
+	cl_git_pass(git_repository_head_for_worktree(&head, fixture.repo, "testrepo-worktree"));
+
+	cl_assert_equal_oid(&ref->target.oid, &head->target.oid);
 
 	git_reference_free(ref);
+	git_reference_free(head);
 }
 
 void test_worktree_repository__head_detached_fails_for_invalid_worktree(void)
diff --git a/tests/worktree/worktree.c b/tests/worktree/worktree.c
index 716d0aa0a..cd20bed82 100644
--- a/tests/worktree/worktree.c
+++ b/tests/worktree/worktree.c
@@ -581,45 +581,32 @@ void test_worktree_worktree__prune_worktree(void)
 	git_worktree_free(wt);
 }
 
-static int read_head_ref(git_repository *repo, const char *path, void *payload)
-{
-	git_vector *refs = (git_vector *) payload;
-	git_reference *head;
-
-	GIT_UNUSED(repo);
-
-	cl_git_pass(git_reference__read_head(&head, repo, path));
+static int foreach_worktree_cb(git_repository *worktree, void *payload)
+{
+	int *counter = (int *)payload;
+
+	switch (*counter) {
+	case 0:
+		cl_assert_equal_s(git_repository_path(fixture.repo),
+				  git_repository_path(worktree));
+		cl_assert(!git_repository_is_worktree(worktree));
+		break;
+	case 1:
+		cl_assert_equal_s(git_repository_path(fixture.worktree),
+				  git_repository_path(worktree));
+		cl_assert(git_repository_is_worktree(worktree));
+		break;
+	default:
+		cl_fail("more worktrees found than expected");
+	}
 
-	git_vector_insert(refs, head);
+	(*counter)++;
 
 	return 0;
 }
 
-void test_worktree_worktree__foreach_head_gives_same_results_in_wt_and_repo(void)
+void test_worktree_worktree__foreach_worktree_lists_all_worktrees(void)
 {
-	git_vector repo_refs = GIT_VECTOR_INIT, worktree_refs = GIT_VECTOR_INIT;
-	git_reference *heads[2];
-	size_t i;
-
-	cl_git_pass(git_reference_lookup(&heads[0], fixture.repo, GIT_HEAD_FILE));
-	cl_git_pass(git_reference_lookup(&heads[1], fixture.worktree, GIT_HEAD_FILE));
-
-	cl_git_pass(git_repository_foreach_head(fixture.repo, read_head_ref, 0, &repo_refs));
-	cl_git_pass(git_repository_foreach_head(fixture.worktree, read_head_ref, 0, &worktree_refs));
-
-	cl_assert_equal_i(repo_refs.length, ARRAY_SIZE(heads));
-	cl_assert_equal_i(worktree_refs.length, ARRAY_SIZE(heads));
-
-	for (i = 0; i < ARRAY_SIZE(heads); i++) {
-		cl_assert_equal_s(heads[i]->name, ((git_reference *) repo_refs.contents[i])->name);
-		cl_assert_equal_s(heads[i]->name, ((git_reference *) repo_refs.contents[i])->name);
-		cl_assert_equal_s(heads[i]->name, ((git_reference *) worktree_refs.contents[i])->name);
-
-		git_reference_free(heads[i]);
-		git_reference_free(repo_refs.contents[i]);
-		git_reference_free(worktree_refs.contents[i]);
-	}
-
-	git_vector_free(&repo_refs);
-	git_vector_free(&worktree_refs);
+	int counter = 0;
+	cl_git_pass(git_repository_foreach_worktree(fixture.repo, foreach_worktree_cb, &counter));
 }