* "git-stash --keep-index" crashes when there are no tracked files
@ 2024-08-13 17:36 Piotr Siupa
2024-08-16 10:39 ` Patrick Steinhardt
2024-08-16 10:42 ` [PATCH] builtin/stash: fix `--keep-index --include-untracked` with empty HEAD Patrick Steinhardt
0 siblings, 2 replies; 3+ messages in thread
From: Piotr Siupa @ 2024-08-13 17:36 UTC (permalink / raw)
To: git
Hello,
I've found a bug.
It's something that's unlikely to happen in a normal workflow but it
breaks a script I'm trying to write.
Here is the complete report from "git bugreport":
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.
What did you do before the bug happened? (Steps to reproduce your issue)
git init
git commit --allow-empty --message='initial commit'
touch foo
git stash push --include-untracked --keep-index
What did you expect to happen? (Expected behavior)
A stash entry storing the untracked file "foo" should be created. The
working directory and index should become empty.
What happened instead? (Actual behavior)
The command did what was expected but after that it crashed with the
following error message:
error: pathspec ':/' did not match any file(s) known to git
What's different between what you expected and what actually happened?
There should be no error message and the exit code should be 0.
Anything else you want to add:
This happens only if there are no files in the current HEAD. Adding
any file to the initial commit prevents the error from occurring (even
if it is unchanged when the stash is created).
I suspect that the command has trouble with the corner case of
restoring the state of the index when there are no tracked files.
I also tested it using executables of "git-stash" compiled from
current branches "maint" and "next". The behavior is the same.
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.34.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.8.0-39-generic #39~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC
Wed Jul 10 15:35:09 UTC 2 x86_64
compiler info: gnuc: 11.4
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /bin/bash
[Enabled Hooks]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: "git-stash --keep-index" crashes when there are no tracked files
2024-08-13 17:36 "git-stash --keep-index" crashes when there are no tracked files Piotr Siupa
@ 2024-08-16 10:39 ` Patrick Steinhardt
2024-08-16 10:42 ` [PATCH] builtin/stash: fix `--keep-index --include-untracked` with empty HEAD Patrick Steinhardt
1 sibling, 0 replies; 3+ messages in thread
From: Patrick Steinhardt @ 2024-08-16 10:39 UTC (permalink / raw)
To: Piotr Siupa; +Cc: git
On Tue, Aug 13, 2024 at 07:36:15PM +0200, Piotr Siupa wrote:
> Hello,
>
> I've found a bug.
> It's something that's unlikely to happen in a normal workflow but it
> breaks a script I'm trying to write.
>
> Here is the complete report from "git bugreport":
>
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
>
> What did you do before the bug happened? (Steps to reproduce your issue)
> git init
> git commit --allow-empty --message='initial commit'
> touch foo
> git stash push --include-untracked --keep-index
>
> What did you expect to happen? (Expected behavior)
> A stash entry storing the untracked file "foo" should be created. The
> working directory and index should become empty.
>
> What happened instead? (Actual behavior)
> The command did what was expected but after that it crashed with the
> following error message:
> error: pathspec ':/' did not match any file(s) known to git
I would clarify that this is not "crashing", but that's basically
splitting hairs. In any case, thanks for this clear bug report with a
nice and simple reproducer. I agree that this is something that should
be fixed indeed.
Will send a patch shortly.
Patrick
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] builtin/stash: fix `--keep-index --include-untracked` with empty HEAD
2024-08-13 17:36 "git-stash --keep-index" crashes when there are no tracked files Piotr Siupa
2024-08-16 10:39 ` Patrick Steinhardt
@ 2024-08-16 10:42 ` Patrick Steinhardt
1 sibling, 0 replies; 3+ messages in thread
From: Patrick Steinhardt @ 2024-08-16 10:42 UTC (permalink / raw)
To: git; +Cc: Piotr Siupa
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-16 10:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-13 17:36 "git-stash --keep-index" crashes when there are no tracked files Piotr Siupa
2024-08-16 10:39 ` Patrick Steinhardt
2024-08-16 10:42 ` [PATCH] builtin/stash: fix `--keep-index --include-untracked` with empty HEAD Patrick Steinhardt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).