Avoid removing the current working directory, even if it becomes empty

Traditionally, if folks run git commands such as checkout or rebase from a subdirectory, that git command could remove their current working directory and result in subsequent git and non-git commands either getting confused or printing messages that confuse the user (e.g. "fatal: Unable to read current working directory: No such file or directory"). Many commands either silently avoid removing directories that are not empty (i.e. those that have untracked or modified files in them)[1], or show an error and abort, depending on which is more appropriate for the command in question. With this series, we augment the reasons to avoid removing directories to include not just has-untracked-or-modified-files, but also to avoid removing the original_cwd as well.

Peff and Junio provided some good pros/cons, if it helps:

* Pros: Peff (original suggester of the idea)[2], and Junio[3]
* Cons: Peff [2, again -- see the "P.S."], and Junio[4]

[1] well, with a few exceptions; see
https://lore.kernel.org/git/[email protected]/
[2] https://lore.kernel.org/git/[email protected]/
[3] https://lore.kernel.org/git/[email protected]/ [4]
https://lore.kernel.org/git/[email protected]/

Changes since v5:

* It's been about a week without changes, so including Acks that have come in (let me know if I was wrong in my interpretation of these statements as Acks):
* from Stolee ("This version is good to go",

     https://lore.kernel.org/git/[email protected]/)
   * from Ævar ("I've got no objections to these changes going in as they
     stand.",
     https://lore.kernel.org/git/[email protected]/)

Changes since v4:

* actually fix bashism

Changes since v3:

* fixed one codepath from v2 so that the series really is only about the working tree
* used test-tool getcwd instead of pwd -P as suggested by Ævar for some less common platforms
* fixed bashism
* check for clean index/worktree after verifying that expected-to-abort codepaths do abort, to make it clearer that we expect an early abort
* remove a leftover (and confusing) is_absolute_dir() check in sequencer and stash from an earlier round of the series

Changes since v2:

* the series is now only about the working tree. So if the original cwd is outside the worktree (or we're in a bare repo), then the new code is a no-op.
* fixed ugly early die() possibility (uses strbuf_getcwd() instead of xgetcwd())
* modified the initial tests to show both expected and desired behavior. subsequent patches fix the tests. One new patch added at the end which simplifies the tests to only check for desired behavior.
* NULLify startup_info->original_cwd when it matches the toplevel worktree; that is already protected and we don't need secondary protection for it. This simplified some other codepaths so we don't have to check for startup_info->original_cwd == "".
* clarified some commit messages

Changes since v1:

* clarified multiple commit messages
* renamed the_cwd to startup_info->original_cwd to make it clearer that it's our parent process'es cwd that really matters, which we inherited at program startup. Also pulls it out of the global namespace.
* Normalize the path for startup_info->original_cwd, and ensure that it's actually the original cwd even if -C is passed to git.
* small code cleanups suggested by René and Ævar
* split the final patch (which got the most comments) into two -- one for each function being modified. Also, add a bunch more history to the first of the two resulting commit messages
* no longer has a content conflict with so/stash-staged
* add another value for the flags parameter that remove_dir_recursively() takes so that it can opt into either the old or the new behavior. Use that for the one special corner case I could find where it matters, and add a few tests around it to highlight the utility of the flag.

Elijah Newren (11):
t2501: add various tests for removing the current working directory
setup: introduce startup_info->original_cwd
unpack-trees: refuse to remove startup_info->original_cwd
unpack-trees: add special cwd handling
symlinks: do not include startup_info->original_cwd in dir removal
clean: do not attempt to remove startup_info->original_cwd
rebase: do not attempt to remove startup_info->original_cwd
stash: do not attempt to remove startup_info->original_cwd
dir: avoid incidentally removing the original_cwd in remove_path()
dir: new flag to remove_dir_recurse() to spare the original_cwd
t2501: simplify the tests since we can now assume desired behavior

 builtin/clean.c      |  44 +++++--
 builtin/rm.c         |   3 +-
 builtin/stash.c      |   4 +-
 cache.h              |   2 +
 common-main.c        |   4 +
 dir.c                |  15 ++-
 dir.h                |   9 +-
 sequencer.c          |   2 +
 setup.c              |  65 ++++++++++
 symlinks.c           |   8 +-
 t/t2501-cwd-empty.sh | 277 +++++++++++++++++++++++++++++++++++++++++++
 unpack-trees.c       |  30 ++++-
 unpack-trees.h       |   1 +
 13 files changed, 442 insertions(+), 22 deletions(-)
 create mode 100755 t/t2501-cwd-empty.sh

base-commit: 88d915a634b449147855041d44875322de2b286d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1140%2Fnewren%2Fcwd_removal-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1140/newren/cwd_removal-v6
Pull-Request: https://github.com/git/git/pull/1140

Range-diff vs v5:

1: 7c72e888d2e ! 1: 1fd76d4f8a8 t2501: add various tests for removing the current working directory
@@ Commit message
Also add a few tests suggested during the review of earlier rounds of
this patch series.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## t/t2501-cwd-empty.sh (new) ##
2: 37f333b2024 ! 2: cd1f564c4c0 setup: introduce startup_info->original_cwd
@@ Commit message

Subsequent commits will make use of this new variable.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## cache.h ##
3: b611c73bd15 ! 3: 4a1f62b662c unpack-trees: refuse to remove startup_info->original_cwd
@@ Commit message
error out if the directory is the current working directory we inherited from our parent process.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## t/t2501-cwd-empty.sh ##
4: 706415a4547 ! 4: 03f2b0b8fb7 unpack-trees: add special cwd handling
@@ Commit message
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 <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## t/t2501-cwd-empty.sh ##
5: 66ef6b4d943 ! 5: e75f6c3f9ad symlinks: do not include startup_info->original_cwd in dir removal
@@ Commit message
commands) that would otherwise report confusing messages about being
unable to read the current working directory.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## symlinks.c ##
6: 54d059c683e ! 6: d4f8784a6e4 clean: do not attempt to remove startup_info->original_cwd
@@ Metadata
## Commit message ##
clean: do not attempt to remove startup_info->original_cwd

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## builtin/clean.c ##
7: f8efb7446c3 ! 7: fd500cc1843 rebase: do not attempt to remove startup_info->original_cwd
@@ Commit message
the startup_info->original_cwd directory, so that the checkout process knows to protect that directory.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## sequencer.c ##
8: 2855ed45112 ! 8: 6f3c4fd49b9 stash: do not attempt to remove startup_info->original_cwd
@@ Commit message
longer run from the toplevel, pass the ':/' magic pathspec to ensure we still clean from the toplevel.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## builtin/stash.c ##
9: 23dfc3e399d ! 9: 1a66b1a2386 dir: avoid incidentally removing the original_cwd in remove_path()
@@ Commit message
looked through every caller of remove_path() in the current codebase to
make sure that all should take this change.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## dir.c ##
10: fe47c0f0c17 ! 10: 4a18efd51ef dir: new flag to remove_dir_recurse() to spare the original_cwd
@@ Commit message
highlight that very specific case involving submodules && --git-dir && --work-tree.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## builtin/rm.c ##
11: 431dd651a7e ! 11: 63781ed8b8e t2501: simplify the tests since we can now assume desired behavior
@@ Commit message
We no longer are dealing with a mixture of previous and desired
behavior, so simplify the tests a bit.

+ Acked-by: Derrick Stolee <[email protected]>
+ Acked-by: Ævar Arnfjörð Bjarmason <[email protected]>

## t/t2501-cwd-empty.sh ##

gitgitgadget