Merge pull request #5300 from tiennou/fix/branch-documentation

branch: clarify documentation around branches

6 files changed, 169 insertions, 92 deletions

diff --git a/include/git2/branch.h b/include/git2/branch.h
index 8a4ce29a9..68fe402ed 100644
--- a/include/git2/branch.h
+++ b/include/git2/branch.h
@@ -75,9 +75,9 @@ GIT_EXTERN(int) git_branch_create_from_annotated(
 /**
  * Delete an existing branch reference.
  *
- * If the branch is successfully deleted, the passed reference
- * object will be invalidated. The reference must be freed manually
- * by the user.
+ * Note that if the deletion succeeds, the reference object will not
+ * be valid anymore, and should be freed immediately by the user using
+ * `git_reference_free()`.
  *
  * @param branch A valid reference representing a branch
  * @return 0 on success, or an error code.
@@ -126,6 +126,12 @@ GIT_EXTERN(void) git_branch_iterator_free(git_branch_iterator *iter);
  * The new branch name will be checked for validity.
  * See `git_tag_create()` for rules about valid names.
  *
+ * Note that if the move succeeds, the old reference object will not
+ + be valid anymore, and should be freed immediately by the user using
+ + `git_reference_free()`.
+ *
+ * @param out New reference object for the updated name.
+ *
  * @param branch Current underlying reference of the branch.
  *
  * @param new_branch_name Target name of the branch once the move
@@ -145,17 +151,14 @@ GIT_EXTERN(int) git_branch_move(
  * Lookup a branch by its name in a repository.
  *
  * The generated reference must be freed by the user.
- *
  * The branch name will be checked for validity.
- * See `git_tag_create()` for rules about valid names.
  *
- * @param out pointer to the looked-up branch reference
+ * @see git_tag_create for rules about valid names.
  *
+ * @param out pointer to the looked-up branch reference
  * @param repo the repository to look up the branch
- *
  * @param branch_name Name of the branch to be looked-up;
  * this name is validated for consistency.
- *
  * @param branch_type Type of the considered branch. This should
  * be valued with either GIT_BRANCH_LOCAL or GIT_BRANCH_REMOTE.
  *
@@ -169,65 +172,74 @@ GIT_EXTERN(int) git_branch_lookup(
 	git_branch_t branch_type);
 
 /**
- * Return the name of the given local or remote branch.
+ * Get the branch name
+ *
+ * Given a reference object, this will check that it really is a branch (ie.
+ * it lives under "refs/heads/" or "refs/remotes/"), and return the branch part
+ * of it.
  *
- * The name of the branch matches the definition of the name
- * for git_branch_lookup. That is, if the returned name is given
- * to git_branch_lookup() then the reference is returned that
- * was given to this function.
+ * @param out Pointer to the abbreviated reference name.
+ *        Owned by ref, do not free.
  *
- * @param out where the pointer of branch name is stored;
- * this is valid as long as the ref is not freed.
- * @param ref the reference ideally pointing to a branch
+ * @param ref A reference object, ideally pointing to a branch
  *
- * @return 0 on success; otherwise an error code (e.g., if the
- *  ref is no local or remote branch).
+ * @return 0 on success; GIT_EINVALID if the reference isn't either a local or
+ *         remote branch, otherwise an error code.
  */
 GIT_EXTERN(int) git_branch_name(
 		const char **out,
 		const git_reference *ref);
 
 /**
- * Return the reference supporting the remote tracking branch,
- * given a local branch reference.
+ * Get the upstream of a branch
  *
- * @param out Pointer where to store the retrieved
- * reference.
+ * Given a reference, this will return a new reference object corresponding
+ * to its remote tracking branch. The reference must be a local branch.
  *
+ * @see git_branch_upstream_name for details on the resolution.
+ *
+ * @param out Pointer where to store the retrieved reference.
  * @param branch Current underlying reference of the branch.
  *
  * @return 0 on success; GIT_ENOTFOUND when no remote tracking
- * reference exists, otherwise an error code.
+ *         reference exists, otherwise an error code.
  */
 GIT_EXTERN(int) git_branch_upstream(
 	git_reference **out,
 	const git_reference *branch);
 
 /**
- * Set the upstream configuration for a given local branch
+ * Set a branch's upstream branch
  *
- * @param branch the branch to configure
+ * This will update the configuration to set the branch named `name` as the upstream of `branch`.
+ * Pass a NULL name to unset the upstream information.
  *
- * @param upstream_name remote-tracking or local branch to set as
- * upstream. Pass NULL to unset.
+ * @note the actual tracking reference must have been already created for the
+ * operation to succeed.
  *
- * @return 0 or an error code
+ * @param branch the branch to configure
+ * @param branch_name remote-tracking or local branch to set as upstream.
+ *
+ * @return 0 on success; GIT_ENOTFOUND if there's no branch named `branch_name`
+ *         or an error code
  */
-GIT_EXTERN(int) git_branch_set_upstream(git_reference *branch, const char *upstream_name);
+GIT_EXTERN(int) git_branch_set_upstream(
+	git_reference *branch,
+	const char *branch_name);
 
 /**
- * Return the name of the reference supporting the remote tracking branch,
- * given the name of a local branch reference.
- *
- * @param out Pointer to the user-allocated git_buf which will be
- * filled with the name of the reference.
+ * Get the upstream name of a branch
  *
- * @param repo the repository where the branches live
+ * Given a local branch, this will return its remote-tracking branch information,
+ * as a full reference name, ie. "feature/nice" would become
+ * "refs/remote/origin/feature/nice", depending on that branch's configuration.
  *
+ * @param out the buffer into which the name will be written.
+ * @param repo the repository where the branches live.
  * @param refname reference name of the local branch.
  *
- * @return 0, GIT_ENOTFOUND when no remote tracking reference exists,
- *     otherwise an error code.
+ * @return 0 on success, GIT_ENOTFOUND when no remote tracking reference exists,
+ *         or an error code.
  */
 GIT_EXTERN(int) git_branch_upstream_name(
 	git_buf *out,
@@ -235,50 +247,55 @@ GIT_EXTERN(int) git_branch_upstream_name(
 	const char *refname);
 
 /**
- * Determine if the current local branch is pointed at by HEAD.
+ * Determine if HEAD points to the given branch
  *
- * @param branch Current underlying reference of the branch.
+ * @param branch A reference to a local branch.
  *
- * @return 1 if HEAD points at the branch, 0 if it isn't,
- * error code otherwise.
+ * @return 1 if HEAD points at the branch, 0 if it isn't, or a negative value
+ * 		   as an error code.
  */
 GIT_EXTERN(int) git_branch_is_head(
 	const git_reference *branch);
 
 /**
- * Determine if the current branch is checked out in any linked
- * repository.
+ * Determine if any HEAD points to the current branch
  *
- * @param branch Reference to the branch.
+ * This will iterate over all known linked repositories (usually in the form of
+ * worktrees) and report whether any HEAD is pointing at the current branch.
  *
- * @return 1 if branch is checked out, 0 if it isn't,
- * error code otherwise.
+ * @param branch A reference to a local branch.
+ *
+ * @return 1 if branch is checked out, 0 if it isn't, an error code otherwise.
  */
 GIT_EXTERN(int) git_branch_is_checked_out(
 	const git_reference *branch);
 
 /**
- * Return the name of remote that the remote tracking branch belongs to.
+ * Find the remote name of a remote-tracking branch
  *
- * @param out Pointer to the user-allocated git_buf which will be filled with the name of the remote.
+ * This will return the name of the remote whose fetch refspec is matching
+ * the given branch. E.g. given a branch "refs/remotes/test/master", it will
+ * extract the "test" part. If refspecs from multiple remotes match,
+ * the function will return GIT_EAMBIGUOUS.
  *
+ * @param out The buffer into which the name will be written.
  * @param repo The repository where the branch lives.
+ * @param refname complete name of the remote tracking branch.
  *
- * @param canonical_branch_name name of the remote tracking branch.
- *
- * @return 0, GIT_ENOTFOUND
- *     when no remote matching remote was found,
- *     GIT_EAMBIGUOUS when the branch maps to several remotes,
- *     otherwise an error code.
+ * @return 0 on success, GIT_ENOTFOUND when no matching remote was found,
+ *         GIT_EAMBIGUOUS when the branch maps to several remotes,
+ *         otherwise an error code.
  */
 GIT_EXTERN(int) git_branch_remote_name(
 	git_buf *out,
 	git_repository *repo,
-	const char *canonical_branch_name);
-
+	const char *refname);
 
 /**
- * Retrieve the name of the upstream remote of a local branch
+ * Retrieve the upstream remote of a local branch
+ *
+ * This will return the currently configured "branch.*.remote" for a given
+ * branch. This branch must be local.
  *
  * @param buf the buffer into which to write the name
  * @param repo the repository in which to look
diff --git a/src/branch.c b/src/branch.c
index a93db7e5b..8926c8956 100644
--- a/src/branch.c
+++ b/src/branch.c
@@ -597,35 +597,36 @@ on_error:
 	return -1;
 }
 
-int git_branch_set_upstream(git_reference *branch, const char *upstream_name)
+int git_branch_set_upstream(git_reference *branch, const char *branch_name)
 {
-	git_buf key = GIT_BUF_INIT, value = GIT_BUF_INIT;
+	git_buf key = GIT_BUF_INIT, remote_name = GIT_BUF_INIT, merge_refspec = GIT_BUF_INIT;
 	git_reference *upstream;
 	git_repository *repo;
 	git_remote *remote = NULL;
 	git_config *config;
-	const char *name, *shortname;
+	const char *refname, *shortname;
 	int local, error;
 	const git_refspec *fetchspec;
 
-	name = git_reference_name(branch);
-	if (!git_reference__is_branch(name))
-		return not_a_local_branch(name);
+	refname = git_reference_name(branch);
+	if (!git_reference__is_branch(refname))
+		return not_a_local_branch(refname);
 
 	if (git_repository_config__weakptr(&config, git_reference_owner(branch)) < 0)
 		return -1;
 
-	shortname = name + strlen(GIT_REFS_HEADS_DIR);
+	shortname = refname + strlen(GIT_REFS_HEADS_DIR);
 
-	if (upstream_name == NULL)
+	/* We're unsetting, delegate and bail-out */
+	if (branch_name == NULL)
 		return unset_upstream(config, shortname);
 
 	repo = git_reference_owner(branch);
 
-	/* First we need to figure out whether it's a branch or remote-tracking */
-	if (git_branch_lookup(&upstream, repo, upstream_name, GIT_BRANCH_LOCAL) == 0)
+	/* First we need to resolve name to a branch */
+	if (git_branch_lookup(&upstream, repo, branch_name, GIT_BRANCH_LOCAL) == 0)
 		local = 1;
-	else if (git_branch_lookup(&upstream, repo, upstream_name, GIT_BRANCH_REMOTE) == 0)
+	else if (git_branch_lookup(&upstream, repo, branch_name, GIT_BRANCH_REMOTE) == 0)
 		local = 0;
 	else {
 		git_error_set(GIT_ERROR_REFERENCE,
@@ -634,60 +635,63 @@ int git_branch_set_upstream(git_reference *branch, const char *upstream_name)
 	}
 
 	/*
-	 * If it's local, the remote is "." and the branch name is
-	 * simply the refname. Otherwise we need to figure out what
-	 * the remote-tracking branch's name on the remote is and use
-	 * that.
+	 * If it's a local-tracking branch, its remote is "." (as "the local
+	 * repository"), and the branch name is simply the refname.
+	 * Otherwise we need to figure out what the remote-tracking branch's
+	 * name on the remote is and use that.
 	 */
 	if (local)
-		error = git_buf_puts(&value, ".");
+		error = git_buf_puts(&remote_name, ".");
 	else
-		error = git_branch_remote_name(&value, repo, git_reference_name(upstream));
+		error = git_branch_remote_name(&remote_name, repo, git_reference_name(upstream));
 
 	if (error < 0)
 		goto on_error;
 
+	/* Update the upsteam branch config with the new name */
 	if (git_buf_printf(&key, "branch.%s.remote", shortname) < 0)
 		goto on_error;
 
-	if (git_config_set_string(config, git_buf_cstr(&key), git_buf_cstr(&value)) < 0)
+	if (git_config_set_string(config, git_buf_cstr(&key), git_buf_cstr(&remote_name)) < 0)
 		goto on_error;
 
 	if (local) {
-		git_buf_clear(&value);
-		if (git_buf_puts(&value, git_reference_name(upstream)) < 0)
+		/* A local branch uses the upstream refname directly */
+		if (git_buf_puts(&merge_refspec, git_reference_name(upstream)) < 0)
 			goto on_error;
 	} else {
-		/* Get the remoe-tracking branch's refname in its repo */
-		if (git_remote_lookup(&remote, repo, git_buf_cstr(&value)) < 0)
+		/* We transform the upstream branch name according to the remote's refspecs */
+		if (git_remote_lookup(&remote, repo, git_buf_cstr(&remote_name)) < 0)
 			goto on_error;
 
 		fetchspec = git_remote__matching_dst_refspec(remote, git_reference_name(upstream));
-		git_buf_clear(&value);
-		if (!fetchspec || git_refspec_rtransform(&value, fetchspec, git_reference_name(upstream)) < 0)
+		if (!fetchspec || git_refspec_rtransform(&merge_refspec, fetchspec, git_reference_name(upstream)) < 0)
 			goto on_error;
 
 		git_remote_free(remote);
 		remote = NULL;
 	}
 
+	/* Update the merge branch config with the refspec */
 	git_buf_clear(&key);
 	if (git_buf_printf(&key, "branch.%s.merge", shortname) < 0)
 		goto on_error;
 
-	if (git_config_set_string(config, git_buf_cstr(&key), git_buf_cstr(&value)) < 0)
+	if (git_config_set_string(config, git_buf_cstr(&key), git_buf_cstr(&merge_refspec)) < 0)
 		goto on_error;
 
 	git_reference_free(upstream);
 	git_buf_dispose(&key);
-	git_buf_dispose(&value);
+	git_buf_dispose(&remote_name);
+	git_buf_dispose(&merge_refspec);
 
 	return 0;
 
 on_error:
 	git_reference_free(upstream);
 	git_buf_dispose(&key);
-	git_buf_dispose(&value);
+	git_buf_dispose(&remote_name);
+	git_buf_dispose(&merge_refspec);
 	git_remote_free(remote);
 
 	return -1;
diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index 77b72dc2a..a721f9841 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -1502,7 +1502,7 @@ static int refdb_fs_backend__rename(
 	const char *message)
 {
 	refdb_fs_backend *backend = GIT_CONTAINER_OF(_backend, refdb_fs_backend, parent);
-	git_reference *old, *new;
+	git_reference *old, *new = NULL;
 	git_filebuf file = GIT_FILEBUF_INIT;
 	int error;
 
@@ -1518,7 +1518,7 @@ static int refdb_fs_backend__rename(
 		return error;
 	}
 
-	new = git_reference__set_name(old, new_name);
+	new = git_reference__realloc(&old, new_name);
 	if (!new) {
 		git_reference_free(old);
 		return -1;
diff --git a/src/refs.c b/src/refs.c
index 07784302b..29dd1bdeb 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -91,18 +91,23 @@ git_reference *git_reference__alloc(
 	return ref;
 }
 
-git_reference *git_reference__set_name(
-	git_reference *ref, const char *name)
+git_reference *git_reference__realloc(
+	git_reference **ptr_to_ref, const char *name)
 {
-	size_t namelen = strlen(name);
-	size_t reflen;
+	size_t namelen, reflen;
 	git_reference *rewrite = NULL;
 
+	assert(ptr_to_ref && name);
+
+	namelen = strlen(name);
+
 	if (!GIT_ADD_SIZET_OVERFLOW(&reflen, sizeof(git_reference), namelen) &&
 		!GIT_ADD_SIZET_OVERFLOW(&reflen, reflen, 1) &&
-		(rewrite = git__realloc(ref, reflen)) != NULL)
+		(rewrite = git__realloc(*ptr_to_ref, reflen)) != NULL)
 		memcpy(rewrite->name, name, namelen + 1);
 
+	*ptr_to_ref = NULL;
+
 	return rewrite;
 }
 
diff --git a/src/refs.h b/src/refs.h
index 46df95eba..adc345a12 100644
--- a/src/refs.h
+++ b/src/refs.h
@@ -75,7 +75,14 @@ struct git_reference {
 	char name[GIT_FLEX_ARRAY];
 };
 
-git_reference *git_reference__set_name(git_reference *ref, const char *name);
+/**
+ * Reallocate the reference with a new name
+ *
+ * Note that this is a dangerous operation, as on success, all existing
+ * pointers to the old reference will now be dangling. Only call this on objects
+ * you control, possibly using `git_reference_dup`.
+ */
+git_reference *git_reference__realloc(git_reference **ptr_to_ref, const char *name);
 
 int git_reference__normalize_name(git_buf *buf, const char *name, unsigned int flags);
 int git_reference__update_terminal(git_repository *repo, const char *ref_name, const git_oid *oid, const git_signature *sig, const char *log_message);
diff --git a/tests/refs/basic.c b/tests/refs/basic.c
new file mode 100644
index 000000000..ed0c0bde6
--- /dev/null
+++ b/tests/refs/basic.c
@@ -0,0 +1,44 @@
+#include "clar_libgit2.h"
+
+#include "futils.h"
+#include "refs.h"
+#include "ref_helpers.h"
+
+static git_repository *g_repo;
+
+static const char *loose_tag_ref_name = "refs/tags/e90810b";
+
+void test_refs_basic__initialize(void)
+{
+	g_repo = cl_git_sandbox_init("testrepo");
+	cl_git_pass(git_repository_set_ident(g_repo, "me", "foo@example.com"));
+}
+
+void test_refs_basic__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+void test_refs_basic__reference_realloc(void)
+{
+	git_reference *ref;
+	git_reference *new_ref;
+	const char *new_name = "refs/tags/awful/name-which-is/clearly/really-that-much/longer-than/the-old-one";
+
+	/* Retrieval of the reference to rename */
+	cl_git_pass(git_reference_lookup(&ref, g_repo, loose_tag_ref_name));
+
+	new_ref = git_reference__realloc(&ref, new_name);
+	cl_assert(new_ref != NULL);
+	git_reference_free(new_ref);
+	git_reference_free(ref);
+
+	/* Reload, so we restore the value */
+	cl_git_pass(git_reference_lookup(&ref, g_repo, loose_tag_ref_name));
+
+	cl_git_pass(git_reference_rename(&new_ref, ref, new_name, 1, "log message"));
+	cl_assert(ref != NULL);
+	cl_assert(new_ref != NULL);
+	git_reference_free(new_ref);
+	git_reference_free(ref);
+}