>From IRC: <Bert> julianf: We changed error codes in libsvn_wc all over the place. I don't think we see strict error codes as part of the documented behavior, unless it is in the function documentation
<julianf> Bert: OK, but I wonder if we really should accept changes to a libsvn_client response (which this is, I assume). Even if not documented, I wanted somebody else's opinion. Would you mind responding to the thread? (I'm thinking: if this is something that "svn" checks for explicitly, then it's likely that other clients also check for it explicitly.) - Julian On Thu, 2010-12-23 at 17:32 +0530, Noorul Islam K M wrote: > 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);