"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