[PATCH 11/16] refs: omit dangling symrefs when using GIT_REF_PARANOIA
- From
Jeff King <peff@peff.net>
- Date
- Sep 24, 2021, 18:42 UTC
- Message-ID
- <YU4cHlGaEwBzTJ0k@coredump.intra.peff.net>
- In-Reply-To
- <YU4ZOF9+ubmoItmK@coredump.intra.peff.net>
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> --- refs.c | 12 ++++++++---- t/t5312-prune-corruption.sh | 7 +++++++ 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c index c28bd6a818..e72510813e 100644 --- a/refs.c +++ b/refs.c @@ -1418,10 +1418,14 @@ struct ref_iterator *refs_ref_iterator_begin( { struct ref_iterator *iter; - if (ref_paranoia < 0) - ref_paranoia = git_env_bool("GIT_REF_PARANOIA", 0); - if (ref_paranoia) - flags |= DO_FOR_EACH_INCLUDE_BROKEN; + if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) { + if (ref_paranoia < 0) + ref_paranoia = git_env_bool("GIT_REF_PARANOIA", 0); + if (ref_paranoia) { + flags |= DO_FOR_EACH_INCLUDE_BROKEN; + flags |= DO_FOR_EACH_OMIT_DANGLING_SYMREFS; + } + } iter = refs->be->iterator_begin(refs, prefix, flags); diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh index 1522a4ba8e..d8ec5a7462 100755 --- a/t/t5312-prune-corruption.sh +++ b/t/t5312-prune-corruption.sh @@ -62,6 +62,13 @@ test_expect_success 'destructive repack keeps packed object' ' 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 # main branch points to "two", which is deleted, # corrupting the repository. But we'd like to make sure
-- 2.33.0.1071.gb37e412355