On Thu, Mar 19, 2015 at 01:04:16PM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > +test_expect_success 'create history with missing tip commit' ' > > + test_tick && git commit --allow-empty -m one && > > + recoverable=$(git rev-parse HEAD) && > > + git cat-file commit $recoverable >saved && > > + test_tick && git commit --allow-empty -m two && > > + missing=$(git rev-parse HEAD) && > > + # point HEAD elsewhere > > + git checkout $base && > > Could you spell this as "$base^0" (or "--detach") to clarify the > intention? I have been scraching my head for a few minutes just > now, trying to figure out what you are doing here. I _think_ you > wanted master to point at the missing "two" and wanted to make sure > all other refs (including HEAD) to point away from it. Yes, exactly. I've squashed in your suggestion and added a comment explaining it: diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh index 1001a69..1cdbd9f 100755 --- a/t/t5312-prune-corruption.sh +++ b/t/t5312-prune-corruption.sh @@ -50,14 +50,24 @@ test_expect_success 'clean up bogus ref' ' rm .git/refs/heads/bogus..name ' +# We create two new objects here, "one" and "two". Our +# master branch points to "two", which is deleted, +# corrupting the repository. But we'd like to make sure +# that the otherwise unreachable "one" is not pruned +# (since it is the user's best bet for recovering +# from the corruption). +# +# Note that we also point HEAD somewhere besides "two", +# as we want to make sure we test the case where we +# pick up the reference to "two" by iterating the refs, +# not by resolving HEAD. test_expect_success 'create history with missing tip commit' ' test_tick && git commit --allow-empty -m one && recoverable=$(git rev-parse HEAD) && git cat-file commit $recoverable >saved && test_tick && git commit --allow-empty -m two && missing=$(git rev-parse HEAD) && - # point HEAD elsewhere - git checkout $base && + git checkout --detach $base && rm .git/objects/$(echo $missing | sed "s,..,&/,") && test_must_fail git cat-file -e $missing ' > > +# we do not want to count on running pack-refs to > > +# actually pack it, as it is perfectly reasonable to > > +# skip processing a broken ref > > +test_expect_success 'create packed-refs file with broken ref' ' > > + rm -f .git/refs/heads/master && > > + cat >.git/packed-refs <<-EOF > > + $missing refs/heads/master > > + $recoverable refs/heads/other > > + EOF > > I do not know offhand if the lack of the pack-refs feature header > matters here; I assume it does not? It doesn't matter. We also do similarly gross things in other corruption-related tests, but I suspect if you git-blamed them all you would find that I am responsible. :) > A safer check may be to pack and then make it missing, I guess, but > I do not know if the difference matters. Yeah, I considered that. The trouble is that we are relying on the earlier setup that made the object go missing. We cannot pack the refs in the setup step, because the earlier tests are checking the loose-ref behavior. So we would have to actually restore the object, pack, and then re-delete it. Another option would be to restructure the whole test script to perform each individual corruption in its own sub-repo. I thought that would end up making things harder to understand due to the extra setup boilerplate, but it would make the tests less fragile with respect to each other (e.g., see the "clean up bogus ref" step which exists only to clean up our earlier corruption that could influence later tests). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html