From 8a0d52dfd870af50b9c28baf66347f5eaaf14e6e Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:25 +0000 Subject: [PATCH 01/11] t2501: add various tests for removing the current working directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Numerous commands will remove directories left empty as a "convenience" after removing files within them. That is normally fine, but removing the current working directory can be rather inconvenient since it can cause confusion for the user when they run subsequent commands. For example, after one git process has removed the current working directory, git status/log/diff will all abort with the message: fatal: Unable to read current working directory: No such file or directory We also have code paths that, when a file needs to be placed where a directory is (due to e.g. checkout, merge, reset, whatever), will check if this is okay and error out if not. These rules include: * all tracked files under that directory are intended to be removed by the operation * none of the tracked files under that directory have uncommitted modification * there are no untracked files under that directory However, if we end up remove the current working directory, we can cause user confusion when they run subsequent commands, so we would prefer if there was a fourth rule added to this list: avoid removing the current working directory. Since there are several code paths that can result in the current working directory being removed, add several tests of various different codepaths. To make it clearer what the difference between the current behavior and the behavior at the end of the series, code both of them into the tests and have the appropriate behavior be selected by a flag. Subsequent commits will toggle the flag from current to desired behavior. Also add a few tests suggested during the review of earlier rounds of this patch series. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t2501-cwd-empty.sh | 342 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 342 insertions(+) create mode 100755 t/t2501-cwd-empty.sh diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh new file mode 100755 index 0000000000..a05abd1818 --- /dev/null +++ b/t/t2501-cwd-empty.sh @@ -0,0 +1,342 @@ +#!/bin/sh + +test_description='Test handling of the current working directory becoming empty' + +. ./test-lib.sh + +test_expect_success setup ' + test_commit init && + + git branch fd_conflict && + + mkdir -p foo/bar && + test_commit foo/bar/baz && + + git revert HEAD && + git tag reverted && + + git checkout fd_conflict && + mkdir dirORfile && + test_commit dirORfile/foo && + + git rm -r dirORfile && + echo not-a-directory >dirORfile && + git add dirORfile && + git commit -m dirORfile && + + git switch -c df_conflict HEAD~1 && + test_commit random_file && + + git switch -c undo_fd_conflict fd_conflict && + git revert HEAD +' + +test_incidental_dir_removal () { + works=$1 && + shift && + + test_when_finished "git reset --hard" && + + git checkout foo/bar/baz^{commit} && + test_path_is_dir foo/bar && + + ( + cd foo && + "$@" && + + # Although we want pwd & git status to pass, test for existing + # rather than desired behavior. + if test "$works" = "success" + then + test-tool getcwd && + git status --porcelain + else + ! test-tool getcwd && + test_might_fail git status --porcelain + fi + ) && + test_path_is_missing foo/bar/baz && + test_path_is_missing foo/bar && + + # Although we want dir to be present, test for existing rather + # than desired behavior. + if test "$works" = "success" + then + test_path_is_dir foo + else + test_path_is_missing foo + fi +} + +test_required_dir_removal () { + works=$1 && + shift && + + git checkout df_conflict^{commit} && + test_when_finished "git clean -fdx" && + + ( + cd dirORfile && + + # We'd like for the command to fail (much as it would if there + # was an untracked file there), and for the index and worktree + # to be left clean with pwd and git status working afterwards. + # But test for existing rather than desired behavior. + if test "$works" = "success" + then + test_must_fail "$@" 2>../error && + grep "Refusing to remove.*current working directory" ../error && + + git diff --exit-code HEAD && + + test-tool getcwd && + git status --porcelain + else + "$@" && + ! test-tool getcwd && + test_might_fail git status --porcelain + fi + ) && + + # Although we want dirORfile to be present, test for existing rather + # than desired behavior. + if test "$works" = "success" + then + test_path_is_dir dirORfile + else + test_path_is_file dirORfile + fi +} + +test_expect_success 'checkout does not clean cwd incidentally' ' + test_incidental_dir_removal failure git checkout init +' + +test_expect_success 'checkout fails if cwd needs to be removed' ' + test_required_dir_removal failure git checkout fd_conflict +' + +test_expect_success 'reset --hard does not clean cwd incidentally' ' + test_incidental_dir_removal failure git reset --hard init +' + +test_expect_success 'reset --hard fails if cwd needs to be removed' ' + test_required_dir_removal failure git reset --hard fd_conflict +' + +test_expect_success 'merge does not clean cwd incidentally' ' + test_incidental_dir_removal failure git merge reverted +' + +# This file uses some simple merges where +# Base: 'dirORfile/' exists +# Side1: random other file changed +# Side2: 'dirORfile/' removed, 'dirORfile' added +# this should resolve cleanly, but merge-recursive throws merge conflicts +# because it's dumb. Add a special test for checking merge-recursive (and +# merge-ort), then after this just hard require ort for all remaining tests. +# +test_expect_success 'merge fails if cwd needs to be removed; recursive friendly' ' + git checkout foo/bar/baz && + test_when_finished "git clean -fdx" && + + mkdir dirORfile && + ( + cd dirORfile && + + # We would rather this failed, but we test for existing + # rather than desired behavior + git merge fd_conflict 2>../error + ) && + + ## Here is the behavior we would rather have: + #test_path_is_dir dirORfile && + #grep "Refusing to remove the current working directory" error + ## But instead we test for existing behavior + test_path_is_file dirORfile && + test_must_be_empty error +' + +GIT_TEST_MERGE_ALGORITHM=ort + +test_expect_success 'merge fails if cwd needs to be removed' ' + test_required_dir_removal failure git merge fd_conflict +' + +test_expect_success 'cherry-pick does not clean cwd incidentally' ' + test_incidental_dir_removal failure git cherry-pick reverted +' + +test_expect_success 'cherry-pick fails if cwd needs to be removed' ' + test_required_dir_removal failure git cherry-pick fd_conflict +' + +test_expect_success 'rebase does not clean cwd incidentally' ' + test_incidental_dir_removal failure git rebase reverted +' + +test_expect_success 'rebase fails if cwd needs to be removed' ' + test_required_dir_removal failure git rebase fd_conflict +' + +test_expect_success 'revert does not clean cwd incidentally' ' + test_incidental_dir_removal failure git revert HEAD +' + +test_expect_success 'revert fails if cwd needs to be removed' ' + test_required_dir_removal failure git revert undo_fd_conflict +' + +test_expect_success 'rm does not clean cwd incidentally' ' + test_incidental_dir_removal failure git rm bar/baz.t +' + +test_expect_success 'apply does not remove cwd incidentally' ' + git diff HEAD HEAD~1 >patch && + test_incidental_dir_removal failure git apply ../patch +' + +test_incidental_untracked_dir_removal () { + works=$1 && + shift && + + test_when_finished "git reset --hard" && + + git checkout foo/bar/baz^{commit} && + mkdir -p untracked && + mkdir empty + >untracked/random && + + ( + cd untracked && + "$@" && + + # Although we want pwd & git status to pass, test for existing + # rather than desired behavior. + if test "$works" = "success" + then + test-tool getcwd && + git status --porcelain + else + ! test-tool getcwd && + test_might_fail git status --porcelain + fi + ) && + test_path_is_missing empty && + test_path_is_missing untracked/random && + + # Although we want dir to be present, test for existing rather + # than desired behavior. + if test "$works" = "success" + then + test_path_is_dir untracked + else + test_path_is_missing untracked + fi +} + +test_expect_success 'clean does not remove cwd incidentally' ' + test_incidental_untracked_dir_removal failure \ + git -C .. clean -fd -e warnings . >warnings +' + +test_expect_success 'stash does not remove cwd incidentally' ' + test_incidental_untracked_dir_removal failure \ + git stash --include-untracked +' + +test_expect_success '`rm -rf dir` only removes a subset of dir' ' + test_when_finished "rm -rf a/" && + + mkdir -p a/b/c && + >a/b/c/untracked && + >a/b/c/tracked && + git add a/b/c/tracked && + + ( + cd a/b && + git rm -rf ../b + ) && + + test_path_is_dir a/b && + test_path_is_missing a/b/c/tracked && + test_path_is_file a/b/c/untracked +' + +test_expect_success '`rm -rf dir` even with only tracked files will remove something else' ' + test_when_finished "rm -rf a/" && + + mkdir -p a/b/c && + >a/b/c/tracked && + git add a/b/c/tracked && + + ( + cd a/b && + git rm -rf ../b + ) && + + test_path_is_missing a/b/c/tracked && + ## We would prefer if a/b was still present, though empty, since it + ## was the current working directory + #test_path_is_dir a/b + ## But the current behavior is that it not only deletes the directory + ## a/b as requested, but also goes and deletes a + test_path_is_missing a +' + +test_expect_success 'git version continues working from a deleted dir' ' + mkdir tmp && + ( + cd tmp && + rm -rf ../tmp && + git version + ) +' + +test_submodule_removal () { + path_status=$1 && + shift && + + test_status= + test "$path_status" = dir && test_status=test_must_fail + + # Actually, while path_status=dir && test_status=test_must_fail + # reflect our desired behavior, current behavior is: + path_status=missing + test_status= + + test_when_finished "git reset --hard HEAD~1" && + test_when_finished "rm -rf .git/modules/my_submodule" && + + git checkout foo/bar/baz && + + git init my_submodule && + touch my_submodule/file && + git -C my_submodule add file && + git -C my_submodule commit -m "initial commit" && + git submodule add ./my_submodule && + git commit -m "Add the submodule" && + + ( + cd my_submodule && + $test_status "$@" + ) && + + test_path_is_${path_status} my_submodule +} + +test_expect_success 'rm -r with -C leaves submodule if cwd inside' ' + test_submodule_removal dir git -C .. rm -r my_submodule/ +' + +test_expect_success 'rm -r leaves submodule if cwd inside' ' + test_submodule_removal dir \ + git --git-dir=../.git --work-tree=.. rm -r ../my_submodule/ +' + +test_expect_success 'rm -rf removes submodule even if cwd inside' ' + test_submodule_removal missing \ + git --git-dir=../.git --work-tree=.. rm -rf ../my_submodule/ +' + +test_done From e6f8861bd43636b27651150e6a3caa0a937fb418 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:26 +0000 Subject: [PATCH 02/11] setup: introduce startup_info->original_cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing the current working directory causes all subsequent git commands run from that directory to get confused and fail with a message about being unable to read the current working directory: $ git status fatal: Unable to read current working directory: No such file or directory Non-git commands likely have similar warnings or even errors, e.g. $ bash -c 'echo hello' shell-init: error retrieving current directory: getcwd: cannot access parent directories: No such file or directory hello This confuses end users, particularly since the command they get the error from is not the one that caused the problem; the problem came from the side-effect of some previous command. We would like to avoid removing the current working directory of our parent process; towards this end, introduce a new variable, startup_info->original_cwd, that tracks the current working directory that we inherited from our parent process. For convenience of later comparisons, we prefer that this new variable store a path relative to the toplevel working directory (thus much like 'prefix'), except without the trailing slash. Subsequent commits will make use of this new variable. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- cache.h | 2 ++ common-main.c | 4 ++++ setup.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/cache.h b/cache.h index eba12487b9..92e181ea75 100644 --- a/cache.h +++ b/cache.h @@ -1834,8 +1834,10 @@ void overlay_tree_on_index(struct index_state *istate, struct startup_info { int have_repository; const char *prefix; + const char *original_cwd; }; extern struct startup_info *startup_info; +extern const char *tmp_original_cwd; /* merge.c */ struct commit_list; diff --git a/common-main.c b/common-main.c index 71e21dd20a..aa8d5aba5b 100644 --- a/common-main.c +++ b/common-main.c @@ -26,6 +26,7 @@ static void restore_sigpipe_to_default(void) int main(int argc, const char **argv) { int result; + struct strbuf tmp = STRBUF_INIT; trace2_initialize_clock(); @@ -49,6 +50,9 @@ int main(int argc, const char **argv) trace2_cmd_start(argv); trace2_collect_process_info(TRACE2_PROCESS_INFO_STARTUP); + if (!strbuf_getcwd(&tmp)) + tmp_original_cwd = strbuf_detach(&tmp, NULL); + result = cmd_main(argc, argv); trace2_cmd_exit(result); diff --git a/setup.c b/setup.c index 347d7181ae..af3b8c09ab 100644 --- a/setup.c +++ b/setup.c @@ -12,6 +12,7 @@ static int work_tree_config_is_bogus; static struct startup_info the_startup_info; struct startup_info *startup_info = &the_startup_info; +const char *tmp_original_cwd; /* * The input parameter must contain an absolute path, and it must already be @@ -432,6 +433,69 @@ void setup_work_tree(void) initialized = 1; } +static void setup_original_cwd(void) +{ + struct strbuf tmp = STRBUF_INIT; + const char *worktree = NULL; + int offset = -1; + + if (!tmp_original_cwd) + return; + + /* + * startup_info->original_cwd points to the current working + * directory we inherited from our parent process, which is a + * directory we want to avoid removing. + * + * For convience, we would like to have the path relative to the + * worktree instead of an absolute path. + * + * Yes, startup_info->original_cwd is usually the same as 'prefix', + * but differs in two ways: + * - prefix has a trailing '/' + * - if the user passes '-C' to git, that modifies the prefix but + * not startup_info->original_cwd. + */ + + /* Normalize the directory */ + strbuf_realpath(&tmp, tmp_original_cwd, 1); + free((char*)tmp_original_cwd); + tmp_original_cwd = NULL; + startup_info->original_cwd = strbuf_detach(&tmp, NULL); + + /* + * Get our worktree; we only protect the current working directory + * if it's in the worktree. + */ + worktree = get_git_work_tree(); + if (!worktree) + goto no_prevention_needed; + + offset = dir_inside_of(startup_info->original_cwd, worktree); + if (offset >= 0) { + /* + * If startup_info->original_cwd == worktree, that is already + * protected and we don't need original_cwd as a secondary + * protection measure. + */ + if (!*(startup_info->original_cwd + offset)) + goto no_prevention_needed; + + /* + * original_cwd was inside worktree; precompose it just as + * we do prefix so that built up paths will match + */ + startup_info->original_cwd = \ + precompose_string_if_needed(startup_info->original_cwd + + offset); + return; + } + +no_prevention_needed: + free((char*)startup_info->original_cwd); + startup_info->original_cwd = NULL; +} + static int read_worktree_config(const char *var, const char *value, void *vdata) { struct repository_format *data = vdata; @@ -1330,6 +1394,7 @@ const char *setup_git_directory_gently(int *nongit_ok) setenv(GIT_PREFIX_ENVIRONMENT, "", 1); } + setup_original_cwd(); strbuf_release(&dir); strbuf_release(&gitdir); From b817e545338cdb737b3deebf4917afb4a18ede57 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:27 +0000 Subject: [PATCH 03/11] unpack-trees: refuse to remove startup_info->original_cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the past, when a directory needs to be removed to make room for a file, we have always errored out when that directory contains any untracked (but not ignored) files. Add an extra condition on that: also error out if the directory is the current working directory we inherited from our parent process. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t2501-cwd-empty.sh | 20 +++++++------------- unpack-trees.c | 17 +++++++++++++---- unpack-trees.h | 1 + 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index a05abd1818..398908dfc9 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -113,7 +113,7 @@ test_expect_success 'checkout does not clean cwd incidentally' ' ' test_expect_success 'checkout fails if cwd needs to be removed' ' - test_required_dir_removal failure git checkout fd_conflict + test_required_dir_removal success git checkout fd_conflict ' test_expect_success 'reset --hard does not clean cwd incidentally' ' @@ -144,23 +144,17 @@ test_expect_success 'merge fails if cwd needs to be removed; recursive friendly' ( cd dirORfile && - # We would rather this failed, but we test for existing - # rather than desired behavior - git merge fd_conflict 2>../error + test_must_fail git merge fd_conflict 2>../error ) && - ## Here is the behavior we would rather have: - #test_path_is_dir dirORfile && - #grep "Refusing to remove the current working directory" error - ## But instead we test for existing behavior - test_path_is_file dirORfile && - test_must_be_empty error + test_path_is_dir dirORfile && + grep "Refusing to remove the current working directory" error ' GIT_TEST_MERGE_ALGORITHM=ort test_expect_success 'merge fails if cwd needs to be removed' ' - test_required_dir_removal failure git merge fd_conflict + test_required_dir_removal success git merge fd_conflict ' test_expect_success 'cherry-pick does not clean cwd incidentally' ' @@ -168,7 +162,7 @@ test_expect_success 'cherry-pick does not clean cwd incidentally' ' ' test_expect_success 'cherry-pick fails if cwd needs to be removed' ' - test_required_dir_removal failure git cherry-pick fd_conflict + test_required_dir_removal success git cherry-pick fd_conflict ' test_expect_success 'rebase does not clean cwd incidentally' ' @@ -184,7 +178,7 @@ test_expect_success 'revert does not clean cwd incidentally' ' ' test_expect_success 'revert fails if cwd needs to be removed' ' - test_required_dir_removal failure git revert undo_fd_conflict + test_required_dir_removal success git revert undo_fd_conflict ' test_expect_success 'rm does not clean cwd incidentally' ' diff --git a/unpack-trees.c b/unpack-trees.c index 89ca95ce90..6bc16f3a71 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -36,6 +36,9 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_WARNING_TYPES] = { /* ERROR_NOT_UPTODATE_DIR */ "Updating '%s' would lose untracked files in it", + /* ERROR_CWD_IN_THE_WAY */ + "Refusing to remove '%s' since it is the current working directory.", + /* ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN */ "Untracked working tree file '%s' would be overwritten by merge.", @@ -131,6 +134,9 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts, msgs[ERROR_NOT_UPTODATE_DIR] = _("Updating the following directories would lose untracked files in them:\n%s"); + msgs[ERROR_CWD_IN_THE_WAY] = + _("Refusing to remove the current working directory:\n%s"); + if (!strcmp(cmd, "checkout")) msg = advice_enabled(ADVICE_COMMIT_BEFORE_MERGE) ? _("The following untracked working tree files would be removed by checkout:\n%%s" @@ -2146,10 +2152,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce, cnt++; } - /* - * Then we need to make sure that we do not lose a locally - * present file that is not ignored. - */ + /* Do not lose a locally present file that is not ignored. */ pathbuf = xstrfmt("%.*s/", namelen, ce->name); memset(&d, 0, sizeof(d)); @@ -2160,6 +2163,12 @@ static int verify_clean_subdirectory(const struct cache_entry *ce, free(pathbuf); if (i) return add_rejected_path(o, ERROR_NOT_UPTODATE_DIR, ce->name); + + /* Do not lose startup_info->original_cwd */ + if (startup_info->original_cwd && + !strcmp(startup_info->original_cwd, ce->name)) + return add_rejected_path(o, ERROR_CWD_IN_THE_WAY, ce->name); + return cnt; } diff --git a/unpack-trees.h b/unpack-trees.h index 71ffb7eeb0..efb9edfbb2 100644 --- a/unpack-trees.h +++ b/unpack-trees.h @@ -19,6 +19,7 @@ enum unpack_trees_error_types { ERROR_WOULD_OVERWRITE = 0, ERROR_NOT_UPTODATE_FILE, ERROR_NOT_UPTODATE_DIR, + ERROR_CWD_IN_THE_WAY, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, ERROR_WOULD_LOSE_UNTRACKED_REMOVED, ERROR_BIND_OVERLAP, From 0b0ee3388cf080c4200c235ee699bd95c960c167 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:28 +0000 Subject: [PATCH 04/11] unpack-trees: add special cwd handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When running commands such as `git reset --hard` from a subdirectory, if that subdirectory is in the way of adding needed files, bail with an error message. Note that this change looks kind of like it duplicates the new lines of code from the previous commit in verify_clean_subdirectory(). However, when we are preserving untracked files, we would rather any error messages about untracked files being in the way take precedence over error messages about a subdirectory that happens to be the_original_cwd being in the way. But in the UNPACK_RESET_OVERWRITE_UNTRACKED case, there is no untracked checking to be done, so we simply add a special case near the top of verify_absent_1. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t2501-cwd-empty.sh | 2 +- unpack-trees.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index 398908dfc9..5af1fec6fe 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -121,7 +121,7 @@ test_expect_success 'reset --hard does not clean cwd incidentally' ' ' test_expect_success 'reset --hard fails if cwd needs to be removed' ' - test_required_dir_removal failure git reset --hard fd_conflict + test_required_dir_removal success git reset --hard fd_conflict ' test_expect_success 'merge does not clean cwd incidentally' ' diff --git a/unpack-trees.c b/unpack-trees.c index 6bc16f3a71..5852807d2f 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -2261,10 +2261,19 @@ static int verify_absent_1(const struct cache_entry *ce, int len; struct stat st; - if (o->index_only || !o->update || - o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED) + if (o->index_only || !o->update) return 0; + if (o->reset == UNPACK_RESET_OVERWRITE_UNTRACKED) { + /* Avoid nuking startup_info->original_cwd... */ + if (startup_info->original_cwd && + !strcmp(startup_info->original_cwd, ce->name)) + return add_rejected_path(o, ERROR_CWD_IN_THE_WAY, + ce->name); + /* ...but nuke anything else. */ + return 0; + } + len = check_leading_path(ce->name, ce_namelen(ce), 0); if (!len) return 0; From 00fcce285db3db48f85730a183421fdb488c14cc Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:29 +0000 Subject: [PATCH 05/11] symlinks: do not include startup_info->original_cwd in dir removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit symlinks has a pair of schedule_dir_for_removal() and remove_scheduled_dirs() functions that ensure that directories made empty by removing other files also themselves get removed. However, we want to exclude startup_info->original_cwd and leave it around. This avoids the user getting confused by subsequent git commands (and non-git commands) that would otherwise report confusing messages about being unable to read the current working directory. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- symlinks.c | 8 +++++++- t/t2501-cwd-empty.sh | 10 +++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/symlinks.c b/symlinks.c index 5232d02020..c667baa949 100644 --- a/symlinks.c +++ b/symlinks.c @@ -279,7 +279,9 @@ static void do_remove_scheduled_dirs(int new_len) { while (removal.len > new_len) { removal.buf[removal.len] = '\0'; - if (rmdir(removal.buf)) + if ((startup_info->original_cwd && + !strcmp(removal.buf, startup_info->original_cwd)) || + rmdir(removal.buf)) break; do { removal.len--; @@ -293,6 +295,10 @@ void schedule_dir_for_removal(const char *name, int len) { int match_len, last_slash, i, previous_slash; + if (startup_info->original_cwd && + !strcmp(name, startup_info->original_cwd)) + return; /* Do not remove the current working directory */ + match_len = last_slash = i = longest_path_match(name, len, removal.buf, removal.len, &previous_slash); diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index 5af1fec6fe..e4502d24d5 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -109,7 +109,7 @@ test_required_dir_removal () { } test_expect_success 'checkout does not clean cwd incidentally' ' - test_incidental_dir_removal failure git checkout init + test_incidental_dir_removal success git checkout init ' test_expect_success 'checkout fails if cwd needs to be removed' ' @@ -117,7 +117,7 @@ test_expect_success 'checkout fails if cwd needs to be removed' ' ' test_expect_success 'reset --hard does not clean cwd incidentally' ' - test_incidental_dir_removal failure git reset --hard init + test_incidental_dir_removal success git reset --hard init ' test_expect_success 'reset --hard fails if cwd needs to be removed' ' @@ -125,7 +125,7 @@ test_expect_success 'reset --hard fails if cwd needs to be removed' ' ' test_expect_success 'merge does not clean cwd incidentally' ' - test_incidental_dir_removal failure git merge reverted + test_incidental_dir_removal success git merge reverted ' # This file uses some simple merges where @@ -158,7 +158,7 @@ test_expect_success 'merge fails if cwd needs to be removed' ' ' test_expect_success 'cherry-pick does not clean cwd incidentally' ' - test_incidental_dir_removal failure git cherry-pick reverted + test_incidental_dir_removal success git cherry-pick reverted ' test_expect_success 'cherry-pick fails if cwd needs to be removed' ' @@ -174,7 +174,7 @@ test_expect_success 'rebase fails if cwd needs to be removed' ' ' test_expect_success 'revert does not clean cwd incidentally' ' - test_incidental_dir_removal failure git revert HEAD + test_incidental_dir_removal success git revert HEAD ' test_expect_success 'revert fails if cwd needs to be removed' ' From c65744e7d7f512f3666da5964f0ace90325dd94a Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:30 +0000 Subject: [PATCH 06/11] clean: do not attempt to remove startup_info->original_cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/clean.c | 44 +++++++++++++++++++++++++++++++++++--------- t/t2501-cwd-empty.sh | 5 +++-- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/builtin/clean.c b/builtin/clean.c index 98a2860409..3ff02bbbff 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -36,6 +36,8 @@ static const char *msg_skip_git_dir = N_("Skipping repository %s\n"); static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n"); static const char *msg_warn_remove_failed = N_("failed to remove %s"); static const char *msg_warn_lstat_failed = N_("could not lstat %s\n"); +static const char *msg_skip_cwd = N_("Refusing to remove current working directory\n"); +static const char *msg_would_skip_cwd = N_("Would refuse to remove current working directory\n"); enum color_clean { CLEAN_COLOR_RESET = 0, @@ -153,6 +155,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, { DIR *dir; struct strbuf quoted = STRBUF_INIT; + struct strbuf realpath = STRBUF_INIT; + struct strbuf real_ocwd = STRBUF_INIT; struct dirent *e; int res = 0, ret = 0, gone = 1, original_len = path->len, len; struct string_list dels = STRING_LIST_INIT_DUP; @@ -231,16 +235,36 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, strbuf_setlen(path, original_len); if (*dir_gone) { - res = dry_run ? 0 : rmdir(path->buf); - if (!res) - *dir_gone = 1; - else { - int saved_errno = errno; - quote_path(path->buf, prefix, "ed, 0); - errno = saved_errno; - warning_errno(_(msg_warn_remove_failed), quoted.buf); + /* + * Normalize path components in path->buf, e.g. change '\' to + * '/' on Windows. + */ + strbuf_realpath(&realpath, path->buf, 1); + + /* + * path and realpath are absolute; for comparison, we would + * like to transform startup_info->original_cwd to an absolute + * path too. + */ + if (startup_info->original_cwd) + strbuf_realpath(&real_ocwd, + startup_info->original_cwd, 1); + + if (!strbuf_cmp(&realpath, &real_ocwd)) { + printf("%s", dry_run ? _(msg_would_skip_cwd) : _(msg_skip_cwd)); *dir_gone = 0; - ret = 1; + } else { + res = dry_run ? 0 : rmdir(path->buf); + if (!res) + *dir_gone = 1; + else { + int saved_errno = errno; + quote_path(path->buf, prefix, "ed, 0); + errno = saved_errno; + warning_errno(_(msg_warn_remove_failed), quoted.buf); + *dir_gone = 0; + ret = 1; + } } } @@ -250,6 +274,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag, printf(dry_run ? _(msg_would_remove) : _(msg_remove), dels.items[i].string); } out: + strbuf_release(&realpath); + strbuf_release(&real_ocwd); strbuf_release("ed); string_list_clear(&dels, 0); return ret; diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index e4502d24d5..b1182390ba 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -230,8 +230,9 @@ test_incidental_untracked_dir_removal () { } test_expect_success 'clean does not remove cwd incidentally' ' - test_incidental_untracked_dir_removal failure \ - git -C .. clean -fd -e warnings . >warnings + test_incidental_untracked_dir_removal success \ + git -C .. clean -fd -e warnings . >warnings && + grep "Refusing to remove current working directory" warnings ' test_expect_success 'stash does not remove cwd incidentally' ' From bc3ae46b420f58dfe2bfd87714dca096a043d554 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:31 +0000 Subject: [PATCH 07/11] rebase: do not attempt to remove startup_info->original_cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since rebase spawns a `checkout` subprocess, make sure we run that from the startup_info->original_cwd directory, so that the checkout process knows to protect that directory. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- sequencer.c | 2 ++ t/t2501-cwd-empty.sh | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index ea96837cde..83f257e7fa 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4228,6 +4228,8 @@ static int run_git_checkout(struct repository *r, struct replay_opts *opts, cmd.git_cmd = 1; + if (startup_info->original_cwd) + cmd.dir = startup_info->original_cwd; strvec_push(&cmd.args, "checkout"); strvec_push(&cmd.args, commit); strvec_pushf(&cmd.env_array, GIT_REFLOG_ACTION "=%s", action); diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index b1182390ba..52335a8afe 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -166,11 +166,11 @@ test_expect_success 'cherry-pick fails if cwd needs to be removed' ' ' test_expect_success 'rebase does not clean cwd incidentally' ' - test_incidental_dir_removal failure git rebase reverted + test_incidental_dir_removal success git rebase reverted ' test_expect_success 'rebase fails if cwd needs to be removed' ' - test_required_dir_removal failure git rebase fd_conflict + test_required_dir_removal success git rebase fd_conflict ' test_expect_success 'revert does not clean cwd incidentally' ' From 0fce211cccd0dce848d217cd4e2037214d7be1dd Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:32 +0000 Subject: [PATCH 08/11] stash: do not attempt to remove startup_info->original_cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since stash spawns a `clean` subprocess, make sure we run that from the startup_info->original_cwd directory, so that the `clean` processs knows to protect that directory. Also, since the `clean` command might no longer run from the toplevel, pass the ':/' magic pathspec to ensure we still clean from the toplevel. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/stash.c | 4 +++- t/t2501-cwd-empty.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index a0ccc8654d..de0e432a4f 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1485,8 +1485,10 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q struct child_process cp = CHILD_PROCESS_INIT; cp.git_cmd = 1; + if (startup_info->original_cwd) + cp.dir = startup_info->original_cwd; strvec_pushl(&cp.args, "clean", "--force", - "--quiet", "-d", NULL); + "--quiet", "-d", ":/", NULL); if (include_untracked == INCLUDE_ALL_FILES) strvec_push(&cp.args, "-x"); if (run_command(&cp)) { diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index 52335a8afe..be9ef903bd 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -236,7 +236,7 @@ test_expect_success 'clean does not remove cwd incidentally' ' ' test_expect_success 'stash does not remove cwd incidentally' ' - test_incidental_untracked_dir_removal failure \ + test_incidental_untracked_dir_removal success \ git stash --include-untracked ' From 63bbe8beb78ee8af5a7faeee4be747a82d8e2dc7 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:33 +0000 Subject: [PATCH 09/11] dir: avoid incidentally removing the original_cwd in remove_path() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Modern git often tries to avoid leaving empty directories around when removing files. Originally, it did not bother. This behavior started with commit 80e21a9ed809 (merge-recursive::removeFile: remove empty directories, 2005-11-19), stating the reason simply as: When the last file in a directory is removed as the result of a merge, try to rmdir the now-empty directory. This was reimplemented in C and renamed to remove_path() in commit e1b3a2cad7 ("Build-in merge-recursive", 2008-02-07), but was still internal to merge-recursive. This trend towards removing leading empty directories continued with commit d9b814cc97f1 (Add builtin "git rm" command, 2006-05-19), which stated the reasoning as: The other question is what to do with leading directories. The old "git rm" script didn't do anything, which is somewhat inconsistent. This one will actually clean up directories that have become empty as a result of removing the last file, but maybe we want to have a flag to decide the behaviour? remove_path() in dir.c was added in 4a92d1bfb784 (Add remove_path: a function to remove as much as possible of a path, 2008-09-27), because it was noted that we had two separate implementations of the same idea AND both were buggy. It described the purpose of the function as a function to remove as much as possible of a path Why remove as much as possible? Well, at the time we probably would have said something like: * removing leading directories makes things feel tidy * removing leading directories doesn't hurt anything so long as they had no files in them. But I don't believe those reasons hold when the empty directory happens to be the current working directory we inherited from our parent process. Leaving the parent process in a deleted directory can cause user confusion when subsequent processes fail: any git command, for example, will immediately fail with fatal: Unable to read current working directory: No such file or directory Other commands may similarly get confused. Modify remove_path() so that the empty leading directories it also deletes does not include the current working directory we inherited from our parent process. I have looked through every caller of remove_path() in the current codebase to make sure that all should take this change. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- dir.c | 3 +++ dir.h | 6 +++++- t/t2501-cwd-empty.sh | 12 ++++-------- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/dir.c b/dir.c index 94489298f4..97d6b71c87 100644 --- a/dir.c +++ b/dir.c @@ -3327,6 +3327,9 @@ int remove_path(const char *name) slash = dirs + (slash - name); do { *slash = '\0'; + if (startup_info->original_cwd && + !strcmp(startup_info->original_cwd, dirs)) + break; } while (rmdir(dirs) == 0 && (slash = strrchr(dirs, '/'))); free(dirs); } diff --git a/dir.h b/dir.h index 83f46c0fb4..d6a5d03bec 100644 --- a/dir.h +++ b/dir.h @@ -504,7 +504,11 @@ int get_sparse_checkout_patterns(struct pattern_list *pl); */ int remove_dir_recursively(struct strbuf *path, int flag); -/* tries to remove the path with empty directories along it, ignores ENOENT */ +/* + * Tries to remove the path, along with leading empty directories so long as + * those empty directories are not startup_info->original_cwd. Ignores + * ENOENT. + */ int remove_path(const char *path); int fspathcmp(const char *a, const char *b); diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index be9ef903bd..ce2efb9d30 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -182,12 +182,12 @@ test_expect_success 'revert fails if cwd needs to be removed' ' ' test_expect_success 'rm does not clean cwd incidentally' ' - test_incidental_dir_removal failure git rm bar/baz.t + test_incidental_dir_removal success git rm bar/baz.t ' test_expect_success 'apply does not remove cwd incidentally' ' git diff HEAD HEAD~1 >patch && - test_incidental_dir_removal failure git apply ../patch + test_incidental_dir_removal success git apply ../patch ' test_incidental_untracked_dir_removal () { @@ -271,12 +271,8 @@ test_expect_success '`rm -rf dir` even with only tracked files will remove somet ) && test_path_is_missing a/b/c/tracked && - ## We would prefer if a/b was still present, though empty, since it - ## was the current working directory - #test_path_is_dir a/b - ## But the current behavior is that it not only deletes the directory - ## a/b as requested, but also goes and deletes a - test_path_is_missing a + test_path_is_missing a/b/c && + test_path_is_dir a/b ' test_expect_success 'git version continues working from a deleted dir' ' From 580a5d7f75fc7b6c4c369ef429742d9d417acddd Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:34 +0000 Subject: [PATCH 10/11] dir: new flag to remove_dir_recurse() to spare the original_cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit remove_dir_recurse(), and its non-static wrapper called remove_dir_recursively(), both take flags for modifying its behavior. As with the previous commits, we would generally like to protect the original_cwd, but we want to forced user commands (e.g. 'git rm -rf ...') or other special cases to remove it. Add a flag for this purpose. After reading through every caller of remove_dir_recursively() in the current codebase, there was only one that should be adjusted and that one only in a very unusual circumstance. Add a pair of new testcases to highlight that very specific case involving submodules && --git-dir && --work-tree. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/rm.c | 3 ++- dir.c | 12 +++++++++--- dir.h | 3 +++ t/t2501-cwd-empty.sh | 5 ----- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/builtin/rm.c b/builtin/rm.c index 3d0967cdc1..b4132e5d8e 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -399,12 +399,13 @@ int cmd_rm(int argc, const char **argv, const char *prefix) if (!index_only) { int removed = 0, gitmodules_modified = 0; struct strbuf buf = STRBUF_INIT; + int flag = force ? REMOVE_DIR_PURGE_ORIGINAL_CWD : 0; for (i = 0; i < list.nr; i++) { const char *path = list.entry[i].name; if (list.entry[i].is_submodule) { strbuf_reset(&buf); strbuf_addstr(&buf, path); - if (remove_dir_recursively(&buf, 0)) + if (remove_dir_recursively(&buf, flag)) die(_("could not remove '%s'"), path); removed = 1; diff --git a/dir.c b/dir.c index 97d6b71c87..52064345a6 100644 --- a/dir.c +++ b/dir.c @@ -3204,6 +3204,7 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) int ret = 0, original_len = path->len, len, kept_down = 0; int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY); int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL); + int purge_original_cwd = (flag & REMOVE_DIR_PURGE_ORIGINAL_CWD); struct object_id submodule_head; if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) && @@ -3259,9 +3260,14 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up) closedir(dir); strbuf_setlen(path, original_len); - if (!ret && !keep_toplevel && !kept_down) - ret = (!rmdir(path->buf) || errno == ENOENT) ? 0 : -1; - else if (kept_up) + if (!ret && !keep_toplevel && !kept_down) { + if (!purge_original_cwd && + startup_info->original_cwd && + !strcmp(startup_info->original_cwd, path->buf)) + ret = -1; /* Do not remove current working directory */ + else + ret = (!rmdir(path->buf) || errno == ENOENT) ? 0 : -1; + } else if (kept_up) /* * report the uplevel that it is not an error that we * did not rmdir() our directory. diff --git a/dir.h b/dir.h index d6a5d03bec..8e02dfb505 100644 --- a/dir.h +++ b/dir.h @@ -495,6 +495,9 @@ int get_sparse_checkout_patterns(struct pattern_list *pl); /* Remove the contents of path, but leave path itself. */ #define REMOVE_DIR_KEEP_TOPLEVEL 04 +/* Remove the_original_cwd too */ +#define REMOVE_DIR_PURGE_ORIGINAL_CWD 0x08 + /* * Remove path and its contents, recursively. flags is a combination * of the above REMOVE_DIR_* constants. Return 0 on success. diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index ce2efb9d30..bc92230f2f 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -291,11 +291,6 @@ test_submodule_removal () { test_status= test "$path_status" = dir && test_status=test_must_fail - # Actually, while path_status=dir && test_status=test_must_fail - # reflect our desired behavior, current behavior is: - path_status=missing - test_status= - test_when_finished "git reset --hard HEAD~1" && test_when_finished "rm -rf .git/modules/my_submodule" && From 324b170b88475811cb0506a30b4710ffc89ae936 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Thu, 9 Dec 2021 05:08:35 +0000 Subject: [PATCH 11/11] t2501: simplify the tests since we can now assume desired behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We no longer are dealing with a mixture of previous and desired behavior, so simplify the tests a bit. Acked-by: Derrick Stolee Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t2501-cwd-empty.sh | 119 +++++++++++++------------------------------ 1 file changed, 34 insertions(+), 85 deletions(-) diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh index bc92230f2f..f6d8d7d03d 100755 --- a/t/t2501-cwd-empty.sh +++ b/t/t2501-cwd-empty.sh @@ -32,9 +32,6 @@ test_expect_success setup ' ' test_incidental_dir_removal () { - works=$1 && - shift && - test_when_finished "git reset --hard" && git checkout foo/bar/baz^{commit} && @@ -44,88 +41,57 @@ test_incidental_dir_removal () { cd foo && "$@" && - # Although we want pwd & git status to pass, test for existing - # rather than desired behavior. - if test "$works" = "success" - then - test-tool getcwd && - git status --porcelain - else - ! test-tool getcwd && - test_might_fail git status --porcelain - fi + # Make sure foo still exists, and commands needing it work + test-tool getcwd && + git status --porcelain ) && test_path_is_missing foo/bar/baz && test_path_is_missing foo/bar && - # Although we want dir to be present, test for existing rather - # than desired behavior. - if test "$works" = "success" - then - test_path_is_dir foo - else - test_path_is_missing foo - fi + test_path_is_dir foo } test_required_dir_removal () { - works=$1 && - shift && - git checkout df_conflict^{commit} && test_when_finished "git clean -fdx" && ( cd dirORfile && - # We'd like for the command to fail (much as it would if there - # was an untracked file there), and for the index and worktree - # to be left clean with pwd and git status working afterwards. - # But test for existing rather than desired behavior. - if test "$works" = "success" - then - test_must_fail "$@" 2>../error && - grep "Refusing to remove.*current working directory" ../error && + # Ensure command refuses to run + test_must_fail "$@" 2>../error && + grep "Refusing to remove.*current working directory" ../error && - git diff --exit-code HEAD && + # ...and that the index and working tree are left clean + git diff --exit-code HEAD && - test-tool getcwd && - git status --porcelain - else - "$@" && - ! test-tool getcwd && - test_might_fail git status --porcelain - fi + # Ensure that getcwd and git status do not error out (which + # they might if the current working directory had been removed) + test-tool getcwd && + git status --porcelain ) && - # Although we want dirORfile to be present, test for existing rather - # than desired behavior. - if test "$works" = "success" - then - test_path_is_dir dirORfile - else - test_path_is_file dirORfile - fi + test_path_is_dir dirORfile } test_expect_success 'checkout does not clean cwd incidentally' ' - test_incidental_dir_removal success git checkout init + test_incidental_dir_removal git checkout init ' test_expect_success 'checkout fails if cwd needs to be removed' ' - test_required_dir_removal success git checkout fd_conflict + test_required_dir_removal git checkout fd_conflict ' test_expect_success 'reset --hard does not clean cwd incidentally' ' - test_incidental_dir_removal success git reset --hard init + test_incidental_dir_removal git reset --hard init ' test_expect_success 'reset --hard fails if cwd needs to be removed' ' - test_required_dir_removal success git reset --hard fd_conflict + test_required_dir_removal git reset --hard fd_conflict ' test_expect_success 'merge does not clean cwd incidentally' ' - test_incidental_dir_removal success git merge reverted + test_incidental_dir_removal git merge reverted ' # This file uses some simple merges where @@ -154,46 +120,43 @@ test_expect_success 'merge fails if cwd needs to be removed; recursive friendly' GIT_TEST_MERGE_ALGORITHM=ort test_expect_success 'merge fails if cwd needs to be removed' ' - test_required_dir_removal success git merge fd_conflict + test_required_dir_removal git merge fd_conflict ' test_expect_success 'cherry-pick does not clean cwd incidentally' ' - test_incidental_dir_removal success git cherry-pick reverted + test_incidental_dir_removal git cherry-pick reverted ' test_expect_success 'cherry-pick fails if cwd needs to be removed' ' - test_required_dir_removal success git cherry-pick fd_conflict + test_required_dir_removal git cherry-pick fd_conflict ' test_expect_success 'rebase does not clean cwd incidentally' ' - test_incidental_dir_removal success git rebase reverted + test_incidental_dir_removal git rebase reverted ' test_expect_success 'rebase fails if cwd needs to be removed' ' - test_required_dir_removal success git rebase fd_conflict + test_required_dir_removal git rebase fd_conflict ' test_expect_success 'revert does not clean cwd incidentally' ' - test_incidental_dir_removal success git revert HEAD + test_incidental_dir_removal git revert HEAD ' test_expect_success 'revert fails if cwd needs to be removed' ' - test_required_dir_removal success git revert undo_fd_conflict + test_required_dir_removal git revert undo_fd_conflict ' test_expect_success 'rm does not clean cwd incidentally' ' - test_incidental_dir_removal success git rm bar/baz.t + test_incidental_dir_removal git rm bar/baz.t ' test_expect_success 'apply does not remove cwd incidentally' ' git diff HEAD HEAD~1 >patch && - test_incidental_dir_removal success git apply ../patch + test_incidental_dir_removal git apply ../patch ' test_incidental_untracked_dir_removal () { - works=$1 && - shift && - test_when_finished "git reset --hard" && git checkout foo/bar/baz^{commit} && @@ -205,38 +168,24 @@ test_incidental_untracked_dir_removal () { cd untracked && "$@" && - # Although we want pwd & git status to pass, test for existing - # rather than desired behavior. - if test "$works" = "success" - then - test-tool getcwd && - git status --porcelain - else - ! test-tool getcwd && - test_might_fail git status --porcelain - fi + # Make sure untracked still exists, and commands needing it work + test-tool getcwd && + git status --porcelain ) && test_path_is_missing empty && test_path_is_missing untracked/random && - # Although we want dir to be present, test for existing rather - # than desired behavior. - if test "$works" = "success" - then - test_path_is_dir untracked - else - test_path_is_missing untracked - fi + test_path_is_dir untracked } test_expect_success 'clean does not remove cwd incidentally' ' - test_incidental_untracked_dir_removal success \ + test_incidental_untracked_dir_removal \ git -C .. clean -fd -e warnings . >warnings && grep "Refusing to remove current working directory" warnings ' test_expect_success 'stash does not remove cwd incidentally' ' - test_incidental_untracked_dir_removal success \ + test_incidental_untracked_dir_removal \ git stash --include-untracked '