Hi, finally got around to update my patch regarding checkouts to existing directories. The semantics have been changed to accept checkouts iff
- the target directory does not exist - the target directory is empty - the repository to check out is empty - the --force flag is given This should treat all cases pointed out by Ivan. There were quite a lot of test cases that needed adjustment following this change. I've mostly added the '--force' flag, but also replaced a test which didn't make sense anymore with a new one (that is 'co to a dir without --force with obstructions'). The new test case simply check is we reject checking out a non-empty repo to a non-empty dir. All but two unrelated tests pass with this. [[[ Reject checkouts which would create tree conflicts. When a new checkout is done where the target dirctory already exists, subversion will usually create a lot of tree conflicts which are intimidating, especially to new users. This behavior stems from release 1.8, where we started accepting checkouts to existing directories without any safety-checks. This patch changes semantics in that it introduces new safety checks so that the user does not accidentally shoots himself into the foot. We now only allow checkouts if one of the following conditions holds true: - the target directory does not exist - the target directory is empty - the repository to check out is empty - the --force flag is given The main use case solved by the above conditions is for converting existing directories into a repository when the repository is newly created. * subversion/svn/checkout-cmd.c: (listing_cb): New callback to check whether the remote repository is empty. (verify_checkout_target): New function to check whether the target checkout directory is a valid one. (svn_cl__checkout): Now calls `verify_checkout_target` if no --force is specified. * subversion/tests/cmdline/autoprop_tests.py: (autoprops_test, inheritable_autoprops_test): use 'co --force'. * subversion/tests/cmdline/basic_tests.py: (basic_checkout, basic_auth_test): use 'co --force'. * subversion/tests/cmdline/checkout_tests.py: (checkout_with_obstructions): Replace old test and now assert that subversion refuses to checkout to non-empty dirs. * subversion/tests/cmdline/svntest/actions.py: (run_and_verify_checkout): use 'co --force'. * subversion/tests/cmdline/tree_conflict_tests.py: (ensure_tree_conflict): use 'co --force'. ]]] Regards Patrick -- Patrick Steinhardt, Entwickler elego Software Solutions GmbH, http://www.elego.de Gebäude 12 (BIG), Gustav-Meyer-Allee 25, 13355 Berlin, Germany Sitz der Gesellschaft: Berlin, USt-IdNr.: DE 163214194 Handelsregister: Amtsgericht Charlottenburg HRB 77719 Geschäftsführer: Olaf Wagner
diff --git a/subversion/svn/checkout-cmd.c b/subversion/svn/checkout-cmd.c
index 56fd02b..b78ee74 100644
--- a/subversion/svn/checkout-cmd.c
+++ b/subversion/svn/checkout-cmd.c
@@ -61,6 +61,87 @@
Is this the same as CVS? Does it matter if it is not?
*/
+static svn_error_t *
+listing_cb(void *baton,
+ const char *path,
+ const svn_dirent_t *dirent,
+ const svn_lock_t *lock,
+ const char *abs_path,
+ const char *external_parent_url,
+ const char *external_target,
+ apr_pool_t *scratch_pool)
+{
+ size_t *count = (size_t *) baton;
+
+ (*count)++;
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+verify_checkout_target(const char *path_or_url,
+ const char *target_dir,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ svn_node_kind_t kind;
+ const char *absolute_path;
+ size_t count = 0;
+
+ SVN_ERR(svn_dirent_get_absolute(&absolute_path,
+ target_dir,
+ pool));
+
+ SVN_ERR(svn_io_check_path(absolute_path,
+ &kind,
+ pool));
+
+ /* If the directory does not exist yet, it will be created
+ * anew and is thus considered valid. */
+ if (kind == svn_node_none)
+ return SVN_NO_ERROR;
+
+ /* Checking out to a file or symlink cannot work. */
+ if (kind != svn_node_dir)
+ return svn_error_createf(
+ SVN_ERR_ILLEGAL_TARGET,
+ NULL,
+ _("Rejecting checkout to existing %s '%s'"),
+ svn_node_kind_to_word(kind),
+ absolute_path);
+
+ /* Checking out to a non-empty directory will create
+ * tree-conflicts, which is usually not what the user wants. */
+ if (svn_path_is_empty(absolute_path))
+ return SVN_NO_ERROR;
+
+ SVN_ERR(svn_client_list3(path_or_url,
+ peg_revision,
+ revision,
+ svn_depth_immediates,
+ 0,
+ FALSE,
+ FALSE,
+ listing_cb,
+ &count,
+ ctx,
+ pool));
+
+ /* If the remote respotiory has more than one file (that is, it
+ * is not an empty root directory only), we refuse to check out
+ * into a non-empty target directory. Otherwise Subversion
+ * would create tree conflicts. */
+ if (count > 1)
+ return svn_error_createf(
+ SVN_ERR_ILLEGAL_TARGET,
+ NULL,
+ _("Rejecting checkout of non-empty repository into non-empty directory '%s'"),
+ absolute_path);
+
+ return SVN_NO_ERROR;
+}
/* This implements the `svn_opt_subcommand_t' interface. */
svn_error_t *
@@ -165,6 +246,16 @@ svn_cl__checkout(apr_getopt_t *os,
revision.kind = svn_opt_revision_head;
}
+ if (! opt_state->force)
+ {
+ SVN_ERR(verify_checkout_target(true_url,
+ target_dir,
+ &peg_revision,
+ &revision,
+ ctx,
+ subpool));
+ }
+
SVN_ERR(svn_client_checkout3
(NULL, true_url, target_dir,
&peg_revision,
diff --git a/subversion/tests/cmdline/autoprop_tests.py b/subversion/tests/cmdline/autoprop_tests.py
index 0987cf1..b5c538b 100755
--- a/subversion/tests/cmdline/autoprop_tests.py
+++ b/subversion/tests/cmdline/autoprop_tests.py
@@ -169,7 +169,7 @@ def autoprops_test(sbox, cmd, cfgenable, clienable, subdir):
# do an svn co if needed
if cmd == 'import':
svntest.main.run_svn(None, 'checkout', repos_url, files_wc_dir,
- '--config-dir', config_dir)
+ '--force', '--config-dir', config_dir)
# check the properties
if enable_flag:
@@ -514,7 +514,7 @@ def inheritable_autoprops_test(sbox, cmd, cfgenable, clienable, subdir,
# do an svn co if needed
if cmd == 'import':
svntest.main.run_svn(None, 'checkout', repos_url + '/A', files_wc_dir,
- '--config-dir', config_dir)
+ '--force', '--config-dir', config_dir)
check_inheritable_autoprops(sbox, auto_props_cfg_enabled,
inheritable_auto_props_enabled)
diff --git a/subversion/tests/cmdline/basic_tests.py b/subversion/tests/cmdline/basic_tests.py
index 0ad7316..b7d7078 100755
--- a/subversion/tests/cmdline/basic_tests.py
+++ b/subversion/tests/cmdline/basic_tests.py
@@ -88,7 +88,7 @@ def basic_checkout(sbox):
url = sbox.repo_url
svntest.actions.run_and_verify_svn(None, [],
- 'co', url,
+ 'co', '--force', url,
wc_dir)
# lambda is restored, modifications remain, deletes remain scheduled
@@ -2546,22 +2546,22 @@ def basic_auth_test(sbox):
# Checkout with jrandom
exit_code, output, errput = svntest.main.run_command(
svntest.main.svn_binary, None, True, 'co', sbox.repo_url, wc_dir,
- '--username', 'jrandom', '--password', 'rayjandom',
+ '--force', '--username', 'jrandom', '--password', 'rayjandom',
'--config-dir', config_dir)
exit_code, output, errput = svntest.main.run_command(
svntest.main.svn_binary, None, True, 'co', sbox.repo_url, wc_dir,
- '--username', 'jrandom', '--non-interactive', '--config-dir', config_dir)
+ '--force', '--username', 'jrandom', '--non-interactive', '--config-dir', config_dir)
# Checkout with jconstant
exit_code, output, errput = svntest.main.run_command(
svntest.main.svn_binary, None, True, 'co', sbox.repo_url, wc_dir,
- '--username', 'jconstant', '--password', 'rayjandom',
+ '--force', '--username', 'jconstant', '--password', 'rayjandom',
'--config-dir', config_dir)
exit_code, output, errput = svntest.main.run_command(
svntest.main.svn_binary, None, True, 'co', sbox.repo_url, wc_dir,
- '--username', 'jconstant', '--non-interactive',
+ '--force', '--username', 'jconstant', '--non-interactive',
'--config-dir', config_dir)
# Checkout with jrandom which should fail since we do not provide
@@ -2569,7 +2569,7 @@ def basic_auth_test(sbox):
expected_err = ["authorization failed: Could not authenticate to server:"]
exit_code, output, errput = svntest.main.run_command(
svntest.main.svn_binary, expected_err, True, 'co', sbox.repo_url, wc_dir,
- '--username', 'jrandom', '--non-interactive', '--config-dir', config_dir)
+ '--force', '--username', 'jrandom', '--non-interactive', '--config-dir', config_dir)
def basic_add_svn_format_file(sbox):
'test add --parents .svn/format'
diff --git a/subversion/tests/cmdline/checkout_tests.py b/subversion/tests/cmdline/checkout_tests.py
index 49165e7..93415b9 100755
--- a/subversion/tests/cmdline/checkout_tests.py
+++ b/subversion/tests/cmdline/checkout_tests.py
@@ -158,94 +158,20 @@ def make_local_tree(sbox, mod_files=False, add_unversioned=False):
#----------------------------------------------------------------------
def checkout_with_obstructions(sbox):
- """co with obstructions conflicts without --force"""
+ """obstructed co without --force"""
make_local_tree(sbox, False, False)
- #svntest.factory.make(sbox,
- # """# Checkout with unversioned obstructions lying around.
- # svn co url wc_dir
- # svn status""")
- #svntest.factory.make(sbox,
- # """# Now see to it that we can recover from the obstructions.
- # rm -rf A iota
- # svn up""")
- #exit(0)
-
wc_dir = sbox.wc_dir
url = sbox.repo_url
# Checkout with unversioned obstructions causes tree conflicts.
# svn co url wc_dir
- expected_output = svntest.wc.State(wc_dir, {
- 'iota' : Item(status=' ', treeconflict='C'),
- 'A' : Item(status=' ', treeconflict='C'),
- # And the updates below the tree conflict
- 'A/D' : Item(status=' ', treeconflict='A'),
- 'A/D/gamma' : Item(status=' ', treeconflict='A'),
- 'A/D/G' : Item(status=' ', treeconflict='A'),
- 'A/D/G/rho' : Item(status=' ', treeconflict='A'),
- 'A/D/G/pi' : Item(status=' ', treeconflict='A'),
- 'A/D/G/tau' : Item(status=' ', treeconflict='A'),
- 'A/D/H' : Item(status=' ', treeconflict='A'),
- 'A/D/H/chi' : Item(status=' ', treeconflict='A'),
- 'A/D/H/omega' : Item(status=' ', treeconflict='A'),
- 'A/D/H/psi' : Item(status=' ', treeconflict='A'),
- 'A/B' : Item(status=' ', treeconflict='A'),
- 'A/B/E' : Item(status=' ', treeconflict='A'),
- 'A/B/E/beta' : Item(status=' ', treeconflict='A'),
- 'A/B/E/alpha' : Item(status=' ', treeconflict='A'),
- 'A/B/F' : Item(status=' ', treeconflict='A'),
- 'A/B/lambda' : Item(status=' ', treeconflict='A'),
- 'A/C' : Item(status=' ', treeconflict='A'),
- 'A/mu' : Item(status=' ', treeconflict='A'),
- })
+ expected_err = ".*Rejecting checkout of non-empty repository into non-empty directory.*"
expected_disk = svntest.main.greek_state.copy()
- expected_disk.remove('A/B', 'A/B/E', 'A/B/E/beta', 'A/B/E/alpha', 'A/B/F',
- 'A/B/lambda', 'A/D', 'A/D/G', 'A/D/G/rho', 'A/D/G/pi', 'A/D/G/tau',
- 'A/D/H', 'A/D/H/psi', 'A/D/H/omega', 'A/D/H/chi', 'A/D/gamma', 'A/C')
-
- actions.run_and_verify_checkout(url, wc_dir, expected_output,
- expected_disk)
-
- # svn status
- expected_status = actions.get_virginal_state(wc_dir, 1)
- # A and iota are tree conflicted and obstructed
- expected_status.tweak('A', 'iota', status='D ', wc_rev=1,
- treeconflict='C')
-
- expected_status.tweak('A/D', 'A/D/G', 'A/D/G/rho', 'A/D/G/pi', 'A/D/G/tau',
- 'A/D/H', 'A/D/H/chi', 'A/D/H/omega', 'A/D/H/psi', 'A/D/gamma', 'A/B',
- 'A/B/E', 'A/B/E/beta', 'A/B/E/alpha', 'A/B/F', 'A/B/lambda', 'A/C',
- status='D ')
- # A/mu exists on disk, but is deleted
- expected_status.tweak('A/mu', status='D ')
-
- actions.run_and_verify_unquiet_status(wc_dir, expected_status)
-
-
- # Now see to it that we can recover from the obstructions.
- # rm -rf A iota
- svntest.main.safe_rmtree( os.path.join(wc_dir, 'A') )
- os.remove( os.path.join(wc_dir, 'iota') )
-
-
- svntest.main.run_svn(None, 'revert', '-R', os.path.join(wc_dir, 'A'),
- os.path.join(wc_dir, 'iota'))
-
- # svn up
- expected_output = svntest.wc.State(wc_dir, {
- })
-
- expected_disk = svntest.main.greek_state.copy()
-
- expected_status = actions.get_virginal_state(wc_dir, 1)
-
- actions.run_and_verify_update(wc_dir, expected_output, expected_disk,
- expected_status,)
-
+ actions.run_and_verify_svn(None, expected_err, "co", url, wc_dir)
#----------------------------------------------------------------------
diff --git a/subversion/tests/cmdline/svntest/actions.py b/subversion/tests/cmdline/svntest/actions.py
index ef6e2dd..fb91e34 100644
--- a/subversion/tests/cmdline/svntest/actions.py
+++ b/subversion/tests/cmdline/svntest/actions.py
@@ -548,7 +548,7 @@ def run_and_verify_checkout(URL, wc_dir_name, output_tree, disk_tree,
### repositories with no auth/auth requirements
exit_code, output, errput = run_and_verify_svn(None, expected_stderr,
'co', URL, wc_dir_name,
- *args)
+ '--force', *args)
actual = tree.build_tree_from_checkout(output)
# Verify actual output against expected output.
diff --git a/subversion/tests/cmdline/tree_conflict_tests.py b/subversion/tests/cmdline/tree_conflict_tests.py
index 84a78fa..324d1f6 100755
--- a/subversion/tests/cmdline/tree_conflict_tests.py
+++ b/subversion/tests/cmdline/tree_conflict_tests.py
@@ -418,7 +418,7 @@ def ensure_tree_conflict(sbox, operation,
head_rev += 1
target_start_rev = head_rev
- main.run_svn(None, 'checkout', '-r', str(target_start_rev), sbox.repo_url,
+ main.run_svn(None, 'checkout', '--force', '-r', str(target_start_rev), sbox.repo_url,
wc_dir)
saved_cwd = os.getcwd()
signature.asc
Description: PGP signature
