[0/8] Avoid removing the current working directory, even if it becomes empty

Message

Phillip Wood via GitGitGadget Nov. 21, 2021, 12:46 a.m. UTC

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"). We already refuse to remove
directories that have untracked files within them[1], preferring to show an
error; with this series, we tweak that rule a bit to also refuse to remove
the current working directory even if it has no untracked files within it.

Peff endorsed the idea behind this series at [2] and even pointed out a
corner case that he suggested (and I agree) would probably be more
problematic to attempt to address than to leave be.

This series has a minor textual conflict with so/stash-staged (in next),
because both series add a new parameter to the do_push_stash() function. The
correct resolution is simple -- just to take both new parameters. However,
let me know if you'd rather I rebased onto so/stash-staged.

(Sorry for the delay; I know I promised this months ago in response to [3],
but it was blocked by some preliminary series and then by my absence during
a migration at $DAYJOB).

[1] well, with a few exceptions; see
https://lore.kernel.org/git/[e-mail]/
[2] https://lore.kernel.org/git/[e-mail]/
[3]
https://lore.kernel.org/git/[e-mail]/

Elijah Newren (8):
  t2501: add various tests for removing the current working directory
  repository, setup: introduce the_cwd
  unpack-trees: refuse to remove the current working directory
  unpack-trees: add special cwd handling
  symlinks: do not include current working directory in dir removal
  clean: do not attempt to remove current working directory
  stash: do not attempt to remove current working directory
  dir: avoid removing the current working directory

 builtin/clean.c      |  29 +++--
 builtin/stash.c      |  13 ++-
 dir.c                |  11 +-
 repository.c         |   1 +
 repository.h         |   1 +
 setup.c              |   2 +
 symlinks.c           |  12 +-
 t/t2501-cwd-empty.sh | 255 +++++++++++++++++++++++++++++++++++++++++++
 unpack-trees.c       |  28 ++++-
 unpack-trees.h       |   1 +
 10 files changed, 329 insertions(+), 24 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-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1140/newren/cwd_removal-v1
Pull-Request: https://github.com/git/git/pull/1140

Comments

Junio C Hamano Nov. 21, 2021, 8:11 a.m. UTC | #1

"Elijah Newren via GitGitGadget" <[e-mail]> writes:

> 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"). We already refuse to remove
> directories that have untracked files within them[1], preferring to show an
> error; with this series, we tweak that rule a bit to also refuse to remove
> the current working directory even if it has no untracked files within it.

The goal is roughly that we do not allow rmdir() of a directory at
"prefix" in the working tree, or any parent directory of it, for
functions that uses RUN_SETUP?  By the time we attempt to rmdir(),
most likely we'd chdir()'ed up to the top of the working tree
ourselves, so we need to remember where the original cwd was when we
started, and protecting the original cwd would mean that we protect
the directory in which the process that spawned us, like the user's
interactive shell, is still sitting, which makes sense.