Skip to content

pr-1036/newren/untracked_removal-v3

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. We also have some codepaths that
preserve ignored files, which shouldn't. 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.

Changes since v2 (all due to Junio's request to consolidate
unpack_trees_options.dir handling):

 * fix some (pre-existing) memory leaks, in preparation for consolidating
   some common code (new patch 2)
 * New patches (3 & 6) to make a few more commands remove ignored files by
   default -- which also fixes an existing testcase
 * New patches (4 & 5) to consolidate the various places handling
   unpack_trees_options.dir and default to treating ignored files as
   expendable within unpack_trees(). These change also make it very easy to
   add --no-overwrite-ignore options in the future to additional commands
   (checkout and merge already have such an option, though merge only passes
   that along to the fast-forwarding backend)

Changes since v1:

 * Various small cleanups (suggested by Ævar)
 * Fixed memory leaks of unpack_trees_opts->dir (also suggested by Ævar)
 * Use an enum for unpack_trees_options->reset, instead of multiple fields
   (suggested by Phillip)
 * Avoid changing behavior for cases not setting unpack_trees_options.reset
   > 0 (even if it may make sense to nuke ignored files when running either
   read-tree -m -u or the various reset flavors run internally by
   rebase/sequencer); we can revisit that later.

SIDENOTE about treating ignored files as precious:

The patches are now getting pretty close to being able to handle ignored
files as precious. The only things left would be making merge pass the
--no-overwrite-ignore option along to more backends, and adding the
--no-overwrite-ignore option that both checkout and merge take to more
commands. There's already comments in the code about what boolean would need
to be set by that flag. And then perhaps also make a global
core.overwrite_ignored config option to affect all of these. Granted, doing
this 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. 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 (11):
  t2500: add various tests for nuking untracked files
  checkout, read-tree: fix leak of unpack_trees_options.dir
  read-tree, merge-recursive: overwrite ignored files by default
  unpack-trees: introduce preserve_ignored to unpack_trees_options
  unpack-trees: make dir an internal-only struct
  Remove ignored files by default when they are in the way
  Change unpack_trees' 'reset' flag into an enum
  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  |  23 +--
 Documentation/git-reset.txt      |   3 +-
 builtin/am.c                     |   3 +-
 builtin/checkout.c               |  10 +-
 builtin/clone.c                  |   1 +
 builtin/merge.c                  |   1 +
 builtin/read-tree.c              |  26 ++--
 builtin/reset.c                  |  10 +-
 builtin/stash.c                  |   5 +-
 builtin/submodule--helper.c      |   4 +
 contrib/rerere-train.sh          |   2 +-
 merge-ort.c                      |   8 +-
 merge-recursive.c                |   5 +-
 merge.c                          |   8 +-
 reset.c                          |   3 +-
 sequencer.c                      |   1 +
 submodule.c                      |   1 +
 t/t1013-read-tree-submodule.sh   |   1 -
 t/t2500-untracked-overwriting.sh | 244 +++++++++++++++++++++++++++++++
 t/t7112-reset-submodule.sh       |   1 -
 unpack-trees.c                   |  61 +++++++-
 unpack-trees.h                   |  14 +-
 23 files changed, 366 insertions(+), 74 deletions(-)
 create mode 100755 t/t2500-untracked-overwriting.sh

base-commit: ddb1055343948e0d0bc81f8d20245f1ada6430a0

Submitted-As: https://lore.kernel.org/git/pull.1036.v3.git.1632760428.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1036.git.1632006923.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1036.v2.git.1632465429.gitgitgadget@gmail.com
Assets 2