Skip to content

pr-1036/newren/untracked_removal-v1

This series depends on en/am-abort-fix.

We have multiple codepaths that delete untracked files/directories but
shouldn't. There are also some codepaths where we delete untracked
files/directories intentionally (based on mailing list discussion), but
where that intent is not documented. Fix the documentation, add several new
(mostly failing) testcases, fix some of the new testcases, and add comments
about some potential remaining problems. (I found these as a side-effect of
looking at [1], though [2] pointed out one explicitly while I was working on
it.)

Note that I'm using Junio's declaration about checkout -f and reset --hard
(and also presuming that since read-tree --reset is porcelain that its
behavior should be left alone)[3] in this series.

SIDENOTE about treating (some) ignored files as precious:

There's another related topic here that came up in the mailing list threads
that is separate even if similar: namely, treating ignored files as precious
instead of deleting them. I do not try to handle that here, but I believe
that would actually be relatively easy to handle. If you leave
unpack_trees_options->dir as NULL, then ignored files are treated as
precious (my original patch 2 made that mistake). There's a few other
locations that already optionally set up unpack_trees_options->dir (a quick
search for "overwrite_ignore" and "overwrite-ignore" will find them), so
we'd just need to implement that option flag in more places corresponding to
the new callsites (and perhaps make a global core.overwrite_ignored config
option to affect all of these). Of course, doing so would globally treat
ignored files as precious rather than allowing them to be configured on a
per-path basis, but honestly I think the idea of configuring ignored files
as precious on a per-path basis sounds like insanity. (We have enough bugs
with untracked and ignored files without adding yet another type. Also,
tla/baz was excessively confusing to me due in part to the number of types
of files and I'd rather not see such ideas ported to git. And, of course,
configuring per-path rules sounds like lots of work for end users to
configure. There may be additional reasons against it.) So, if someone wants
to pursue the precious-ignored concept then I'd much rather see it done as a
global setting. Just my $0.02.

[1] https://lore.kernel.org/git/xmqqv93n7q1v.fsf@gitster.g/ [2]
https://lore.kernel.org/git/C357A648-8B13-45C3-9388-C0C7F7D40DAE@gmail.com/
[3] https://lore.kernel.org/git/xmqqr1e2ejs9.fsf@gitster.g/

Elijah Newren (6):
  t2500: add various tests for nuking untracked files
  Split unpack_trees 'reset' flag into two for untracked handling
  unpack-trees: avoid nuking untracked dir in way of unmerged file
  unpack-trees: avoid nuking untracked dir in way of locally deleted
    file
  Comment important codepaths regarding nuking untracked files/dirs
  Documentation: call out commands that nuke untracked files/directories

 Documentation/git-checkout.txt   |   5 +-
 Documentation/git-read-tree.txt  |   5 +-
 Documentation/git-reset.txt      |   3 +-
 builtin/am.c                     |   6 +-
 builtin/checkout.c               |  10 +-
 builtin/read-tree.c              |  11 +-
 builtin/reset.c                  |  15 +-
 builtin/stash.c                  |   3 +-
 builtin/submodule--helper.c      |   4 +
 builtin/worktree.c               |   5 +
 contrib/rerere-train.sh          |   2 +-
 reset.c                          |   9 +-
 submodule.c                      |   1 +
 t/t1013-read-tree-submodule.sh   |   4 +-
 t/t2500-untracked-overwriting.sh | 244 +++++++++++++++++++++++++++++++
 unpack-trees.c                   |  56 +++++--
 unpack-trees.h                   |   4 +-
 17 files changed, 359 insertions(+), 28 deletions(-)
 create mode 100755 t/t2500-untracked-overwriting.sh

base-commit: c5ead19ea282a288e01d86536349a4ae4a093e4b

Submitted-As: https://lore.kernel.org/git/pull.1036.git.1632006923.gitgitgadget@gmail.com
Assets 2