Re: [PATCH v3 2/6] stash: remove the second index in stash_working_tree()

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

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

>> @@ -1091,8 +1088,6 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps
>>  	argv_array_pushl(&cp_upd_index.args, "update-index",
>>  			 "--ignore-skip-worktree-entries",
>>  			 "-z", "--add", "--remove", "--stdin", NULL);
>> -	argv_array_pushf(&cp_upd_index.env_array, "GIT_INDEX_FILE=%s",
>> -			 stash_index_path.buf);
>>  
>>  	if (pipe_command(&cp_upd_index, diff_output.buf, diff_output.len,
>>  			 NULL, 0, NULL, 0)) {
>
> And then the new code now lets "update-index" work directly on the
> main index (which does make an observable difference to the outside
> world, but we are not letting any hook to look at this intermediate
> state, so it might be OK---I cannot tell at this point in the code).

This changes the behaviour of the command a lot when a fatal happens
anywhere after this point til we reset the index to match the HEAD,
doesn't it, I wonder.  The original code refrained from touching the
index so after such a "fatal:" error message, the user would have
seen the original state (with previous "git add" etc. the user did)
in the index.  New code now added all the working tree changes to
the index, so the user would lose the work made so far in the main
index, no?  With later patch in the series applied, we will end up
having even untracked paths in the main index---I am not sure to
what extent that step would make things even worse, but it cannot be
harmless.

> How is the use of second on-disk index hurting us?  I must be
> missing something obvious, but at this point, it is not clear what
> we are gaining---I only see downsides.