Re: [PATCH v8 4/4] cache-tree: Write updated cache-tree after commit

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

Duy Nguyen <pclouds@gmail.com> writes:

> On Sat, Jul 12, 2014 at 11:44 AM, David Turner <dturner@twopensource.com> wrote:
>> @@ -342,6 +342,15 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
>>
>>                 discard_cache();
>>                 read_cache_from(index_lock.filename);
>> +               if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
>> +                       fd = open(index_lock.filename, O_WRONLY);
>> +                       if (fd >= 0)
>> +                               if (write_cache(fd, active_cache, active_nr) == 0) {
>> +                                       close_lock_file(&index_lock);
>
> If write_cache() returns a negative value, index.lock is probably
> corrupted. Should we die() instead of moving on and returning
> index_lock.filename to the caller? The caller may move index.lock to
> index later on and officially ruin "index".

Perhaps true, but worse yet, this will not play nicely together with
your split index series, no?  After taking the lock and writing and
closing, we spawn the interactive while still holding the lock, and
the "open" we see here is because we want to further update the same
under the same lock.  Perhaps write_locked_index() API in the split
index series can notice that the underlying fd in index_lock has
been closed earlier, realize that the call is to re-update the
index under the same lock and open the file again for writing?

Then we can update the above "open() then write_cache()" sequence
with just a call to "write_locked_index()".