Re: svn commit: r1767190 - in /subversion/trunk/subversion: include/svn_ra.h libsvn_client/list.c libsvn_ra/ra_loader.c libsvn_ra/ra_loader.h libsvn_ra_local/ra_plugin.c libsvn_ra_serf/serf.c libsvn_ra_svn/client.c

[Stefan Fuhrmann <st...@apache.org>]

On 31.10.2016 02:05, Daniel Shahaf wrote:
> stefan2@apache.org wrote on Sun, Oct 30, 2016 at 22:13:57 -0000:
>> Author: stefan2
>> Date: Sun Oct 30 22:13:57 2016
>> New Revision: 1767190

[snip]

>> +svn_error_t *
>> +svn_ra_list(svn_ra_session_t *session,
>> +            const char *path,
>> +            svn_revnum_t revision,
>> +            apr_array_header_t *patterns,
>> +            svn_depth_t depth,
>> +            apr_uint32_t dirent_fields,
>> +            svn_ra_dirent_receiver_t receiver,
>> +            void *receiver_baton,
>> +            apr_pool_t *scratch_pool)
>> +{
>> +  SVN_ERR_ASSERT(svn_relpath_is_canonical(path));
>> +  if (!session->vtable->list)
>> +    return svn_error_create(SVN_ERR_RA_NOT_IMPLEMENTED, NULL, NULL);
>
> Judging by its error message, SVN_ERR_RA_NOT_IMPLEMENTED is supposed to
> mean "protocol scheme not recognised", not "feature not supported".
>
> I think this should use SVN_ERR_UNSUPPORTED_FEATURE instead, like
> svn_ra__assert_capable_server() does.
>
> (There are a few more instances of this mixup in libsvn_ra_svn/client.c.)

There there are also a few instances in ra_serf where we pass on
SVN_ERR_RA_NOT_IMPLEMENTED upon receiving SVN_ERR_UNSUPPORTED_FEATURE.

But you are right that SVN_ERR_UNSUPPORTED_FEATURE is the better
choice here, so I changed it in r1768311.

Thanks for the reviews!

-- Stefan^2.