Daniel Shahaf <d...@daniel.shahaf.name> writes:
> 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.
In 1.6 for a non-existent wc-entry the API returns
00150000 SVN_ERR_ENTRY_NOT_FOUND
With 1.7
00155010 SVN_ERR_WC_PATH_NOT_FOUND
>
> (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.
>
We are actually consuming the error here to print it and proceed with
the other targets.
Thanks and Regards
Noorul
>> }
>> - 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);
