Stephen Butler wrote:
> Quoting Julian Foad <julianfoad_at_btopenworld.com>:
>>> + err = editor->delete_entry(path, item->revision, parent_baton,
>>> pool);
>>> + if (err)
>>> + {
>>> + if (err->apr_err == SVN_ERR_FS_NOT_FOUND
>>> + || err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND)
>>
>> So there are two errors that definitely mean "out of date", and these
>> are them? I've no reason to doubt it, but nor do I know why. I'd be
>> happy to hear you confirm, "Yes, there are exactly two such errors and
>> those are them, in all four cases that we deal with in this function."
>
> Yes indeed, SVN_ERR_FS_NOT_FOUND is sent by the file or svn RA layers,
> and SVN_ERR_RA_DAV_PATH_NOT_FOUND is sent by the http/https RA layers,
> with the same meaning.
OK.
It sounds to me like all the RA layers ought to be made to return the same
error code for this case. That's a separate issue, though.
>>> + if (cmt_err->apr_err == SVN_ERR_FS_NOT_FOUND
>>> + || cmt_err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND)
>>> + {
>>> + svn_error_clear(cmt_err);
>>> + cmt_err = svn_error_createf(SVN_ERR_WC_NOT_UP_TO_DATE, NULL,
>>> + _("Directory '%s' is out of
>>> date"),
>>> + base_dir);
>>
>> I can see the point of giving an "out of date" error for consistency. I
>> can't see any value in hiding the lower-level errors that lead to this
>> diagnosis. I would expect you to add the "out of date" error to the
>> chain of errors. I tried doing so and your tests still pass. Not a big
>> deal, but do you have an opinion one way or the other?
>>
>> The same question applies to the four cases in commit_util.c above.
>
> I suggest we give the SVN_ERR_WC_NOT_UP_TO_DATE error code in all cases,
> because the not-found state is due to changes in the repo.
I agree we should return an SVN_ERR_WC_NOT_UP_TO_DATE error code in all cases.
I was asking if there is any reason for the "svn_error_clear()" which removes
the underlying error(s) from the chain of errors. Usually we append the
high-level error code to the chain of low-level errors that we receive from a
called function, in order to be able to display more detail about why the error
happened.
So, your patch returns this error chain:
SVN_ERR_WC_NOT_UP_TO_DATE
I am suggesting it should instead return either this error chain:
SVN_ERR_WC_NOT_UP_TO_DATE,
SVN_ERR_FS_NOT_FOUND
or this error chain:
SVN_ERR_WC_NOT_UP_TO_DATE,
SVN_ERR_RA_DAV_PATH_NOT_FOUND
by executing "cmt_err = svn_error_createf(SVN_ERR_WC_NOT_UP_TO_DATE, cmt_err,
...)".
Regards,
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-06 11:25:02 CET