From: Junio C Hamano Date: Fri, 26 Sep 2014 21:39:43 +0000 (-0700) Subject: Merge branch 'js/fsck-tag-validation' X-Git-Tag: v2.2.0-rc0~89 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/13f4f046929de00a8c16171c5e08cdcae887b54d?hp=-c Merge branch 'js/fsck-tag-validation' Teach "git fsck" to inspect the contents of annotated tag objects. * js/fsck-tag-validation: Make sure that index-pack --strict checks tag objects Add regression tests for stricter tag fsck'ing fsck: check tag objects' headers Make sure fsck_commit_buffer() does not run out of the buffer fsck_object(): allow passing object data separately from the object itself Refactor type_from_string() to allow continuing after detecting an error --- 13f4f046929de00a8c16171c5e08cdcae887b54d diff --combined builtin/fsck.c index 0928a98a71,d9f4e6e38c..e9ba576c1f --- a/builtin/fsck.c +++ b/builtin/fsck.c @@@ -298,7 -298,7 +298,7 @@@ static int fsck_obj(struct object *obj if (fsck_walk(obj, mark_used, NULL)) objerror(obj, "broken links"); - if (fsck_object(obj, check_strict, fsck_error_func)) + if (fsck_object(obj, NULL, 0, check_strict, fsck_error_func)) return -1; if (obj->type == OBJ_TREE) { @@@ -388,8 -388,7 +388,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; @@@ -489,7 -488,6 +489,7 @@@ static int fsck_handle_ref(const char * 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; } @@@ -506,7 -504,7 +506,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 --combined builtin/index-pack.c index eebf1a8fc2,f2465ff18e..f89df017c2 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@@ -112,10 -112,6 +112,10 @@@ static pthread_mutex_t deepest_delta_mu #define deepest_delta_lock() lock_mutex(&deepest_delta_mutex) #define deepest_delta_unlock() unlock_mutex(&deepest_delta_mutex) +static pthread_mutex_t type_cas_mutex; +#define type_cas_lock() lock_mutex(&type_cas_mutex) +#define type_cas_unlock() unlock_mutex(&type_cas_mutex) + static pthread_key_t key; static inline void lock_mutex(pthread_mutex_t *mutex) @@@ -139,7 -135,6 +139,7 @@@ static void init_thread(void init_recursive_mutex(&read_mutex); pthread_mutex_init(&counter_mutex, NULL); pthread_mutex_init(&work_mutex, NULL); + pthread_mutex_init(&type_cas_mutex, NULL); if (show_stat) pthread_mutex_init(&deepest_delta_mutex, NULL); pthread_key_create(&key, NULL); @@@ -162,7 -157,6 +162,7 @@@ static void cleanup_thread(void pthread_mutex_destroy(&read_mutex); pthread_mutex_destroy(&counter_mutex); pthread_mutex_destroy(&work_mutex); + pthread_mutex_destroy(&type_cas_mutex); if (show_stat) pthread_mutex_destroy(&deepest_delta_mutex); for (i = 0; i < nr_threads; i++) @@@ -779,7 -773,8 +779,8 @@@ static void sha1_object(const void *dat if (!obj) die(_("invalid %s"), typename(type)); if (do_fsck_object && - fsck_object(obj, 1, fsck_error_function)) + fsck_object(obj, buf, size, 1, + fsck_error_function)) die(_("Error in object")); if (fsck_walk(obj, mark_link, NULL)) die(_("Not all child objects of %s are reachable"), sha1_to_hex(obj->sha1)); @@@ -868,6 -863,7 +869,6 @@@ static void resolve_delta(struct object { void *base_data, *delta_data; - delta_obj->real_type = base->obj->real_type; if (show_stat) { delta_obj->delta_depth = base->obj->delta_depth + 1; deepest_delta_lock(); @@@ -893,26 -889,6 +894,26 @@@ counter_unlock(); } +/* + * Standard boolean compare-and-swap: atomically check whether "*type" is + * "want"; if so, swap in "set" and return true. Otherwise, leave it untouched + * and return false. + */ +static int compare_and_swap_type(enum object_type *type, + enum object_type want, + enum object_type set) +{ + enum object_type old; + + type_cas_lock(); + old = *type; + if (old == want) + *type = set; + type_cas_unlock(); + + return old == want; +} + static struct base_data *find_unresolved_deltas_1(struct base_data *base, struct base_data *prev_base) { @@@ -940,10 -916,7 +941,10 @@@ struct object_entry *child = objects + deltas[base->ref_first].obj_no; struct base_data *result = alloc_base_data(); - assert(child->real_type == OBJ_REF_DELTA); + if (!compare_and_swap_type(&child->real_type, OBJ_REF_DELTA, + base->obj->real_type)) + die("BUG: child->real_type != OBJ_REF_DELTA"); + resolve_delta(child, base, result); if (base->ref_first == base->ref_last && base->ofs_last == -1) free_base_data(base); @@@ -957,7 -930,6 +958,7 @@@ struct base_data *result = alloc_base_data(); assert(child->real_type == OBJ_OFS_DELTA); + child->real_type = base->obj->real_type; resolve_delta(child, base, result); if (base->ofs_first == base->ofs_last) free_base_data(base); diff --combined t/t1450-fsck.sh index b52397afd3,1b96b4045b..c23408ec07 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@@ -69,7 -69,7 +69,7 @@@ test_expect_success 'object with bad sh 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 +101,7 @@@ test_expect_success 'email with embedde 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 +113,7 @@@ test_expect_success 'missing < email de 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 +125,7 @@@ test_expect_success 'missing email is r 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 +137,7 @@@ test_expect_success '> in name is repor 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,31 -151,11 +151,31 @@@ test_expect_success 'integer overflow i 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 @@@ -214,6 -194,25 +214,25 @@@ test_expect_success 'tag pointing to so test_must_fail git fsck --tags ' + test_expect_success 'tag with incorrect tag name & missing tagger' ' + sha=$(git rev-parse HEAD) && + cat >wrong-tag <<-EOF && + object $sha + type commit + tag wrong name format + + This is an invalid tag. + EOF + + tag=$(git hash-object -t tag -w --stdin .git/refs/tags/wrong && + test_when_finished "git update-ref -d refs/tags/wrong" && + git fsck --tags 2>out && + grep "invalid .tag. name" out && + grep "expected .tagger. line" out + ' + test_expect_success 'cleaned up' ' git fsck >actual 2>&1 && test_cmp empty actual @@@ -302,60 -301,4 +321,60 @@@ test_expect_success 'fsck notices ".git ) ' +# 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