Hi Torsten
On 23/07/2023 21:52, Torsten Bögershausen wrote:
I think whenever git overwrites an untracked file without the user passing
some option indicating that they want to do so it is a bug.
OK, agreed after reading the next sentence.
For example "git
checkout" refuses to overwrite untracked files by default. Sadly this seems
to be a known bug in do_push_stash() where we are using "git reset --hard"
to remove the stashed changes from the working copy. This was documented in
94b7f1563a (Comment important codepaths regarding nuking untracked
files/dirs, 2021-09-27). The stash implementation does a lot of necessary
forking of subprocesses, in this case I think it would be better to call
unpack_trees() directly with UNPACK_RESET_PROTECT_UNTRACKED.
Thanks for the fast response.
This is not an area of Git, where I have much understanding of the code.
But is seems as if pop_stash() in builtin/stash.c
(and the called functions) seems to be the problem here ?
Confusingly it is creating the stash that deletes the untracked file because
it recreates README/README. do_push_stash() in builtin/stash.c is the culprit
I think. I had hoped the diff below would fix the problem but it does not
seem to and breaks half a dozen test cases that seem to rely on removing
untracked files. Unfortunately I don't really have time to dig any
further at the moment.
Best Wishes
Phillip
diff --git a/builtin/stash.c b/builtin/stash.c
index fe64cde9ce3..c8bbfe56d26 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -29,6 +29,8 @@
#include "exec-cmd.h"
#include "reflog.h"
#include "add-interactive.h"
+#include "reset.h"
+#include "submodule.h"
#define INCLUDE_ALL_FILES 2
@@ -336,7 +338,7 @@ static int apply_cached(struct strbuf *out)
return pipe_command(&cp, out->buf, out->len, NULL, 0, NULL, 0);
}
-static int reset_head(void)
+static int stash_reset_head(void)
{
struct child_process cp = CHILD_PROCESS_INIT;
@@ -569,7 +571,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
get_index_file(), 0, NULL))
return error(_("could not save index tree"));
- reset_head();
+ stash_reset_head();
discard_index(&the_index);
repo_read_index(the_repository);
}
@@ -1649,12 +1651,14 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
goto done;
}
} else {
- struct child_process cp = CHILD_PROCESS_INIT;
- cp.git_cmd = 1;
- /* BUG: this nukes untracked files in the way */
- strvec_pushl(&cp.args, "reset", "--hard", "-q",
- "--no-recurse-submodules", NULL);
- if (run_command(&cp)) {
+ struct reset_head_opts opts = {
+ .flags = RESET_HEAD_HARD,
+ };
+
+ if (should_update_submodules())
+ BUG("stash should not update submodules");
+
+ if (reset_head(the_repository, &opts)) {
ret = -1;
goto done;
}