Merge pull request #1378 from jamill/clone_no_delete

Clone should not clean up directories it did not create

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;

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

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.

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));