On Sun, Mar 25, 2018 at 9:43 AM, Thomas Gummerer <t.gumme...@gmail.com> wrote:
> On 03/24, Joel Teichroeb wrote:
>> ---
>
> Missing sign-off? I saw it's missing in the other patches as well.
>
Thanks! I always forget to add a sign-off.
>> [...]
>> +
>> + if (info->has_u) {
>> + struct child_process cp = CHILD_PROCESS_INIT;
>> + struct child_process cp2 = CHILD_PROCESS_INIT;
>> + int res;
>> +
>> + cp.git_cmd = 1;
>> + argv_array_push(&cp.args, "read-tree");
>> + argv_array_push(&cp.args, sha1_to_hex(info->u_tree.hash));
>> + argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s",
>> stash_index_path);
>> +
>> + cp2.git_cmd = 1;
>> + argv_array_pushl(&cp2.args, "checkout-index", "--all", NULL);
>> + argv_array_pushf(&cp2.env_array, "GIT_INDEX_FILE=%s",
>> stash_index_path);
>> +
>> + res = run_command(&cp) || run_command(&cp2);
>> + remove_path(stash_index_path);
>> + if (res)
>> + return error(_("Could not restore untracked files from
>> stash"));
>
> A minor change in behaviour here is that we are removing the temporary
> index file unconditionally here, while we would previously only remove
> it if both 'read-tree' and 'checkout-index' would succeed.
>
> I don't think that's a bad thing, we probably don't want users to try
> and use that index file in any way, and I doubt that's part of anyones
> workflow, so I think cleaning it up makes sense.
>
I'm not sure about that. The shell script has a trap near the start in
order to clean up the temp index, unless I'm understanding the shell
script incorrectly.
