From: Stefan Sperling <stsp@stsp.name>
Subject: improve got branch -d behaviour
To: gameoftrees@openbsd.org
Date: Fri, 27 Aug 2021 16:01:07 +0200

Removing branches in the refs/remotes namespace currently requires
'got ref -d'. However, because 'got branch -l' includes branches
under refs/remotes in its output it is not unreasonable to expect
'got branch -d origin/foo' to succeed.

This patch implements support for this and relaxes requirements
on the 'name' argument when adding and deleting branches.

It is now possible to pass absolute references such as:

  got branch refs/heads/new-branch
  got branch -d refs/heads/old-branch
  got branch -d refs/remotes/origin/old-branch

The above have the same efect as:

  got branch new-branch
  got branch -d old-branch
  got branch -d origin/old-branch

ok?

diff a099809f2873564368fcd20d3d7be32ce4a5bc12 /home/stsp/src/got
blob - 6cbbf9b01db61e8aab969ebcb566b2c29f7940a4
file + got/got.1
--- got/got.1
+++ got/got.1
@@ -1014,8 +1014,17 @@ Local branches are managed via references which live i
 reference namespace.
 The
 .Cm got branch
-command creates or deletes references in this namespace only.
+command creates references in this namespace only.
 .Pp
+When deleting branches the specified
+.Ar name
+is searched in the
+.Dq refs/heads
+reference namespace first.
+If no corresponding branch is found the
+.Dq refs/remotes
+namespace will be searched next.
+.Pp
 If invoked in a work tree without any arguments, print the name of the
 work tree's current branch.
 .Pp
@@ -1073,7 +1082,14 @@ with one the following annotations:
 .It \(a~ Ta work tree's base commit is out-of-date
 .El
 .It Fl d Ar name
-Delete the branch with the specified name from the repository.
+Delete the branch with the specified
+.Ar name
+from the
+.Dq refs/heads
+or
+.Dq refs/remotes
+reference namespace.
+.Pp
 Only the branch reference is deleted.
 Any commit, tree, and blob objects belonging to the branch
 remain in the repository and may be removed separately with
blob - 5f57676c71bfe6ae726a39f6cba1b9fb3dafb730
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -5732,15 +5732,40 @@ delete_branch(struct got_repository *repo, struct got_
 {
 	const struct got_error *err = NULL;
 	struct got_reference *ref = NULL;
-	char *refname;
+	char *refname, *remote_refname = NULL;
 
+	if (strncmp(branch_name, "refs/", 5) == 0)
+		branch_name += 5;
+	if (strncmp(branch_name, "heads/", 6) == 0)
+		branch_name += 6;
+	else if (strncmp(branch_name, "remotes/", 8) == 0)
+		branch_name += 8;
+
 	if (asprintf(&refname, "refs/heads/%s", branch_name) == -1)
 		return got_error_from_errno("asprintf");
 
-	err = got_ref_open(&ref, repo, refname, 0);
-	if (err)
+	if (asprintf(&remote_refname, "refs/remotes/%s",
+	    branch_name) == -1) {
+		err = got_error_from_errno("asprintf");
 		goto done;
+	}
 
+	err = got_ref_open(&ref, repo, refname, 0);
+	if (err) {
+		const struct got_error *err2;
+		if (err->code != GOT_ERR_NOT_REF)
+			goto done;
+		/*
+		 * Keep 'err' intact such that if neither branch exists
+		 * we report "refs/heads" rather than "refs/remotes" in
+		 * our error message.
+		 */
+		err2 = got_ref_open(&ref, repo, remote_refname, 0);
+		if (err2)
+			goto done;
+		err = NULL;
+	}
+
 	if (worktree &&
 	    strcmp(got_worktree_get_head_ref_name(worktree),
 	    got_ref_get_name(ref)) == 0) {
@@ -5754,6 +5779,7 @@ done:
 	if (ref)
 		got_ref_close(ref);
 	free(refname);
+	free(remote_refname);
 	return err;
 }
 
@@ -5773,6 +5799,9 @@ add_branch(struct got_repository *repo, const char *br
 	if (branch_name[0] == '-')
 		return got_error_path(branch_name, GOT_ERR_REF_NAME_MINUS);
 
+	if (strncmp(branch_name, "refs/heads/", 11) == 0)
+		branch_name += 11;
+
 	if (asprintf(&refname, "refs/heads/%s", branch_name) == -1) {
 		err = got_error_from_errno("asprintf");
 		goto done;
blob - a766241048c3ca1094a34b2c4977c80ec15d69e5
file + regress/cmdline/branch.sh
--- regress/cmdline/branch.sh
+++ regress/cmdline/branch.sh
@@ -59,7 +59,7 @@ test_branch_create() {
 	fi
 
 	# Create a branch based on the work tree's branch
-	(cd $testroot/wt && got branch -n anotherbranch)
+	(cd $testroot/wt && got branch -n refs/heads/anotherbranch)
 	ret="$?"
 	if [ "$ret" != "0" ]; then
 		test_done "$testroot" "$ret"
@@ -250,7 +250,7 @@ test_branch_delete() {
 	got branch -d branch2 -r $testroot/repo > $testroot/stdout
 	ret="$?"
 	if [ "$ret" != "0" ]; then
-		echo "got update command failed unexpectedly"
+		echo "got branch command failed unexpectedly"
 		test_done "$testroot" "$ret"
 		return 1
 	fi
@@ -284,7 +284,7 @@ test_branch_delete() {
 		> $testroot/stdout 2> $testroot/stderr
 	ret="$?"
 	if [ "$ret" = "0" ]; then
-		echo "got update succeeded unexpectedly"
+		echo "got branch succeeded unexpectedly"
 		test_done "$testroot" "$ret"
 		return 1
 	fi
@@ -295,7 +295,62 @@ test_branch_delete() {
 	ret="$?"
 	if [ "$ret" != "0" ]; then
 		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" "$ret"
+		return 1
 	fi
+
+	got ref -r $testroot/repo -c master refs/remotes/origin/master
+	got ref -r $testroot/repo -c branch1 refs/remotes/origin/branch1
+	got ref -r $testroot/repo -c branch3 refs/remotes/origin/branch3
+
+	got ref -l -r $testroot/repo > $testroot/stdout
+	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+	echo "refs/heads/branch1: $commit_id" >> $testroot/stdout.expected
+	echo "refs/heads/branch3: $commit_id" >> $testroot/stdout.expected
+	echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+	echo "refs/remotes/origin/branch1: $commit_id" \
+		>> $testroot/stdout.expected
+	echo "refs/remotes/origin/branch3: $commit_id" \
+		>> $testroot/stdout.expected
+	echo "refs/remotes/origin/master: $commit_id" \
+		>> $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got branch -d origin/branch1 -r $testroot/repo > $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got branch command failed unexpectedly"
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got branch -d refs/remotes/origin/branch3 -r $testroot/repo \
+		> $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got branch command failed unexpectedly"
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	got ref -l -r $testroot/repo > $testroot/stdout
+	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
+	echo "refs/heads/branch1: $commit_id" >> $testroot/stdout.expected
+	echo "refs/heads/branch3: $commit_id" >> $testroot/stdout.expected
+	echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected
+	echo "refs/remotes/origin/master: $commit_id" \
+		>> $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
 	test_done "$testroot" "$ret"
 }
 
@@ -340,7 +395,7 @@ test_branch_delete_packed() {
 
 	(cd $testroot/repo && git pack-refs --all)
 
-	got branch -d branch2 -r $testroot/repo > $testroot/stdout
+	got branch -d refs/heads/branch2 -r $testroot/repo > $testroot/stdout
 	ret="$?"
 	if [ "$ret" != "0" ]; then
 		echo "got update command failed unexpectedly"