[PATCH v3 0/2] [GSoC] read-cache: use index state repository for trace2 logging

["Jayesh Daga via GitGitGadget" <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