From 78b8f0392cebb0d9e2c47b36f8d97ec53c1f0346 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Feb 2017 14:00:38 +0100 Subject: [PATCH 1/4] tests: fix indentation in checkout::head::with_index_only_tree --- tests/checkout/head.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/checkout/head.c b/tests/checkout/head.c index 07cc1d209..018ee3545 100644 --- a/tests/checkout/head.c +++ b/tests/checkout/head.c @@ -38,7 +38,7 @@ void test_checkout_head__with_index_only_tree(void) cl_git_pass(git_repository_index(&index, g_repo)); p_mkdir("testrepo/newdir", 0777); - cl_git_mkfile("testrepo/newdir/newfile.txt", "new file\n"); + cl_git_mkfile("testrepo/newdir/newfile.txt", "new file\n"); cl_git_pass(git_index_add_bypath(index, "newdir/newfile.txt")); cl_git_pass(git_index_write(index)); From 0ef405b3c8c0213282ef15c48462f16d03442bac Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Feb 2017 14:05:10 +0100 Subject: [PATCH 2/4] checkout: do not delete directories with untracked entries 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. This commit adds two tests to verify behavior. --- tests/checkout/head.c | 76 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/checkout/head.c b/tests/checkout/head.c index 018ee3545..ded86df33 100644 --- a/tests/checkout/head.c +++ b/tests/checkout/head.c @@ -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")); +} From 83989d70ecd39324a9c262e75ce27b6a33cf78fc Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 8 Jun 2017 22:23:53 +0100 Subject: [PATCH 3/4] checkout: cope with untracked files in directory deletion When deleting a directory during checkout, do not simply delete the directory, since there may be untracked files. Instead, go into the iterator and examine each file. In the original code (the code with the faulty assumption), we look to see if there's an index entry beneath the directory that we want to remove. Eg, it looks to see if we have a workdir entry foo and an index entry foo/bar.txt. If this is not the case, then the working directory must have precious files in that directory. This part is okay. The part that's not okay is if there is an index entry foo/bar.txt. It just blows away the whole damned directory. That's not cool. Instead, by simply pushing the directory itself onto the stack and iterating each entry, we will deal with the files one by one - whether they're in the index (and can be force removed) or not (and are precious). The original code was a bad optimization, assuming that we didn't need to git_iterator_advance_into if there was any index entry in the folder. That's wrong - we could have optimized this iff all folder entries are in the index. Instead, we need to simply dig into the directory and analyze its entries. --- src/checkout.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 9d1eed59f..25018d291 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -370,10 +370,8 @@ 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) { - notify = GIT_CHECKOUT_NOTIFY_DIRTY; - remove = ((data->strategy & GIT_CHECKOUT_FORCE) != 0); - } + if (e != NULL && data->diff->pfxcomp(e->path, wd->path) == 0) + return git_iterator_advance_into(wditem, workdir); } } From 4a0df574114f331a2428278c5f72aae7a49fa214 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sat, 10 Jun 2017 18:46:35 +0100 Subject: [PATCH 4/4] git_futils_rmdir: only allow `EBUSY` when asked Only ignore `EBUSY` from `rmdir` when the `GIT_RMDIR_SKIP_NONEMPTY` bit is set. --- src/fileops.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/fileops.c b/src/fileops.c index f9552a5f8..2f86ba1e4 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -766,6 +766,9 @@ static int futils__rmdir_empty_parent(void *opaque, const char *path) if (en == ENOENT || en == ENOTDIR) { /* do nothing */ + } else if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) == 0 && + en == EBUSY) { + error = git_path_set_error(errno, path, "rmdir"); } else if (en == ENOTEMPTY || en == EEXIST || en == EBUSY) { error = GIT_ITEROVER; } else {