Skip to content

Commit bd9a755

Browse files
committed
Sparse Index: log why the index is being expanded (#691)
I will intend to send this upstream after the 2.47.0 release cycle, but this should get to our microsoft/git users for maximum impact. Customers have been struggling with explaining why the sparse index expansion advice message is showing up. The advice to run 'git clean' has not always helped folks, and sometimes it is very unclear why we are running into trouble. These changes introduce a way to log a reason for the expansion into the trace2 logs so it can be found by requesting that a user enable tracing. While testing this, I created the most standard case that happens, which is to have an existing directory match a sparse directory in the index. In this case, it showed that two log messages were required. See the last commit for this new log message. Together, these two places show this kind of message in the `GIT_TRACE2_PERF` output (trimmed for clarity): ``` region_enter | index | label:clear_skip_worktree_from_present_files_sparse data | sparse-index | ..skip-worktree sparsedir:<my-sparse-path>/ data | index | ..sparse_path_count:362 data | index | ..sparse_lstat_count:732 region_leave | index | label:clear_skip_worktree_from_present_files_sparse data | sparse-index | expansion-reason:failed to clear skip-worktree while sparse ``` I added some tests to demonstrate that these logs are recorded, but it also seems difficult to hit some of these cases.
2 parents 5d7750d + c69d14f commit bd9a755

26 files changed

+111
-37
lines changed

Documentation/technical/sparse-index.txt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,3 +206,10 @@ Here are some commands that might be useful to update:
206206
* `git am`
207207
* `git clean`
208208
* `git stash`
209+
210+
In order to help identify the cases where remaining index expansion is
211+
occurring in user machines, calls to `ensure_full_index()` have been
212+
replaced with `ensure_full_index_with_reason()` or with
213+
`ensure_full_index_unaudited()`. These versions add tracing that should
214+
help identify the reason for the index expansion without needing full
215+
access to someone's repository.

builtin/checkout-index.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ static int checkout_all(const char *prefix, int prefix_length)
156156
* first entry inside the expanded sparse directory).
157157
*/
158158
if (ignore_skip_worktree) {
159-
ensure_full_index(the_repository->index);
159+
ensure_full_index_with_reason(the_repository->index,
160+
"checkout-index");
160161
ce = the_repository->index->cache[i];
161162
}
162163
}

builtin/commit.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ static int list_paths(struct string_list *list, const char *with_tree,
385385
}
386386

387387
/* TODO: audit for interaction with sparse-index. */
388-
ensure_full_index(the_repository->index);
388+
ensure_full_index_unaudited(the_repository->index);
389389
for (i = 0; i < the_repository->index->cache_nr; i++) {
390390
const struct cache_entry *ce = the_repository->index->cache[i];
391391
struct string_list_item *item;
@@ -1133,7 +1133,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
11331133
int i, ita_nr = 0;
11341134

11351135
/* TODO: audit for interaction with sparse-index. */
1136-
ensure_full_index(the_repository->index);
1136+
ensure_full_index_unaudited(the_repository->index);
11371137
for (i = 0; i < the_repository->index->cache_nr; i++)
11381138
if (ce_intent_to_add(the_repository->index->cache[i]))
11391139
ita_nr++;

builtin/difftool.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
592592
ret = run_command(&cmd);
593593

594594
/* TODO: audit for interaction with sparse-index. */
595-
ensure_full_index(&wtindex);
595+
ensure_full_index_unaudited(&wtindex);
596596

597597
/*
598598
* If the diff includes working copy files and those

builtin/fsck.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,7 @@ static void fsck_index(struct index_state *istate, const char *index_path,
821821
unsigned int i;
822822

823823
/* TODO: audit for interaction with sparse-index. */
824-
ensure_full_index(istate);
824+
ensure_full_index_unaudited(istate);
825825
for (i = 0; i < istate->cache_nr; i++) {
826826
unsigned int mode;
827827
struct blob *blob;

builtin/ls-files.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ static void show_files(struct repository *repo, struct dir_struct *dir)
413413
return;
414414

415415
if (!show_sparse_dirs)
416-
ensure_full_index(repo->index);
416+
ensure_full_index_with_reason(repo->index, "ls-files");
417417

418418
for (i = 0; i < repo->index->cache_nr; i++) {
419419
const struct cache_entry *ce = repo->index->cache[i];

builtin/merge-index.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ static void merge_all(void)
6666
{
6767
int i;
6868
/* TODO: audit for interaction with sparse-index. */
69-
ensure_full_index(the_repository->index);
69+
ensure_full_index_unaudited(the_repository->index);
7070
for (i = 0; i < the_repository->index->cache_nr; i++) {
7171
const struct cache_entry *ce = the_repository->index->cache[i];
7272
if (!ce_stage(ce))
@@ -93,7 +93,7 @@ int cmd_merge_index(int argc,
9393
repo_read_index(the_repository);
9494

9595
/* TODO: audit for interaction with sparse-index. */
96-
ensure_full_index(the_repository->index);
96+
ensure_full_index_unaudited(the_repository->index);
9797

9898
i = 1;
9999
if (!strcmp(argv[i], "-o")) {

builtin/read-tree.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,8 @@ int cmd_read_tree(int argc,
226226
setup_work_tree();
227227

228228
if (opts.skip_sparse_checkout)
229-
ensure_full_index(the_repository->index);
229+
ensure_full_index_with_reason(the_repository->index,
230+
"read-tree");
230231

231232
if (opts.merge) {
232233
switch (stage - 1) {

builtin/reset.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ static int read_from_tree(const struct pathspec *pathspec,
262262
opt.add_remove = diff_addremove;
263263

264264
if (pathspec->nr && pathspec_needs_expanded_index(the_repository->index, pathspec))
265-
ensure_full_index(the_repository->index);
265+
ensure_full_index_with_reason(the_repository->index,
266+
"reset pathspec");
266267

267268
if (do_diff_cache(tree_oid, &opt))
268269
return 1;

builtin/rm.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,8 @@ int cmd_rm(int argc,
313313
seen = xcalloc(pathspec.nr, 1);
314314

315315
if (pathspec_needs_expanded_index(the_repository->index, &pathspec))
316-
ensure_full_index(the_repository->index);
316+
ensure_full_index_with_reason(the_repository->index,
317+
"rm pathspec");
317318

318319
for (i = 0; i < the_repository->index->cache_nr; i++) {
319320
const struct cache_entry *ce = the_repository->index->cache[i];

0 commit comments

Comments
 (0)