[PATCH v3 0/2] [GSoC] read-cache: use index state repository for trace2 logging
- From
Jayesh Daga via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Mar 30, 2026, 17:27 UTC
- Message-ID
- <pull.2253.v3.git.git.1774891667.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.2253.v2.git.git.1774682046750.gitgitgadget@gmail.com>
trace2 calls in read-cache.c use the global 'the_repository', even though the relevant index_state provides an explicit repository pointer via 'istate->repo'.
Using the global repository can result in incorrect trace2 output when multiple repository instances are in use, as events may be attributed to the wrong repository.
Use 'istate->repo' instead in these call sites to ensure correct repository attribution.
Changes since v2:
* Apply the change consistently across read-cache.c * Drop unnecessary intermediate variable * Remove obsolete TODO comments * Update commit message
Jayesh Daga (1): read-cache: use istate->repo for trace2 logging
jayesh0104 (1): repo: add paths.git_dir repo info key
Documentation/git-repo.adoc | 5 +++++ builtin/repo.c | 7 +++++++ read-cache.c | 32 ++++++++------------------------ t/t1900-repo-info.sh | 10 ++++++++++ 4 files changed, 30 insertions(+), 24 deletions(-)
base-commit: ca1db8a0f7dc0dbea892e99f5b37c5fe5861be71 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2253%2Fjayesh0104%2Ftrace2-istate-repo-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2253/jayesh0104/trace2-istate-repo-v3 Pull-Request: https://github.com/git/git/pull/2253
Range-diff vs v2:
-: ---------- > 1: 5a8165b05d repo: add paths.git_dir repo info key
1: 7fdfaaef9b ! 2: 9bb6d0fa01 read-cache: use istate->repo for trace2 logging
@@ Metadata
## Commit message ##
read-cache: use istate->repo for trace2 logging
- trace2_data_intmax() calls in do_read_index() currently use the
- global 'the_repository' instance, even though the index_state
- already carries an explicit repository pointer (istate->repo).
+ trace2 calls in read-cache.c use the global 'the_repository',
+ even though the relevant index_state provides an explicit
+ repository pointer via 'istate->repo'.
- index_state instances are initialized via INDEX_STATE_INIT(r),
- which sets istate->repo. Using the_repository here is therefore
- redundant and obscures the actual repository context associated
- with the index being read.
+ Using the global repository can result in incorrect trace2
+ output when multiple repository instances are in use, as
+ events may be attributed to the wrong repository.
- In particular, using the global repository can lead to misleading
- trace2 output in scenarios where multiple repository instances are
- in use (such as tests or future refactoring toward better library
- boundaries), as events may be attributed to the wrong repository.
-
- Switch these calls to use istate->repo directly, making the
- association between the index and its repository explicit.
-
- Since istate->repo is expected to be non-NULL, enforce this
- assumption with a BUG() check so that any violation of this
- invariant is caught early.
-
- Also remove the now-obsolete TODO comment.
+ Use 'istate->repo' instead to ensure correct repository
+ attribution.
Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
## read-cache.c ##
-@@ read-cache.c: int do_read_index(struct index_state *istate, const char *path, int must_exist)
- size_t extension_offset = 0;
- int nr_threads, cpus;
- struct index_entry_offset_table *ieot = NULL;
-+ struct repository *r;
-
- if (istate->initialized)
- return istate->cache_nr;
@@ read-cache.c: int do_read_index(struct index_state *istate, const char *path, int must_exist)
}
munmap((void *)mmap, mmap_size);
@@ read-cache.c: int do_read_index(struct index_state *istate, const char *path, in
- * that is associated with the given "istate".
- */
- trace2_data_intmax("index", the_repository, "read/version",
-+ r=istate->repo;
-+ if (!r)
-+ BUG("istate->repo is NULL in do_read_index");
-+ trace2_data_intmax("index", r, "read/version",
++ trace2_data_intmax("index", istate->repo, "read/version",
istate->version);
- trace2_data_intmax("index", the_repository, "read/cache_nr",
-+ trace2_data_intmax("index", r, "read/cache_nr",
++ trace2_data_intmax("index", istate->repo, "read/cache_nr",
istate->cache_nr);
/*
+@@ read-cache.c: int read_index_from(struct index_state *istate, const char *path,
+ if (istate->initialized)
+ return istate->cache_nr;
+
+- /*
+- * TODO trace2: replace "the_repository" with the actual repo instance
+- * that is associated with the given "istate".
+- */
+- trace2_region_enter_printf("index", "do_read_index", the_repository,
++ trace2_region_enter_printf("index", "do_read_index", istate->repo,
+ "%s", path);
+ trace_performance_enter();
+ ret = do_read_index(istate, path, 0);
+ trace_performance_leave("read cache %s", path);
+- trace2_region_leave_printf("index", "do_read_index", the_repository,
++ trace2_region_leave_printf("index", "do_read_index", istate->repo,
+ "%s", path);
+
+ split_index = istate->split_index;
+@@ read-cache.c: static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
+ istate->timestamp.nsec = ST_MTIME_NSEC(st);
+ trace_performance_since(start, "write index, changed mask = %x", istate->cache_changed);
+
+- /*
+- * TODO trace2: replace "the_repository" with the actual repo instance
+- * that is associated with the given "istate".
+- */
+- trace2_data_intmax("index", the_repository, "write/version",
++ trace2_data_intmax("index", istate->repo, "write/version",
+ istate->version);
+- trace2_data_intmax("index", the_repository, "write/cache_nr",
++ trace2_data_intmax("index", istate->repo, "write/cache_nr",
+ istate->cache_nr);
+
+ ret = 0;
+@@ read-cache.c: static int do_write_locked_index(struct index_state *istate,
+ return ret;
+ }
+
+- /*
+- * TODO trace2: replace "the_repository" with the actual repo instance
+- * that is associated with the given "istate".
+- */
+- trace2_region_enter_printf("index", "do_write_index", the_repository,
++ trace2_region_enter_printf("index", "do_write_index", istate->repo,
+ "%s", get_lock_file_path(lock));
+ ret = do_write_index(istate, lock->tempfile, write_extensions, flags);
+- trace2_region_leave_printf("index", "do_write_index", the_repository,
++ trace2_region_leave_printf("index", "do_write_index", istate->repo,
+ "%s", get_lock_file_path(lock));
+
+ if (was_full)-- gitgitgadget