fsck: mention file path for index errors

If we encounter an error in an index file, we may say something like: error: 1234abcd: invalid sha1 pointer in resolve-undo But if you have multiple worktrees, each with its own index, it can be very helpful to know which file had the problem. So let's pass that path down through the various index-fsck functions and use it where appropriate. After this patch you should get something like: error: 1234abcd: invalid sha1 pointer in resolve-undo of .git/worktrees/wt/index That's a bit verbose, but since the point is that you shouldn't see this normally, we're better to err on the side of more details. I've also added the index filename to the name used by "fsck --name-objects", which will show up if we find the object to be missing, etc. This is bending the rules a little there, as the option claims to write names that can be fed to rev-parse. But there is no revision syntax to access the index of another worktree, so the best we can do is make up something that a human will probably understand. I did take care to retain the existing ":file" syntax for the current worktree. So the uglier output should kick in only when it's actually necessary. See the included tests for examples of both forms. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 24, 2023 at 03:12 UTC 592ec63b38ca7e2fb069ce1bf41b47a6f5a4ef8a
2 files changed +46 -16
builtin/fsck.c
+27 -15
@@ -731,19 +731,19 @@ static int fsck_head_link(const char *head_ref_name,
731 return 0;
732 }
733
734 -static int fsck_cache_tree(struct cache_tree *it)
734 +static int fsck_cache_tree(struct cache_tree *it, const char *index_path)
735 {
736 int i;
737 int err = 0;
738
739 if (verbose)
740 - fprintf_ln(stderr, _("Checking cache tree"));
740 + fprintf_ln(stderr, _("Checking cache tree of %s"), index_path);
741
742 if (0 <= it->entry_count) {
743 struct object *obj = parse_object(the_repository, &it->oid);
744 if (!obj) {
745 - error(_("%s: invalid sha1 pointer in cache-tree"),
746 - oid_to_hex(&it->oid));
745 + error(_("%s: invalid sha1 pointer in cache-tree of %s"),
746 + oid_to_hex(&it->oid), index_path);
747 errors_found |= ERROR_REFS;
748 return 1;
749 }
@@ -754,11 +754,12 @@ static int fsck_cache_tree(struct cache_tree *it)
754 err |= objerror(obj, _("non-tree in cache-tree"));
755 }
756 for (i = 0; i < it->subtree_nr; i++)
757 - err |= fsck_cache_tree(it->down[i]->cache_tree);
757 + err |= fsck_cache_tree(it->down[i]->cache_tree, index_path);
758 return err;
759 }
760
761 -static int fsck_resolve_undo(struct index_state *istate)
761 +static int fsck_resolve_undo(struct index_state *istate,
762 + const char *index_path)
763 {
764 struct string_list_item *item;
765 struct string_list *resolve_undo = istate->resolve_undo;
@@ -781,8 +782,9 @@ static int fsck_resolve_undo(struct index_state *istate)
782
783 obj = parse_object(the_repository, &ru->oid[i]);
784 if (!obj) {
784 - error(_("%s: invalid sha1 pointer in resolve-undo"),
785 - oid_to_hex(&ru->oid[i]));
785 + error(_("%s: invalid sha1 pointer in resolve-undo of %s"),
786 + oid_to_hex(&ru->oid[i]),
787 + index_path);
788 errors_found |= ERROR_REFS;
789 continue;
790 }
@@ -795,7 +797,8 @@ static int fsck_resolve_undo(struct index_state *istate)
797 return 0;
798 }
799
798 -static void fsck_index(struct index_state *istate)
800 +static void fsck_index(struct index_state *istate, const char *index_path,
801 + int is_main_index)
802 {
803 unsigned int i;
804
@@ -816,12 +819,14 @@ static void fsck_index(struct index_state *istate)
819 obj = &blob->object;
820 obj->flags |= USED;
821 fsck_put_object_name(&fsck_walk_options, &obj->oid,
819 - ":%s", istate->cache[i]->name);
822 + "%s:%s",
823 + is_main_index ? "" : index_path,
824 + istate->cache[i]->name);
825 mark_object_reachable(obj);
826 }
827 if (istate->cache_tree)
823 - fsck_cache_tree(istate->cache_tree);
824 - fsck_resolve_undo(istate);
828 + fsck_cache_tree(istate->cache_tree, index_path);
829 + fsck_resolve_undo(istate, index_path);
830 }
831
832 static void mark_object_for_connectivity(const struct object_id *oid)
@@ -994,12 +999,19 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
999 struct worktree *wt = *p;
1000 struct index_state istate =
1001 INDEX_STATE_INIT(the_repository);
1002 + char *path;
1003
998 - if (read_index_from(&istate,
999 - worktree_git_path(wt, "index"),
1004 + /*
1005 + * Make a copy since the buffer is reusable
1006 + * and may get overwritten by other calls
1007 + * while we're examining the index.
1008 + */
1009 + path = xstrdup(worktree_git_path(wt, "index"));
1010 + if (read_index_from(&istate, path,
1011 get_worktree_git_dir(wt)) > 0)
1001 - fsck_index(&istate);
1012 + fsck_index(&istate, path, wt->is_current);
1013 discard_index(&istate);
1014 + free(path);
1015 }
1016 free_worktrees(worktrees);
1017 }
t/t1450-fsck.sh
+19 -1
@@ -1032,7 +1032,25 @@ test_expect_success 'fsck detects problems in worktree index' '
1032 blob=$(git -C wt rev-parse :file) &&
1033 remove_object $blob &&
1034
1035 - test_must_fail git fsck
1035 + test_must_fail git fsck --name-objects >actual 2>&1 &&
1036 + cat >expect <<-EOF &&
1037 + missing blob $blob (.git/worktrees/wt/index:file)
1038 + EOF
1039 + test_cmp expect actual
1040 +'
1041 +
1042 +test_expect_success 'fsck reports problems in main index without filename' '
1043 + test_when_finished "rm -f .git/index && git read-tree HEAD" &&
1044 + echo "this object will be removed to break the main index" >file &&
1045 + git add file &&
1046 + blob=$(git rev-parse :file) &&
1047 + remove_object $blob &&
1048 +
1049 + test_must_fail git fsck --name-objects >actual 2>&1 &&
1050 + cat >expect <<-EOF &&
1051 + missing blob $blob (:file)
1052 + EOF
1053 + test_cmp expect actual
1054 '
1055
1056 test_done