diff --git a/Documentation/fsck-msgids.adoc b/Documentation/fsck-msgids.adoc index acac9683af..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. @@ -41,6 +44,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 f671288026..0512f78a87 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -596,10 +596,6 @@ 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 snapshot_refs(struct snapshot *snap, int argc, const char **argv) { struct worktree **worktrees, **p; @@ -636,7 +632,10 @@ static void snapshot_refs(struct snapshot *snap, int argc, const char **argv) 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, @@ -803,43 +802,6 @@ 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) -{ - int null_is_error = 0; - - 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; - return error(_("invalid %s"), head_ref_name); - } - if (!strcmp(*head_points_at, head_ref_name)) - /* detached HEAD */ - null_is_error = 1; - else if (!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); - } - return 0; -} - static int fsck_cache_tree(struct cache_tree *it, const char *index_path) { int i; diff --git a/fsck.c b/fsck.c index 97805035e0..070f8eb6bf 100644 --- a/fsck.c +++ b/fsck.c @@ -1296,11 +1296,6 @@ int fsck_refs_error_function(struct fsck_options *options UNUSED, strbuf_addstr(&sb, report->path); - if (report->oid) - strbuf_addf(&sb, " -> (%s)", oid_to_hex(report->oid)); - else if (report->referent) - strbuf_addf(&sb, " -> (%s)", report->referent); - if (msg_type == FSCK_WARN) warning("%s: %s", sb.buf, message); else diff --git a/fsck.h b/fsck.h index 336917c045..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) \ @@ -39,6 +40,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) \ @@ -162,8 +164,6 @@ struct fsck_object_report { struct fsck_ref_report { const char *path; - const struct object_id *oid; - const char *referent; }; struct fsck_options { diff --git a/refs.c b/refs.c index 1d29d9b1c9..2ed6c38229 100644 --- a/refs.c +++ b/refs.c @@ -320,6 +320,49 @@ 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, + 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; +} + +int refs_fsck_symref(struct ref_store *refs UNUSED, struct fsck_options *o, + struct fsck_ref_report *report, + 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; + + if (check_refname_format(target, 0) && + fsck_report_ref(o, report, FSCK_MSG_BAD_REFERENT_NAME, + "points to invalid refname '%s'", target)) + return -1; + + if (!starts_with(target, "refs/") && + !starts_with(target, "worktrees/") && + fsck_report_ref(o, report, FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF, + "points to non-ref target '%s'", target)) + return -1; + + return 0; +} + int refs_fsck(struct ref_store *refs, struct fsck_options *o, struct worktree *wt) { diff --git a/refs.h b/refs.h index d9051bbb04..61c56cca36 100644 --- a/refs.h +++ b/refs.h @@ -653,6 +653,24 @@ int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_dat */ int check_refname_format(const char *refname, int flags); +struct fsck_ref_report; + +/* + * Perform generic checks for a specific symref target. This function is + * expected to be called by the ref backends for every symbolic ref. + */ +int refs_fsck_ref(struct ref_store *refs, struct fsck_options *o, + struct fsck_ref_report *report, + const char *refname, const struct object_id *oid); + +/* + * Perform generic checks for a specific symref target. This function is + * expected to be called by the ref backends for every symbolic ref. + */ +int refs_fsck_symref(struct ref_store *refs, struct fsck_options *o, + struct fsck_ref_report *report, + const char *refname, const char *target); + /* * Check the reference database for consistency. Return 0 if refs and * reflogs are consistent, and non-zero otherwise. The errors will be diff --git a/refs/files-backend.c b/refs/files-backend.c index 6f6f76a8d8..e59794f5da 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -354,13 +354,11 @@ static int for_each_root_ref(struct files_ref_store *refs, void *cb_data) { struct strbuf path = STRBUF_INIT, refname = STRBUF_INIT; - const char *dirname = refs->loose->root->name; struct dirent *de; - size_t dirnamelen; int ret; DIR *d; - files_ref_path(refs, &path, dirname); + files_ref_path(refs, &path, ""); d = opendir(path.buf); if (!d) { @@ -368,9 +366,6 @@ static int for_each_root_ref(struct files_ref_store *refs, return -1; } - strbuf_addstr(&refname, dirname); - dirnamelen = refname.len; - while ((de = readdir(d)) != NULL) { unsigned char dtype; @@ -378,6 +373,8 @@ static int for_each_root_ref(struct files_ref_store *refs, continue; if (ends_with(de->d_name, ".lock")) continue; + + strbuf_reset(&refname); strbuf_addstr(&refname, de->d_name); dtype = get_dtype(de, &path, 1); @@ -386,8 +383,6 @@ static int for_each_root_ref(struct files_ref_store *refs, if (ret) goto done; } - - strbuf_setlen(&refname, dirnamelen); } ret = 0; @@ -3720,64 +3715,50 @@ static int files_ref_store_remove_on_disk(struct ref_store *ref_store, typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store, struct fsck_options *o, const char *refname, - struct dir_iterator *iter); + const char *path, + int mode); -static int files_fsck_symref_target(struct fsck_options *o, +static int files_fsck_symref_target(struct ref_store *ref_store, + struct fsck_options *o, struct fsck_ref_report *report, + const char *refname, struct strbuf *referent, unsigned int symbolic_link) { - int is_referent_root; char orig_last_byte; size_t orig_len; int ret = 0; orig_len = referent->len; orig_last_byte = referent->buf[orig_len - 1]; - if (!symbolic_link) + + if (!symbolic_link) { strbuf_rtrim(referent); - is_referent_root = is_root_ref(referent->buf); - if (!is_referent_root && - !starts_with(referent->buf, "refs/") && - !starts_with(referent->buf, "worktrees/")) { - ret = fsck_report_ref(o, report, - FSCK_MSG_SYMREF_TARGET_IS_NOT_A_REF, - "points to non-ref target '%s'", referent->buf); + if (referent->len == orig_len || + (referent->len < orig_len && orig_last_byte != '\n')) { + ret |= fsck_report_ref(o, report, + FSCK_MSG_REF_MISSING_NEWLINE, + "misses LF at the end"); + } + if (referent->len != orig_len && referent->len != orig_len - 1) { + ret |= fsck_report_ref(o, report, + FSCK_MSG_TRAILING_REF_CONTENT, + "has trailing whitespaces or newlines"); + } } - if (!is_referent_root && check_refname_format(referent->buf, 0)) { - ret = fsck_report_ref(o, report, - FSCK_MSG_BAD_REFERENT_NAME, - "points to invalid refname '%s'", referent->buf); - goto out; - } + ret |= refs_fsck_symref(ref_store, o, report, refname, referent->buf); - if (symbolic_link) - goto out; - - if (referent->len == orig_len || - (referent->len < orig_len && orig_last_byte != '\n')) { - ret = fsck_report_ref(o, report, - FSCK_MSG_REF_MISSING_NEWLINE, - "misses LF at the end"); - } - - if (referent->len != orig_len && referent->len != orig_len - 1) { - ret = fsck_report_ref(o, report, - FSCK_MSG_TRAILING_REF_CONTENT, - "has trailing whitespaces or newlines"); - } - -out: - return ret; + return ret ? -1 : 0; } static int files_fsck_refs_content(struct ref_store *ref_store, struct fsck_options *o, const char *target_name, - struct dir_iterator *iter) + const char *path, + int mode) { struct strbuf ref_content = STRBUF_INIT; struct strbuf abs_gitdir = STRBUF_INIT; @@ -3791,7 +3772,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, report.path = target_name; - if (S_ISLNK(iter->st.st_mode)) { + if (S_ISLNK(mode)) { const char *relative_referent_path = NULL; ret = fsck_report_ref(o, &report, @@ -3803,7 +3784,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, if (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1])) strbuf_addch(&abs_gitdir, '/'); - strbuf_add_real_path(&ref_content, iter->path.buf); + strbuf_add_real_path(&ref_content, path); skip_prefix(ref_content.buf, abs_gitdir.buf, &relative_referent_path); @@ -3812,11 +3793,12 @@ static int files_fsck_refs_content(struct ref_store *ref_store, else strbuf_addbuf(&referent, &ref_content); - ret |= files_fsck_symref_target(o, &report, &referent, 1); + ret |= files_fsck_symref_target(ref_store, o, &report, + target_name, &referent, 1); goto cleanup; } - if (strbuf_read_file(&ref_content, iter->path.buf, 0) < 0) { + if (strbuf_read_file(&ref_content, path, 0) < 0) { /* * Ref file could be removed by another concurrent process. We should * ignore this error and continue to the next ref. @@ -3824,7 +3806,7 @@ static int files_fsck_refs_content(struct ref_store *ref_store, if (errno == ENOENT) goto cleanup; - ret = error_errno(_("cannot read ref file '%s'"), iter->path.buf); + ret = error_errno(_("cannot read ref file '%s'"), path); goto cleanup; } @@ -3851,8 +3833,11 @@ static int files_fsck_refs_content(struct ref_store *ref_store, "has trailing garbage: '%s'", trailing); goto cleanup; } + + ret = refs_fsck_ref(ref_store, o, &report, target_name, &oid); } else { - ret = files_fsck_symref_target(o, &report, &referent, 0); + ret = files_fsck_symref_target(ref_store, o, &report, + target_name, &referent, 0); goto cleanup; } @@ -3866,21 +3851,25 @@ cleanup: static int files_fsck_refs_name(struct ref_store *ref_store UNUSED, struct fsck_options *o, const char *refname, - struct dir_iterator *iter) + const char *path, + int mode UNUSED) { struct strbuf sb = STRBUF_INIT; + const char *filename; int ret = 0; + filename = basename((char *) path); + /* * Ignore the files ending with ".lock" as they may be lock files * However, do not allow bare ".lock" files. */ - if (iter->basename[0] != '.' && ends_with(iter->basename, ".lock")) + if (filename[0] != '.' && ends_with(filename, ".lock")) + goto cleanup; + + if (is_root_ref(refname)) goto cleanup; - /* - * This works right now because we never check the root refs. - */ if (check_refname_format(refname, 0)) { struct fsck_ref_report report = { 0 }; @@ -3895,11 +3884,44 @@ cleanup: return ret; } +static const files_fsck_refs_fn fsck_refs_fn[]= { + files_fsck_refs_name, + files_fsck_refs_content, + NULL, +}; + +static int files_fsck_ref(struct ref_store *ref_store, + struct fsck_options *o, + const char *refname, + const char *path, + int mode) +{ + int ret = 0; + + if (o->verbose) + fprintf_ln(stderr, "Checking %s", refname); + + if (!S_ISREG(mode) && !S_ISLNK(mode)) { + struct fsck_ref_report report = { .path = refname }; + + if (fsck_report_ref(o, &report, + FSCK_MSG_BAD_REF_FILETYPE, + "unexpected file type")) + ret = -1; + goto out; + } + + for (size_t i = 0; fsck_refs_fn[i]; i++) + if (fsck_refs_fn[i](ref_store, o, refname, path, mode)) + ret = -1; + +out: + return ret; +} + static int files_fsck_refs_dir(struct ref_store *ref_store, struct fsck_options *o, - const char *refs_check_dir, - struct worktree *wt, - files_fsck_refs_fn *fsck_refs_fn) + struct worktree *wt) { struct strbuf refname = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; @@ -3907,7 +3929,7 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, int iter_status; int ret = 0; - strbuf_addf(&sb, "%s/%s", ref_store->gitdir, refs_check_dir); + strbuf_addf(&sb, "%s/refs", ref_store->gitdir); iter = dir_iterator_begin(sb.buf, 0); if (!iter) { @@ -3919,31 +3941,17 @@ static int files_fsck_refs_dir(struct ref_store *ref_store, } while ((iter_status = dir_iterator_advance(iter)) == ITER_OK) { - if (S_ISDIR(iter->st.st_mode)) { + if (S_ISDIR(iter->st.st_mode)) continue; - } else if (S_ISREG(iter->st.st_mode) || - S_ISLNK(iter->st.st_mode)) { - strbuf_reset(&refname); - if (!is_main_worktree(wt)) - strbuf_addf(&refname, "worktrees/%s/", wt->id); - strbuf_addf(&refname, "%s/%s", refs_check_dir, - iter->relative_path); + strbuf_reset(&refname); + if (!is_main_worktree(wt)) + strbuf_addf(&refname, "worktrees/%s/", wt->id); + strbuf_addf(&refname, "refs/%s", iter->relative_path); - if (o->verbose) - fprintf_ln(stderr, "Checking %s", refname.buf); - - for (size_t i = 0; fsck_refs_fn[i]; i++) { - if (fsck_refs_fn[i](ref_store, o, refname.buf, iter)) - ret = -1; - } - } else { - struct fsck_ref_report report = { .path = iter->basename }; - if (fsck_report_ref(o, &report, - FSCK_MSG_BAD_REF_FILETYPE, - "unexpected file type")) - ret = -1; - } + if (files_fsck_ref(ref_store, o, refname.buf, + iter->path.buf, iter->st.st_mode) < 0) + ret = -1; } if (iter_status != ITER_DONE) @@ -3956,17 +3964,36 @@ out: return ret; } -static int files_fsck_refs(struct ref_store *ref_store, - struct fsck_options *o, - struct worktree *wt) -{ - files_fsck_refs_fn fsck_refs_fn[]= { - files_fsck_refs_name, - files_fsck_refs_content, - NULL, - }; +struct files_fsck_root_ref_data { + struct files_ref_store *refs; + struct fsck_options *o; + struct worktree *wt; + struct strbuf refname; + struct strbuf path; + bool errors_found; +}; - return files_fsck_refs_dir(ref_store, o, "refs", wt, fsck_refs_fn); +static int files_fsck_root_ref(const char *refname, void *cb_data) +{ + struct files_fsck_root_ref_data *data = cb_data; + struct stat st; + + strbuf_reset(&data->refname); + if (!is_main_worktree(data->wt)) + strbuf_addf(&data->refname, "worktrees/%s/", data->wt->id); + strbuf_addstr(&data->refname, refname); + + strbuf_reset(&data->path); + strbuf_addf(&data->path, "%s/%s", data->refs->gitcommondir, data->refname.buf); + + if (stat(data->path.buf, &st)) { + if (errno == ENOENT) + return 0; + return error_errno("failed to read ref: '%s'", data->path.buf); + } + + return files_fsck_ref(&data->refs->base, data->o, data->refname.buf, + data->path.buf, st.st_mode); } static int files_fsck(struct ref_store *ref_store, @@ -3975,9 +4002,28 @@ static int files_fsck(struct ref_store *ref_store, { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ, "fsck"); + struct files_fsck_root_ref_data data = { + .refs = refs, + .o = o, + .wt = wt, + .refname = STRBUF_INIT, + .path = STRBUF_INIT, + }; + int ret = 0; - return files_fsck_refs(ref_store, o, wt) | - refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt); + if (files_fsck_refs_dir(ref_store, o, wt) < 0) + ret = -1; + + if (for_each_root_ref(refs, files_fsck_root_ref, &data) < 0 || + data.errors_found) + ret = -1; + + if (refs->packed_ref_store->be->fsck(refs->packed_ref_store, o, wt) < 0) + ret = -1; + + strbuf_release(&data.refname); + strbuf_release(&data.path); + return ret; } struct ref_storage_be refs_be_files = { diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 4319a4eacb..fe74af73af 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -10,9 +10,10 @@ #include "../gettext.h" #include "../hash.h" #include "../hex.h" -#include "../iterator.h" #include "../ident.h" +#include "../iterator.h" #include "../object.h" +#include "../parse.h" #include "../path.h" #include "../refs.h" #include "../reftable/reftable-basics.h" @@ -25,8 +26,8 @@ #include "../setup.h" #include "../strmap.h" #include "../trace2.h" +#include "../worktree.h" #include "../write-or-die.h" -#include "parse.h" #include "refs-internal.h" /* @@ -172,6 +173,37 @@ static struct reftable_ref_store *reftable_be_downcast(struct ref_store *ref_sto return refs; } +static int backend_for_worktree(struct reftable_backend **out, + struct reftable_ref_store *store, + const char *worktree_name) +{ + struct strbuf worktree_dir = STRBUF_INIT; + int ret; + + *out = strmap_get(&store->worktree_backends, worktree_name); + if (*out) { + ret = 0; + goto out; + } + + strbuf_addf(&worktree_dir, "%s/worktrees/%s/reftable", + store->base.repo->commondir, worktree_name); + + CALLOC_ARRAY(*out, 1); + store->err = ret = reftable_backend_init(*out, worktree_dir.buf, + &store->write_options); + if (ret < 0) { + free(*out); + goto out; + } + + strmap_put(&store->worktree_backends, worktree_name, *out); + +out: + strbuf_release(&worktree_dir); + return ret; +} + /* * Some refs are global to the repository (refs/heads/{*}), while others are * local to the worktree (eg. HEAD, refs/bisect/{*}). We solve this by having @@ -191,19 +223,19 @@ static int backend_for(struct reftable_backend **out, const char **rewritten_ref, int reload) { - struct reftable_backend *be; const char *wtname; int wtname_len; + int ret; if (!refname) { - be = &store->main_backend; + *out = &store->main_backend; + ret = 0; goto out; } switch (parse_worktree_ref(refname, &wtname, &wtname_len, rewritten_ref)) { case REF_WORKTREE_OTHER: { static struct strbuf wtname_buf = STRBUF_INIT; - struct strbuf wt_dir = STRBUF_INIT; /* * We're using a static buffer here so that we don't need to @@ -223,20 +255,8 @@ static int backend_for(struct reftable_backend **out, * already and error out when trying to write a reference via * both stacks. */ - be = strmap_get(&store->worktree_backends, wtname_buf.buf); - if (!be) { - strbuf_addf(&wt_dir, "%s/worktrees/%s/reftable", - store->base.repo->commondir, wtname_buf.buf); + ret = backend_for_worktree(out, store, wtname_buf.buf); - CALLOC_ARRAY(be, 1); - store->err = reftable_backend_init(be, wt_dir.buf, - &store->write_options); - assert(store->err != REFTABLE_API_ERROR); - - strmap_put(&store->worktree_backends, wtname_buf.buf, be); - } - - strbuf_release(&wt_dir); goto out; } case REF_WORKTREE_CURRENT: @@ -245,27 +265,24 @@ static int backend_for(struct reftable_backend **out, * main worktree. We thus return the main stack in that case. */ if (!store->worktree_backend.stack) - be = &store->main_backend; + *out = &store->main_backend; else - be = &store->worktree_backend; + *out = &store->worktree_backend; + ret = 0; goto out; case REF_WORKTREE_MAIN: case REF_WORKTREE_SHARED: - be = &store->main_backend; + *out = &store->main_backend; + ret = 0; goto out; default: BUG("unhandled worktree reference type"); } out: - if (reload) { - int ret = reftable_stack_reload(be->stack); - if (ret) - return ret; - } - *out = be; - - return 0; + if (reload && !ret) + ret = reftable_stack_reload((*out)->stack); + return ret; } static int should_write_log(struct reftable_ref_store *refs, const char *refname) @@ -2746,24 +2763,92 @@ static int reftable_fsck_error_handler(struct reftable_fsck_info *info, } static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o, - struct worktree *wt UNUSED) + struct worktree *wt) { - struct reftable_ref_store *refs; - struct strmap_entry *entry; - struct hashmap_iter iter; - int ret = 0; + struct reftable_ref_store *refs = + reftable_be_downcast(ref_store, REF_STORE_READ, "fsck"); + struct reftable_ref_iterator *iter = NULL; + struct reftable_ref_record ref = { 0 }; + struct fsck_ref_report report = { 0 }; + struct strbuf refname = STRBUF_INIT; + struct reftable_backend *backend; + int ret, errors = 0; - refs = reftable_be_downcast(ref_store, REF_STORE_READ, "fsck"); - - ret |= reftable_fsck_check(refs->main_backend.stack, reftable_fsck_error_handler, - reftable_fsck_verbose_handler, o); - - strmap_for_each_entry(&refs->worktree_backends, &iter, entry) { - struct reftable_backend *b = (struct reftable_backend *)entry->value; - ret |= reftable_fsck_check(b->stack, reftable_fsck_error_handler, - reftable_fsck_verbose_handler, o); + if (is_main_worktree(wt)) { + backend = &refs->main_backend; + } else { + ret = backend_for_worktree(&backend, refs, wt->id); + if (ret < 0) { + ret = error(_("reftable stack for worktree '%s' is broken"), + wt->id); + goto out; + } } + errors |= reftable_fsck_check(backend->stack, reftable_fsck_error_handler, + reftable_fsck_verbose_handler, o); + + iter = ref_iterator_for_stack(refs, backend->stack, "", NULL, 0); + if (!iter) { + ret = error(_("could not create iterator for worktree '%s'"), wt->id); + goto out; + } + + while (1) { + ret = reftable_iterator_next_ref(&iter->iter, &ref); + if (ret > 0) + break; + if (ret < 0) { + ret = error(_("could not read record for worktree '%s'"), wt->id); + goto out; + } + + strbuf_reset(&refname); + if (!is_main_worktree(wt)) + strbuf_addf(&refname, "worktrees/%s/", wt->id); + strbuf_addstr(&refname, ref.refname); + report.path = refname.buf; + + switch (ref.value_type) { + case REFTABLE_REF_VAL1: + case REFTABLE_REF_VAL2: { + struct object_id oid; + unsigned hash_id; + + switch (reftable_stack_hash_id(backend->stack)) { + case REFTABLE_HASH_SHA1: + hash_id = GIT_HASH_SHA1; + break; + case REFTABLE_HASH_SHA256: + hash_id = GIT_HASH_SHA256; + break; + default: + BUG("unhandled hash ID %d", + reftable_stack_hash_id(backend->stack)); + } + + oidread(&oid, reftable_ref_record_val1(&ref), + &hash_algos[hash_id]); + + errors |= refs_fsck_ref(ref_store, o, &report, ref.refname, &oid); + break; + } + case REFTABLE_REF_SYMREF: + errors |= refs_fsck_symref(ref_store, o, &report, ref.refname, + ref.value.symref); + break; + default: + BUG("unhandled reference value type %d", ref.value_type); + } + } + + ret = errors ? -1 : 0; + +out: + if (iter) + ref_iterator_free(&iter->base); + reftable_ref_record_release(&ref); + strbuf_release(&refname); return ret; } diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh index 0ef483659d..3c1f553b81 100755 --- a/t/t0602-reffiles-fsck.sh +++ b/t/t0602-reffiles-fsck.sh @@ -905,4 +905,34 @@ test_expect_success '--[no-]references option should apply to fsck' ' ) ' +test_expect_success 'complains about broken root ref' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + 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/heads/../HEAD${SQ} + EOF + test_cmp expect err + ) +' + +test_expect_success 'complains about broken root ref in worktree' ' + test_when_finished "rm -rf repo worktree" && + git init repo && + ( + cd repo && + test_commit initial && + git worktree add ../worktree && + 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/heads/../HEAD${SQ} + EOF + test_cmp expect err + ) +' + test_done diff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh index 677eb9143c..d24b87f961 100755 --- a/t/t0614-reftable-fsck.sh +++ b/t/t0614-reftable-fsck.sh @@ -55,4 +55,48 @@ for TABLE_NAME in "foo-bar-e4d12d59.ref" \ ' done +test_expect_success 'worktree stacks can be verified' ' + test_when_finished "rm -rf repo worktree" && + git init repo && + test_commit -C repo initial && + git -C repo worktree add ../worktree && + + git -C worktree refs verify 2>err && + test_must_be_empty err && + + REFTABLE_DIR=$(git -C worktree rev-parse --git-dir)/reftable && + EXISTING_TABLE=$(head -n1 "$REFTABLE_DIR/tables.list") && + mv "$REFTABLE_DIR/$EXISTING_TABLE" "$REFTABLE_DIR/broken.ref" && + + for d in repo worktree + do + echo "broken.ref" >"$REFTABLE_DIR/tables.list" && + git -C "$d" refs verify 2>err && + cat >expect <<-EOF && + warning: broken.ref: badReftableTableName: invalid reftable table name + EOF + test_cmp expect err && + + echo garbage >"$REFTABLE_DIR/tables.list" && + test_must_fail git -C "$d" refs verify 2>err && + cat >expect <<-EOF && + error: reftable stack for worktree ${SQ}worktree${SQ} is broken + EOF + test_cmp expect err || return 1 + + done +' + +test_expect_success 'invalid symref gets reported' ' + test_when_finished "rm -rf repo" && + git init repo && + test_commit -C repo initial && + git -C repo symbolic-ref refs/heads/symref garbage && + test_must_fail git -C repo refs verify 2>err && + cat >expect <<-EOF && + error: refs/heads/symref: badReferentName: points to invalid refname ${SQ}garbage${SQ} + EOF + test_cmp expect err +' + test_done diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index c4b651c2dc..3fae05f9d9 100755 --- 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' ' @@ -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)' ' @@ -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' ' @@ -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' '