On Thu, Mar 26, 2009 at 16:40, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Greg Stein wrote:
>> On Wed, Mar 25, 2009 at 22:24, C. Michael Pilato <cmpilato_at_collab.net> wrote:
>>> ...
>>> +++ trunk/subversion/libsvn_ra_serf/merge.c Wed Mar 25 14:24:32 2009 (r36782)
>>> @@ -287,40 +287,41 @@ end_merge(svn_ra_serf__xml_parser_t *par
>>> apr_hash_get(info->props,
>>> "post-commit-err", APR_HASH_KEY_STRING));
>>> }
>>> - else if (ctx->session->wc_callbacks->push_wc_prop)
>>> + else
>>> {
>>> - const char *href, *checked_in;
>>> - svn_string_t checked_in_str;
>>> + const char *href;
>>>
>>> href = apr_hash_get(info->props, "href", APR_HASH_KEY_STRING);
>>> - checked_in = apr_hash_get(info->props, "checked-in",
>>> - APR_HASH_KEY_STRING);
>>> -
>>> if (! svn_path_is_ancestor(ctx->merge_url, href))
>>> {
>>> /* ### need something better than APR_EGENERAL */
>>> return svn_error_createf(APR_EGENERAL, NULL,
>>> - _("A MERGE response for '%s' is not a child "
>>> - "of the destination ('%s')"),
>>> + _("A MERGE response for '%s' is not "
>>> + "a child of the destination ('%s')"),
>>> href, ctx->merge_url);
>>> }
>
> Hi. Remember me?
Ah. Missed that usage.
>>> href = svn_path_is_child(ctx->merge_url, href, NULL);
>>> if (! href) /* the paths are equal */
>>> href = "";
>>>
>>> - checked_in_str.data = checked_in;
>>> - checked_in_str.len = strlen(checked_in);
>>> -
>>> /* We now need to dive all the way into the WC to update the
>>> * base VCC url.
>>> */
>>> - SVN_ERR(ctx->session->wc_callbacks->push_wc_prop(
>>> - ctx->session->wc_callback_baton,
>>> - href,
>>> - SVN_RA_SERF__WC_CHECKED_IN_URL,
>>> - &checked_in_str,
>>> - info->pool));
>>> + if ((! SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(ctx->session))
>>> + && ctx->session->wc_callbacks->push_wc_prop)
>>> + {
>>> + svn_string_t checked_in_str;
>>> + const char *checked_in;
>>>
>>> + checked_in = apr_hash_get(info->props, "checked-in",
>>> + APR_HASH_KEY_STRING);
>>> + checked_in_str.data = checked_in;
>>> + checked_in_str.len = strlen(checked_in);
>>> +
>>> + SVN_ERR(ctx->session->wc_callbacks->push_wc_prop(
>>> + ctx->session->wc_callback_baton, href,
>>> + SVN_RA_SERF__WC_CHECKED_IN_URL, &checked_in_str, info->pool));
>>
>> href is only needed here, so you can tighten up its computation/scope.
>
> No, I expressly wish to always hit the sanity check above, which previously
> only "ran" if there was a push_wc_prop() callback function to use. We've
> seen enough bugs over time in this area to make me want to always perform
> the sanity check. When this error triggers, it has traditionally meant that
> a commit has gone to the *wrong* FS node ... like changes intended for
> /trunk winding up on some branch. Users will want to know about this
> scenario, even if they aren't writing out wcprops.
Yup. Fair enough.
So the second bits of href usage can be shifted inside the block.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1432872
Received on 2009-03-26 18:44:27 CET