Re: svn commit: r31900 - in trunk/subversion: libsvn_client tests/cmdline

I got the crux of the problem.

Our 'serf_response_handler_t' implementation is wrong.

According to 'serf_response_handler_t' doc implementation can return
either one of the following,

APR_EAGAIN, APR_EOF, APR_SUCCESS.

Our libsvn_ra_serf's implementation of
'serf_response_handler_t'(handle_response) returns arbitrary svn error
codes and hence 'serf system' thinks that 'EOF on a request has not
reached' and hence this failure.

I am attaching the work in progress patch here.(It fails few tests over
my 'maintainer-mode' build).

With regards
Kamesh Jayachandran

Kamesh Jayachandran wrote:
>
> One more update about this which is still closer.
>
> Post r31900 When we merge -rX:Y, we do run location-segments report on
> each corresponding_src_url_of_subtree-with-mergeinfo_at_Y.
>
> corresponding_src_url_of_subtree-with-mergeinfo_at_Y may be non-existent.
>
> In merge-tests-61 we run location_segments_report for
>
> http://localhost/svn/merge_tests-61/A/D/gamma@5
>
> which is non-existent since r4.
>
> In 'svn_ra_serf__get_location_segments' we have the following code.
>
> <snip>
> svn_ra_serf__request_create(handler);
>
> err = svn_ra_serf__context_run_wait(&gls_ctx->done, session, pool);
>
> if (gls_ctx->error || parser_ctx->error)
> {
> svn_error_clear(err);
> err = SVN_NO_ERROR;
> SVN_ERR(gls_ctx->error);
> SVN_ERR(parser_ctx->error);
> }
> </snip>
>
> When requests fail like above the request is not getting removed from
> the serf's opaque data structure 'handler->conn->conn->requests' and
> hence subsequent request on the same connection fails while reading the
> first request's response(which should have been removed upon failure).
>
>
> With regards
> Kamesh Jayachandran
>
>
>
>
>
> Kamesh Jayachandran wrote:
>> Let me record my study so far on this.
>
>> Context from test merge_tests-61.
>
>> On Pristine working copy of merge_tests-61
>> 1)svn merge -r1:4 http://localhost/svn/merge_tests-61/A/D/gamma@4
>> A_copy/D/gamma
>> 2)svn merge -r1:5 http://localhost/svn/merge_tests-61/A A_copy --force
>
>
>> Prior to this commit when we run 2)
>> ->we open the diff editor for '/A' with default_start=2 and end
>> revision=5.
>> ->we call set_path on /A/D/gamma for start_rev=4
>> ->finish_report()
>
>> After this commit
>> ->we open the diff editor for '/A' with default_start=2 and end
>> revision=5.
>> *No set_path on /A/D/gamma is called as it is anyway going to be deleted.
>> ->finish_report()
>
>> Not calling set_path on /A/D/gamma causes the serf to fail on
>> 'finish_report'.
>
>> It fails at 'svn_ra_serf__get_baseline_info' called via the callback
>> which calls 'svn_ra_get_dir2'.
>
>> I could see neon also makes a call to 'svn_ra_get_dir2' with almost same
>> set of parameters.
>
>> One difference I could observe that may be the cause is not sure though.
>
>> When neon tries to retrieve the 'dir properties at rev 2' it makes a
>> request to '/svn/merge_tests-61/!svn/bc/2/A'.
>
>> Whereas serf makes a request to '/svn/merge_tests-1/A'.
>
>
>> With regards
>> Kamesh Jayachandran
>> Lieven Govaerts wrote:
>>> kameshj_at_tigris.org wrote:
>>>> Author: kameshj
>>>> Date: Fri Jun 27 07:56:04 2008
>>>> New Revision: 31900
>>>>
>>>> Log:
>>>> Fix issue #3067, 'subtrees that don't exist at the start or end of a
>>>> merge
>>>> range shouldn't break the merge'
>>>>
>>>> Fixes the core problem of issue #3076: Don't try to describe invalid
>>>> subtrees to the merge report editor. Also fixes the ancillary problem
>>>> described in
>>>> http://subversion.tigris.org/issues/show_bug.cgi?id=3067#desc34
>>>> where subtrees with empty mergeinfo don't get their mergeinfo updated
>>>> properly.
>>>>
>>>> * subversion/libsvn_client/merge.c
>>>> (init_rangelist, push_range): New helpers.
>>>> (prepare_subtree_ranges): New helper for filter_merged_revisions() when
>>>> operating on subtrees.
>>>> (filter_merged_revisions): Use new prepare_subtree_ranges() helper to
>>>> address various issue #3067 problems. Do away with entry argument
>>>> and instead pass a svn_client__merge_path_t representing the working
>>>> copy
>>>> target path, along with another svn_client__merge_path_t
>>>> representing the
>>>> path's parent if the path is a subtree. Cascade some additional
>>>> args from
>>>> calculate_remaining_ranges().
>>>> (calculate_remaining_ranges): Improve doc string. Tweak signature
>>>> to get
>>>> svn_client__merge_path_t for target and its nearest parent if target
>>>> is a
>>>> subtree. Update call to filter_merged_revisions().
>>>> (populate_remaining_ranges, do_file_merge): Update calls to
>>>> calculate_remaining_ranges().
>>>> (get_mergeinfo_walk_cb): Stop doing any 'preemptive' elision on
>>>> subtrees.
>>>> This leaves a local mod on the subtree and the subtree is to be
>>>> deleted by
>>>> the requested merge,the local mod will cause the subtree to be skipped.
>>>> Just add subtrees with empty mergeinfo and let prepare_subtree_ranges()
>>>> handle it.
>>>> (get_mergeinfo_paths): Note in doc string that we always include
>>>> children
>>>> with empty mergeinfo.
>>>>
>>> This commit seems to break 4 merge tests over ra_serf only.
>>> The error message for each of this is:
>>> svn: Premature EOF seen from server
>>> I don't know what specifically in your patch can cause this situation,
>>> and I don't have a lot of time right now to look into it. If anyone else
>>> can, great. Otherwise I'll use this as a todo reminder mail.
>>> Lieven
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org

Index: subversion/libsvn_ra_serf/util.c
===================================================================
--- subversion/libsvn_ra_serf/util.c (revision 32028)
+++ subversion/libsvn_ra_serf/util.c (working copy)
@@ -1074,8 +1074,6 @@
           XML_ParserFree(ctx->xmlp);
           if (xml_status == XML_STATUS_ERROR && ctx->ignore_errors == FALSE)
             {
- status = SVN_ERR_RA_DAV_REQUEST_FAILED;
-
               svn_error_clear(ctx->error);
             }
 
@@ -1184,7 +1182,7 @@
           ctx->session->pending_error =
               svn_error_create(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
                                _("Premature EOF seen from server"));
- return ctx->session->pending_error->apr_err;
+ return status;
         }
     }
 
@@ -1236,7 +1234,7 @@
               svn_error_create(APR_EGENERAL, NULL,
                                _("Unspecified error message"));
         }
- return ctx->session->pending_error->apr_err;
+ return APR_EOF;
     }
   else
     {