Merge pull request #1378 from jamill/clone_no_delete Clone should not clean up directories it did not create
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155
diff --git a/src/clone.c b/src/clone.c
index 409a77f..0bbccd4 100644
--- a/src/clone.c
+++ b/src/clone.c
@@ -429,6 +429,7 @@ int git_clone(
int retcode = GIT_ERROR;
git_repository *repo = NULL;
git_clone_options normOptions;
+ int remove_directory_on_failure = 0;
assert(out && url && local_path);
@@ -439,11 +440,19 @@ int git_clone(
return GIT_ERROR;
}
+ /* Only remove the directory on failure if we create it */
+ remove_directory_on_failure = !git_path_exists(local_path);
+
if (!(retcode = git_repository_init(&repo, local_path, normOptions.bare))) {
if ((retcode = setup_remotes_and_fetch(repo, url, &normOptions)) < 0) {
/* Failed to fetch; clean up */
git_repository_free(repo);
- git_futils_rmdir_r(local_path, NULL, GIT_RMDIR_REMOVE_FILES);
+
+ if (remove_directory_on_failure)
+ git_futils_rmdir_r(local_path, NULL, GIT_RMDIR_REMOVE_FILES);
+ else
+ git_futils_cleanupdir_r(local_path);
+
} else {
*out = repo;
retcode = 0;
diff --git a/src/fileops.c b/src/fileops.c
index 3531e75..6429e55 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -519,6 +519,41 @@ int git_futils_rmdir_r(
return error;
}
+int git_futils_cleanupdir_r(const char *path)
+{
+ int error;
+ git_buf fullpath = GIT_BUF_INIT;
+ futils__rmdir_data data;
+
+ if ((error = git_buf_put(&fullpath, path, strlen(path)) < 0))
+ goto clean_up;
+
+ data.base = "";
+ data.baselen = 0;
+ data.flags = GIT_RMDIR_REMOVE_FILES;
+ data.error = 0;
+
+ if (!git_path_exists(path)) {
+ giterr_set(GITERR_OS, "Path does not exist: %s" , path);
+ error = GIT_ERROR;
+ goto clean_up;
+ }
+
+ if (!git_path_isdir(path)) {
+ giterr_set(GITERR_OS, "Path is not a directory: %s" , path);
+ error = GIT_ERROR;
+ goto clean_up;
+ }
+
+ error = git_path_direach(&fullpath, futils__rmdir_recurs_foreach, &data);
+ if (error == GIT_EUSER)
+ error = data.error;
+
+clean_up:
+ git_buf_free(&fullpath);
+ return error;
+}
+
int git_futils_find_system_file(git_buf *path, const char *filename)
{
#ifdef GIT_WIN32
diff --git a/src/fileops.h b/src/fileops.h
index 5495b12..f01f227 100644
--- a/src/fileops.h
+++ b/src/fileops.h
@@ -128,7 +128,7 @@ typedef enum {
/**
* Remove path and any files and directories beneath it.
*
- * @param path Path to to top level directory to process.
+ * @param path Path to the top level directory to process.
* @param base Root for relative path.
* @param flags Combination of git_futils_rmdir_flags values
* @return 0 on success; -1 on error.
@@ -136,6 +136,14 @@ typedef enum {
extern int git_futils_rmdir_r(const char *path, const char *base, uint32_t flags);
/**
+ * Remove all files and directories beneath the specified path.
+ *
+ * @param path Path to the top level directory to process.
+ * @return 0 on success; -1 on error.
+ */
+extern int git_futils_cleanupdir_r(const char *path);
+
+/**
* Create and open a temporary file with a `_git2_` suffix.
* Writes the filename into path_out.
* @return On success, an open file descriptor, else an error code < 0.
diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c
index 3d327cb..2c4cba4 100644
--- a/tests-clar/clone/nonetwork.c
+++ b/tests-clar/clone/nonetwork.c
@@ -52,6 +52,43 @@ void test_clone_nonetwork__bad_url(void)
cl_assert(!git_path_exists("./foo"));
}
+static int dont_call_me(void *state, git_buf *path)
+{
+ GIT_UNUSED(state);
+ GIT_UNUSED(path);
+ return GIT_ERROR;
+}
+
+void test_clone_nonetwork__do_not_clean_existing_directory(void)
+{
+ git_buf path_buf = GIT_BUF_INIT;
+
+ git_buf_put(&path_buf, "./foo", 5);
+
+ /* Clone should not remove the directory if it already exists, but
+ * Should clean up entries it creates. */
+ p_mkdir("./foo", GIT_DIR_MODE);
+ cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options));
+ cl_assert(git_path_exists("./foo"));
+
+ /* Make sure the directory is empty. */
+ cl_git_pass(git_path_direach(&path_buf,
+ dont_call_me,
+ NULL));
+
+ /* Try again with a bare repository. */
+ g_options.bare = true;
+ cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options));
+ cl_assert(git_path_exists("./foo"));
+
+ /* Make sure the directory is empty. */
+ cl_git_pass(git_path_direach(&path_buf,
+ dont_call_me,
+ NULL));
+
+ git_buf_free(&path_buf);
+}
+
void test_clone_nonetwork__local(void)
{
cl_git_pass(git_clone(&g_repo, cl_git_fixture_url("testrepo.git"), "./foo", &g_options));