git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 03/16] t5600: provide detached HEAD for corruption failures

From
Jeff King <peff@peff.net>
Date
Sep 24, 2021, 18:34 UTC
Message-ID
<YU4aHBX/cMht6rRX@coredump.intra.peff.net>
In-Reply-To
<YU4ZOF9+ubmoItmK@coredump.intra.peff.net>

When checking how git-clone behaves when it fails, we stimulate some failures by trying to do a clone from a local repository whose objects have been removed. Because these clones use local optimizations, there's a subtle dependency in how the corruption is handled on the sending side.

If upload-pack does not show us the broken refs (which it does not currently), then we see only HEAD (which is itself broken), and clone that as a detached HEAD. When we try to write the ref, we notice that we never got the object and bail.

But if upload-pack _does_ show us the broken refs (which it may in a future patch), then we'll realize that HEAD is a symref and just write that. You'd think we'd fail when writing out the refs themselves, but we don't; we do a bulk write and skip the connectivity check because of our --local optimizations. For the non-bare case, we do notice the problem when we try to checkout. But for a bare repository, we unexpectedly complete the clone successfully!

At first glance this may seem like a bug. But the whole point of those local optimizations is to give up some safety for speed. If you want to be careful, you should be using "--no-local", which would notice that the pack did not transfer sufficient objects. We could do that in these tests, but part of the point is for them to fail at specific moments (and indeed, we have a later test that checks for transport failure).

However, we can make this less subtle and future-proof it against changes on the upload-pack side by just having an explicit detached HEAD in the corrupted repo. Now we'll fail as expected during the ref write if any ref _or_ HEAD is corrupt, whether we're --bare or not.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5600-clone-fail-cleanup.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh
index 5bf10261d3..34b3df4027 100755
--- a/t/t5600-clone-fail-cleanup.sh
+++ b/t/t5600-clone-fail-cleanup.sh
@@ -35,7 +35,9 @@ test_expect_success 'create a repo to clone' '
 '
 
 test_expect_success 'create objects in repo for later corruption' '
-	test_commit -C foo file
+	test_commit -C foo file &&
+	git -C foo checkout --detach &&
+	test_commit -C foo detached
 '
 
 # source repository given to git clone should be relative to the
-- 
2.33.0.1071.gb37e412355
Previous: Jeff KingNext: Jeff King
Message 6 of 22 in “enabling GIT_REF_PARANOIA by default”
  1. 0/16 enabling GIT_REF_PARANOIA by defaultJeff King, Sep 24, 2021
  2. 01/16 t7900: clean up some more broken refsJeff King, Sep 24, 2021
  3. Jonathan TanSep 27, 2021
  4. Jeff KingSep 27, 2021
  5. 02/16 t5516: don't use HEAD ref for invalid ref-deletion testsJeff King, Sep 24, 2021
  6. 03/16 t5600: provide detached HEAD for corruption failuresJeff King, Sep 24, 2021
  7. 04/16 t5312: drop "verbose" helperJeff King, Sep 24, 2021
  8. 05/16 t5312: create bogus ref as necessaryJeff King, Sep 24, 2021
  9. 06/16 t5312: test non-destructive repackJeff King, Sep 24, 2021
  10. 07/16 t5312: be more assertive about command failureJeff King, Sep 24, 2021
  11. 08/16 refs-internal.h: move DO_FOR_EACH_* flags next to each otherJeff King, Sep 24, 2021
  12. 09/16 refs-internal.h: reorganize DO_FOR_EACH_* flag documentationJeff King, Sep 24, 2021
  13. 10/16 refs: add DO_FOR_EACH_OMIT_DANGLING_SYMREFS flagJeff King, Sep 24, 2021
  14. 11/16 refs: omit dangling symrefs when using GIT_REF_PARANOIAJeff King, Sep 24, 2021
  15. 12/16 refs: turn on GIT_REF_PARANOIA by defaultJeff King, Sep 24, 2021
  16. Jonathan TanSep 27, 2021
  17. 13/16 repack, prune: drop GIT_REF_PARANOIA settingsJeff King, Sep 24, 2021
  18. 14/16 ref-filter: stop setting FILTER_REFS_INCLUDE_BROKENJeff King, Sep 24, 2021
  19. 15/16 ref-filter: drop broken-ref code entirelyJeff King, Sep 24, 2021
  20. 16/16 refs: drop "broken" flag from for_each_fullref_in()Jeff King, Sep 24, 2021
  21. Jonathan TanSep 27, 2021
  22. Junio C HamanoSep 24, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.