[PATCH] builtin/stash: fix `--keep-index --include-untracked` with empty HEAD

From: Patrick Steinhardt <ps@pks.im>
Date: Aug 16, 2024, 10:42 UTC
Message-ID: <6067675b5edf36e2eb28e98119a1c02d0a7ae670.1723804926.git.ps@pks.im>
In-Reply-To: <CAPM0=yCJ0snEznK0C+zkmGofWHE3OPWoMXSVGH6=TdM7=rZ84g@mail.gmail.com>

It was reported that creating a stash with `--keep-index --include-untracked` causes an error when HEAD points to a commit whose tree is empty:

    $ git stash push --keep-index --include-untracked
    error: pathspec ':/' did not match any file(s) known to git

This error comes from `git checkout --no-overlay $i_tree -- :/`, which we execute to reset the working tree to the state in our index. As the tree generated from the index is empty in our case, ':/' does not match any files and thus causes git-checkout(1) to error out.

Fix the issue by skipping the checkout when the index tree is empty. As explained in the in-code comment, this should be the correct thing to do as there is nothing that we'd have to reset in the first place.

Reported-by: Piotr Siupa <piotrsiupa@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/stash.c  | 23 ++++++++++++++++++++++-
 t/t3903-stash.sh | 15 +++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index d90e072ddc..fcfd97972a 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1671,7 +1671,28 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 			}
 		}
 
-		if (keep_index == 1 && !is_null_oid(&info.i_tree)) {
+		/*
+		 * When keeping staged entries, we need to reset the working
+		 * directory to match the state of our index. This can be
+		 * skipped when the index is the empty tree, because there is
+		 * nothing to reset in that case:
+		 *
+		 *   - When the index has any file, regardless of whether
+		 *     staged or not, the tree cannot be empty by definition
+		 *     and thus we enter the condition.
+		 *
+		 *   - When the index has no files, the only thing we need to
+		 *     care about is untracked files when `--include-untracked`
+		 *     is given. But as we already execute git-clean(1) further
+		 *     up to delete such untracked files we don't have to do
+		 *     anything here, either.
+		 *
+		 * We thus skip calling git-checkout(1) in this case, also
+		 * because running it on an empty tree will cause it to fail
+		 * due to the pathspec not matching anything.
+		 */
+		if (keep_index == 1 && !is_null_oid(&info.i_tree) &&
+		    !is_empty_tree_oid(&info.i_tree, the_repository->hash_algo)) {
 			struct child_process cp = CHILD_PROCESS_INIT;
 
 			cp.git_cmd = 1;
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e4c0937f61..c87592ee2f 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1398,6 +1398,21 @@ test_expect_success 'stash --keep-index with file deleted in index does not resu
 	test_path_is_missing to-remove
 '
 
+test_expect_success 'stash --keep-index --include-untracked with empty tree' '
+	test_when_finished "rm -rf empty" &&
+	git init empty &&
+	(
+		cd empty &&
+		git commit --allow-empty --message "empty" &&
+		echo content >file &&
+		git stash push --keep-index --include-untracked &&
+		test_path_is_missing file &&
+		git stash pop &&
+		echo content >expect &&
+		test_cmp expect file
+	)
+'
+
 test_expect_success 'stash apply should succeed with unmodified file' '
 	echo base >file &&
 	git add file &&

base-commit: 477ce5ccd69bda5c44f53e3b230f8a9c54217ed6

-- 
2.46.0.46.g406f326d27.dirty