Re: [PATCH v4 5/7] stash create: rework temporary index creation

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> Use create_index_from_tree() instead of reset_tree() so that we
> can create temporary index file without overwriting the in-memory
> index or relying on set_alternate_index_output().

This is very nice simplification.

> As create_index_from_tree() does not
> modify the in-memory index it is clearly safe to remove the call to
> repo_read_index_preload() which saves one lstat() call per changed
> path.

;-)

> It would probably be safe to remove that call without moving away
> from reset_tree() but it is much easier to reason about now that the
> in-memory index is not modified when we create the temporary index file.

Indeed.

> @@ -1458,22 +1458,15 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps
>  	repo_init_revisions(the_repository, &rev, NULL);
>  	copy_pathspec(&rev.prune_data, ps);
>  
> -	set_alternate_index_output(stash_index_path.buf);
> -	if (reset_tree(&info->i_tree, 0, 0)) {
> +	if (create_index_from_tree(&info->i_tree, stash_index_path.buf, false)) {
>  		ret = -1;
>  		goto done;
>  	}
> -	set_alternate_index_output(NULL);
>  
>  	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
>  	rev.diffopt.format_callback = add_diff_to_buf;
>  	rev.diffopt.format_callback_data = &diff_output;
>  
> -	if (repo_read_index_preload(the_repository, &rev.diffopt.pathspec, 0) < 0) {
> -		ret = -1;
> -		goto done;
> -	}
> -

Yup, removal of these is indeed very nice.

>  	add_pending_object(&rev, parse_object(the_repository, &info->b_commit),
>  			   "");
>  	run_diff_index(&rev, 0);