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

Am 26.08.21 um 21:05 schrieb Junio C Hamano:

René Scharfe [off-list ref] writes:

quoted

+	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.

quoted

+	mv $objpath $objpath.x &&
+	git branch --delete --force dangling &&

quoted

+	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é