Re: [PATCH 01/12] cache-tree: remove dependency on `the_repository`

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> diff --git a/cache-tree.c b/cache-tree.c
> index a220372a42..6e4d82f3e8 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> ...
> @@ -775,7 +781,7 @@ struct tree *write_in_core_index_as_tree(struct repository *repo,
>  	int was_valid, ret;
>  
>  	was_valid = index_state->cache_tree &&
> -		    cache_tree_fully_valid(index_state->cache_tree);
> +		    cache_tree_fully_valid(repo, index_state->cache_tree);

Here you use "repo".  I do not know offhand if the two callers of
this function write_in_core_index_as_tree() are correct (they are in
builtin/history.c and not my code), but if they are, then "repo"
should match "index_state->repo".  And if they aren't, this code is
broken.

Either way, passing index_state->repo would be the right thing to do
here.  Perhaps write_in_core_index_as_tree() should be corrected to
drop its "repo" parameter.

> diff --git a/unpack-trees.c b/unpack-trees.c
> index 154d6d40a1..20e6d6d824 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> ...
> @@ -2086,7 +2087,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>  			}
>  
>  			if (!o->skip_cache_tree_update &&
> -			    !cache_tree_fully_valid(o->internal.result.cache_tree))
> +			    !cache_tree_fully_valid(the_repository, o->internal.result.cache_tree))

The o->internal.result is a full fledged istate instance and comes
with its own repository.  It may or may not be the same as
the_repository.

If this were a misconversion, it would argue strongly for a thin
wrapper I suggested in the other message, that only takes an istate
instance and infers which repository and which cache-tree to work
with.