From 4dbf7f30b1d6469474d6a7e6759c9f84d305b95c Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Fri, 10 Sep 2021 10:29:54 +0000 Subject: [PATCH 1/3] t3903: document a pair of directory/file bugs There are three tests here, because the second bug is documented with two tests: a file -> directory change and a directory -> file change. The reason for the two tests is just to verify that both are indeed broken but that both will be fixed by the same simple change (which will be provided in a subsequent patch). Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t3903-stash.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 873aa56e35..7346f8d103 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1307,4 +1307,62 @@ test_expect_success 'stash -c stash.useBuiltin=false warning ' ' test_must_be_empty err ' +test_expect_failure 'git stash succeeds despite directory/file change' ' + test_create_repo directory_file_switch_v1 && + ( + cd directory_file_switch_v1 && + test_commit init && + + test_write_lines this file has some words >filler && + git add filler && + git commit -m filler && + + git rm filler && + mkdir filler && + echo contents >filler/file && + git stash push + ) +' + +test_expect_failure 'git stash can pop file -> directory saved changes' ' + test_create_repo directory_file_switch_v2 && + ( + cd directory_file_switch_v2 && + test_commit init && + + test_write_lines this file has some words >filler && + git add filler && + git commit -m filler && + + git rm filler && + mkdir filler && + echo contents >filler/file && + cp filler/file expect && + git stash push --include-untracked && + git stash apply --index && + test_cmp expect filler/file + ) +' + +test_expect_failure 'git stash can pop directory -> file saved changes' ' + test_create_repo directory_file_switch_v3 && + ( + cd directory_file_switch_v3 && + test_commit init && + + mkdir filler && + test_write_lines some words >filler/file1 && + test_write_lines and stuff >filler/file2 && + git add filler && + git commit -m filler && + + git rm -rf filler && + echo contents >filler && + cp filler expect && + git stash push --include-untracked && + git stash apply --index && + test_cmp expect filler + ) +' + test_done From 3d40e3723b1bc86d22136ff01b0787809a3267a4 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Fri, 10 Sep 2021 10:29:55 +0000 Subject: [PATCH 2/3] stash: avoid feeding directories to update-index When a file is removed from the cache, but there is a file of the same name present in the working directory, we would normally treat that file in the working directory as untracked. However, in the case of stash, doing that would prevent a simple 'git stash push', because the untracked file would be in the way of restoring the deleted file. git stash, however, blindly assumes that whatever is in the working directory for a deleted file is wanted and passes that path along to update-index. That causes problems when the working directory contains a directory with the same name as the deleted file. Add some code for this special case that will avoid passing directory names to update-index. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/stash.c | 14 ++++++++++++++ t/t3903-stash.sh | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/builtin/stash.c b/builtin/stash.c index 8f42360ca9..9ad2940f87 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -313,6 +313,17 @@ static int reset_head(void) return run_command(&cp); } +static int is_path_a_directory(const char *path) +{ + /* + * This function differs from abspath.c:is_directory() in that + * here we use lstat() instead of stat(); we do not want to + * follow symbolic links here. + */ + struct stat st; + return (!lstat(path, &st) && S_ISDIR(st.st_mode)); +} + static void add_diff_to_buf(struct diff_queue_struct *q, struct diff_options *options, void *data) @@ -320,6 +331,9 @@ static void add_diff_to_buf(struct diff_queue_struct *q, int i; for (i = 0; i < q->nr; i++) { + if (is_path_a_directory(q->queue[i]->one->path)) + continue; + strbuf_addstr(data, q->queue[i]->one->path); /* NUL-terminate: will be fed to update-index -z */ diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 7346f8d103..34d1ad9837 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1307,7 +1307,7 @@ test_expect_success 'stash -c stash.useBuiltin=false warning ' ' test_must_be_empty err ' -test_expect_failure 'git stash succeeds despite directory/file change' ' +test_expect_success 'git stash succeeds despite directory/file change' ' test_create_repo directory_file_switch_v1 && ( cd directory_file_switch_v1 && From bee8691f197ae6b74ca26081c1a3fa218e2b9db7 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Fri, 10 Sep 2021 10:29:56 +0000 Subject: [PATCH 3/3] stash: restore untracked files AFTER restoring tracked files 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 counterpart problem to worry about with the user deleting an untracked file and then add a tracked one in its place. Git does not track untracked files, and so will not know the untracked file was deleted, and thus won't be able to stash the removal of that file. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/stash.c | 6 +++--- t/t3903-stash.sh | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index 9ad2940f87..5512f4942c 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -535,9 +535,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"; @@ -572,6 +569,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 34d1ad9837..f0a82be9de 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1324,7 +1324,7 @@ test_expect_success 'git stash succeeds despite directory/file change' ' ) ' -test_expect_failure 'git stash can pop file -> directory saved changes' ' +test_expect_success 'git stash can pop file -> directory saved changes' ' test_create_repo directory_file_switch_v2 && ( cd directory_file_switch_v2 && @@ -1344,7 +1344,7 @@ test_expect_failure 'git stash can pop file -> directory saved changes' ' ) ' -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_v3 && ( cd directory_file_switch_v3 &&