cache-tree: do not die when the index is locked

When a background process such as `git status` holds `index.lock` while
`git rebase` picks a commit, the rebase can die in the middle of the
pick and leave the sequencer in a state from which `git rebase
--continue` refuses to run:

    error: you have staged changes in your working tree

This happens because `try_to_commit()` calls `write_index_as_tree()`
right after the pick has updated the index and written
`CHERRY_PICK_HEAD`, and `write_index_as_tree()` dies when it cannot
take the index lock. We then die before writing `rebase-merge/message`.
Since 356ee4659b (sequencer: try to commit without forking 'git commit',
2017-11-24), this call happens in-process. Before that commit the
`die()` happened in a `git commit` child process, and the sequencer
stopped cleanly with its state saved.

`write_index_as_tree()` does two different things:

  - It turns the index into a tree object. This part must succeed.

  - When that succeeds, it also rewrites the index to populate the
    cache-tree extension. This part is allowed to fail without
    impacting the result; as the comment next to
    `write_locked_index()` already notes, failing to write the index
    here is fine.

Yet the function locks the index with `LOCK_DIE_ON_ERROR`, even though
we only need the lock for the optional second part.

Drop `LOCK_DIE_ON_ERROR` so that we take the lock gently, just like
other callers that only refresh the index, and skip writing the index
when the lock is held by someone else. This also lets `git write-tree`
and `git am` proceed while another process holds the lock.

Signed-off-by: Szymon Wilczek <swilczek.lx@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

cache-tree.c

@@ -809,11 +809,15 @@ struct tree *write_in_core_index_as_tree(struct repository *repo,
 
 int write_index_as_tree(struct object_id *oid, struct index_state *index_state, const char *index_path, int flags, const char *prefix)
 {
-	int entries, was_valid;
+	int entries, was_valid, fd;
 	struct lock_file lock_file = LOCK_INIT;
 	int ret;
 
-	hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
+	/*
+	 * The lock only protects the cache-tree write-back below; if somebody
+	 * else holds it, compute the tree without updating the index.
+	 */
+	fd = hold_lock_file_for_update(&lock_file, index_path, 0);
 
 	entries = read_index_from(index_state, index_path,
 				  repo_get_git_dir(index_state->repo));
@@ -828,7 +832,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
 
 	ret = write_index_as_tree_internal(oid, index_state, was_valid, flags,
 					   prefix);
-	if (!ret && !was_valid) {
+	if (!ret && !was_valid && fd >= 0) {
 		write_locked_index(index_state, &lock_file, COMMIT_LOCK);
 		/* Not being able to write is fine -- we are only interested
 		 * in updating the cache-tree part, and if the next caller

t/t0090-cache-tree.sh

@@ -286,4 +286,17 @@ test_expect_success 'cache-tree is used by write-tree when valid' '
 	test_grep ! region_enter.*cache_tree.*update trace.output
 '
 
+test_expect_success 'write-tree succeeds while the index is locked' '
+	test_when_finished "rm -f .git/index.lock" &&
+	test_commit locked &&
+	>uncommitted &&
+	git add uncommitted &&
+	>.git/index.lock &&
+	git write-tree >actual &&
+	test_path_is_file .git/index.lock &&
+	rm .git/index.lock &&
+	git write-tree >expect &&
+	test_cmp expect actual
+'
+
 test_done