Skip to content

Commit

Permalink
checkout: do not delete directories with untracked entries
Browse files Browse the repository at this point in the history
If the `GIT_CHECKOUT_FORCE` flag is given to any of the `git_checkout`
invocations, we remove files which were previously staged. But while
doing so, we unfortunately also remove unstaged files in a directory
which contains at least one staged file, resulting in potential data
loss.

The issue resides in our logic which tries to determine which action to
excute for a tree entry. We currently assume that a directory is safe to
be deleted as soon as it contains at least one versioned entry - which
is ovbiously wrong. Instead, we first have to determine whether the
directory contains any unversioned files and, if so, advance into the
directory itself. As such, the directory will not be deleted, but all
versioned entries inside of the directory will be reset correctly.

This commit also adds two tests to verify behavior.
  • Loading branch information
pks-t committed Jun 6, 2017
1 parent 78b8f03 commit f2d5ded
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 0 deletions.
46 changes: 46 additions & 0 deletions src/checkout.c
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,44 @@ static int checkout_queue_remove(checkout_data *data, const char *path)
return git_vector_insert(&data->removes, copy);
}

static int contains_unversioned(int *unversioned, git_index *index, const char *dir)
{
git_vector entries = GIT_VECTOR_INIT;
git_buf path = GIT_BUF_INIT;
const char *workdir, *entry;
int error = 0;
size_t i;

if ((workdir = git_repository_workdir(index->rc.owner)) == NULL) {
error = -1;
goto out;
}

if ((error = git_buf_joinpath(&path, workdir, dir)) < 0)
goto out;

if ((error = git_path_dirload(&entries, path.ptr, strlen(workdir), 0)) < 0)
goto out;

*unversioned = 0;

git_vector_foreach(&entries, i, entry) {
const git_index_entry *ientry =
git_index_get_bypath(index, entry, 0);

if (ientry == NULL) {
*unversioned = 1;
break;
}
}

out:
git_vector_free_deep(&entries);
git_buf_free(&path);

return error;
}

/* note that this advances the iterator over the wd item */
static int checkout_action_wd_only(
checkout_data *data,
Expand Down Expand Up @@ -371,6 +409,14 @@ static int checkout_action_wd_only(
const git_index_entry *e = git_index_get_byindex(data->index, pos);

if (e != NULL && data->diff->pfxcomp(e->path, wd->path) == 0) {
int unversioned;
if ((error = !contains_unversioned(
&unversioned, data->index, wd->path)) < 0)
return error;

if (unversioned)
return git_iterator_advance_into(wditem, workdir);

notify = GIT_CHECKOUT_NOTIFY_DIRTY;
remove = ((data->strategy & GIT_CHECKOUT_FORCE) != 0);
}
Expand Down
76 changes: 76 additions & 0 deletions tests/checkout/head.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,79 @@ void test_checkout_head__with_index_only_tree(void)

git_index_free(index);
}

void test_checkout_head__do_not_remove_untracked_file(void)
{
git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
git_index *index;

cl_git_pass(p_mkdir("testrepo/tracked", 0755));
cl_git_mkfile("testrepo/tracked/tracked", "tracked\n");
cl_git_mkfile("testrepo/tracked/untracked", "untracked\n");

cl_git_pass(git_repository_index(&index, g_repo));
cl_git_pass(git_index_add_bypath(index, "tracked/tracked"));
cl_git_pass(git_index_write(index));

git_index_free(index);

opts.checkout_strategy = GIT_CHECKOUT_FORCE;
cl_git_pass(git_checkout_head(g_repo, &opts));

cl_assert(!git_path_isfile("testrepo/tracked/tracked"));
cl_assert(git_path_isfile("testrepo/tracked/untracked"));
}

void test_checkout_head__do_not_remove_untracked_file_in_subdir(void)
{
git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
git_index *index;

cl_git_pass(p_mkdir("testrepo/tracked", 0755));
cl_git_pass(p_mkdir("testrepo/tracked/subdir", 0755));
cl_git_mkfile("testrepo/tracked/tracked", "tracked\n");
cl_git_mkfile("testrepo/tracked/subdir/tracked", "tracked\n");
cl_git_mkfile("testrepo/tracked/subdir/untracked", "untracked\n");

cl_git_pass(git_repository_index(&index, g_repo));
cl_git_pass(git_index_add_bypath(index, "tracked/tracked"));
cl_git_pass(git_index_add_bypath(index, "tracked/subdir/tracked"));
cl_git_pass(git_index_write(index));

git_index_free(index);

opts.checkout_strategy = GIT_CHECKOUT_FORCE;
cl_git_pass(git_checkout_head(g_repo, &opts));

cl_assert(!git_path_isfile("testrepo/tracked/tracked"));
cl_assert(!git_path_isfile("testrepo/tracked/subdir/tracked"));
cl_assert(git_path_isfile("testrepo/tracked/subdir/untracked"));
}

void test_checkout_head__do_remove_tracked_subdir(void)
{
git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
git_index *index;

cl_git_pass(p_mkdir("testrepo/subdir", 0755));
cl_git_pass(p_mkdir("testrepo/subdir/tracked", 0755));
cl_git_mkfile("testrepo/subdir/tracked-file", "tracked\n");
cl_git_mkfile("testrepo/subdir/untracked-file", "untracked\n");
cl_git_mkfile("testrepo/subdir/tracked/tracked1", "tracked\n");
cl_git_mkfile("testrepo/subdir/tracked/tracked2", "tracked\n");

cl_git_pass(git_repository_index(&index, g_repo));
cl_git_pass(git_index_add_bypath(index, "subdir/tracked-file"));
cl_git_pass(git_index_add_bypath(index, "subdir/tracked/tracked1"));
cl_git_pass(git_index_add_bypath(index, "subdir/tracked/tracked2"));
cl_git_pass(git_index_write(index));

git_index_free(index);

opts.checkout_strategy = GIT_CHECKOUT_FORCE;
cl_git_pass(git_checkout_head(g_repo, &opts));

cl_assert(!git_path_isdir("testrepo/subdir/tracked"));
cl_assert(!git_path_isfile("testrepo/subdir/tracked-file"));
cl_assert(git_path_isfile("testrepo/subdir/untracked-file"));
}

0 comments on commit f2d5ded

Please sign in to comment.