Re: [PATCH RESEND] branch: allow deleting dangling branches with --force

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Aug 25 2021, René Scharfe wrote:

> git branch only allows deleting branches that point to valid commits.
> Skip that check if --force is given, as the caller is indicating with
> it that they know what they are doing and accept the consequences.
> This allows deleting dangling branches, which previously had to be
> reset to a valid start-point using --force first.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> Original submission:
> http://public-inbox.org/git/52847a99-db7c-9634-b3b1-fd9b1342bc32@web.de/
>
>  Documentation/git-branch.txt | 3 ++-
>  builtin/branch.c             | 2 +-
>  t/t3200-branch.sh            | 7 +++++++
>  3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 94dc9a54f2..5449767121 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -118,7 +118,8 @@ OPTIONS
>  	Reset <branchname> to <startpoint>, even if <branchname> exists
>  	already. Without `-f`, 'git branch' refuses to change an existing branch.
>  	In combination with `-d` (or `--delete`), allow deleting the
> -	branch irrespective of its merged status. In combination with
> +	branch irrespective of its merged status, or whether it even
> +	points to a valid commit. In combination with
>  	`-m` (or `--move`), allow renaming the branch even if the new
>  	branch name already exists, the same applies for `-c` (or `--copy`).
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index b23b1d1752..03c7b7253a 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -168,7 +168,7 @@ static int check_branch_commit(const char *branchname, const char *refname,
>  			       int kinds, int force)
>  {
>  	struct commit *rev = lookup_commit_reference(the_repository, oid);
> -	if (!rev) {
> +	if (!force && !rev) {
>  		error(_("Couldn't look up commit object for '%s'"), refname);
>  		return -1;
>  	}
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index cc4b10236e..ec61a10c29 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1272,6 +1272,13 @@ test_expect_success 'attempt to delete a branch merged to its base' '
>  	test_must_fail git branch -d my10
>  '
>
> +test_expect_success 'branch --delete --force removes dangling branch' '
> +	test_when_finished "rm -f .git/refs/heads/dangling" &&
> +	echo $ZERO_OID >.git/refs/heads/dangling &&
> +	git branch --delete --force dangling &&
> +	test_path_is_missing .git/refs/heads/dangling
> +'

Isn't a more meaningful test here to use a "real" SHA, instead of the
$ZERO_OID? You can use $(test_oid deadbeef) to get one of those.

That way we know that this this test & logic is really testing that we
can delete a branch that's been racily GC'd away or whatever, and not
one in the already-broken state of referring to the $ZERO_OID.

Also: How does "git tag -d" handle this scenario if the same sort of
data were added to .git/refs/tags/* ?