Merge branch 'jk/fsck-connectivity-check-fix'
authorJunio C Hamano <gitster@pobox.com>
Tue, 31 Jan 2017 21:15:01 +0000 (13:15 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 31 Jan 2017 21:15:01 +0000 (13:15 -0800)
"git fsck --connectivity-check" was not working at all.

* jk/fsck-connectivity-check-fix:
fsck: lazily load types under --connectivity-only
fsck: move typename() printing to its own function
t1450: use "mv -f" within loose object directory
fsck: check HAS_OBJ more consistently
fsck: do not fallback "git fsck <bogus>" to "git fsck"
fsck: tighten error-checks of "git fsck <head>"
fsck: prepare dummy objects for --connectivity-check
fsck: report trees as dangling
t1450: clean up sub-objects in duplicate-entry test

builtin/fsck.c
fsck.c
t/t1450-fsck.sh
index 4b91ee95e66396285d2eb8e67ab597e84d76fab7..1a5caccd0f5ee3fa56af06e8f536271f58312665 100644 (file)
@@ -56,6 +56,23 @@ static const char *describe_object(struct object *obj)
        return buf.buf;
 }
 
+static const char *printable_type(struct object *obj)
+{
+       const char *ret;
+
+       if (obj->type == OBJ_NONE) {
+               enum object_type type = sha1_object_info(obj->oid.hash, NULL);
+               if (type > 0)
+                       object_as_type(obj, type, 0);
+       }
+
+       ret = typename(obj->type);
+       if (!ret)
+               ret = "unknown";
+
+       return ret;
+}
+
 static int fsck_config(const char *var, const char *value, void *cb)
 {
        if (strcmp(var, "fsck.skiplist") == 0) {
@@ -83,7 +100,7 @@ static void objreport(struct object *obj, const char *msg_type,
                        const char *err)
 {
        fprintf(stderr, "%s in %s %s: %s\n",
-               msg_type, typename(obj->type), describe_object(obj), err);
+               msg_type, printable_type(obj), describe_object(obj), err);
 }
 
 static int objerror(struct object *obj, const char *err)
@@ -114,7 +131,7 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt
        if (!obj) {
                /* ... these references to parent->fld are safe here */
                printf("broken link from %7s %s\n",
-                          typename(parent->type), describe_object(parent));
+                          printable_type(parent), describe_object(parent));
                printf("broken link from %7s %s\n",
                           (type == OBJ_ANY ? "unknown" : typename(type)), "unknown");
                errors_found |= ERROR_REACHABLE;
@@ -131,9 +148,9 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt
        if (!(obj->flags & HAS_OBJ)) {
                if (parent && !has_object_file(&obj->oid)) {
                        printf("broken link from %7s %s\n",
-                                typename(parent->type), describe_object(parent));
+                                printable_type(parent), describe_object(parent));
                        printf("              to %7s %s\n",
-                                typename(obj->type), describe_object(obj));
+                                printable_type(obj), describe_object(obj));
                        errors_found |= ERROR_REACHABLE;
                }
                return 1;
@@ -205,9 +222,7 @@ static void check_reachable_object(struct object *obj)
        if (!(obj->flags & HAS_OBJ)) {
                if (has_sha1_pack(obj->oid.hash))
                        return; /* it is in pack - forget about it */
-               if (connectivity_only && has_object_file(&obj->oid))
-                       return;
-               printf("missing %s %s\n", typename(obj->type),
+               printf("missing %s %s\n", printable_type(obj),
                        describe_object(obj));
                errors_found |= ERROR_REACHABLE;
                return;
@@ -225,7 +240,7 @@ static void check_unreachable_object(struct object *obj)
         * to complain about it being unreachable (since it does
         * not exist).
         */
-       if (!obj->parsed)
+       if (!(obj->flags & HAS_OBJ))
                return;
 
        /*
@@ -233,7 +248,7 @@ static void check_unreachable_object(struct object *obj)
         * since this is something that is prunable.
         */
        if (show_unreachable) {
-               printf("unreachable %s %s\n", typename(obj->type),
+               printf("unreachable %s %s\n", printable_type(obj),
                        describe_object(obj));
                return;
        }
@@ -252,7 +267,7 @@ static void check_unreachable_object(struct object *obj)
         */
        if (!obj->used) {
                if (show_dangling)
-                       printf("dangling %s %s\n", typename(obj->type),
+                       printf("dangling %s %s\n", printable_type(obj),
                               describe_object(obj));
                if (write_lost_and_found) {
                        char *filename = git_pathdup("lost-found/%s/%s",
@@ -326,7 +341,7 @@ static int fsck_obj(struct object *obj)
 
        if (verbose)
                fprintf(stderr, "Checking %s %s\n",
-                       typename(obj->type), describe_object(obj));
+                       printable_type(obj), describe_object(obj));
 
        if (fsck_walk(obj, NULL, &fsck_obj_options))
                objerror(obj, "broken links");
@@ -352,7 +367,7 @@ static int fsck_obj(struct object *obj)
                struct tag *tag = (struct tag *) obj;
 
                if (show_tags && tag->tagged) {
-                       printf("tagged %s %s", typename(tag->tagged->type),
+                       printf("tagged %s %s", printable_type(tag->tagged),
                                describe_object(tag->tagged));
                        printf(" (%s) in %s\n", tag->tag,
                                describe_object(&tag->object));
@@ -388,7 +403,7 @@ static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
 
        if (!is_null_sha1(sha1)) {
                obj = lookup_object(sha1);
-               if (obj) {
+               if (obj && (obj->flags & HAS_OBJ)) {
                        if (timestamp && name_objects)
                                add_decoration(fsck_walk_options.object_names,
                                        obj,
@@ -604,6 +619,29 @@ static int fsck_cache_tree(struct cache_tree *it)
        return err;
 }
 
+static void mark_object_for_connectivity(const unsigned char *sha1)
+{
+       struct object *obj = lookup_unknown_object(sha1);
+       obj->flags |= HAS_OBJ;
+}
+
+static int mark_loose_for_connectivity(const unsigned char *sha1,
+                                      const char *path,
+                                      void *data)
+{
+       mark_object_for_connectivity(sha1);
+       return 0;
+}
+
+static int mark_packed_for_connectivity(const unsigned char *sha1,
+                                       struct packed_git *pack,
+                                       uint32_t pos,
+                                       void *data)
+{
+       mark_object_for_connectivity(sha1);
+       return 0;
+}
+
 static char const * const fsck_usage[] = {
        N_("git fsck [<options>] [<object>...]"),
        NULL
@@ -660,38 +698,41 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
        git_config(fsck_config, NULL);
 
        fsck_head_link();
-       if (!connectivity_only) {
+       if (connectivity_only) {
+               for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
+               for_each_packed_object(mark_packed_for_connectivity, NULL, 0);
+       } else {
                fsck_object_dir(get_object_directory());
 
                prepare_alt_odb();
                for (alt = alt_odb_list; alt; alt = alt->next)
                        fsck_object_dir(alt->path);
-       }
 
-       if (check_full) {
-               struct packed_git *p;
-               uint32_t total = 0, count = 0;
-               struct progress *progress = NULL;
+               if (check_full) {
+                       struct packed_git *p;
+                       uint32_t total = 0, count = 0;
+                       struct progress *progress = NULL;
 
-               prepare_packed_git();
+                       prepare_packed_git();
 
-               if (show_progress) {
+                       if (show_progress) {
+                               for (p = packed_git; p; p = p->next) {
+                                       if (open_pack_index(p))
+                                               continue;
+                                       total += p->num_objects;
+                               }
+
+                               progress = start_progress(_("Checking objects"), total);
+                       }
                        for (p = packed_git; p; p = p->next) {
-                               if (open_pack_index(p))
-                                       continue;
-                               total += p->num_objects;
+                               /* verify gives error messages itself */
+                               if (verify_pack(p, fsck_obj_buffer,
+                                               progress, count))
+                                       errors_found |= ERROR_PACK;
+                               count += p->num_objects;
                        }
-
-                       progress = start_progress(_("Checking objects"), total);
+                       stop_progress(&progress);
                }
-               for (p = packed_git; p; p = p->next) {
-                       /* verify gives error messages itself */
-                       if (verify_pack(p, fsck_obj_buffer,
-                                       progress, count))
-                               errors_found |= ERROR_PACK;
-                       count += p->num_objects;
-               }
-               stop_progress(&progress);
        }
 
        heads = 0;
@@ -701,9 +742,11 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
                if (!get_sha1(arg, sha1)) {
                        struct object *obj = lookup_object(sha1);
 
-                       /* Error is printed by lookup_object(). */
-                       if (!obj)
+                       if (!obj || !(obj->flags & HAS_OBJ)) {
+                               error("%s: object missing", sha1_to_hex(sha1));
+                               errors_found |= ERROR_OBJECT;
                                continue;
+                       }
 
                        obj->used = 1;
                        if (name_objects)
@@ -714,6 +757,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
                        continue;
                }
                error("invalid parameter: expected sha1, got '%s'", arg);
+               errors_found |= ERROR_OBJECT;
        }
 
        /*
@@ -721,7 +765,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
         * default ones from .git/refs. We also consider the index file
         * in this case (ie this implies --cache).
         */
-       if (!heads) {
+       if (!argc) {
                get_default_heads();
                keep_cache_objects = 1;
        }
diff --git a/fsck.c b/fsck.c
index 4a3069e204ea555bfb6d1ce05a0163860f852437..939792752bf39c72cc536f00c21e2af8ed854c3e 100644 (file)
--- a/fsck.c
+++ b/fsck.c
@@ -458,6 +458,10 @@ int fsck_walk(struct object *obj, void *data, struct fsck_options *options)
 {
        if (!obj)
                return -1;
+
+       if (obj->type == OBJ_NONE)
+               parse_object(obj->oid.hash);
+
        switch (obj->type) {
        case OBJ_BLOB:
                return 0;
index 8975b4d1bcc4675be4d1869783ee4b7cc17c750f..33a51c9a67fe833e31e51099f7568b64be385d07 100755 (executable)
@@ -189,14 +189,16 @@ test_expect_success 'commit with NUL in header' '
 '
 
 test_expect_success 'tree object with duplicate entries' '
-       test_when_finished "remove_object \$T" &&
+       test_when_finished "for i in \$T; do remove_object \$i; done" &&
        T=$(
                GIT_INDEX_FILE=test-index &&
                export GIT_INDEX_FILE &&
                rm -f test-index &&
                >x &&
                git add x &&
+               git rev-parse :x &&
                T=$(git write-tree) &&
+               echo $T &&
                (
                        git cat-file tree $T &&
                        git cat-file tree $T
@@ -521,9 +523,21 @@ test_expect_success 'fsck --connectivity-only' '
                touch empty &&
                git add empty &&
                test_commit empty &&
+
+               # Drop the index now; we want to be sure that we
+               # recursively notice the broken objects
+               # because they are reachable from refs, not because
+               # they are in the index.
+               rm -f .git/index &&
+
+               # corrupt the blob, but in a way that we can still identify
+               # its type. That lets us see that --connectivity-only is
+               # not actually looking at the contents, but leaves it
+               # free to examine the type if it chooses.
                empty=.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 &&
-               rm -f $empty &&
-               echo invalid >$empty &&
+               blob=$(echo unrelated | git hash-object -w --stdin) &&
+               mv -f $(sha1_file $blob) $empty &&
+
                test_must_fail git fsck --strict &&
                git fsck --strict --connectivity-only &&
                tree=$(git rev-parse HEAD:) &&
@@ -535,6 +549,19 @@ test_expect_success 'fsck --connectivity-only' '
        )
 '
 
+test_expect_success 'fsck --connectivity-only with explicit head' '
+       rm -rf connectivity-only &&
+       git init connectivity-only &&
+       (
+               cd connectivity-only &&
+               test_commit foo &&
+               rm -f .git/index &&
+               tree=$(git rev-parse HEAD^{tree}) &&
+               remove_object $(git rev-parse HEAD:foo.t) &&
+               test_must_fail git fsck --connectivity-only $tree
+       )
+'
+
 test_expect_success 'fsck --name-objects' '
        rm -rf name-objects &&
        git init name-objects &&
@@ -619,4 +646,47 @@ test_expect_success 'fsck detects trailing loose garbage (blob)' '
        test_i18ngrep "garbage.*$blob" out
 '
 
+# for each of type, we have one version which is referenced by another object
+# (and so while unreachable, not dangling), and another variant which really is
+# dangling.
+test_expect_success 'fsck notices dangling objects' '
+       git init dangling &&
+       (
+               cd dangling &&
+               blob=$(echo not-dangling | git hash-object -w --stdin) &&
+               dblob=$(echo dangling | git hash-object -w --stdin) &&
+               tree=$(printf "100644 blob %s\t%s\n" $blob one | git mktree) &&
+               dtree=$(printf "100644 blob %s\t%s\n" $blob two | git mktree) &&
+               commit=$(git commit-tree $tree) &&
+               dcommit=$(git commit-tree -p $commit $tree) &&
+
+               cat >expect <<-EOF &&
+               dangling blob $dblob
+               dangling commit $dcommit
+               dangling tree $dtree
+               EOF
+
+               git fsck >actual &&
+               # the output order is non-deterministic, as it comes from a hash
+               sort <actual >actual.sorted &&
+               test_cmp expect actual.sorted
+       )
+'
+
+test_expect_success 'fsck $name notices bogus $name' '
+       test_must_fail git fsck bogus &&
+       test_must_fail git fsck $_z40
+'
+
+test_expect_success 'bogus head does not fallback to all heads' '
+       # set up a case that will cause a reachability complaint
+       echo to-be-deleted >foo &&
+       git add foo &&
+       blob=$(git rev-parse :foo) &&
+       test_when_finished "git rm --cached foo" &&
+       remove_object $blob &&
+       test_must_fail git fsck $_z40 >out 2>&1 &&
+       ! grep $blob out
+'
+
 test_done