Merge branch 'js/fsck-tag-validation'
authorJunio C Hamano <gitster@pobox.com>
Fri, 26 Sep 2014 21:39:43 +0000 (14:39 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 26 Sep 2014 21:39:43 +0000 (14:39 -0700)
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

1  2 
builtin/fsck.c
builtin/index-pack.c
t/t1450-fsck.sh
diff --combined builtin/fsck.c
index 0928a98a71ca2b764d4fbb59976f1c4bdc421dde,d9f4e6e38c9c0a98a0340c60dc1af64853303918..e9ba576c1fe85cab505eb13fc20ae68dcfc19ff3
@@@ -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 eebf1a8fc21fd551f3917ad23ab5a45dff302f23,f2465ff18e413e37e92a4828429effa6bd566707..f89df017c200fbf7e2d5ec016d5f85975f1e7109
@@@ -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();
        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)
  {
                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);
                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 b52397afd3352b873c9c7fabb91eb850aaf0db3b,1b96b4045bd270c5f3af2dd10d126d7b4b4ce706..c23408ec07ae0743504806fcfbb43a41c643006d
@@@ -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 <wrong-tag) &&
+       test_when_finished "remove_object $tag" &&
+       echo $tag >.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