From: Jayesh Daga via GitGitGadget Date: Mon, 30 Mar 2026 17:27:45 GMT Subject: [PATCH v3 0/2] [GSoC] read-cache: use index state repository for trace2 logging Message-ID: In-Reply-To: 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 ## 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