[PATCH] unpack-trees: use explicit repository in trace2 calls

[PATCH] unpack-trees: use explicit repository in trace2 calls

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@gmail.com>

trace2 calls in unpack-trees.c use the global 'the_repository',
even though the relevant context provides an explicit repository
pointer via 'istate->repo' or the local 'repo' variable.

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 explicit repository pointers instead to ensure correct
repository attribution.

Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
    unpack-trees: use explicit repository in trace2 calls
    
    trace2 calls in unpack-trees.c use the global 'the_repository', even
    though the relevant context provides an explicit repository pointer via
    'istate->repo' or the local 'repo' variable.
    
    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 explicit repository pointers instead in these call sites to ensure
    correct repository attribution.
    
    Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com
    
    cc :Karthik Nayak karthik.188@gmail.com

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

 unpack-trees.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 998a1e6dc7..191b9d4769 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1780,14 +1780,14 @@ static int clear_ce_flags(struct index_state *istate,
 
 	xsnprintf(label, sizeof(label), "clear_ce_flags(0x%08lx,0x%08lx)",
 		  (unsigned long)select_mask, (unsigned long)clear_mask);
-	trace2_region_enter("unpack_trees", label, the_repository);
+	trace2_region_enter("unpack_trees", label, istate->repo);
 	rval = clear_ce_flags_1(istate,
 				istate->cache,
 				istate->cache_nr,
 				&prefix,
 				select_mask, clear_mask,
 				pl, 0, 0);
-	trace2_region_leave("unpack_trees", label, the_repository);
+	trace2_region_leave("unpack_trees", label, istate->repo);
 
 	stop_progress(&istate->progress);
 	return rval;
@@ -1903,7 +1903,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		BUG("o->df_conflict_entry is an output only field");
 
 	trace_performance_enter();
-	trace2_region_enter("unpack_trees", "unpack_trees", the_repository);
+	trace2_region_enter("unpack_trees", "unpack_trees", repo);
 
 	prepare_repo_settings(repo);
 	if (repo->settings.command_requires_full_index) {
@@ -2007,9 +2007,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		}
 
 		trace_performance_enter();
-		trace2_region_enter("unpack_trees", "traverse_trees", the_repository);
+		trace2_region_enter("unpack_trees", "traverse_trees", repo);
 		ret = traverse_trees(o->src_index, len, t, &info);
-		trace2_region_leave("unpack_trees", "traverse_trees", the_repository);
+		trace2_region_leave("unpack_trees", "traverse_trees", repo);
 		trace_performance_leave("traverse_trees");
 		if (ret < 0)
 			goto return_failed;
@@ -2106,7 +2106,7 @@ done:
 		dir_clear(o->internal.dir);
 		o->internal.dir = NULL;
 	}
-	trace2_region_leave("unpack_trees", "unpack_trees", the_repository);
+	trace2_region_leave("unpack_trees", "unpack_trees", repo);
 	trace_performance_leave("unpack_trees");
 	return ret;
 

base-commit: 5361983c075154725be47b65cca9a2421789e410
-- 
gitgitgadget

Re: [PATCH] unpack-trees: use explicit repository in trace2 calls

[Patrick Steinhardt]

On Mon, Mar 30, 2026 at 08:13:27PM +0000, Jayesh Daga via GitGitGadget wrote:
> From: Jayesh Daga <jayeshdaga99@gmail.com>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 998a1e6dc7..191b9d4769 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1903,7 +1903,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>  		BUG("o->df_conflict_entry is an output only field");
>  
>  	trace_performance_enter();
> -	trace2_region_enter("unpack_trees", "unpack_trees", the_repository);
> +	trace2_region_enter("unpack_trees", "unpack_trees", repo);
>  
>  	prepare_repo_settings(repo);
>  	if (repo->settings.command_requires_full_index) {

The changes in `unpack_trees()` are a bit misleading -- while it reads
as if we don't use `the_repository` anymore, we still do because the
function starts with:

  int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
  {
  	struct repository *repo = the_repository;

So would it make sense to maybe have a separate patch where we inject a
repository as a parameter to `unpack_trees()`?

Once that's done we only have a handful of other places, and in all but
two cases we have a repository available via the index. Do we maybe want
to go all the way so that we can drop `USE_THE_REPOSITORY_VARIABLE` at
the end of this series?

Thanks!

Patrick

Re: [PATCH] unpack-trees: use explicit repository in trace2 calls

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> The changes in `unpack_trees()` are a bit misleading -- while it reads
> as if we don't use `the_repository` anymore, we still do because the
> function starts with:
>
>   int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
>   {
>   	struct repository *repo = the_repository;
>
> So would it make sense to maybe have a separate patch where we inject a
> repository as a parameter to `unpack_trees()`?

We can see that "struct unpack_trees_options" is rich enough in the
merge context that it would be a natural place to have it unless it
is already tehre.

In fact, o->dst_index->repo should probably be what you want, and
because it would be insane to start from an index in a repo and
store the resulting updated index in another repo, there probably
needs an assert(o->dst_index->repo == o->src_index->repo) somewhere.

Re: [PATCH] unpack-trees: use explicit repository in trace2 calls

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Patrick Steinhardt <ps@pks.im> writes:
>
>> The changes in `unpack_trees()` are a bit misleading -- while it reads
>> as if we don't use `the_repository` anymore, we still do because the
>> function starts with:
>>
>>   int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
>>   {
>>   	struct repository *repo = the_repository;
>>
>> So would it make sense to maybe have a separate patch where we inject a
>> repository as a parameter to `unpack_trees()`?
>
> We can see that "struct unpack_trees_options" is rich enough in the
> merge context that it would be a natural place to have it unless it
> is already tehre.
>
> In fact, o->dst_index->repo should probably be what you want, and
> because it would be insane to start from an index in a repo and
> store the resulting updated index in another repo, there probably
> needs an assert(o->dst_index->repo == o->src_index->repo) somewhere.

Actually, assert(dst_index->repo == src_index->repo) is probably not
what we want, as dst_index can legitimately be NULL, even since
34110cd4 (Make 'unpack_trees()' have a separate source and
destination index, 2008-03-06) introduced srparete src/dst indices
to unpack_trees() API.

    We will always unpack into our own internal index, but we will take the
    source from wherever specified, and we will optionally write the result
    to a specified index (optionally, because not everybody even _wants_ any
    result: the index diffing really wants to just walk the tree and index
    in parallel).

So o->src_index->repo is what we want in this case, I think.

[PATCH v2 0/2] unpack-trees: use explicit repository in trace2 calls

[Jayesh Daga via GitGitGadget]

trace2 calls in unpack-trees.c use the global 'the_repository', even though
the relevant context provides an explicit repository pointer via
'istate->repo' or the local 'repo' variable.

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 explicit repository pointers instead in these call sites to ensure
correct repository attribution.

Signed-off-by: Jayesh Daga jayeshdaga99@gmail.com

v2:

 * Use repository from src_index instead of the_repository
 * Address review feedback from Patrick Steinhardt
 * Avoid introducing new API or struct fields

cc :Karthik Nayak karthik.188@gmail.com

Jayesh Daga (2):
  unpack-trees: use repository from index instead of global
  unpack-trees: use repository from index instead of global

 unpack-trees.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)


base-commit: 5361983c075154725be47b65cca9a2421789e410
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2258%2Fjayesh0104%2Funpack-trees-trace2-repo-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2258/jayesh0104/unpack-trees-trace2-repo-v2
Pull-Request: https://github.com/git/git/pull/2258

Range-diff vs v1:

 1:  717da16044 ! 1:  f03ea194e3 unpack-trees: use explicit repository in trace2 calls
     @@ Metadata
      Author: Jayesh Daga <jayeshdaga99@gmail.com>
      
       ## Commit message ##
     -    unpack-trees: use explicit repository in trace2 calls
     +    unpack-trees: use repository from index instead of global
      
     -    trace2 calls in unpack-trees.c use the global 'the_repository',
     -    even though the relevant context provides an explicit repository
     -    pointer via 'istate->repo' or the local 'repo' variable.
     +    unpack_trees() currently initializes its repository from the
     +    global 'the_repository', even though a repository instance is
     +    already available via the source index.
      
     -    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 'o->src_index->repo' instead of the global variable,
     +    reducing reliance on global repository state.
      
     -    Use explicit repository pointers instead to ensure correct
     -    repository attribution.
     +    This is a step towards eliminating global repository usage in
     +    unpack_trees().
      
     +    Suggested-by: Patrick Steinhardt <ps@pks.im>
          Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
      
       ## unpack-trees.c ##
 -:  ---------- > 2:  fbdf3271b7 unpack-trees: use repository from index instead of global

-- 
gitgitgadget

[PATCH v2 1/2] unpack-trees: use repository from index instead of global

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@gmail.com>

unpack_trees() currently initializes its repository from the
global 'the_repository', even though a repository instance is
already available via the source index.

Use 'o->src_index->repo' instead of the global variable,
reducing reliance on global repository state.

This is a step towards eliminating global repository usage in
unpack_trees().

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
 unpack-trees.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 998a1e6dc7..191b9d4769 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1780,14 +1780,14 @@ static int clear_ce_flags(struct index_state *istate,
 
 	xsnprintf(label, sizeof(label), "clear_ce_flags(0x%08lx,0x%08lx)",
 		  (unsigned long)select_mask, (unsigned long)clear_mask);
-	trace2_region_enter("unpack_trees", label, the_repository);
+	trace2_region_enter("unpack_trees", label, istate->repo);
 	rval = clear_ce_flags_1(istate,
 				istate->cache,
 				istate->cache_nr,
 				&prefix,
 				select_mask, clear_mask,
 				pl, 0, 0);
-	trace2_region_leave("unpack_trees", label, the_repository);
+	trace2_region_leave("unpack_trees", label, istate->repo);
 
 	stop_progress(&istate->progress);
 	return rval;
@@ -1903,7 +1903,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		BUG("o->df_conflict_entry is an output only field");
 
 	trace_performance_enter();
-	trace2_region_enter("unpack_trees", "unpack_trees", the_repository);
+	trace2_region_enter("unpack_trees", "unpack_trees", repo);
 
 	prepare_repo_settings(repo);
 	if (repo->settings.command_requires_full_index) {
@@ -2007,9 +2007,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		}
 
 		trace_performance_enter();
-		trace2_region_enter("unpack_trees", "traverse_trees", the_repository);
+		trace2_region_enter("unpack_trees", "traverse_trees", repo);
 		ret = traverse_trees(o->src_index, len, t, &info);
-		trace2_region_leave("unpack_trees", "traverse_trees", the_repository);
+		trace2_region_leave("unpack_trees", "traverse_trees", repo);
 		trace_performance_leave("traverse_trees");
 		if (ret < 0)
 			goto return_failed;
@@ -2106,7 +2106,7 @@ done:
 		dir_clear(o->internal.dir);
 		o->internal.dir = NULL;
 	}
-	trace2_region_leave("unpack_trees", "unpack_trees", the_repository);
+	trace2_region_leave("unpack_trees", "unpack_trees", repo);
 	trace_performance_leave("unpack_trees");
 	return ret;
 
-- 
gitgitgadget

[PATCH v2 2/2] unpack-trees: use repository from index instead of global

[Jayesh Daga via GitGitGadget]

From: Jayesh Daga <jayeshdaga99@gmail.com>

unpack_trees() currently initializes its repository from the
global 'the_repository', even though a repository instance is
already available via the source index.

Use 'o->src_index->repo' instead of the global variable,
reducing reliance on global repository state.

This is a step towards eliminating global repository usage in
unpack_trees().

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Jayesh Daga <jayeshdaga99@gmail.com>
---
 unpack-trees.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 191b9d4769..b42020f16b 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1882,7 +1882,7 @@ static int verify_absent(const struct cache_entry *,
  */
 int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
 {
-	struct repository *repo = the_repository;
+	struct repository *repo = o->src_index->repo;
 	int i, ret;
 	static struct cache_entry *dfc;
 	struct pattern_list pl;
-- 
gitgitgadget

Re: [PATCH v2 0/2] unpack-trees: use explicit repository in trace2 calls

[Junio C Hamano]

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

> Jayesh Daga (2):
>   unpack-trees: use repository from index instead of global
>   unpack-trees: use repository from index instead of global

That is unusual to have two commits with identical title and
identical proposed log messages yet with different patch text.

Do you perhaps want to squash them into a single commit?