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

[PATCH 1/5] t5312: test object deletion code paths in a corrupted repository

From
Jeff King <peff@peff.net>
Date
Mar 17, 2015, 07:28 UTC
Message-ID
<20150317072844.GA25191@peff.net>
In-Reply-To
<20150317072750.GA22155@peff.net>

When we are doing a destructive operation like "git prune", we want to be extra careful that the set of reachable tips we compute is valid. If there is any corruption or oddity, we are better off aborting the operation and letting the user figure things out rather than plowing ahead and possibly deleting some data that cannot be recovered.

The tests here include:
  1. Pruning objects mentioned only be refs with invalid
     names. This used to abort prior to d0f810f (refs.c:
     allow listing and deleting badly named refs,
     2014-09-03), but since then we silently ignore the tip.
     Likewise, we test repacking that can drop objects
     (either "-ad", which drops anything unreachable,
     or "-Ad --unpack-unreachable=<time>", which tries to
     optimize out a loose object write that would be
     directly pruned).
  2. Pruning objects when some refs point to missing
     objects. We don't know whether any dangling objects
     would have been reachable from the missing objects. We
     are better to keep them around, as they are better than
     nothing for helping the user recover history.
  3. Packed refs that point to missing objects can sometimes
     be dropped. By itself, this is more of an annoyance
     (you do not have the object anyway; even if you can
     recover it from elsewhere, all you are losing is a
     placeholder for your state at the time of corruption).
     But coupled with (2), if we drop the ref and then go
     on to prune, we may lose unrecoverable objects.

Note that we use test_might_fail for some of the operations. In some cases, it would be appropriate to abort the operation, and in others, it might be acceptable to continue but taking the information into account. The tests don't care either way, and check only for data loss.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5312-prune-corruption.sh | 104 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100755 t/t5312-prune-corruption.sh
diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh
new file mode 100755
index 0000000..167031e
--- /dev/null
+++ b/t/t5312-prune-corruption.sh
@@ -0,0 +1,104 @@
+#!/bin/sh
+
+test_description='
+Test pruning of repositories with minor corruptions. The goal
+here is that we should always be erring on the side of safety. So
+if we see, for example, a ref with a bogus name, it is OK either to
+bail out or to proceed using it as a reachable tip, but it is _not_
+OK to proceed as if it did not exist. Otherwise we might silently
+delete objects that cannot be recovered.
+'
+. ./test-lib.sh
+
+test_expect_success 'disable reflogs' '
+	git config core.logallrefupdates false &&
+	rm -rf .git/logs
+'
+
+test_expect_success 'create history reachable only from a bogus-named ref' '
+	test_tick && git commit --allow-empty -m master &&
+	base=$(git rev-parse HEAD) &&
+	test_tick && git commit --allow-empty -m bogus &&
+	bogus=$(git rev-parse HEAD) &&
+	git cat-file commit $bogus >saved &&
+	echo $bogus >.git/refs/heads/bogus:name &&
+	git reset --hard HEAD^
+'
+
+test_expect_failure 'pruning does not drop bogus object' '
+	test_when_finished "git hash-object -w -t commit saved" &&
+	test_might_fail git prune --expire=now &&
+	verbose git cat-file -e $bogus
+'
+
+test_expect_success 'put bogus object into pack' '
+	git tag reachable $bogus &&
+	git repack -ad &&
+	git tag -d reachable &&
+	verbose git cat-file -e $bogus
+'
+
+test_expect_failure 'destructive repack keeps packed object' '
+	test_might_fail git repack -Ad --unpack-unreachable=now &&
+	verbose git cat-file -e $bogus &&
+	test_might_fail git repack -ad &&
+	verbose git cat-file -e $bogus
+'
+
+# subsequent tests will have different corruptions
+test_expect_success 'clean up bogus ref' '
+	rm .git/refs/heads/bogus:name
+'
+
+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 &&
+	rm .git/objects/$(echo $missing | sed "s,..,&/,") &&
+	test_must_fail git cat-file -e $missing
+'
+
+test_expect_failure 'pruning with a corrupted tip does not drop history' '
+	test_when_finished "git hash-object -w -t commit saved" &&
+	test_might_fail git prune --expire=now &&
+	verbose git cat-file -e $recoverable
+'
+
+test_expect_success 'pack-refs does not silently delete broken loose ref' '
+	git pack-refs --all --prune &&
+	echo $missing >expect &&
+	git rev-parse refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+# 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
+	echo $missing >expect &&
+	git rev-parse refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'pack-refs does not silently delete broken packed ref' '
+	git pack-refs --all --prune &&
+	git rev-parse refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'pack-refs does not drop broken refs during deletion' '
+	git update-ref -d refs/heads/other &&
+	git rev-parse refs/heads/master >actual &&
+	test_cmp expect actual
+'
+
+test_done
-- 
2.3.3.520.g3cfbb5d
Previous: Jeff KingNext: Johannes Sixt
Message 2 of 35 in “not making corruption worse”
  1. 0/5 not making corruption worseJeff King, Mar 17, 2015
  2. 1/5 t5312: test object deletion code paths in a corrupted repositoryJeff King, Mar 17, 2015
  3. Johannes SixtMar 17, 2015
  4. Jeff KingMar 17, 2015
  5. Johannes SixtMar 18, 2015
  6. Junio C HamanoMar 19, 2015
  7. Jeff KingMar 19, 2015
  8. Junio C HamanoMar 19, 2015
  9. Jeff KingMar 19, 2015
  10. Junio C HamanoMar 19, 2015
  11. Jeff KingMar 19, 2015
  12. Eric SunshineMar 20, 2015
  13. Jeff KingMar 20, 2015
  14. Eric SunshineMar 20, 2015
  15. test &&-chain lint (was: [PATCH 1/5] t5312: test object deletion code paths in a corrupted repository)Jeff King, Mar 20, 2015
  16. Jeff KingMar 20, 2015
  17. Jeff KingMar 20, 2015
  18. Eric SunshineMar 20, 2015
  19. Junio C HamanoMar 20, 2015
  20. Junio C HamanoMar 20, 2015
  21. Jeff KingMar 20, 2015
  22. Junio C HamanoMar 20, 2015
  23. Jeff KingMar 20, 2015
  24. 2/5 refs: introduce a "ref paranoia" flagJeff King, Mar 17, 2015
  25. Junio C HamanoMar 19, 2015
  26. Jeff KingMar 19, 2015
  27. Junio C HamanoMar 19, 2015
  28. Jeff KingMar 19, 2015
  29. 3/5 prune: turn on ref_paranoia flagJeff King, Mar 17, 2015
  30. 4/5 repack: turn on "ref paranoia" when doing a destructive repackJeff King, Mar 17, 2015
  31. 5/5 refs.c: drop curate_packed_refsJeff King, Mar 17, 2015
  32. Eric SunshineMar 20, 2015
  33. Jeff KingMar 17, 2015
  34. Junio C HamanoMar 17, 2015
  35. Jeff KingMar 18, 2015

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.