Re: Lost files after git stash && git stash pop

[Phillip Wood <phillip.wood123@gmail.com>]

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;
                         }