refs: omit dangling symrefs when using GIT_REF_PARANOIA

Dangling symrefs aren't actually a corruption problem. It's perfectly
fine for refs/remotes/origin/HEAD to point to an unborn branch. And in
particular, if you are trying to establish reachability, a symref that
points nowhere doesn't matter either way. Any ref it could point to will
be examined during the rest of the traversal.

It's possible that a symref pointing nowhere _could_ be a sign that the
ref it was meant to point to was deleted accidentally (e.g., via
corruption). But there is no particular reason to think that is true for
any given case, and in the meantime, GIT_REF_PARANOIA kicking in
automatically for some operations means they'll fail unnecessarily.

So let's loosen it just a bit. The new test in t5312 shows off an
example that is safe, but currently fails (and no longer does after this
patch).

Note that we don't do anything if the caller explicitly asked for
DO_FOR_EACH_INCLUDE_BROKEN. In that case they may be looking for
dangling symrefs themselves, and setting GIT_REF_PARANOIA should not
_loosen_ things from what the caller asked for.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Jeff King 2021-09-24 14:42:38 -04:00 committed by Junio C Hamano
parent 8dccb2244c
commit 6d751be4b6
2 changed files with 15 additions and 4 deletions

12
refs.c
View File

@ -1418,10 +1418,14 @@ struct ref_iterator *refs_ref_iterator_begin(
{ {
struct ref_iterator *iter; struct ref_iterator *iter;
if (ref_paranoia < 0) if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) {
ref_paranoia = git_env_bool("GIT_REF_PARANOIA", 0); if (ref_paranoia < 0)
if (ref_paranoia) ref_paranoia = git_env_bool("GIT_REF_PARANOIA", 0);
flags |= DO_FOR_EACH_INCLUDE_BROKEN; if (ref_paranoia) {
flags |= DO_FOR_EACH_INCLUDE_BROKEN;
flags |= DO_FOR_EACH_OMIT_DANGLING_SYMREFS;
}
}
iter = refs->be->iterator_begin(refs, prefix, flags); iter = refs->be->iterator_begin(refs, prefix, flags);

View File

@ -62,6 +62,13 @@ test_expect_success 'destructive repack keeps packed object' '
git cat-file -e $bogus git cat-file -e $bogus
' '
test_expect_success 'destructive repack not confused by dangling symref' '
test_when_finished "git symbolic-ref -d refs/heads/dangling" &&
git symbolic-ref refs/heads/dangling refs/heads/does-not-exist &&
git repack -ad &&
test_must_fail git cat-file -e $bogus
'
# We create two new objects here, "one" and "two". Our # We create two new objects here, "one" and "two". Our
# main branch points to "two", which is deleted, # main branch points to "two", which is deleted,
# corrupting the repository. But we'd like to make sure # corrupting the repository. But we'd like to make sure