@matheusbsbs / matheus-bagliano / commits / 94b7f1563a

Comment important codepaths regarding nuking untracked files/dirs

In the last few commits we focused on code in unpack-trees.c that mistakenly removed untracked files or directories. There may be more of those, but in this commit we change our focus: callers of toplevel commands that are expected to remove untracked files or directories. As noted previously, we have toplevel commands that are expected to delete untracked files such as 'read-tree --reset', 'reset --hard', and 'checkout --force'. However, that does not mean that other highlevel commands that happen to call these other commands thought about or conveyed to users the possibility that untracked files could be removed. Audit the code for such callsites, and add comments near existing callsites to mention whether these are safe or not. My auditing is somewhat incomplete, though; it skipped several cases: * git-rebase--preserve-merges.sh: is in the process of being deprecated/removed, so I won't leave a note that there are likely more bugs in that script. * contrib/git-new-workdir: why is the -f flag being used in a new empty directory?? It shouldn't hurt, but it seems useless. * git-p4.py: Don't see why -f is needed for a new dir (maybe it's not and is just superfluous), but I'm not at all familiar with the p4 stuff * git-archimport.perl: Don't care; arch is long since dead * git-cvs*.perl: Don't care; cvs is long since dead Also, the reset --hard in builtin/worktree.c looks safe, due to only running in an empty directory. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Sep 27, 2021 at 16:33 UTC 94b7f1563ace91af823125e5b8895cb24b2c0e4a

4 files changed +7 -1

builtin/stash.c

@@ -1521,6 +1521,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q

1521 } else {

1522 struct child_process cp = CHILD_PROCESS_INIT;

1523 cp.git_cmd = 1;

1524 + /* BUG: this nukes untracked files in the way */

1525 strvec_pushl(&cp.args, "reset", "--hard", "-q",

1526 "--no-recurse-submodules", NULL);

1527 if (run_command(&cp)) {

builtin/submodule--helper.c

@@ -2864,6 +2864,10 @@ static int add_submodule(const struct add_data *add_data)

2864 prepare_submodule_repo_env(&cp.env_array);

2865 cp.git_cmd = 1;

2866 cp.dir = add_data->sm_path;

2867 + /*

2868 + * NOTE: we only get here if add_data->force is true, so

2869 + * passing --force to checkout is reasonable.

2870 + */

2871 strvec_pushl(&cp.args, "checkout", "-f", "-q", NULL);

2872

2873 if (add_data->branch) {

contrib/rerere-train.sh

+1 -1

@@ -91,7 +91,7 @@ do

91 git checkout -q $commit -- .

92 git rerere

93 fi

94 - git reset -q --hard

94 + git reset -q --hard # Might nuke untracked files...

95 done

97 if test -z "$branch"

submodule.c

@@ -1866,6 +1866,7 @@ static void submodule_reset_index(const char *path)

1866

1867 strvec_pushf(&cp.args, "--super-prefix=%s%s/",

1868 get_super_prefix_or_empty(), path);

1869 + /* TODO: determine if this might overwright untracked files */

1870 strvec_pushl(&cp.args, "read-tree", "-u", "--reset", NULL);

1871

1872 strvec_push(&cp.args, empty_tree_oid_hex());