Merge branch 'jk/fsck-exit-code-fix'

"git fsck" failed to report that it found corrupt objects via its
exit status in some cases.

* jk/fsck-exit-code-fix:
  fsck: return non-zero status on missing ref tips
  fsck: exit with non-zero status upon error from fsck_obj()

2 files changed, 83 insertions, 7 deletions

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 8c739c9613..b52397afd3 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -69,7 +69,7 @@ test_expect_success 'object with bad sha1' '
 	git update-ref refs/heads/bogus $cmt &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
 
-	test_might_fail git fsck 2>out &&
+	test_must_fail git fsck 2>out &&
 	cat out &&
 	grep "$sha.*corrupt" out
 '
@@ -101,7 +101,7 @@ test_expect_success 'email with embedded > is not okay' '
 	test_when_finished "remove_object $new" &&
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
-	git fsck 2>out &&
+	test_must_fail git fsck 2>out &&
 	cat out &&
 	grep "error in commit $new" out
 '
@@ -113,7 +113,7 @@ test_expect_success 'missing < email delimiter is reported nicely' '
 	test_when_finished "remove_object $new" &&
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
-	git fsck 2>out &&
+	test_must_fail git fsck 2>out &&
 	cat out &&
 	grep "error in commit $new.* - bad name" out
 '
@@ -125,7 +125,7 @@ test_expect_success 'missing email is reported nicely' '
 	test_when_finished "remove_object $new" &&
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
-	git fsck 2>out &&
+	test_must_fail git fsck 2>out &&
 	cat out &&
 	grep "error in commit $new.* - missing email" out
 '
@@ -137,7 +137,7 @@ test_expect_success '> in name is reported' '
 	test_when_finished "remove_object $new" &&
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
-	git fsck 2>out &&
+	test_must_fail git fsck 2>out &&
 	cat out &&
 	grep "error in commit $new" out
 '
@@ -151,11 +151,31 @@ test_expect_success 'integer overflow in timestamps is reported' '
 	test_when_finished "remove_object $new" &&
 	git update-ref refs/heads/bogus "$new" &&
 	test_when_finished "git update-ref -d refs/heads/bogus" &&
-	git fsck 2>out &&
+	test_must_fail git fsck 2>out &&
 	cat out &&
 	grep "error in commit $new.*integer overflow" out
 '
 
+test_expect_success 'malformatted tree object' '
+	test_when_finished "git update-ref -d refs/tags/wrong" &&
+	test_when_finished "remove_object \$T" &&
+	T=$(
+		GIT_INDEX_FILE=test-index &&
+		export GIT_INDEX_FILE &&
+		rm -f test-index &&
+		>x &&
+		git add x &&
+		T=$(git write-tree) &&
+		(
+			git cat-file tree $T &&
+			git cat-file tree $T
+		) |
+		git hash-object -w -t tree --stdin
+	) &&
+	test_must_fail git fsck 2>out &&
+	grep "error in tree .*contains duplicate file entries" out
+'
+
 test_expect_success 'tag pointing to nonexistent' '
 	cat >invalid-tag <<-\EOF &&
 	object ffffffffffffffffffffffffffffffffffffffff
@@ -282,4 +302,60 @@ test_expect_success 'fsck notices ".git" in trees' '
 	)
 '
 
+# create a static test repo which is broken by omitting
+# one particular object ($1, which is looked up via rev-parse
+# in the new repository).
+create_repo_missing () {
+	rm -rf missing &&
+	git init missing &&
+	(
+		cd missing &&
+		git commit -m one --allow-empty &&
+		mkdir subdir &&
+		echo content >subdir/file &&
+		git add subdir/file &&
+		git commit -m two &&
+		unrelated=$(echo unrelated | git hash-object --stdin -w) &&
+		git tag -m foo tag $unrelated &&
+		sha1=$(git rev-parse --verify "$1") &&
+		path=$(echo $sha1 | sed 's|..|&/|') &&
+		rm .git/objects/$path
+	)
+}
+
+test_expect_success 'fsck notices missing blob' '
+	create_repo_missing HEAD:subdir/file &&
+	test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices missing subtree' '
+	create_repo_missing HEAD:subdir &&
+	test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices missing root tree' '
+	create_repo_missing HEAD^{tree} &&
+	test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices missing parent' '
+	create_repo_missing HEAD^ &&
+	test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices missing tagged object' '
+	create_repo_missing tag^{blob} &&
+	test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices ref pointing to missing commit' '
+	create_repo_missing HEAD &&
+	test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices ref pointing to missing tag' '
+	create_repo_missing tag &&
+	test_must_fail git -C missing fsck
+'
+
 test_done
diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh
index 58b792bf20..67bd8ec020 100755
--- a/t/t4212-log-corrupt.sh
+++ b/t/t4212-log-corrupt.sh
@@ -14,7 +14,7 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'fsck notices broken commit' '
-	git fsck 2>actual &&
+	test_must_fail git fsck 2>actual &&
 	test_i18ngrep invalid.author actual
 '