git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4] cache-tree.c: remove implicit dependency on the_repository

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 7, 2021, 23:03 UTC
Message-ID
<xmqqsg41wvdi.fsf@gitster.g>
In-Reply-To
<pull.915.v4.git.1617778489719.gitgitgadget@gmail.com>
"Chinmoy via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 8 quoted lines
> From: Chinmoy Chakraborty <chinmoy12c@gmail.com>
>
> This kills the_repository dependency in cache_tree_update()
> and prime_cache_tree().
>
> Signed-off-by: Chinmoy Chakraborty <chinmoy12c@gmail.com>
> ---
>     Replace the_repository with r
Huh???
Show 31 quoted lines
> diff --git a/cache-tree.c b/cache-tree.c
> index add1f0771317..4928a9f0f13b 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -446,10 +446,10 @@ int cache_tree_update(struct index_state *istate, int flags)
>  		istate->cache_tree = cache_tree();
>  
>  	trace_performance_enter();
> -	trace2_region_enter("cache_tree", "update", the_repository);
> +	trace2_region_enter("cache_tree", "update", istate->repo);
>  	i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
>  		       "", 0, &skip, flags);
> -	trace2_region_leave("cache_tree", "update", the_repository);
> +	trace2_region_leave("cache_tree", "update", istate->repo);
>  	trace_performance_leave("cache_tree_update");
>  	if (i < 0)
>  		return i;
> @@ -746,13 +746,13 @@ void prime_cache_tree(struct repository *r,
>  		      struct index_state *istate,
>  		      struct tree *tree)
>  {
> -	trace2_region_enter("cache-tree", "prime_cache_tree", the_repository);
> +	trace2_region_enter("cache-tree", "prime_cache_tree", r);
>  	cache_tree_free(&istate->cache_tree);
>  	istate->cache_tree = cache_tree();
>  
>  	prime_cache_tree_rec(r, istate->cache_tree, tree);
>  	istate->cache_changed |= CACHE_TREE_CHANGED;
> -	trace2_region_leave("cache-tree", "prime_cache_tree", the_repository);
> +	trace2_region_leave("cache-tree", "prime_cache_tree", r);
>  }

The patch assumes that istate->repo will always set, but it does not even try to justify why that assumption is safe to make (e.g. "the entire codebase that leads to this function has been audited and made sure istate at this point will always have its .repo member is set" in the log message, if such an audit has actually been done, may have been convincing), which I find quite troubling.

Previous: Chinmoy via GitGitGadgetNext: Chinmoy Chakraborty
Message 15 of 17 in “cache-tree.c: remove implicit dependency on the_repository”
  1. cache-tree.c: remove implicit dependency on the_repositoryChinmoy via GitGitGadget, Mar 25, 2021
  2. Derrick StoleeMar 25, 2021
  3. Chinmoy ChakrabortyMar 26, 2021
  4. Chinmoy ChakrabortyMar 26, 2021
  5. cache-tree.c: remove implicit dependency on the_repositoryChinmoy via GitGitGadget, Mar 26, 2021
  6. cache-tree.c: remove implicit dependency on the_repositoryChinmoy via GitGitGadget, Apr 3, 2021
  7. Junio C HamanoApr 4, 2021
  8. Chinmoy ChakrabortyApr 4, 2021
  9. Junio C HamanoApr 4, 2021
  10. Chinmoy ChakrabortyApr 4, 2021
  11. Junio C HamanoApr 4, 2021
  12. Derrick StoleeApr 5, 2021
  13. Junio C HamanoApr 5, 2021
  14. cache-tree.c: remove implicit dependency on the_repositoryChinmoy via GitGitGadget, Apr 7, 2021
  15. Junio C HamanoApr 7, 2021
  16. Chinmoy ChakrabortyApr 8, 2021
  17. Junio C HamanoApr 8, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.