From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 31 Mar 2026 05:31:09 GMT
Subject: Re: [PATCH] unpack-trees: use explicit repository in trace2 calls
Message-ID: <actcHT_ZHkb58ndi@pks.im>
In-Reply-To: <pull.2258.git.git.1774901607564.gitgitgadget@gmail.com>

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