You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ivan Zhakov <iv...@visualsvn.com> on 2015/06/26 15:15:42 UTC
Re: svn commit: r1687769 - in /subversion/trunk/subversion:
bindings/javahl/native/ bindings/javahl/src/org/apache/subversion/javahl/
include/ libsvn_repos/ svnadmin/ tests/cmdline/ tests/libsvn_fs_fs/
On 26 June 2015 at 15:44, <ko...@apache.org> wrote:
>
> Author: kotkov
> Date: Fri Jun 26 12:44:26 2015
> New Revision: 1687769
>
> URL: http://svn.apache.org/r1687769
> Log:
> Reimplement svn_repos_verify_fs3() to support an arbitrary callback that
> receives the information about an encountered problem and lets the caller
> decide on what happens next. This supersedes the keep_going argument for
> this API. A callback is optional; the behavior of this API if the callback
> is not provided is equivalent to how svn_repos_verify_fs2() deals with
> encountered errors. This allows seamless migration to the new API, if the
> callback is not necessary. The idea is partly taken from how our existing
> svn_fs_lock_many() API works with a svn_fs_lock_callback_t and passes error
> information to the caller.
>
[...]
>
> Modified: subversion/trunk/subversion/include/svn_repos.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_repos.h?rev=1687769&r1=1687768&r2=1687769&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_repos.h (original)
> +++ subversion/trunk/subversion/include/svn_repos.h Fri Jun 26 12:44:26 2015
[..]
> @@ -2825,6 +2817,22 @@ enum svn_repos_load_uuid
> svn_repos_load_uuid_force
> };
>
> +/** Callback type for use with svn_repos_verify_fs3(). @a revision
> + * and @a verify_err are the details of a single verification failure
> + * that occurred during the svn_repos_verify_fs3() call. @a baton is
> + * the same baton given to svn_repos_verify_fs3(). @a scratch_pool is
> + * provided for the convenience of the implementor, who should not
> + * expect it to live longer than a single callback call.
> + *
Who is responsible for clearing verify_err: caller or callback implementor?
> + * @see svn_repos_verify_fs3
> + *
> + * @since New in 1.9.
> + */
> +typedef svn_error_t *(*svn_repos_verify_callback_t)(void *baton,
> + svn_revnum_t revision,
> + svn_error_t *verify_err,
> + apr_pool_t *scratch_pool);
> +
--
Ivan Zhakov
Re: svn commit: r1687769 - in /subversion/trunk/subversion:
bindings/javahl/native/ bindings/javahl/src/org/apache/subversion/javahl/
include/ libsvn_repos/ svnadmin/ tests/cmdline/ tests/libsvn_fs_fs/
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 26 June 2015 at 17:44, Evgeny Kotkov <ev...@visualsvn.com> wrote:
> Ivan Zhakov <iv...@visualsvn.com> writes:
>
>>> +/** Callback type for use with svn_repos_verify_fs3(). @a revision
>>> + * and @a verify_err are the details of a single verification failure
>>> + * that occurred during the svn_repos_verify_fs3() call. @a baton is
>>> + * the same baton given to svn_repos_verify_fs3(). @a scratch_pool is
>>> + * provided for the convenience of the implementor, who should not
>>> + * expect it to live longer than a single callback call.
>>> + *
>> Who is responsible for clearing verify_err: caller or callback implementor?
>
> That is the caller's responsibility. I documented this in the docstring
> that precedes the svn_repos_verify_fs3() declaration ...
>
> + * failure. Set @c revision callback argument to #SVN_INVALID_REVNUM or
> + * to the revision number respectively. Set @c verify_err to svn_error_t
> + * describing the reason of the failure. @c verify_err will be cleared
> + * after the callback returns, use svn_error_dup() to preserve the error.
>
> ...but now I think that it makes sense to say this again in the documentation
> for svn_repos_verify_callback_t. So, I committed that in r1687776 [1].
>
> Anticipating further possible questions, I chose the approach where a calling
> site of the callback is responsible for clearing the error because I think
> that the reverse design would be rather painful for potential implementors,
> including ourselves. The callback can potentially return (other) errors while
> processing the verification error, hence transferring the verify_err lifetime
> to the callback would require a bigger amount of efforts — just to avoid an
> error leak. We also have svn_fs_lock_callback_t that sticks to the same
> approach, and I chose not to come up with something new here.
>
> [1] https://svn.apache.org/r1687776
>
OK, makes sense. Thanks for explanation and docstring clarification!
--
Ivan Zhakov
Re: svn commit: r1687769 - in /subversion/trunk/subversion:
bindings/javahl/native/ bindings/javahl/src/org/apache/subversion/javahl/
include/ libsvn_repos/ svnadmin/ tests/cmdline/ tests/libsvn_fs_fs/
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Ivan Zhakov <iv...@visualsvn.com> writes:
>> +/** Callback type for use with svn_repos_verify_fs3(). @a revision
>> + * and @a verify_err are the details of a single verification failure
>> + * that occurred during the svn_repos_verify_fs3() call. @a baton is
>> + * the same baton given to svn_repos_verify_fs3(). @a scratch_pool is
>> + * provided for the convenience of the implementor, who should not
>> + * expect it to live longer than a single callback call.
>> + *
> Who is responsible for clearing verify_err: caller or callback implementor?
That is the caller's responsibility. I documented this in the docstring
that precedes the svn_repos_verify_fs3() declaration ...
+ * failure. Set @c revision callback argument to #SVN_INVALID_REVNUM or
+ * to the revision number respectively. Set @c verify_err to svn_error_t
+ * describing the reason of the failure. @c verify_err will be cleared
+ * after the callback returns, use svn_error_dup() to preserve the error.
...but now I think that it makes sense to say this again in the documentation
for svn_repos_verify_callback_t. So, I committed that in r1687776 [1].
Anticipating further possible questions, I chose the approach where a calling
site of the callback is responsible for clearing the error because I think
that the reverse design would be rather painful for potential implementors,
including ourselves. The callback can potentially return (other) errors while
processing the verification error, hence transferring the verify_err lifetime
to the callback would require a bigger amount of efforts — just to avoid an
error leak. We also have svn_fs_lock_callback_t that sticks to the same
approach, and I chose not to come up with something new here.
[1] https://svn.apache.org/r1687776
Regards,
Evgeny Kotkov