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

["René Scharfe" <l.s.r@web.de>]

Am 26.08.21 um 21:05 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> +	hash=$(git rev-parse HEAD) &&
>> +	objpath=$(echo $hash | sed -e "s|^..|.git/objects/&/|") &&
>> +	git branch --no-track dangling &&
>> +	test_when_finished "test -f $objpath.x && mv $objpath.x $objpath" &&
>
> Do we need test -f here?

If the mv in the next line fails, then test in the cleanup prevents it
from adding another confusing error.  So it's not really needed, but
kinda nice to have.

>> +	mv $objpath $objpath.x &&
>> +	git branch --delete --force dangling &&
>
>> +	test -z "$(git for-each-ref refs/heads/dangling)"
>
> It is not wrong per-se, but maybe
>
> 	git show-ref --quiet refs/heads/dangling
>
> is more straight-forward.

Actually it *is* wrong, because that check passes even if the dangling
ref still exists due to for-each-ref checking if the ref target exists
and just erroring out if it doesn't.  I somehow assumed it wouldn't do
this needless verification.  So we'd need to check its return value:

	git for-each-ref refs/heads/dangling >actual &&
	test_must_be_empty actual

git show-ref fails both if the ref is missing and if it's dangling, so
we'd need to check its stderr to distinguish between those cases:

	test_must_fail git show-ref --quiet refs/heads/dangling 2>err &&
	test_must_be_empty err

To avoid these complications we could ask git branch itself:

	test -z $(git branch --list dangling)

René