philip@tigris.org writes:
> Log:
> Prevent checkout into an existing working-copy.
>
> * subversion/libsvn_wc/update_editor.c (svn_wc_get_checkout_editor): Fail
> if destination is already a working copy.
>
> * subversion/tests/clients/cmdline/basic_tests.py (basic_checkout): Test
> that checkout into a working copy fails.
Hmmm. Interesting fix. Shouldn't it be farther down in the code,
though -- in the editor itself, effectively?
svn_wc_make_checkout_editor() just returns an editor. No checkout
takes place until someone starts driving that editor. It seems
improper to error because the target is a wc directory *at the time
the editor is created*. It should only be an error if the target is a
wc at the time the checkout is actually attempted (and, ideally, only
if it's a wc from a different URL than is being checked out, but
that's an optimization).
Let's see... open_root() calls prep_directory(), which looks like
this:
static svn_error_t *
prep_directory (svn_stringbuf_t *path,
svn_stringbuf_t *ancestor_url,
svn_revnum_t ancestor_revision,
svn_boolean_t force,
apr_pool_t *pool)
{
/* kff todo: how about a sanity check that it's not a dir of the
same name from a different repository or something?
Well, that will be later on down the line... */
if (force) /* Make sure the directory exists. */
SVN_ERR (svn_wc__ensure_directory (path, pool));
/* Make sure it's the right working copy, either by creating it so,
or by checking that it is so already. */
return svn_wc__ensure_adm (path, ancestor_url, ancestor_revision,
pool);
}
I think that `todo' comment is a red herring. We could implement the
check directly here, but I think doing it in svn_wc__ensure_adm()
makes more sense. In fact, svn_wc__ensure_adm() is supposed to be
doing it, but (as its doc string says) it's currently not. It would
really depend on check_adm_exists(), doing it correctly, anyway, and
there's the root of the problem:
/* Set *EXISTS to non-zero iff there's an adm area for PATH, and it
* matches URL and REVISION.
* ### todo: The url/rev match is not currently implemented.
*
* If an error occurs, just return the error and don't touch *EXISTS.
*/
static svn_error_t *
check_adm_exists (int *exists,
svn_stringbuf_t *path,
svn_stringbuf_t *url,
svn_revnum_t revision,
apr_pool_t *pool)
{
[...]
}
If that url match were working right, this problem and maybe some
others would be solved.
So, I think the "right" way to fix this is to make check_adm_exists()
finally implement that check, and the behavior will propagate up.
Do you agree?
-Karl
> Modified: trunk/subversion/libsvn_wc/update_editor.c
> ==============================================================================
> --- trunk/subversion/libsvn_wc/update_editor.c (original)
> +++ trunk/subversion/libsvn_wc/update_editor.c Thu May 2 18:17:44 2002
> @@ -1780,6 +1780,21 @@
> void **edit_baton,
> apr_pool_t *pool)
> {
> + svn_node_kind_t kind;
> +
> + /* Checkout into an existing working-copy isn't supported. */
> + SVN_ERR (svn_io_check_path (dest->data, &kind, pool));
> + if (kind == svn_node_dir)
> + {
> + svn_boolean_t is_wc;
> + SVN_ERR (svn_wc_check_wc (dest, &is_wc, pool));
> + if (is_wc)
> + return svn_error_createf
> + (SVN_ERR_UNSUPPORTED_FEATURE, 0, NULL, pool,
> + "checkout into '%s' which is already a working copy",
> + dest->data);
> + }
> +
> return make_editor (dest, NULL, target_revision,
> TRUE, ancestor_url,
> FALSE, NULL,
>
> Modified: trunk/subversion/tests/clients/cmdline/basic_tests.py
> ==============================================================================
> --- trunk/subversion/tests/clients/cmdline/basic_tests.py (original)
> +++ trunk/subversion/tests/clients/cmdline/basic_tests.py Thu May 2 18:17:45 2002
> @@ -37,7 +37,19 @@
> def basic_checkout(sbox):
> "basic checkout of a wc"
>
> - return sbox.build()
> + if sbox.build():
> + return 1
> +
> + # second checkout is expected to fail
> + wc_dir = sbox.wc_dir
> + url = svntest.main.current_repo_url
> + stdout_lines, stderr_lines = svntest.main.run_svn (1, 'checkout', url,
> + '-d', wc_dir)
> + for stderr_line in stderr_lines:
> + if re.match('.*already a working copy', stderr_line):
> + return 0
> +
> + return 1
>
> #----------------------------------------------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 3 02:52:10 2002