[PATCH] read-cache: use index state repository for trace2 logging

[PATCH] read-cache: use index state repository for trace2 logging

[Jayesh Daga via GitGitGadget]

From: jayesh0104 <jayeshdaga99@gmail.com>

Replace uses of the_repository in trace2_data_intmax() with
istate->repo, which represents the repository associated with
the index state.

This avoids relying on global repository state and aligns with
other parts of the codebase (e.g., sparse-index.c) that pass the
repository instance explicitly.

No functional change intended.

Signed-off-by: jayesh0104 <jayeshdaga99@gmail.com>
---
    [GSoC] read-cache: use index state repository for trace2 logging
    
    
    HIGH LEVEL
    ==========
    
    The current implementation of trace2_data_intmax() in read-cache.c
    relies on the global the_repository instance.
    
    As part of the ongoing effort to "lib-ify" the Git codebase and reduce
    dependence on global state, this patch transitions those calls to use
    the repository instance associated with the index_state.
    
    
    Low-level (Implementation & Justification)
    ==========================================
    
    In read-cache.c, the index_state (istate) typically carries a pointer to
    its associated repository. However, because istate->repo is not
    guaranteed to be initialized in all code paths (e.g., certain low-level
    utility or testing contexts), this patch implements a defensive fallback
    pattern.
    
    Changes:
    
    Introduced a local repository pointer r that prefers istate->repo but
    falls back to the_repository if the former is NULL.
    
    Updated trace2_data_intmax() calls to use this context-aware pointer.
    
    + struct repository *r = istate->repo ? istate->repo : the_repository;
    - trace2_data_intmax("index", the_repository, "read/version", istate->version);
    + trace2_data_intmax("index", r, "read/version", istate->version);
    
    
    Benefits:
    
    Thread Safety & Modernization: Aligns with the project's goal of moving
    away from the_repository.
    
    Robustness: The ternary fallback ensures we avoid potential NULL pointer
    dereferences while maintaining existing logging behavior in edge cases.
    
    Consistency: Follows patterns seen in other modernized areas of the
    codebase.
    
    
    Summary
    =======
    
    Transitioned trace2 logging in read-cache.c from global to local
    repository context.
    
    Implemented a safety fallback to the_repository to handle uninitialized
    istate->repo pointers.
    
    No functional changes to telemetry output are intended.
    
    cc :Karthik Nayak karthik.188@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2253%2Fjayesh0104%2Ftrace2-istate-repo-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2253/jayesh0104/trace2-istate-repo-v1
Pull-Request: https://github.com/git/git/pull/2253

 read-cache.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 5049f9baca..2c5c5165e0 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2206,6 +2206,7 @@ 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;
@@ -2313,9 +2314,10 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
 	 * TODO trace2: replace "the_repository" with the actual repo instance
 	 * that is associated with the given "istate".
 	 */
-	trace2_data_intmax("index", the_repository, "read/version",
+	r = istate->repo ? istate->repo : the_repository;
+	trace2_data_intmax("index", r, "read/version",
 			   istate->version);
-	trace2_data_intmax("index", the_repository, "read/cache_nr",
+	trace2_data_intmax("index", r, "read/cache_nr",
 			   istate->cache_nr);
 
 	/*

base-commit: ca1db8a0f7dc0dbea892e99f5b37c5fe5861be71
-- 
gitgitgadget

Re: [PATCH] read-cache: use index state repository for trace2 logging

[Derrick Stolee]

On 3/27/2026 6:08 AM, Jayesh Daga via GitGitGadget wrote:

>     Robustness: The ternary fallback ensures we avoid potential NULL pointer
>     dereferences while maintaining existing logging behavior in edge cases.

> +	r = istate->repo ? istate->repo : the_repository;

If I understand correctly, it is a bug if istate->repo is NULL.

Did you try running the test suite with istate->repo as a replacement for
the_repository in these tracing calls? Is there a legitimate scenario
where this would be NULL?

> +	trace2_data_intmax("index", r, "read/version",
>  			   istate->version);
> -	trace2_data_intmax("index", the_repository, "read/cache_nr",
> +	trace2_data_intmax("index", r, "read/cache_nr",
>  			   istate->cache_nr);

Other than that, this is a minor improvement in the right direction. I'd
rather that it be more complete if you are working in this file.

There are several places where we use the_repository and there is an
'istate' right there. If this change works, then why not apply that same
transformation in the other places?

There are TODO comments for many of these, including the hunk you are
editing (be sure to remove these!).

There are other cases that I see in this file:

* refresh_index() uses the_repository for progress.

* tweak_untracked_cache() has a local pointer 'r' that could
  be set to istate->repo

* tweak_split_index() passes the_repository to
  repo_config_get_split_index().

* do_read_index() uses the_repository when it could use
  istate->repo.

* read_index_from() uses the_repository for tracing.

* verify_index_from() uses the_repository->hash_algo but
  has an istate->repo that could be used.

* do_write_index() has several instances.

(At this point, I stopped taking inventory.)

There are other uses of the_repository that will be harder to remove as
there isn't a local istate at the time, so those aren't worth combining
with this kind of effort.

If you are already working in this space, then I recommend figuring out
how much we can rely on istate->repo and then apply that knowledge to
these cases as separate commits:

1. Replace the uses in the trace2 calls with istate->repo
   and delete the TODO comments.

2. Replace the other uses of the_repository when an istate
   exists already.

Thanks, -Stolee

Re: [PATCH] read-cache: use index state repository for trace2 logging

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

> On 3/27/2026 6:08 AM, Jayesh Daga via GitGitGadget wrote:
>
>>     Robustness: The ternary fallback ensures we avoid potential NULL pointer
>>     dereferences while maintaining existing logging behavior in edge cases.
>
>> +	r = istate->repo ? istate->repo : the_repository;
>
> If I understand correctly, it is a bug if istate->repo is NULL.

Because INDEX_STATE_INIT(r) assigns the repository as the first
thing, I tend to agree.  A (bare) repository can lack the index
so repo->index might be NULL, but if you have an istate instance,
it should always know which repository it came from.

>> +	trace2_data_intmax("index", r, "read/version",
>>  			   istate->version);
>> -	trace2_data_intmax("index", the_repository, "read/cache_nr",
>> +	trace2_data_intmax("index", r, "read/cache_nr",
>>  			   istate->cache_nr);
>
> Other than that, this is a minor improvement in the right direction. I'd
> rather that it be more complete if you are working in this file.

;-)  Long timers always aim higher than posted patches.

> If you are already working in this space, then I recommend figuring out
> how much we can rely on istate->repo and then apply that knowledge to
> these cases as separate commits:
>
> 1. Replace the uses in the trace2 calls with istate->repo
>    and delete the TODO comments.
>
> 2. Replace the other uses of the_repository when an istate
>    exists already.

Excellent suggestion.
Thanks.

By the way, Jeyesh, do you really want to be known with a numbered
"jayesh0104" as your name?  These author identities are cast in
stone in commit objects and will stay with the project.

Also see Documentation/SubmittingPatches::[dco,real-name].

Re: [PATCH] read-cache: use index state repository for trace2 logging

[jayesh0104]

Hi Junio, Derrick,

Thanks for the detailed review and suggestions.

On the fallback to `the_repository`: I agree with your observation that `istate->repo` being NULL would indicate a bug rather than a scenario to defensively handle. My initial intent was to be conservative in case there were edge paths where `istate->repo` might not be initialized, but given that INDEX_STATE_INIT(r) sets this unconditionally, it makes sense to rely on that invariant instead of masking potential issues. I will drop the fallback and use `istate->repo` directly (and verify via the test suite).

Regarding scope, Derrick’s suggestion to split this into separate commits makes sense. I’ll proceed as follows:

1. A focused patch that replaces `the_repository` with `istate->repo` in the trace2 calls within this file and removes the associated TODO comments.

2. A follow-up patch that replaces other uses of `the_repository` in places where an `istate` is already available (e.g., `refresh_index()`, `tweak_untracked_cache()`, `do_write_index()`, etc.), keeping changes logically grouped for easier review.

I’ll also run the full test suite after removing the fallback to confirm there are no hidden assumptions.

Junio, thanks also for pointing out the author identity. I’ll update it to use my real name in the next version.

Thanks again for the guidance.

Best,
Jayesh

[PATCH v2] read-cache: use istate->repo for trace2 logging

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@gmail.com>

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).

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.

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.

Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
    [GSoC] read-cache: use index state repository for trace2 logging
    
    
    HIGH LEVEL
    ==========
    
    trace2_data_intmax() calls in read-cache.c currently use the global
    'the_repository' instance, even though index_state already carries an
    explicit repository pointer (istate->repo).
    
    Use istate->repo directly in these calls, making the repository context
    associated with the index explicit.
    
    
    Low-level (Implementation & Justification)
    ==========================================
    
    index_state instances are initialized via INDEX_STATE_INIT(r), which
    sets istate->repo. Therefore, istate->repo is expected to be non-NULL in
    normal code paths.
    
    Using the global 'the_repository' in this context is redundant and can
    obscure the actual repository associated with the index being read.
    
    In particular, relying on the global repository may lead to misleading
    trace2 output in scenarios where multiple repository instances are in
    use (e.g., tests or future refactoring toward improved library
    boundaries), as events could be attributed to the wrong repository.
    
    Changes:
    
     * Replace uses of the_repository in trace2_data_intmax() with
       istate->repo
     * Enforce the expectation that istate->repo is non-NULL via BUG()
     * Remove the now-obsolete TODO comment
    
    Example:
    
     * trace2_data_intmax("index", the_repository, "read/version", ...)
    
     * trace2_data_intmax("index", istate->repo, "read/version", ...)
    
    
    Summary
    =======
    
    Use the repository associated with index_state for trace2 logging
    instead of the global repository instance, making the repository context
    explicit and consistent with existing initialization guarantees.
    
    
    Changes since v1:
    =================
    
     * Dropped fallback to the_repository; rely on istate->repo invariant
     * Added BUG() check for NULL istate->repo
     * Removed TODO comment
     * Improved commit message with detailed rationale
     * Cleaned up duplicate commits and fixed history

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2253%2Fjayesh0104%2Ftrace2-istate-repo-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2253/jayesh0104/trace2-istate-repo-v2
Pull-Request: https://github.com/git/git/pull/2253

Range-diff vs v1:

 1:  ba6d2add1d ! 1:  7fdfaaef9b read-cache: use index state repository for trace2 logging
     @@
       ## Metadata ##
     -Author: jayesh0104 <jayeshdaga99@gmail.com>
     +Author: Jayesh Daga <jayeshdaga99@gmail.com>
      
       ## Commit message ##
     -    read-cache: use index state repository for trace2 logging
     +    read-cache: use istate->repo for trace2 logging
      
     -    Replace uses of the_repository in trace2_data_intmax() with
     -    istate->repo, which represents the repository associated with
     -    the index state.
     +    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).
      
     -    This avoids relying on global repository state and aligns with
     -    other parts of the codebase (e.g., sparse-index.c) that pass the
     -    repository instance explicitly.
     +    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.
      
     -    No functional change intended.
     +    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.
      
     -    Signed-off-by: jayesh0104 <jayeshdaga99@gmail.com>
     +    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.
     +
     +    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)
     @@ read-cache.c: int do_read_index(struct index_state *istate, const char *path, in
       	if (istate->initialized)
       		return istate->cache_nr;
      @@ read-cache.c: int do_read_index(struct index_state *istate, const char *path, int must_exist)
     - 	 * TODO trace2: replace "the_repository" with the actual repo instance
     - 	 * that is associated with the given "istate".
     - 	 */
     + 	}
     + 	munmap((void *)mmap, mmap_size);
     + 
     +-	/*
     +-	 * TODO trace2: replace "the_repository" with the actual repo instance
     +-	 * that is associated with the given "istate".
     +-	 */
      -	trace2_data_intmax("index", the_repository, "read/version",
     -+	r = istate->repo ? istate->repo : the_repository;
     ++	r=istate->repo;
     ++	if (!r)
     ++	    BUG("istate->repo is NULL in do_read_index");
      +	trace2_data_intmax("index", r, "read/version",
       			   istate->version);
      -	trace2_data_intmax("index", the_repository, "read/cache_nr",


 read-cache.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 5049f9baca..46ffb49cab 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2206,6 +2206,7 @@ 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;
@@ -2309,13 +2310,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
 	}
 	munmap((void *)mmap, mmap_size);
 
-	/*
-	 * TODO trace2: replace "the_repository" with the actual repo instance
-	 * 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",
 			   istate->version);
-	trace2_data_intmax("index", the_repository, "read/cache_nr",
+	trace2_data_intmax("index", r, "read/cache_nr",
 			   istate->cache_nr);
 
 	/*

base-commit: ca1db8a0f7dc0dbea892e99f5b37c5fe5861be71
-- 
gitgitgadget

Re: [PATCH v2] read-cache: use istate->repo for trace2 logging

[Junio C Hamano]

"Jayesh Daga via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jayesh Daga <jayeshdaga99@gmail.com>
>
> 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).

Drop "currently".  The canonical order in log messages in this
project is to describe the current state of the relevant part of the
codebase in present tense, pulling attention of readers to what the
author finds problematic, and propose a solution.  And finally give
orders to somebody sitting in front of the keyboard to make the
change.

> 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.

Even worse, it could do a wrong thing, so saying "redundant" here
somewhat misses the point.

> 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.

Exactly.

> diff --git a/read-cache.c b/read-cache.c
> index 5049f9baca..46ffb49cab 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2206,6 +2206,7 @@ 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;
> @@ -2309,13 +2310,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
>  	}
>  	munmap((void *)mmap, mmap_size);
>  
> -	/*
> -	 * TODO trace2: replace "the_repository" with the actual repo instance
> -	 * that is associated with the given "istate".
> -	 */
> -	trace2_data_intmax("index", the_repository, "read/version",
> +	r=istate->repo;

Have SP on both sides of an assignment operator '=', i.e.

	r = istate->repo;

> +	if (!r)
> +	    BUG("istate->repo is NULL in do_read_index");
> +	trace2_data_intmax("index", r, "read/version",
>  			   istate->version);
> -	trace2_data_intmax("index", the_repository, "read/cache_nr",
> +	trace2_data_intmax("index", r, "read/cache_nr",
>  			   istate->cache_nr);

Or you can do without an intermediate variable 'r'.  Replacing
"the_repository" with "istate->repo" would make the resulting line
shorter already, and more importantly, readers do not have to
remember that 'r' is an alias for 'istate->repo' while reading the
code.

Thanks.

Re: [PATCH v2] read-cache: use istate->repo for trace2 logging

[Derrick Stolee]

On 3/28/2026 2:36 PM, Junio C Hamano wrote:
> "Jayesh Daga via GitGitGadget" <gitgitgadget@gmail.com> writes:

>> -	/*
>> -	 * TODO trace2: replace "the_repository" with the actual repo instance
>> -	 * that is associated with the given "istate".
>> -	 */
>> -	trace2_data_intmax("index", the_repository, "read/version",
>> +	r=istate->repo;
> 
> Have SP on both sides of an assignment operator '=', i.e.
> 
> 	r = istate->repo;
> 
>> +	if (!r)
>> +	    BUG("istate->repo is NULL in do_read_index");
>> +	trace2_data_intmax("index", r, "read/version",
>>  			   istate->version);
>> -	trace2_data_intmax("index", the_repository, "read/cache_nr",
>> +	trace2_data_intmax("index", r, "read/cache_nr",
>>  			   istate->cache_nr);
> 
> Or you can do without an intermediate variable 'r'.  Replacing
> "the_repository" with "istate->repo" would make the resulting line
> shorter already, and more importantly, readers do not have to
> remember that 'r' is an alias for 'istate->repo' while reading the
> code.

And I don't think we need these BUG() statements, so I'm sorry if
that was inferred from my statement. This kind of bug isn't
something that a developer will stumble upon by accidentally
calling the method incorrectly, but instead would be a substantial
break of the index_state structure. A segfault would be enough to
catch this in testing.

Have you thought about applying this pattern to the rest of the
trace2 statements in this file? Or did you want review to solidify
on this section first?

Thanks,
-Stolee

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

[Jayesh Daga via GitGitGadget]

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

[PATCH v3 1/2] repo: add paths.git_dir repo info key

[jayesh0104 via GitGitGadget]

From: jayesh0104 <jayeshdaga99@gmail.com>

Introduce a new repo info key `paths.git_dir` to expose the
repository's gitdir path, equivalent to `git rev-parse --git-dir`.

This improves consistency and allows tools to retrieve the gitdir
path without invoking external commands.

The implementation adds support in repo.c and integrates it into
the repo info reporting mechanism. Documentation is updated to
describe the new key, and tests are added to verify that the value
matches the output of `git rev-parse --git-dir`.

Signed-off-by: jayesh0104 <jayeshdaga99@gmail.com>
---
 Documentation/git-repo.adoc |  5 +++++
 builtin/repo.c              |  7 +++++++
 t/t1900-repo-info.sh        | 10 ++++++++++
 3 files changed, 22 insertions(+)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 42262c1983..d17d911ec6 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -95,6 +95,11 @@ In order to obtain a set of values from `git repo info`, you should provide
 the keys that identify them. Here's a list of the available keys and the
 values that they return:
 
+`paths.git_dir`::
+	The path to the Git directory for the repository (equivalent to
+	`git rev-parse --git-dir`).
+
+
 `layout.bare`::
 	`true` if this is a bare repository, otherwise `false`.
 
diff --git a/builtin/repo.c b/builtin/repo.c
index 55f9b9095c..3067107cad 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -66,11 +66,18 @@ static int get_references_format(struct repository *repo, struct strbuf *buf)
 	return 0;
 }
 
+static int get_paths_git_dir(struct repository *repo, struct strbuf *buf)
+{
+	strbuf_addstr(buf, repo_get_git_dir(repo));
+	return 0;
+}
+
 /* repo_info_field keys must be in lexicographical order */
 static const struct repo_info_field repo_info_field[] = {
 	{ "layout.bare", get_layout_bare },
 	{ "layout.shallow", get_layout_shallow },
 	{ "object.format", get_object_format },
+	{ "paths.git_dir", get_paths_git_dir },
 	{ "references.format", get_references_format },
 };
 
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index a9eb07abe8..63be0849c4 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -149,4 +149,14 @@ test_expect_success 'git repo info --keys uses lines as its default output forma
 	test_cmp expect actual
 '
 
+test_expect_success 'paths.git_dir matches rev-parse --git-dir' '
+	git init repo &&
+	(
+		cd repo &&
+		git repo info paths.git_dir >actual &&
+		echo "paths.git_dir=$(git rev-parse --git-dir)" >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
gitgitgadget

[PATCH v3 2/2] read-cache: use istate->repo for trace2 logging

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@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 to ensure correct repository
attribution.

Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
 read-cache.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 5049f9baca..b1074fbf06 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2309,13 +2309,9 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
 	}
 	munmap((void *)mmap, mmap_size);
 
-	/*
-	 * TODO trace2: replace "the_repository" with the actual repo instance
-	 * that is associated with the given "istate".
-	 */
-	trace2_data_intmax("index", the_repository, "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", istate->repo, "read/cache_nr",
 			   istate->cache_nr);
 
 	/*
@@ -2360,16 +2356,12 @@ 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;
@@ -3096,13 +3088,9 @@ 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;
@@ -3144,14 +3132,10 @@ 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

[PATCH v4] read-cache: use istate->repo for trace2 logging

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@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 to ensure correct repository
attribution.

Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
    [GSoC] read-cache: use index state repository for trace2 logging
    
    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.
    
    v4:
    
     * reroll as clean series; drop unrelated commit.
    
    v2:
    
     * Apply the change consistently across read-cache.c
     * Drop unnecessary intermediate variable
     * Remove obsolete TODO comments
     * Update commit message

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2253%2Fjayesh0104%2Ftrace2-istate-repo-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2253/jayesh0104/trace2-istate-repo-v4
Pull-Request: https://github.com/git/git/pull/2253

Range-diff vs v3:

 1:  5a8165b05d < -:  ---------- repo: add paths.git_dir repo info key
 2:  9bb6d0fa01 = 1:  c99d731efa read-cache: use istate->repo for trace2 logging


 read-cache.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 5049f9baca..b1074fbf06 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2309,13 +2309,9 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
 	}
 	munmap((void *)mmap, mmap_size);
 
-	/*
-	 * TODO trace2: replace "the_repository" with the actual repo instance
-	 * that is associated with the given "istate".
-	 */
-	trace2_data_intmax("index", the_repository, "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", istate->repo, "read/cache_nr",
 			   istate->cache_nr);
 
 	/*
@@ -2360,16 +2356,12 @@ 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;
@@ -3096,13 +3088,9 @@ 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;
@@ -3144,14 +3132,10 @@ 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)

base-commit: 5361983c075154725be47b65cca9a2421789e410
-- 
gitgitgadget

Re: [PATCH v4] read-cache: use istate->repo for trace2 logging

[Junio C Hamano]

"Jayesh Daga via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jayesh Daga <jayeshdaga99@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 to ensure correct repository
> attribution.
>
> Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
> ---
> Range-diff vs v3:
>
>  1:  5a8165b05d < -:  ---------- repo: add paths.git_dir repo info key
>  2:  9bb6d0fa01 = 1:  c99d731efa read-cache: use istate->repo for trace2 logging

A range-diff with v2 would have been much more relevant, but the
patch below looks super boring compared to v2 which is very good.
Just replacing the_repository with istate->repo and nothing else,
which is exactly we expect to see from the patch title above ;-)

Will queue.  Thanks.

Re: [PATCH v4] read-cache: use istate->repo for trace2 logging

[Derrick Stolee]

On 3/30/26 4:04 PM, Junio C Hamano wrote:
> "Jayesh Daga via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Jayesh Daga <jayeshdaga99@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 to ensure correct repository
>> attribution.
>>
>> Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
>> ---
>> Range-diff vs v3:
>>
>>   1:  5a8165b05d < -:  ---------- repo: add paths.git_dir repo info key
>>   2:  9bb6d0fa01 = 1:  c99d731efa read-cache: use istate->repo for trace2 logging
> 
> A range-diff with v2 would have been much more relevant, but the
> patch below looks super boring compared to v2 which is very good.
> Just replacing the_repository with istate->repo and nothing else,
> which is exactly we expect to see from the patch title above ;-)
> 
> Will queue.  Thanks.

Thanks for putting in the work to create this complete
commit. LGTM.

-Stolee

Re: [PATCH] read-cache: use istate->repo for trace2 logging

[Jayesh Daga]

Hi Derrick,

Thanks for the review and for taking the time to look at this.

Appreciate it!

Thanks,
Jayesh