[PATCH] builtin/stash: report failure to write to index

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]

The git-stash(1) command needs to write to the index for many of its
operations. When the index is locked by a concurrent writer it will thus
fail to operate, which is expected. What is not expected though is that
we do not print any error message at all in this case. The user can thus
easily miss the fact that the command didn't do what they expected it to
do and would be left wondering why that is.

Fix this bug and report failures to write to the index. Add tests for
the subcommands which hit the respective code paths.

Note that the chosen error message ("Cannot write to the index") does
not match our guidelines as it starts with a capitalized letter. This is
intentional though and matches the style of all the other messages used
in git-stash(1).

Reported-by: moti sd <motisd8@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/stash.c  |  6 +++---
 t/t3903-stash.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index b2813c614c..9df072b459 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 	repo_read_index_preload(the_repository, NULL, 0);
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL))
-		return -1;
+		return error(_("Cannot write to the index"));
 
 	if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0,
 				NULL))
@@ -1364,7 +1364,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 	repo_read_index_preload(the_repository, NULL, 0);
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL) < 0) {
-		ret = -1;
+		ret = error(_("Cannot write to the index"));
 		goto done;
 	}
 
@@ -1555,7 +1555,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 
 	if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0,
 					 NULL, NULL, NULL)) {
-		ret = -1;
+		ret = error(_("Cannot write to the index"));
 		goto done;
 	}
 
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3319240515..770881e537 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1516,4 +1516,56 @@ test_expect_success 'restore untracked files even when we hit conflicts' '
 	)
 '
 
+test_expect_success 'stash create reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: Cannot write to the index
+		EOF
+		test_must_fail git stash create 2>err &&
+		test_cmp expect err
+	)
+'
+
+test_expect_success 'stash push reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: Cannot write to the index
+		EOF
+		test_must_fail git stash push 2>err &&
+		test_cmp expect err
+	)
+'
+
+test_expect_success 'stash apply reports a locked index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A A.file &&
+		echo change >A.file &&
+		git stash push &&
+		touch .git/index.lock &&
+
+		cat >expect <<-EOF &&
+		error: Cannot write to the index
+		EOF
+		test_must_fail git stash apply 2>err &&
+		test_cmp expect err
+	)
+'
+
 test_done
-- 
2.43.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]