From 2e770fe47ef9c0b20bc687e37f3eb50f1bf919d0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 29 Aug 2014 16:31:46 -0400 Subject: fsck: exit with non-zero status upon error from fsck_obj() Upon finding a corrupt loose object, we forgot to note the error to signal it with the exit status of the entire process. [jc: adjusted t1450 and added another test] Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/fsck.c b/builtin/fsck.c index 1affdd5..8abe644 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -389,7 +389,8 @@ static void fsck_sha1_list(void) unsigned char *sha1 = entry->sha1; sha1_list.entry[i] = NULL; - fsck_sha1(sha1); + if (fsck_sha1(sha1)) + errors_found |= ERROR_OBJECT; free(entry); } sha1_list.nr = 0; diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 8c739c9..0de755c 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 diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh index 58b792b..67bd8ec 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 ' -- cgit v0.10.2-6-g49f6 From 30d1038d1bd538bf2f8abfb5f5dccf80327a26c9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Sep 2014 23:38:30 -0400 Subject: fsck: return non-zero status on missing ref tips Fsck tries hard to detect missing objects, and will complain (and exit non-zero) about any inter-object links that are missing. However, it will not exit non-zero for any missing ref tips, meaning that a severely broken repository may still pass "git fsck && echo ok". The problem is that we use for_each_ref to iterate over the ref tips, which hides broken tips. It does at least print an error from the refs.c code, but fsck does not ever see the ref and cannot note the problem in its exit code. We can solve this by using for_each_rawref and noting the error ourselves. In addition to adding tests for this case, we add tests for all types of missing-object links (all of which worked, but which we were not testing). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/fsck.c b/builtin/fsck.c index 8abe644..ca3018c 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -495,6 +495,7 @@ static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int f obj = parse_object(sha1); if (!obj) { error("%s: invalid sha1 pointer %s", refname, sha1_to_hex(sha1)); + errors_found |= ERROR_REACHABLE; /* We'll continue with the rest despite the error.. */ return 0; } @@ -511,7 +512,7 @@ static void get_default_heads(void) { if (head_points_at && !is_null_sha1(head_sha1)) fsck_handle_ref("HEAD", head_sha1, 0, NULL); - for_each_ref(fsck_handle_ref, NULL); + for_each_rawref(fsck_handle_ref, NULL); if (include_reflogs) for_each_reflog(fsck_handle_reflog, NULL); diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 0de755c..b52397a 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -302,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 -- cgit v0.10.2-6-g49f6