[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: rev 1865 - trunk/subversion/libsvn_wc trunk/subversion/tests/clients/cmdline

From: Karl Fogel <kfogel_at_newton.ch.collab.net>
Date: 2002-05-03 02:53:10 CEST

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

This is an archived mail posted to the Subversion Dev mailing list.