After a little discussion with Daniel on IRC I've decided to use
'compatible-version' approach. Setting 'compatible-version' to 1.0.0 should
force to fail creation of new FSFS filesystem, since it first appeared in
Subversion 1.1. This should also be true for FSX (or any other hypothetical
backend of the future). If this is not true for some FS backend, the test
would fail, which makes it future-proof.
The test is skipped for BDB (which is deprecated).
I've attached a new version of my patch (log message remains the same).
Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 1669349)
+++ subversion/include/svn_io.h (working copy)
@@ -1662,6 +1662,22 @@ svn_error_t *
svn_io_remove_dir(const char *path,
apr_pool_t *pool);
+/** Recursively remove contents of directory specified by @a path. @a path is
+ * utf8-encoded. If @a ignore_enoent is @c TRUE, don't fail if the target
+ * directory doesn't exist. Use @a pool for temporary allocations.
+ *
+ * Because recursive delete of a directory tree can be a lengthy operation,
+ * provide @a cancel_func and @a cancel_baton for interruptibility.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_io_remove_dir_contents(const char *path,
+ svn_boolean_t ignore_enoent,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
+ apr_pool_t *pool);
+
/** Read all of the disk entries in directory @a path, a utf8-encoded
* path. Set @a *dirents to a hash mapping dirent names (<tt>char *</tt>) to
* undefined non-NULL values, allocated in @a pool.
Index: subversion/libsvn_repos/repos.c
===================================================================
--- subversion/libsvn_repos/repos.c (revision 1669349)
+++ subversion/libsvn_repos/repos.c (working copy)
@@ -1136,6 +1136,7 @@ svn_repos_create(svn_repos_t **repos_p,
apr_pool_t *scratch_pool = svn_pool_create(result_pool);
const char *root_path;
const char *local_abspath;
+ svn_node_kind_t kind;
/* Allocate a repository object, filling in the format we will create. */
repos = create_svn_repos_t(path, result_pool);
@@ -1167,6 +1168,9 @@ svn_repos_create(svn_repos_t **repos_p,
scratch_pool));
}
+ /* Check if directory exists before creating the repository structure. */
+ SVN_ERR(svn_io_check_path(path, &kind, scratch_pool));
+
/* Create the various files and subdirectories for the repository. */
SVN_ERR_W(create_repos_structure(repos, path, fs_config, scratch_pool),
_("Repository creation failed"));
@@ -1180,16 +1184,28 @@ svn_repos_create(svn_repos_t **repos_p,
{
/* If there was an error making the filesytem, e.g. unknown/supported
* filesystem type. Clean up after ourselves. Yes this is safe because
- * create_repos_structure will fail if the path existed before we started
- * so we can't accidentally remove a directory that previously existed.
- */
+ * create_repos_structure will fail if the top-level directory existed
+ * and was non-empty before we started. */
svn_pool_destroy(scratch_pool); /* Release lock to allow deleting dir */
- return svn_error_trace(
- svn_error_compose_create(
- err,
- svn_io_remove_dir2(path, FALSE, NULL, NULL,
- result_pool)));
+ if (kind == svn_node_dir)
+ {
+ /* Don't remove the top-level repository directory if it existed
+ * before repository creation. */
+ return svn_error_trace(
+ svn_error_compose_create(
+ err,
+ svn_io_remove_dir_contents(path, FALSE, NULL, NULL,
+ result_pool)));
+ }
+ else
+ {
+ return svn_error_trace(
+ svn_error_compose_create(
+ err,
+ svn_io_remove_dir2(path, FALSE, NULL, NULL,
+ result_pool)));
+ }
}
/* This repository is ready. Stamp it with a format number. */
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 1669349)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -2549,14 +2549,13 @@ svn_io_remove_dir(const char *path, apr_pool_t *po
http://subversion.tigris.org/issues/show_bug.cgi?id=3501.
*/
-/* Neither windows nor unix allows us to delete a non-empty
- directory.
-
- This is a function to perform the equivalent of 'rm -rf'. */
-svn_error_t *
-svn_io_remove_dir2(const char *path, svn_boolean_t ignore_enoent,
- svn_cancel_func_t cancel_func, void *cancel_baton,
- apr_pool_t *pool)
+static svn_error_t *
+remove_dir_helper(const char *path,
+ svn_boolean_t ignore_enoent,
+ svn_boolean_t remove_top_dir,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
+ apr_pool_t *pool)
{
svn_error_t *err;
apr_pool_t *subpool;
@@ -2594,8 +2593,8 @@ svn_io_remove_dir(const char *path, apr_pool_t *po
if (dirent->kind == svn_node_dir)
{
/* Don't check for cancellation, the callee will immediately do so */
- SVN_ERR(svn_io_remove_dir2(fullpath, FALSE, cancel_func,
- cancel_baton, subpool));
+ SVN_ERR(remove_dir_helper(fullpath, FALSE, TRUE, cancel_func,
+ cancel_baton, subpool));
}
else
{
@@ -2612,10 +2611,37 @@ svn_io_remove_dir(const char *path, apr_pool_t *po
svn_pool_destroy(subpool);
- return svn_io_dir_remove_nonrecursive(path, pool);
+ if (remove_top_dir)
+ SVN_ERR(svn_io_dir_remove_nonrecursive(path, pool));
+
+ return SVN_NO_ERROR;
}
+/* Neither windows nor unix allows us to delete a non-empty
+ directory.
+
+ This is a function to perform the equivalent of 'rm -rf'. */
svn_error_t *
+svn_io_remove_dir2(const char *path, svn_boolean_t ignore_enoent,
+ svn_cancel_func_t cancel_func, void *cancel_baton,
+ apr_pool_t *pool)
+{
+ return svn_error_trace(remove_dir_helper(path, ignore_enoent, TRUE,
+ cancel_func, cancel_baton, pool));
+}
+
+svn_error_t *
+svn_io_remove_dir_contents(const char *path,
+ svn_boolean_t ignore_enoent,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
+ apr_pool_t *pool)
+{
+ return svn_error_trace(remove_dir_helper(path, ignore_enoent, FALSE,
+ cancel_func, cancel_baton, pool));
+}
+
+svn_error_t *
svn_io_get_dir_filenames(apr_hash_t **dirents,
const char *path,
apr_pool_t *pool)
Index: subversion/tests/libsvn_repos/repos-test.c
===================================================================
--- subversion/tests/libsvn_repos/repos-test.c (revision 1669349)
+++ subversion/tests/libsvn_repos/repos-test.c (working copy)
@@ -3612,6 +3612,45 @@ deprecated_access_context_api(const svn_test_opts_
return SVN_NO_ERROR;
}
+static svn_error_t *
+fail_to_create_repos_over_empty_dir(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ const char *test_dir = "test-fail-to-create-repos-over-empty-dir";
+ svn_repos_t *repos;
+ apr_hash_t *fs_config;
+ svn_boolean_t empty;
+
+ if (strcmp(opts->fs_type, SVN_FS_TYPE_BDB) == 0)
+ return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL, NULL);
+
+ /* Remove the test directory from previous runs. */
+ SVN_ERR(svn_io_remove_dir2(test_dir, TRUE, NULL, NULL, pool));
+
+ /* Create an empty directory. */
+ SVN_ERR(svn_io_dir_make(test_dir, APR_OS_DEFAULT, pool));
+ svn_test_add_dir_cleanup(test_dir);
+
+ /* Try to create repository over an existing empty directory. We're forcing
+ svn_repos_create() to fail by specifying an invalid compatible version
(àny
+ FS backends (except BDB) is not compatible with Subversion prior to 1.1.).
+ */
+ fs_config = apr_hash_make(pool);
+ svn_hash_sets(fs_config, SVN_FS_CONFIG_FS_TYPE, opts->fs_type);
+ svn_hash_sets(fs_config, SVN_FS_CONFIG_COMPATIBLE_VERSION, "1.0.0");
+
+ SVN_TEST_ASSERT_ANY_ERROR(svn_repos_create(&repos, test_dir, NULL, NULL,
+ NULL, fs_config, pool));
+
+ /* The svn_io_dir_empty() would fail for non-existing directory. */
+ SVN_ERR(svn_io_dir_empty(&empty, test_dir, pool));
+
+ /* Directory should be empty. */
+ SVN_TEST_ASSERT(empty);
+
+ return SVN_NO_ERROR;
+}
+
/* The test table. */
static int max_threads = 4;
@@ -3667,6 +3706,8 @@ static struct svn_test_descriptor_t test_funcs[] =
"test test_repos_fs_type"),
SVN_TEST_OPTS_PASS(deprecated_access_context_api,
"test deprecated access context api"),
+ SVN_TEST_OPTS_PASS(fail_to_create_repos_over_empty_dir,
+ "test repository creation over existing empty dir"),
SVN_TEST_NULL
};
