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

Re: Confusing error: "Cannot replace a directory from within"

From: Karl Fogel <kfogel_at_red-bean.com>
Date: 2007-05-10 04:52:55 CEST

"David Glasser" <glasser@mit.edu> writes:
> Karl, do you think the following patch is fixing the problem at the
> right level? (Also, have a better suggestion for the error message?)
>
> [[[
> Provide a less cryptic error message when you try to "svn switch"
> to an URL that doesn't exist.
>
> * subversion/libsvn_client/switch.c
> (svn_client__switch_internal): Check to make sure that switch_url is actually
> a directory or file before going too far.
> ]]]

Hmm, how about just "Make sure destination exists before proceeding."?
(The problem is more about existence than type.)

> === subversion/libsvn_client/switch.c
> ==================================================================
> --- subversion/libsvn_client/switch.c (revision 117906)
> +++ subversion/libsvn_client/switch.c (local)
> @@ -67,9 +67,10 @@
> const svn_ra_reporter3_t *reporter;
> void *report_baton;
> const svn_wc_entry_t *entry;
> - const char *URL, *anchor, *target;
> + const char *URL, *anchor, *target, *source_root;
> svn_ra_session_t *ra_session;
> svn_revnum_t revnum;
> + svn_node_kind_t switch_url_kind;
> svn_error_t *err = SVN_NO_ERROR;
> svn_wc_adm_access_t *adm_access, *dir_access;
> const char *diff3_cmd;
> @@ -125,7 +126,32 @@
> ctx, pool));
> SVN_ERR(svn_client__get_revision_number
> (&revnum, ra_session, revision, path, pool));
> + SVN_ERR(svn_ra_get_repos_root(ra_session, &source_root, pool));
> +
> + /* Disallow a switch operation to change the repository root of the
> target. */
> + if (! svn_path_is_ancestor(source_root, switch_url))
> + return svn_error_createf
> + (SVN_ERR_WC_INVALID_SWITCH, NULL,
> + _("'%s'\n"
> + "is not the same repository as\n"
> + "'%s'"), switch_url, source_root);

[Odd line-wrapping from your mailer there, but no big deal.]

So, just to make sure I understand correctly: here you're introducing
a sanity check (make sure they're the same repository) that's not
really related to the bug reported, right? Not objecting, just making
sure I understand -- and the log message might want to mention it.

When someone wants to switch from, say, "svn+ssh://" to "svn://",
should they use 'switch --relocate'? If not, and the recommended way
is plain 'svn switch', then I think this check would block them...

> + /* Check to make sure that the switch target actually exists. */
> + SVN_ERR(svn_ra_reparent(ra_session, source_root, pool));
> + SVN_ERR(svn_ra_check_path(ra_session,
> + svn_path_is_child(source_root, switch_url, pool),
> + revnum,
> + &switch_url_kind,
> + pool));
> +
> + if (switch_url_kind == svn_node_none)
> + return svn_error_createf
> + (SVN_ERR_WC_INVALID_SWITCH, NULL,
> + _("No such path '%s'"), switch_url);
> +
> + SVN_ERR(svn_ra_reparent(ra_session, URL, pool));

Well, we know it's a switch, so how about

   return svn_error_createf
     (SVN_ERR_WC_INVALID_SWITCH, NULL,
      _("Destination does not exist: '%s'"), switch_url);

?

I saw your pastebin of the same patch. I'll apply it here and test
over http:// while you're running the svn:// tests.

Fast work!

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu May 10 04:53:07 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.