Skip to content

Commit 94b7f15

Browse files
newrengitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 56d06fe commit 94b7f15

File tree

4 files changed

+7
-1
lines changed

4 files changed

+7
-1
lines changed

builtin/stash.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,6 +1521,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
15211521
} else {
15221522
struct child_process cp = CHILD_PROCESS_INIT;
15231523
cp.git_cmd = 1;
1524+
/* BUG: this nukes untracked files in the way */
15241525
strvec_pushl(&cp.args, "reset", "--hard", "-q",
15251526
"--no-recurse-submodules", NULL);
15261527
if (run_command(&cp)) {

builtin/submodule--helper.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2864,6 +2864,10 @@ static int add_submodule(const struct add_data *add_data)
28642864
prepare_submodule_repo_env(&cp.env_array);
28652865
cp.git_cmd = 1;
28662866
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+
*/
28672871
strvec_pushl(&cp.args, "checkout", "-f", "-q", NULL);
28682872

28692873
if (add_data->branch) {

contrib/rerere-train.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ do
9191
git checkout -q $commit -- .
9292
git rerere
9393
fi
94-
git reset -q --hard
94+
git reset -q --hard # Might nuke untracked files...
9595
done
9696

9797
if test -z "$branch"

submodule.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1866,6 +1866,7 @@ static void submodule_reset_index(const char *path)
18661866

18671867
strvec_pushf(&cp.args, "--super-prefix=%s%s/",
18681868
get_super_prefix_or_empty(), path);
1869+
/* TODO: determine if this might overwright untracked files */
18691870
strvec_pushl(&cp.args, "read-tree", "-u", "--reset", NULL);
18701871

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

0 commit comments

Comments
 (0)