Noorul Islam K M wrote on Thu, Dec 23, 2010 at 15:53:58 +0530:
>
> Forwarding one of the threads to dev@ with [PATCH] tag so that this gets
> attention of more people. Julian said on IRC,
>
> <julianf> noorul: Now we're into the realm of changing behaviour. If the error
> code returned from libsvn_client is different in 1.7 from what it
> was in 1.6, maybe that indicates a backward-compatibility problem
> with our APIs. I think this needs more review. It is not so simple
> as just changing an error message.
> <julianf> noorul: Please could you post this again as a new thread, with
> subject line starting with "[PATCH]"? I'd like other people to
> review this. Thanks.
>
So, to ask the obvious:
What error codes (err->apr_err) were returned in 1.6.x, and what
error codes are returned now?
Noorul: you can use tools/dev/which-error.py to convert numeric error
codes into symbolic ones.
(more below)
> Pasting here the log once more so that there is some clarity about the
> patch attached.
>
> [[[
>
> Make "svn info" to display the actual error message returned by API when
> one of the targets is a non-existent URL or wc-entry.
>
> * subversion/svn/info-cmd.c
> (svn_cl__info): Display the actual error message returned by
> svn_client_info3() instead of a custom one. Catch error
> SVN_ERR_WC_PATH_NOT_FOUND instead of SVN_ERR_UNVERSIONED_RESOURCE
> and SVN_ERR_ENTRY_NOT_FOUND for non-existent wc-entry.
>
> * subversion/tests/cmdline/basic_tests.py
> (info_nonexisting_file): Modify test case
>
> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
> ]]]
>
>
> Thanks and Regards
> Noorul
>
> Index: subversion/tests/cmdline/basic_tests.py
> ===================================================================
> --- subversion/tests/cmdline/basic_tests.py (revision 1051915)
> +++ subversion/tests/cmdline/basic_tests.py (working copy)
> @@ -2260,7 +2260,8 @@
>
> # Check for the correct error message
> for line in errput:
> - if re.match(".*\(Not a valid URL\).*", line):
> + if re.match(".*" + idonotexist_url + ".*non-existent in revision 1.*",
> + line):
> return
>
> # Else never matched the expected error output, so the test failed.
> Index: subversion/svn/info-cmd.c
> ===================================================================
> --- subversion/svn/info-cmd.c (revision 1051915)
> +++ subversion/svn/info-cmd.c (working copy)
> @@ -566,25 +566,15 @@
> {
> /* If one of the targets is a non-existent URL or wc-entry,
> don't bail out. Just warn and move on to the next target. */
> - if (err->apr_err == SVN_ERR_UNVERSIONED_RESOURCE
> - || err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
> + if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND ||
> + err->apr_err == SVN_ERR_RA_ILLEGAL_URL)
> {
> - SVN_ERR(svn_cmdline_fprintf
> - (stderr, subpool,
> - _("%s: (Not a versioned resource)\n\n"),
> - svn_path_is_url(truepath)
> - ? truepath
> - : svn_dirent_local_style(truepath, pool)));
> + char errbuf[256];
> +
> + SVN_ERR(svn_cmdline_fprintf(
> + stderr, subpool, "svn: %s\n\n",
> + svn_err_best_message(err, errbuf, sizeof(errbuf))));
Why do you have to place the "svn: " prefix here explicitly? Normally
svn_handle_error2() would do that for you. This is a red flag ("is
a wheel being reinvented here?") for me.
> }
> - else if (err->apr_err == SVN_ERR_RA_ILLEGAL_URL)
> - {
> - SVN_ERR(svn_cmdline_fprintf
> - (stderr, subpool,
> - _("%s: (Not a valid URL)\n\n"),
> - svn_path_is_url(truepath)
> - ? truepath
> - : svn_dirent_local_style(truepath, pool)));
> - }
> else
> {
> return svn_error_return(err);
