From 46d611cadab500ca2b458b2fda7008c41b174011 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 12 Jan 2026 10:03:04 +0100 Subject: 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 Signed-off-by: Junio C Hamano --- builtin/fsck.c | 41 +++++++++++++++-------------------------- 1 file changed, 15 insertions(+), 26 deletions(-) (limited to 'builtin/fsck.c') diff --git a/builtin/fsck.c b/builtin/fsck.c index 4979bc795e..4dd4d74d1e 100644 --- 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) -- cgit v1.3 From 9727336b31c055e4507248703b8a4a8ed039dc06 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 12 Jan 2026 10:03:05 +0100 Subject: builtin/fsck: move generic HEAD check into `refs_fsck()` Move the check that detects "HEAD" refs that do not point at a branch into `refs_fsck()`. This follows the same motivation as the preceding commit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/fsck-msgids.adoc | 3 +++ builtin/fsck.c | 7 ------- fsck.h | 1 + refs.c | 12 +++++++++++- t/t0602-reffiles-fsck.sh | 8 ++++---- t/t1450-fsck.sh | 4 ++-- 6 files changed, 21 insertions(+), 14 deletions(-) (limited to 'builtin/fsck.c') diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc index 76609321f6..6a4db3a991 100644 --- a/Documentation/fsck-msgids.adoc +++ b/Documentation/fsck-msgids.adoc @@ -13,6 +13,9 @@ `badGpgsig`:: (ERROR) A tag contains a bad (truncated) signature (e.g., `gpgsig`) header. +`badHeadTarget`:: + (ERROR) The `HEAD` ref is a symref that does not refer to a branch. + `badHeaderContinuation`:: (ERROR) A continuation header (such as for `gpgsig`) is unexpectedly truncated. diff --git a/builtin/fsck.c b/builtin/fsck.c index 4dd4d74d1e..5dda441f45 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -728,13 +728,6 @@ static void fsck_head_link(const char *head_ref_name, error(_("invalid %s"), head_ref_name); return; } - if (strcmp(*head_points_at, head_ref_name) && - !starts_with(*head_points_at, "refs/heads/")) { - errors_found |= ERROR_REFS; - error(_("%s points to something strange (%s)"), - head_ref_name, *head_points_at); - return; - } return; } diff --git a/fsck.h b/fsck.h index 1f472b7daa..65ecbb7fe1 100644 --- a/fsck.h +++ b/fsck.h @@ -30,6 +30,7 @@ enum fsck_msg_type { FUNC(BAD_DATE_OVERFLOW, ERROR) \ FUNC(BAD_EMAIL, ERROR) \ FUNC(BAD_GPGSIG, ERROR) \ + FUNC(BAD_HEAD_TARGET, ERROR) \ FUNC(BAD_NAME, ERROR) \ FUNC(BAD_OBJECT_SHA1, ERROR) \ FUNC(BAD_PACKED_REF_ENTRY, ERROR) \ diff --git a/refs.c b/refs.c index c3528862c6..a772d371cd 100644 --- a/refs.c +++ b/refs.c @@ -334,8 +334,18 @@ int refs_fsck_ref(struct ref_store *refs UNUSED, struct fsck_options *o, int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o, struct fsck_ref_report *report, - const char *refname UNUSED, const char *target) + const char *refname, const char *target) { + const char *stripped_refname; + + parse_worktree_ref(refname, NULL, NULL, &stripped_refname); + + if (!strcmp(stripped_refname, "HEAD") && + !starts_with(target, "refs/heads/") && + fsck_report_ref(o, report, FSCK_MSG_BAD_HEAD_TARGET, + "HEAD points to non-branch '%s'", target)) + return -1; + if (is_root_ref(target)) return 0; diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 479f3d528e..3c1f553b81 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -910,10 +910,10 @@ test_expect_success 'complains about broken root ref' ' git init repo && ( cd repo && - echo "ref: refs/../HEAD" >.git/HEAD && + echo "ref: refs/heads/../HEAD" >.git/HEAD && test_must_fail git refs verify 2>err && cat >expect <<-EOF && - error: HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ} + error: HEAD: badReferentName: points to invalid refname ${SQ}refs/heads/../HEAD${SQ} EOF test_cmp expect err ) @@ -926,10 +926,10 @@ test_expect_success 'complains about broken root ref in worktree' ' cd repo && test_commit initial && git worktree add ../worktree && - echo "ref: refs/../HEAD" >.git/worktrees/worktree/HEAD && + echo "ref: refs/heads/../HEAD" >.git/worktrees/worktree/HEAD && test_must_fail git refs verify 2>err && cat >expect <<-EOF && - error: worktrees/worktree/HEAD: badReferentName: points to invalid refname ${SQ}refs/../HEAD${SQ} + error: worktrees/worktree/HEAD: badReferentName: points to invalid refname ${SQ}refs/heads/../HEAD${SQ} EOF test_cmp expect err ) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 900c1b2eb2..3fae05f9d9 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -113,7 +113,7 @@ test_expect_success 'HEAD link pointing at a funny place' ' test-tool ref-store main create-symref HEAD refs/funny/place && # avoid corrupt/broken HEAD from interfering with repo discovery test_must_fail env GIT_DIR=.git git fsck 2>out && - test_grep "HEAD points to something strange" out + test_grep "HEAD: badHeadTarget: HEAD points to non-branch ${SQ}refs/funny/place${SQ}" out ' test_expect_success REFFILES 'HEAD link pointing at a funny object (from different wt)' ' @@ -148,7 +148,7 @@ test_expect_success 'other worktree HEAD link pointing at a funny place' ' git worktree add other && git -C other symbolic-ref HEAD refs/funny/place && test_must_fail git fsck 2>out && - test_grep "worktrees/other/HEAD points to something strange" out + test_grep "worktrees/other/HEAD: badHeadTarget: HEAD points to non-branch ${SQ}refs/funny/place${SQ}" out ' test_expect_success 'commit with multiple signatures is okay' ' -- cgit v1.3 From 8947da018387f146a90e64055b4caf2ab79e39a7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 12 Jan 2026 10:03:06 +0100 Subject: builtin/fsck: drop `fsck_head_link()` The function `fsck_head_link()` was historically used to perform a couple of consistency checks for refs. (Almost) all of these checks have now been moved into the refs subsystem. There's only a single check remaining that verifies whether `refs_resolve_ref_unsafe()` returns a `NULL` pointer. This may happen in a couple of cases: - When `refs_is_safe()` declares the ref to be unsafe. We already have checks for this as we verify refnames with `check_refname_format()`. - When the ref doesn't exist. A repository without "HEAD" is completely broken though, and we would notice this error ahead of time already. - In case the caller passes `RESOLVE_REF_READING` and the ref is a symref that doesn't resolve. We don't pass this flag though. As such, this check doesn't cover anything anymore that isn't already covered by `refs_fsck()`. Drop it, which also allows us to inline the call to `refs_resolve_ref_unsafe()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) (limited to 'builtin/fsck.c') diff --git a/builtin/fsck.c b/builtin/fsck.c index 5dda441f45..f104b7af0e 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -564,10 +564,6 @@ static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED) return 0; } -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) { struct worktree **worktrees, **p; @@ -583,7 +579,10 @@ static void get_default_heads(void) struct strbuf refname = STRBUF_INIT; strbuf_worktree_ref(wt, &refname, "HEAD"); - fsck_head_link(refname.buf, &head_points_at, &head_oid); + + head_points_at = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), + refname.buf, 0, &head_oid, NULL); + if (head_points_at && !is_null_oid(&head_oid)) { struct reference ref = { .name = refname.buf, @@ -713,25 +712,6 @@ static void fsck_source(struct odb_source *source) stop_progress(&progress); } -static void fsck_head_link(const char *head_ref_name, - const char **head_points_at, - struct object_id *head_oid) -{ - if (verbose) - fprintf_ln(stderr, _("Checking %s link"), head_ref_name); - - *head_points_at = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - head_ref_name, 0, head_oid, - NULL); - if (!*head_points_at) { - errors_found |= ERROR_REFS; - error(_("invalid %s"), head_ref_name); - return; - } - - return; -} - static int fsck_cache_tree(struct cache_tree *it, const char *index_path) { int i; -- cgit v1.3