builtin/fsck: move generic object ID checks into `refs_fsck()`

While most of the logic that verifies the consistency of refs is
driven by `refs_fsck()`, we still have a small handful of checks in
`fsck_head_link()`. These checks don't use the git-fsck(1) reporting
infrastructure, and as such it's impossible to for example disable
some of those checks.

One such check detects refs that point to the all-zeroes object ID.
Extract this check into the generic `refs_fsck_ref()` function that is
used by both the "files" and "reftable" backends.

Note that this will cause us to not return an error code from
`fsck_head_link()` anymore in case this error was detected. This is fine
though: the only caller of this function does not check the error code
anyway. To demonstrate this, adapt the function to drop its return value
altogether. The function will be removed in a subsequent commit anyway.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc
index acac9683af83f423527dea75152a778f650cfe4d..76609321f662dd957f9c61b6029be44c0dc1002c 100644 (file)
--- a/Documentation/fsck-msgids.adoc
+++ b/Documentation/fsck-msgids.adoc
@@ -41,6 +41,9 @@
 `badRefName`::
        (ERROR) A ref has an invalid format.
 
+`badRefOid`::
+       (ERROR) A ref points to an invalid object ID.
+
 `badReferentName`::
        (ERROR) The referent name of a symref is invalid.
 

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 4979bc795e5d618b137d1f44db8cf1bdc5bdceca..4dd4d74d1e15da041dc35187131dd9d226ac970c 100644 (file)
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -564,9 +564,9 @@ static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED)
        return 0;
 }
 
-static int fsck_head_link(const char *head_ref_name,
-                         const char **head_points_at,
-                         struct object_id *head_oid);
+static void fsck_head_link(const char *head_ref_name,
+                          const char **head_points_at,
+                          struct object_id *head_oid);
 
 static void get_default_heads(void)
 {
@@ -713,12 +713,10 @@ static void fsck_source(struct odb_source *source)
        stop_progress(&progress);
 }
 
-static int fsck_head_link(const char *head_ref_name,
-                         const char **head_points_at,
-                         struct object_id *head_oid)
+static void fsck_head_link(const char *head_ref_name,
+                          const char **head_points_at,
+                          struct object_id *head_oid)
 {
-       int null_is_error = 0;
-
        if (verbose)
                fprintf_ln(stderr, _("Checking %s link"), head_ref_name);
 
@@ -727,27 +725,18 @@ static int fsck_head_link(const char *head_ref_name,
                                                  NULL);
        if (!*head_points_at) {
                errors_found |= ERROR_REFS;
-               return error(_("invalid %s"), head_ref_name);
+               error(_("invalid %s"), head_ref_name);
+               return;
        }
-       if (!strcmp(*head_points_at, head_ref_name))
-               /* detached HEAD */
-               null_is_error = 1;
-       else if (!starts_with(*head_points_at, "refs/heads/")) {
+       if (strcmp(*head_points_at, head_ref_name) &&
+           !starts_with(*head_points_at, "refs/heads/")) {
                errors_found |= ERROR_REFS;
-               return error(_("%s points to something strange (%s)"),
-                            head_ref_name, *head_points_at);
-       }
-       if (is_null_oid(head_oid)) {
-               if (null_is_error) {
-                       errors_found |= ERROR_REFS;
-                       return error(_("%s: detached HEAD points at nothing"),
-                                    head_ref_name);
-               }
-               fprintf_ln(stderr,
-                          _("notice: %s points to an unborn branch (%s)"),
-                          head_ref_name, *head_points_at + 11);
+               error(_("%s points to something strange (%s)"),
+                     head_ref_name, *head_points_at);
+               return;
        }
-       return 0;
+
+       return;
 }
 
 static int fsck_cache_tree(struct cache_tree *it, const char *index_path)

diff --git a/fsck.h b/fsck.h
index bfe0d9c6d2e44e2dffa025be5e361acd4da16e9e..1f472b7daac38cba6f2c29f999e3a636c1044975 100644 (file)
--- a/fsck.h
+++ b/fsck.h
@@ -39,6 +39,7 @@ enum fsck_msg_type {
        FUNC(BAD_REF_CONTENT, ERROR) \
        FUNC(BAD_REF_FILETYPE, ERROR) \
        FUNC(BAD_REF_NAME, ERROR) \
+       FUNC(BAD_REF_OID, ERROR) \
        FUNC(BAD_TIMEZONE, ERROR) \
        FUNC(BAD_TREE, ERROR) \
        FUNC(BAD_TREE_SHA1, ERROR) \

diff --git a/refs.c b/refs.c
index 4fc1317cb32323893828108a342141f7985ba081..c3528862c68f63a359323c4b22095026c4c4fbd5 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -320,10 +320,15 @@ int check_refname_format(const char *refname, int flags)
        return check_or_sanitize_refname(refname, flags, NULL);
 }
 
-int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o UNUSED,
-                 struct fsck_ref_report *report UNUSED,
-                 const char *refname UNUSED, const struct object_id *oid UNUSED)
+int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o,
+                 struct fsck_ref_report *report,
+                 const char *refname UNUSED, const struct object_id *oid)
 {
+       if (is_null_oid(oid))
+               return fsck_report_ref(o, report, FSCK_MSG_BAD_REF_OID,
+                                      "points to invalid object ID '%s'",
+                                      oid_to_hex(oid));
+
        return 0;
 }
 

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index c4b651c2dc793885ad13236f5caceb7514d00004..900c1b2eb258c14a107ea0bf0f80be79b2ca895f 100755 (executable)
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -105,7 +105,7 @@ test_expect_success REFFILES 'HEAD link pointing at a funny object' '
        echo $ZERO_OID >.git/HEAD &&
        # avoid corrupt/broken HEAD from interfering with repo discovery
        test_must_fail env GIT_DIR=.git git fsck 2>out &&
-       test_grep "detached HEAD points" out
+       test_grep "HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}" out
 '
 
 test_expect_success 'HEAD link pointing at a funny place' '
@@ -123,7 +123,7 @@ test_expect_success REFFILES 'HEAD link pointing at a funny object (from differe
        echo $ZERO_OID >.git/HEAD &&
        # avoid corrupt/broken HEAD from interfering with repo discovery
        test_must_fail git -C wt fsck 2>out &&
-       test_grep "main-worktree/HEAD: detached HEAD points" out
+       test_grep "HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}" out
 '
 
 test_expect_success REFFILES 'other worktree HEAD link pointing at a funny object' '
@@ -131,7 +131,7 @@ test_expect_success REFFILES 'other worktree HEAD link pointing at a funny objec
        git worktree add other &&
        echo $ZERO_OID >.git/worktrees/other/HEAD &&
        test_must_fail git fsck 2>out &&
-       test_grep "worktrees/other/HEAD: detached HEAD points" out
+       test_grep "worktrees/other/HEAD: badRefOid: points to invalid object ID ${SQ}$ZERO_OID${SQ}" out
 '
 
 test_expect_success 'other worktree HEAD link pointing at missing object' '