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

Re: [PATCH][Issue 2287] svn_client_log should take a peg revision

From: Daniel Rall <dlr_at_collab.net>
Date: 2006-01-25 05:55:12 CET

On Tue, 24 Jan 2006, Daniel Rall wrote:

> On Tue, 24 Jan 2006, Julian Foad wrote:
...
> > Note: some confusion may result from the fact that, as far as I recall,
> > this log function has never worked properly for multiple local paths. We
> > (I) changed our command-line client to not support that usage, but it seems
> > we didn't fix the problem in the API. I don't know whether that affects
> > any of the issues below.
> >
> > > /* Find the base URL and condensed targets relative to it. */
> > >- SVN_ERR (svn_path_condense_targets (&base_url, &condensed_targets,
> > >+ SVN_ERR (svn_path_condense_targets (&ignored_url,
> > >&condensed_targets,
> > > target_urls, TRUE, pool));
> >
> > I don't see how it can be right to ignore the base URL determined by that
> > function and yet use the condensed targets that are relative to that base.
>
> PATH is currently passed to svn_client__ra_session_from_path(), after
> being set to the first target. At least for the command-line client,
> this is correct for the URI case, where any URI required to be the
> first target. Is this also correct when svn_client_log3() is used as
> an API (not by the command-line client)?
>
> This certainly does seem questionable for local paths, where the
> path's ".svn/entries" file is eventually used to procure its URI (via
> svn_client_url_from_path). If multiple local paths to different
> directories are specified in the TARGETS argument, will the wrong base
> URI be inferred when opening the RA session?

How about changing things like below, renaming PATH to
BASE_URL_OR_PATH, and using it both when condensing the targets and
when opening the RA session. This only seems necessary when TARGETS
contain multiple local paths, which can only occur via direct API
access (the 'svn' command-line client doesn't support this). This
passes log_tests.py over ra_local.

- Dan

Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 18211)
+++ subversion/libsvn_client/log.c (working copy)
@@ -58,7 +58,7 @@
                  apr_pool_t *pool)
 {
   svn_ra_session_t *ra_session;
- const char *path;
+ const char *base_url_or_path;
   const char *ignored_url;
   const char *base_name = NULL;
   apr_array_header_t *condensed_targets;
@@ -73,10 +73,10 @@
          _("Missing required revision specification"));
     }

- path = (APR_ARRAY_IDX(targets, 0, const char *));
+ base_url_or_path = (APR_ARRAY_IDX(targets, 0, const char *));

   /* Use the passed URL, if there is one. */
- if (svn_path_is_url (path))
+ if (svn_path_is_url (base_url_or_path))
     {
       /* Initialize this array, since we'll be building it below */
       condensed_targets = apr_array_make (pool, 1, sizeof (const char *));
@@ -143,7 +143,8 @@
         return SVN_NO_ERROR;

       /* Find the base URL and condensed targets relative to it. */
- SVN_ERR (svn_path_condense_targets (&ignored_url, &condensed_targets,
+ SVN_ERR (svn_path_condense_targets (&base_url_or_path,
+ &condensed_targets,
                                           target_urls, TRUE, pool));

       if (condensed_targets->nelts == 0)
@@ -166,8 +167,9 @@
     session_opt_rev.kind = svn_opt_revision_unspecified;

   SVN_ERR (svn_client__ra_session_from_path (&ra_session, &ignored_revnum,
- &ignored_url, path, peg_revision,
- &session_opt_rev, ctx, pool));
+ &ignored_url, base_url_or_path,
+ peg_revision, &session_opt_rev,
+ ctx, pool));

   /* It's a bit complex to correctly handle the special revision words
    * such as "BASE", "COMMITTED", and "PREV". For example, if the

  • application/pgp-signature attachment: stored
Received on Wed Jan 25 05:55:27 2006

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.