On 12/21/10 5:57 PM, Daniel Shahaf wrote:
> Blair Zajac wrote on Tue, Dec 21, 2010 at 13:47:40 -0800:
>> On 12/21/10 11:44 AM, Daniel Shahaf wrote:
>>> Daniel Shahaf wrote on Tue, Dec 21, 2010 at 20:40:02 +0200:
>>>> Blair Zajac wrote on Tue, Dec 21, 2010 at 10:16:56 -0800:
>>>>> 4) In svn_repos_fs_commit_txn(), which order should errors be composed?
>>>>> svn_fs_commit_txn()'s error as the parent followed by the
>>>>> SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED error as a child? This seems to be
>>>>> the standard ordering of chained errors. On the other hand, it makes it
>>>>> harder to find a post-commit script error.
>>>>
>>>> Actually, it will make it impossible to detect post-commit errors over
>>>> ra_dav, since that RA layer marshals only the outermost error code in an
>>>> error chain.
>>>
>>> This is now<http://subversion.tigris.org/issues/show_bug.cgi?id=3767>.
>>>
>>> (Details are partly from memory, partly from quick testing I re-did today.)
>>
>> While we're opening tickets, I found an issue with
>> svn_repos_parse_fns2_t.close_revision(), it doesn't support the
>> documented svn_fs_commit_txn() contract, I opened this to track it and
>> put it in 1.7-consider, as it shouldn't block 1.7, but it would be nice
>> to have it in there.
>>
>> http://subversion.tigris.org/issues/show_bug.cgi?id=3768
>>
>
> svn_repos_parse_fns2_t is about parsing a dumpstream, not about
> committing the expressed revisions to some repository, so I don't
> understand why the "NEW_REV" parameter belongs there and not in
> svn_repos_get_fs_build_parser3().
I haven't used nor looked too closely at this API, but reading the API, it can
commit multiple revisions, so shouldn't the errors be returned from each
revision committed?
In svn_repos_parse_dumpstream2(), I see that it commits each revision.
/* If we already have a rev_baton open, we need to close it
and clear the per-revision subpool. */
if (rev_baton != NULL)
{
SVN_ERR(parse_fns->close_revision(rev_baton));
This code couldn't handle a successful commit followed by an internal FS
failure, so close_revision() would need to have a *new_rev argument.
Let me know if I'm reading this correctly.
Blair
Received on 2010-12-22 04:49:32 CET