Re: [PATCH v4 5/5] stash: implement builtin stash

> Joel Teichroeb <joel@teichroeb.net> writes:
>> +/*
>> + * Untracked files are stored by themselves in a parentless commit, for
>> + * ease of unpacking later.
>> + */
>> +static int save_untracked(struct stash_info *info, const char *message,
>> +             int include_untracked, int include_ignored, const char **argv)
>> +{
>> +     struct child_process cp = CHILD_PROCESS_INIT;
>> +     struct strbuf out = STRBUF_INIT;
>> +     struct object_id orig_tree;
>> +     int ret;
>> +     const char *index_file = get_index_file();
>> +
>> +     set_alternate_index_output(stash_index_path);
>> +     untracked_files(&out, include_untracked, include_ignored, argv);
>> +
>> +     cp.git_cmd = 1;
>> +     argv_array_pushl(&cp.args, "update-index", "-z", "--add", "--remove",
>> +             "--stdin", NULL);
>> +     argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", stash_index_path);
>> +
>> +     if (pipe_command(&cp, out.buf, out.len, NULL, 0, NULL, 0)) {
>> +             strbuf_release(&out);
>> +             return 1;
>> +     }
>> +
>
> OK, that's a very straight-forward way of doing this, and as we do
> not care too much about performance in this initial conversion to C,
> it is even sensible.  In a later update after this patch lands, you
> may want to use dir.c's fill_directory() API to find the untracked
> files and add them yourself internally, without running ls-files (in
> untracked_files()) or update-index (here) as subprocesses, but that
> is in the future.  Let's get this round finished.
>
>> +     strbuf_reset(&out);
>> +
>> +     discard_cache();
>> +     read_cache_from(stash_index_path);
>> +
>> +     write_index_as_tree(orig_tree.hash, &the_index, stash_index_path, 0,NULL);
>
> SP before "NULL".
>
>> +     discard_cache();
>> +
>> +     read_cache_from(stash_index_path);
>
> Hmph, what did anybody change in the on-disk stash_index (or
> contents in the_index) since you read_cache_from()?
>
>> +     write_cache_as_tree(info->u_tree.hash, 0, NULL);
>
> Then you write exactly the same index contents again, this time to
> info->u_tree here.  I am not sure why you need to do this twice, and
> I do not see how orig_tree.hash you wrote earlier is used?
>