Hi, this is version three of my patch regarding checkouts to existing directories. Thanks for the feedback on the previous two patches.
This patch requires my currently in-flight patch
`svn_client_list4: accept `NULL patterns'. If it will not go into
trunk, I'll convert `verify_checkout_target` to use an empty
array instead of passing `NULL`.
The new version fixes an issue where I did not check correctly
whether the target directory is empty (that is I previously used
`svn_path_is_empty`, where I now correctly use
`svn_io_dir_empty`). Furthermore, I now check if the target is a
working copy and, if so, compare UUID and relative paths of the
wc and remote repository. These changes allowed me to drop all
previously required '--force' flags in our test suite, indicating
the changes now match more closely with existing use cases.
I stopped short of adding a new '--force-obstructed-checkout'
flag, which might be used instead of '--force' if the new checks
reject the checkout. After reading '--force's doc string, it
should actually do exactly what the new flag would be doing.
Furthermore, I no hope that it's not required that frequently.
Regards
Patrick
[[[
checkout_cmd: refuse obstructed checkouts
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 target directory is a repository and has the same UUID and
relative path as the repository that is to be checked out
- 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 as well as resuming checkouts.
* 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 flag is specified.
* subversion/tests/cmdline/checkout_tests.py:
(checkout_with_obstructions): Replace old test and now assert
that subversion refuses to
checkout to non-empty dirs.
]]]
--
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..b282248 100644
--- a/subversion/svn/checkout-cmd.c
+++ b/subversion/svn/checkout-cmd.c
@@ -61,6 +61,144 @@
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;
+}
+
+/* Check if the target directory which should be checked out to
+ * is a valid target directory. A target directory is valid if we
+ * are sure that a checkout to the directory will not create any
+ * unexpected situations for the user. This is the case if one of
+ * the following criteria is true:
+ *
+ * - the target directory does not exist
+ * - the target directory is empty
+ * - the target directory is the same repository with the same
+ * relative URL as the one that is to be checked out (e.g. we
+ * resume a checkout)
+ * - the repository that is to be checked out is empty
+ */
+static svn_error_t *
+verify_checkout_target(const char *repo_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;
+ apr_pool_t *scratch_pool;
+ const char *absolute_path;
+ size_t count = 0;
+ svn_wc_status3_t *status;
+ svn_error_t *error;
+ svn_boolean_t empty;
+
+ scratch_pool = svn_pool_create(pool);
+
+ 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. */
+ SVN_ERR(svn_io_dir_empty(&empty, absolute_path, scratch_pool));
+ if (empty)
+ return SVN_NO_ERROR;
+
+ error = svn_wc_status3(&status,
+ ctx->wc_ctx,
+ absolute_path,
+ pool,
+ scratch_pool);
+
+ /* If the remote repository UUID and working copy UUID are the
+ * same and if the relative paths inside the repository match,
+ * we assume that the command is a resumed checkout. */
+ if (error == SVN_NO_ERROR)
+ {
+ const char *repo_relpath, *repo_root, *repo_uuid;
+
+ SVN_ERR(svn_client_get_repos_root(&repo_root,
+ &repo_uuid,
+ repo_url,
+ ctx,
+ pool,
+ scratch_pool));
+
+ repo_relpath = svn_uri_skip_ancestor(repo_root,
+ repo_url,
+ scratch_pool);
+
+ if (repo_relpath
+ && !strcmp(status->repos_uuid, repo_uuid)
+ && !strcmp(status->repos_relpath, repo_relpath))
+ return SVN_NO_ERROR;
+ }
+ else
+ {
+ svn_error_clear(error);
+ }
+
+ SVN_ERR(svn_client_list4(repo_url,
+ peg_revision,
+ revision,
+ NULL,
+ 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);
+
+ svn_pool_clear(scratch_pool);
+
+ return SVN_NO_ERROR;
+}
/* This implements the `svn_opt_subcommand_t' interface. */
svn_error_t *
@@ -165,6 +303,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/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)
#----------------------------------------------------------------------
signature.asc
Description: PGP signature
