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

Re: svn commit: r25251 - in trunk/subversion: include libsvn_ra_svn svnserve

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: 2007-06-01 19:52:43 CEST

Malcolm Rowe wrote:
> On Thu, May 31, 2007 at 09:28:05PM -0700, hwright@tigris.org wrote:
>> Really fix the ra_svn compatibility issue for 'svn log -g'. Introduce a new
>> format character, 'B', which can be used to specify an optional boolean
>> value. Use this value for the optional include-merged-revisions paramater.
>
> Nicely done!
>
>> --- trunk/subversion/svnserve/serve.c (original)
>> +++ trunk/subversion/svnserve/serve.c Thu May 31 21:28:05 2007
>> @@ -1551,17 +1551,22 @@
>> apr_array_header_t *paths, *full_paths;
>> svn_ra_svn_item_t *elt;
>> int i;
>> - apr_uint64_t limit;
>> + apr_uint64_t limit, include_merged_revs_param;
>> log_baton_t lb;
>>
>> /* Parse the arguments. The usual optional element pattern "(?n)"
>> isn't used for the limit argument because pre-1.3 clients don't
>> know to send it. Nor is the pattern used for the include_merged_revisions
>> argument, because pre-1.5 clients don't know to send it, either. */
>
> That comment's actually pretty confusing: neither of the limit nor
> include_merged_revisions options are considered optional in the
> protocol, so the format we're using is entirely correct.
>
>> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "l(?r)(?r)bb?n?B", &paths,
>
> I realise you didn't update libsvn_ra_svn/protocol originally to
> document the extract include_merged_revisions option - could you do that
> as well? (And could you include some text that explains what limit
> should be set to to indicate 'unlimited'? We really _should_ have made
> it optional, but too late for that now...).

I'm confused. The protocol document currently has:
  log
    params: ( ( target-path:string ... ) [ start-rev:number ]
                [ end-rev:number ] changed-paths:bool strict-node:bool
                ? limit:number ? include-merged-revisions:bool )

Doesn't the '?' indicate that the limit and include-merged-revisions are
optional? include-merged-revisions is optional because pre-1.5 won't
send it. I guess I'm just failing to see the difference between a
earlier client not sending the parameter, and the parameter being
optional in the protocol.

-Hyrum

Received on Fri Jun 1 19:53:06 2007

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.