Re: svn commit: r28477 - in branches/reintegrate: . subversion/include/private subversion/libsvn_ra_neon subversion/libsvn_ra_serf subversion/mod_dav_svn/reports

[David Glasser <gl...@davidglasser.net>]

On Dec 13, 2007 8:36 PM,  <kf...@tigris.org> wrote:
> Author: kfogel
> Date: Thu Dec 13 20:36:27 2007
> New Revision: 28477
>
> Log:
> Follow up to r28251: transmit the new include_descendants boolean over DAV.
>
> * subversion/include/private/svn_dav_protocol.h
>   (SVN_DAV__INCLUDE_DESCENDANTS): New child element.
>
> * subversion/mod_dav_svn/reports/mergeinfo.c
>   (dav_svn__get_mergeinfo_report): Read "include-descendants", pass
>     along the resulting value to svn_repos_fs_get_mergeinfo().
>
> * subversion/libsvn_ra_serf/mergeinfo.c
>   (struct mergeinfo_context_t): New include_descendants member.
>   (svn_ra_serf__get_mergeinfo): Set include_descendants in mergeinfo_ctx.
>   (create_mergeinfo_body): Add include_descendants element iff
>     mergeinfo_ctx->include_descendants is true.
>
> * subversion/libsvn_ra_neon/mergeinfo.c
>   (svn_ra_neon__get_mergeinfo): Send include_descendants.
>
> * reintegrate-branch-TODO: Remove this item.
>
>
> Modified:
>    branches/reintegrate/reintegrate-branch-TODO
>    branches/reintegrate/subversion/include/private/svn_dav_protocol.h
>    branches/reintegrate/subversion/libsvn_ra_neon/mergeinfo.c
>    branches/reintegrate/subversion/libsvn_ra_serf/mergeinfo.c
>    branches/reintegrate/subversion/mod_dav_svn/reports/mergeinfo.c
>


> Modified: branches/reintegrate/subversion/libsvn_ra_neon/mergeinfo.c
> URL: http://svn.collab.net/viewvc/svn/branches/reintegrate/subversion/libsvn_ra_neon/mergeinfo.c?pathrev=28477&r1=28476&r2=28477
> ==============================================================================
> --- branches/reintegrate/subversion/libsvn_ra_neon/mergeinfo.c  (original)
> +++ branches/reintegrate/subversion/libsvn_ra_neon/mergeinfo.c  Thu Dec 13 20:36:27 2007
> @@ -157,7 +157,7 @@
>                             const apr_array_header_t *paths,
>                             svn_revnum_t revision,
>                             svn_mergeinfo_inheritance_t inherit,
> -                           svn_boolean_t include_descendents, /*### TODO(reint): implement*/
> +                           svn_boolean_t include_descendants,
>                             apr_pool_t *pool)
>  {
>    int i, status_code;
> @@ -185,6 +185,16 @@
>                                          "<S:inherit>%s"
>                                          "</S:inherit>",
>                                          svn_inheritance_to_word(inherit)));
> +
> +  if (include_descendants)
> +    {
> +      /* Send it only if true; server will default to "no". */
> +      svn_stringbuf_appendcstr(request_body,
> +                               apr_pstrdup(pool,
> +                                           "<S:include-descendants>yes"

Why not use the #define here?  (Also, no need to pstrdup a static string.)

Also, does notes/webdav-protocol need to be updated?

--dave


-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r28477 - in branches/reintegrate: . subversion/include/private subversion/libsvn_ra_neon subversion/libsvn_ra_serf subversion/mod_dav_svn/reports

[Karl Fogel <kf...@red-bean.com>]

"David Glasser" <gl...@davidglasser.net> writes:
>> --- branches/reintegrate/subversion/libsvn_ra_neon/mergeinfo.c  (original)
>> +++ branches/reintegrate/subversion/libsvn_ra_neon/mergeinfo.c  Thu Dec 13 20:36:27 2007
>> @@ -157,7 +157,7 @@
>>                             const apr_array_header_t *paths,
>>                             svn_revnum_t revision,
>>                             svn_mergeinfo_inheritance_t inherit,
>> -                           svn_boolean_t include_descendents, /*### TODO(reint): implement*/
>> +                           svn_boolean_t include_descendants,
>>                             apr_pool_t *pool)
>>  {
>>    int i, status_code;
>> @@ -185,6 +185,16 @@
>>                                          "<S:inherit>%s"
>>                                          "</S:inherit>",
>>                                          svn_inheritance_to_word(inherit)));
>> +
>> +  if (include_descendants)
>> +    {
>> +      /* Send it only if true; server will default to "no". */
>> +      svn_stringbuf_appendcstr(request_body,
>> +                               apr_pstrdup(pool,
>> +                                           "<S:include-descendants>yes"
>
> Why not use the #define here?  (Also, no need to pstrdup a static string.)

We're not using the #defines for any of the protocol elements in
ra_neon, I think because the elements are put together in a slightly
different way there than in ra_serf.  Though deploring the inconsistency
between ra_neon and ra_serf, I didn't want to introduce an even finer-
grained inconsistency within ra_neon as part of this change.  (I'd be
all for cleaning this up as a separate change, though!)

Dan also brought up the pstrdup'ing.  I guess you're right.  I didn't
like the lifetime sloppiness of having it last longer than the other
strings in the local pool, but that seems kind of silly in retrospect.
Fixed in r28494.

> Also, does notes/webdav-protocol need to be updated?

I forgot about that file, whups.  Done in r28492.

Thanks for the review, as always,
-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org