S.Ramaswamy wrote:
>
> Fix issue #2287 - add peg revision to svn_client_log2() and add
> peg revision support to the command line client.
Apologies for the delay in reviewing this.
>
> * subversion/include/svn_client.h:
> (svn_client_log3): New prototype.
> (svn_client_log2): Deprecate.
>
> * subversion/libsvn_client/log.c:
> (svn_client_log3): New function.
> (svn_client_log2): Re-implemented using new function
> svn_client_log3().
> (svn_client_log): Re-implemented using new function
> svn_client_log3().
>
> * subversion/clients/cmdline/log-cmd.c:
> (svn_cl__log): Call svn_client_log3().
>
> * subversion/tests/clients/cmdline/log_tests.py:
> (url_missing_in_head, log_through_copyfrom_history): Use peg
> revisions.
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 15298)
> +++ subversion/include/svn_client.h (working copy)
> @@ -915,7 +915,8 @@
> /**
> * Invoke @a receiver with @a receiver_baton on each log message from @a
> * start to @a end in turn, inclusive (but never invoke @a receiver on a
> - * given log message more than once).
> + * given log message more than once). @a peg_revision indicates in which
> + * revision @a targets are valid.
This needs to specify what svn_opt_revision_unspecified means as a peg revision ...
> *
> * @a targets contains either a URL followed by zero or more relative
> * paths, or a list of working copy paths, as <tt>const char *</tt>,
[...]
> /**
> + * Similar to svn_client_log3(), but with the @a peg_revision
> + * parameter always set to @c svn_opt_revision_unspecified.
... if svn_client_log2() is documented in terms of it. I wasn't easily able to
determine the meaning for myself.
> + *
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> + */
> +svn_error_t *
> +svn_client_log2 (const apr_array_header_t *targets,
[...]
> Index: subversion/libsvn_client/log.c
> ===================================================================
> --- subversion/libsvn_client/log.c (revision 15298)
> +++ subversion/libsvn_client/log.c (working copy)
[...]
> @@ -157,13 +161,17 @@
> targets = real_targets;
> }
>
> - /* Open a repository session to the BASE_URL. */
> - SVN_ERR (svn_path_condense_targets (&base_name, NULL, targets, TRUE, pool));
> - SVN_ERR (svn_client__open_ra_session_internal (&ra_session, base_url,
> - base_name, NULL, NULL,
> - (NULL != base_name), TRUE,
> - ctx, pool));
> + if (start->kind == svn_opt_revision_number &&
> + end->kind == svn_opt_revision_number)
> + revision = (start->value.number > end->value.number) ? *start : *end;
> + else
> + revision.kind = svn_opt_revision_unspecified;
>
> + SVN_ERR (svn_client__ra_session_from_path (&ra_session, &rev,
> + &url_p, path,
> + peg_revision, &revision, ctx,
> + pool));
> +
Please could you explain this change. I don't understand why something special
is happening if both the start and end revisions are specified as numbers
rather than, for example, dates. And I don't really understand _what_ special
behaviour is happening in that case.
I see from the implementation of svn_client__ra_session_from_path() that an
unspecified "revision" means "revision = peg_revision", so wouldn't it be
clearer to make your "else" clause say "revision = *peg_revision" (or an
equivalent pointer assignment) ?
The rest of the code looks fine. (I haven't studued the regression tests.)
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 1 02:19:00 2005