fsck: avoid misleading variable name

Commit Message

Eric Sunshine June 29, 2023, 6:13 p.m. UTC

From: Eric Sunshine <[e-mail]>

When reporting a problem, `git fsck` emits a message such as:

    missing blob 1234abcd (:file)

However, this can be ambiguous when the problem is detected in the index
of a worktree other than the one in which `git fsck` was invoked. To
address this shortcoming, 592ec63b38 (fsck: mention file path for index
errors, 2023-02-24) enhanced the output to mention the path of the index
when the problem is detected in some other worktree:

    missing blob 1234abcd (.git/worktrees/wt/index:file)

Unfortunately, the variable in fsck_index() which controls whether the
index path should be shown is misleadingly named "is_main_index" which
can be misunderstood as referring to the main worktree (i.e. the one
housing the .git/ repository) rather than to the current worktree (i.e.
the one in which `git fsck` was invoked). Avoid such potential confusion
by choosing a name more reflective of its actual purpose.

Signed-off-by: Eric Sunshine <[e-mail]>
---

The associated discussion which led to this patch begins at [1].

[1]: https://lore.kernel.org/git/[e-mail]/

 builtin/fsck.c  | 4 ++--
 t/t1450-fsck.sh | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Jeff King June 29, 2023, 7:04 p.m. UTC | #1

On Thu, Jun 29, 2023 at 02:13:33PM -0400, Eric Sunshine wrote:

> From: Eric Sunshine <[e-mail]>
> 
> When reporting a problem, `git fsck` emits a message such as:
> 
>     missing blob 1234abcd (:file)
> 
> However, this can be ambiguous when the problem is detected in the index
> of a worktree other than the one in which `git fsck` was invoked. To
> address this shortcoming, 592ec63b38 (fsck: mention file path for index
> errors, 2023-02-24) enhanced the output to mention the path of the index
> when the problem is detected in some other worktree:
> 
>     missing blob 1234abcd (.git/worktrees/wt/index:file)
> 
> Unfortunately, the variable in fsck_index() which controls whether the
> index path should be shown is misleadingly named "is_main_index" which
> can be misunderstood as referring to the main worktree (i.e. the one
> housing the .git/ repository) rather than to the current worktree (i.e.
> the one in which `git fsck` was invoked). Avoid such potential confusion
> by choosing a name more reflective of its actual purpose.

This looks good to me. Thanks for following up!

-Peff

Patch

diff --git a/builtin/fsck.c b/builtin/fsck.c
index d9aa4db828..0c00920703 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -808,7 +808,7 @@  static int fsck_resolve_undo(struct index_state *istate,
 }
 
 static void fsck_index(struct index_state *istate, const char *index_path,
-		       int is_main_index)
+		       int is_current_worktree)
 {
 	unsigned int i;
 
@@ -830,7 +830,7 @@  static void fsck_index(struct index_state *istate, const char *index_path,
 		obj->flags |= USED;
 		fsck_put_object_name(&fsck_walk_options, &obj->oid,
 				     "%s:%s",
-				     is_main_index ? "" : index_path,
+				     is_current_worktree ? "" : index_path,
 				     istate->cache[i]->name);
 		mark_object_reachable(obj);
 	}
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 8c442adb1a..5805d47eb9 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -1036,9 +1036,9 @@  test_expect_success 'fsck detects problems in worktree index' '
 	test_cmp expect actual
 '
 
-test_expect_success 'fsck reports problems in main index without filename' '
+test_expect_success 'fsck reports problems in current worktree index without filename' '
 	test_when_finished "rm -f .git/index && git read-tree HEAD" &&
-	echo "this object will be removed to break the main index" >file &&
+	echo "this object will be removed to break current worktree index" >file &&
 	git add file &&
 	blob=$(git rev-parse :file) &&
 	remove_object $blob &&