Am 26.08.21 um 21:05 schrieb Junio C Hamano: > René Scharfe <l.s.r@xxxxxx> 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é
