[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r29360 - trunk/subversion/mod_dav_svn/reports

From: David Glasser <glasser_at_davidglasser.net>
Date: Thu, 14 Feb 2008 16:22:04 -0800

Out of curiosity, why aren't you using svn_repos_has_capability in
mod_dav_svn (like the other two servers)? That would also let you
sidestep this issue for now...

--dave

On Thu, Feb 14, 2008 at 1:34 PM, <kfogel_at_tigris.org> wrote:
> Author: kfogel
> Date: Thu Feb 14 13:34:11 2008
> New Revision: 29360
>
> Log:
> For issue #3089: cause mod_dav_svn to report an error more recognizeably.
>
> There is some question as to whether this is the right solution, see:
>
> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=135089
> From: Karl Fogel <kfogel_at_red-bean.com>
> To: dev_at_subversion.tigris.org
> Subject: Better error reporting from mod_dav_svn?
> Date: Thu, 14 Feb 2008 00:36:40 -0500
> Message-Id: <87r6fgumjb.fsf_at_red-bean.com>
>
> We may stick with this method, or we may switch to some other method
> of letting the client detect the "mergeinfo is unimplemented" error.
> Either way, it is independent of other changes related to issue #3089,
> so I've extracted it from r29314 on the mergeinfo-capability branch.
> See the r29358 log message for more about how changes from that branch
> are being ported to trunk.
>
> * subversion/mod_dav_svn/reports/mergeinfo.c
> (dav_svn__get_mergeinfo_report): Don't flush before returning error,
> as flushing might obscure the error due to certain intricacies of
> mod_dav's plumbing.
>
>
> Modified:
> trunk/subversion/mod_dav_svn/reports/mergeinfo.c
>
> Modified: trunk/subversion/mod_dav_svn/reports/mergeinfo.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/mod_dav_svn/reports/mergeinfo.c?pathrev=29360&r1=29359&r2=29360
> ==============================================================================
> --- trunk/subversion/mod_dav_svn/reports/mergeinfo.c (original)
> +++ trunk/subversion/mod_dav_svn/reports/mergeinfo.c Thu Feb 14 13:34:11 2008
> @@ -199,9 +199,25 @@
> svn_inheritance_to_word(inherit));
> dav_svn__operational_log(resource->info, action);
>
> - /* Flush the contents of the brigade (returning an error only if we
> - don't already have one). */
> - if ((apr_err = ap_fflush(output, bb)) && !derr)
> + /* If we already have an error, don't even flush the contents of the
> + brigade, because we don't want mod_dav.c:dav_method_report() to
> + think we've sent anything back to the client yet -- as long as it
> + doesn't think that, it'll send the real error to the client,
> + which is what we'd prefer. This situation is described in more
> + detail in httpd-2.2.6/modules/dav/main/mod_dav.c, line 4066, in
> + the comment in dav_method_report() that says:
> +
> + If an error occurred during the report delivery, there's
> + basically nothing we can do but abort the connection and
> + log an error. This is one of the limitations of HTTP; it
> + needs to "know" the entire status of the response before
> + generating it, which is just impossible in these streamy
> + response situations.
> +
> + In other words, flushing the brigade here would cause
> + r->sent_bodyct (see dav_method_report()) to flip from 0 to 1.
> + */
> + if (!derr && (apr_err = ap_fflush(output, bb)))
> derr = dav_svn__convert_err(svn_error_create(apr_err, 0, NULL),
> HTTP_INTERNAL_SERVER_ERROR,
> "Error flushing brigade.",
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: svn-help_at_subversion.tigris.org
>
>

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-15 01:22:22 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.