diff mbox series

[3/3] stash: restore untracked files AFTER restoring tracked files

Message ID ac8ca07481d2ed4156036c2441d10712a1b92b0e.1631065427.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Fix D/F issues in stash | expand

Commit Message

Elijah Newren Sept. 8, 2021, 1:43 a.m. UTC
From: Elijah Newren <newren@gmail.com>

If a user deletes a file and places a directory of untracked files
there, then stashes all these changes, the untracked directory of files
cannot be restored until after the corresponding file in the way is
removed.  So, restore changes to tracked files before restoring
untracked files.

There is no similar problem to worry about in the opposite directory,
because untracked files are always additive.  Said another way, there's
no way to "stash a removal of an untracked file" because if an untracked
file is removed, git simply doesn't know about it.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/stash.c  | 6 +++---
 t/t3903-stash.sh | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Derrick Stolee Sept. 8, 2021, 1:15 p.m. UTC | #1
On 9/7/2021 9:43 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> If a user deletes a file and places a directory of untracked files
> there, then stashes all these changes, the untracked directory of files
> cannot be restored until after the corresponding file in the way is
> removed.  So, restore changes to tracked files before restoring
> untracked files.
> 
> There is no similar problem to worry about in the opposite directory,

s/directory/direction/ ?

> because untracked files are always additive.  Said another way, there's
> no way to "stash a removal of an untracked file" because if an untracked
> file is removed, git simply doesn't know about it.

Makes sense.

Thanks,
-Stolee
Junio C Hamano Sept. 8, 2021, 4:30 p.m. UTC | #2
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> If a user deletes a file and places a directory of untracked files
> there, then stashes all these changes, the untracked directory of files
> cannot be restored until after the corresponding file in the way is
> removed.  So, restore changes to tracked files before restoring
> untracked files.
>
> There is no similar problem to worry about in the opposite directory,

direction, I think.

If a user deletes a directory with tracked files in it and places an
untracked file at the path the directory used to be, and then stash
all these changes, that would be the opposite direction, I would
presume?  The untracked file would be restorable only after applying
the removal of the tracked files that occupied the directory.

Which would be fine if we apply the changes to the tracked ones first.

OK.

After creating a stash, we clear the working tree.  How do we do
that exactly, and would we have a similar problem there?  We need to
first remove the untracked file that currently occupies where the
directory used to be in HEAD, and then create the directory and
resurrect the tracked files in the directory from HEAD, in my
modified example above.  In the file-becomes-directory scenario, is
the story the same?

Thanks.
diff mbox series

Patch

diff --git a/builtin/stash.c b/builtin/stash.c
index b85cf9d267e..9a1f6a5a854 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -530,9 +530,6 @@  static int do_apply_stash(const char *prefix, struct stash_info *info,
 		}
 	}
 
-	if (info->has_u && restore_untracked(&info->u_tree))
-		return error(_("could not restore untracked files from stash"));
-
 	init_merge_options(&o, the_repository);
 
 	o.branch1 = "Updated upstream";
@@ -567,6 +564,9 @@  static int do_apply_stash(const char *prefix, struct stash_info *info,
 		unstage_changes_unless_new(&c_tree);
 	}
 
+	if (info->has_u && restore_untracked(&info->u_tree))
+		return error(_("could not restore untracked files from stash"));
+
 	if (!quiet) {
 		struct child_process cp = CHILD_PROCESS_INIT;
 
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index fc74918ccc0..bfd3158a502 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1325,7 +1325,7 @@  test_expect_success 'git stash succeeds despite directory/file change' '
 	)
 '
 
-test_expect_failure 'git stash can pop directory/file saved changes' '
+test_expect_success 'git stash can pop directory/file saved changes' '
 	test_create_repo directory_file_switch_v2 &&
 	(
 		cd directory_file_switch_v2 &&