subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pbu...@apache.org
Subject svn commit: r1478220 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/log.c libsvn_client/ra.c tests/cmdline/authz_tests.py tests/cmdline/log_tests.py
Date Wed, 01 May 2013 22:27:32 GMT
Author: pburba
Date: Wed May  1 22:27:31 2013
New Revision: 1478220

URL: http://svn.apache.org/r1478220
Log:
Fix (hopefully for the last time) issue #4355 'svn_client_log5 broken with
multiple revisions which span a rename' and refactor the aforementioned
API, which was a bit of an unwieldy beast.

* subversion/include/private/svn_client_private.h

  (svn_client__resolve_rev_and_url): "New", but actually just the old
   libsvn_client/ra.c:resolve_rev_and_url().

* subversion/libsvn_client/log.c

  (assert.h): New include.

  (resolve_log_targets,
   find_youngest_and_oldest_revs,
   resolve_wc_opt_revs,
   resolve_wc_head_revs,
   resolve_wc_date_revs,
   rev_range_t,
   convert_opt_rev_array_to_rev_range_array,
   compare_rev_to_segment,
   run_ra_get_log): New.

  (svn_client_log5): Refactor this into the new helpers above.  Beyond the
   simple refactoring, this function now uses
   svn_client__repos_location_segments() to get the history spanning the
   requested log revisions and then uses this history to handle issue #4355.
   Credit cmpilato and rhhijben with this idea, see
   http://svn.haxx.se/dev/archive-2013-04/0474.shtml

* subversion/libsvn_client/ra.c

  (resolve_rev_and_url): Convert from static to svn_client private function,
   renaming from this to...

  (svn_client__resolve_rev_and_url): ...this.

* subversion/tests/cmdline/authz_tests.py

  (authz_log_and_tracing_test): Tweak expected error to account for new
   svn_client_log5 implementation.

* subversion/tests/cmdline/log_tests.py

  (log_multiple_revs_spanning_rename): Remove XFail decorator.  Tweak
   comments re failure status.  Add a required 'svn update' for testing
   log with WC target.

Modified:
    subversion/trunk/subversion/include/private/svn_client_private.h
    subversion/trunk/subversion/libsvn_client/log.c
    subversion/trunk/subversion/libsvn_client/ra.c
    subversion/trunk/subversion/tests/cmdline/authz_tests.py
    subversion/trunk/subversion/tests/cmdline/log_tests.py

Modified: subversion/trunk/subversion/include/private/svn_client_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_client_private.h?rev=1478220&r1=1478219&r2=1478220&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_client_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_client_private.h Wed May  1 22:27:31 2013
@@ -147,6 +147,37 @@ svn_client__ra_session_from_path2(svn_ra
                                  svn_client_ctx_t *ctx,
                                  apr_pool_t *pool);
 
+/* Given PATH_OR_URL, which contains either a working copy path or an
+   absolute URL, a peg revision PEG_REVISION, and a desired revision
+   REVISION, find the path at which that object exists in REVISION,
+   following copy history if necessary.  If REVISION is younger than
+   PEG_REVISION, then check that PATH_OR_URL is the same node in both
+   PEG_REVISION and REVISION, and return @c
+   SVN_ERR_CLIENT_UNRELATED_RESOURCES if it is not the same node.
+
+   If PEG_REVISION->kind is 'unspecified', the peg revision is 'head'
+   for a URL or 'working' for a WC path.  If REVISION->kind is
+   'unspecified', the operative revision is the peg revision.
+
+   Store the actual location of the object in *RESOLVED_LOC_P.
+
+   RA_SESSION should be an open RA session pointing at the URL of
+   PATH_OR_URL, or NULL, in which case this function will open its own
+   temporary session.
+
+   Use authentication baton cached in CTX to authenticate against the
+   repository.
+
+   Use POOL for all allocations. */
+svn_error_t *
+svn_client__resolve_rev_and_url(svn_client__pathrev_t **resolved_loc_p,
+                                svn_ra_session_t *ra_session,
+                                const char *path_or_url,
+                                const svn_opt_revision_t *peg_revision,
+                                const svn_opt_revision_t *revision,
+                                svn_client_ctx_t *ctx,
+                                apr_pool_t *pool);
+
 /** Return @c SVN_ERR_ILLEGAL_TARGET if TARGETS contains a mixture of
  * URLs and paths; otherwise return SVN_NO_ERROR.
  *

Modified: subversion/trunk/subversion/libsvn_client/log.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/log.c?rev=1478220&r1=1478219&r2=1478220&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/log.c (original)
+++ subversion/trunk/subversion/libsvn_client/log.c Wed May  1 22:27:31 2013
@@ -42,6 +42,7 @@
 #include "svn_private_config.h"
 #include "private/svn_wc_private.h"
 
+#include <assert.h>
 
 /*** Getting misc. information ***/
 
@@ -260,57 +261,336 @@ limit_receiver(void *baton, svn_log_entr
   return rb->receiver(rb->baton, log_entry, pool);
 }
 
-
-/*** Public Interface. ***/
+/* Resolve the URLs or WC path in TARGETS as per the svn_client_log5 API.
 
+   The limitations on TARGETS specified by svn_client_log5 are enforced here.
+   So TARGETS can only contain a single WC path or a URL and zero or more
+   relative paths -- anything else will raise an error. 
 
-svn_error_t *
-svn_client_log5(const apr_array_header_t *targets,
-                const svn_opt_revision_t *peg_revision,
-                const apr_array_header_t *revision_ranges,
-                int limit,
-                svn_boolean_t discover_changed_paths,
-                svn_boolean_t strict_node_history,
-                svn_boolean_t include_merged_revisions,
-                const apr_array_header_t *revprops,
-                svn_log_entry_receiver_t real_receiver,
-                void *real_receiver_baton,
-                svn_client_ctx_t *ctx,
-                apr_pool_t *pool)
+   PEG_REVISION, TARGETS, and CTX are as per svn_client_log5.
+
+   If TARGETS contains a single WC path then set *RA_TARGET to the absolute
+   path of that single path if PEG_REVISION is dependent on the working copy
+   (e.g. PREV).  Otherwise set *RA_TARGET to the corresponding URL for the
+   single WC path.  Set *RELATIVE_TARGETS to an array with a single
+   element "".
+
+   If TARGETS contains only a single URL, then set *RA_TARGET to a copy of
+   that URL and *RELATIVE_TARGETS to an array with a single element "".
+
+   If TARGETS contains a single URL and one or more relative paths, then
+   set *RA_TARGET to a copy of that URL and *CONDENSED_PATHS to a copy of
+   each relative path after the URL.
+
+   If *PEG_REVISION is svn_opt_revision_unspecified, then *PEG_REVISION is
+   set to svn_opt_revision_head for URLs or svn_opt_revision_working for a
+   WC path.
+
+   *RA_TARGET and *RELATIVE_TARGETS are allocated in RESULT_POOL. */
+static svn_error_t *
+resolve_log_targets(apr_array_header_t **relative_targets,
+                    const char **ra_target,
+                    svn_opt_revision_t *peg_revision,
+                    const apr_array_header_t *targets,
+                    svn_client_ctx_t *ctx,
+                    apr_pool_t *result_pool,
+                    apr_pool_t *scratch_pool)
 {
-  svn_ra_session_t *ra_session;
-  const char *url_or_path;
-  svn_boolean_t has_log_revprops;
-  apr_array_header_t *condensed_targets;
-  svn_opt_revision_t session_opt_rev;
-  const char *ra_target;
-  pre_15_receiver_baton_t rb = {0};
-  apr_pool_t *iterpool;
   int i;
-  svn_opt_revision_t peg_rev;
-  svn_boolean_t url_targets = FALSE;
+  svn_boolean_t url_targets;
+
+  /* Per svn_client_log5, TARGETS contains either a URL followed by zero or
+     more relative paths, or one working copy path. */
+  const char *url_or_path = APR_ARRAY_IDX(targets, 0, const char *);
+
+  /* svn_client_log5 requires at least one target. */
+  if (targets->nelts == 0)
+    return svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL,
+                            _("No valid target found"));
 
-  if (revision_ranges->nelts == 0)
+  /* Initialize the output array. */
+  *relative_targets = apr_array_make(result_pool, 1, sizeof(const char *));
+
+  if (svn_path_is_url(url_or_path))
     {
-      return svn_error_create
-        (SVN_ERR_CLIENT_BAD_REVISION, NULL,
-         _("Missing required revision specification"));
+      /* An unspecified PEG_REVISION for a URL path defaults
+         to svn_opt_revision_head. */
+      if (peg_revision->kind == svn_opt_revision_unspecified)
+          (*peg_revision).kind = svn_opt_revision_head;
+
+      /* The logic here is this: If we get passed one argument, we assume
+         it is the full URL to a file/dir we want log info for. If we get
+         a URL plus some paths, then we assume that the URL is the base,
+         and that the paths passed are relative to it.  */
+      if (targets->nelts > 1)
+        {
+          /* We have some paths, let's use them. Start after the URL.  */
+          for (i = 1; i < targets->nelts; i++)
+            {
+              const char *target;
+
+              target = APR_ARRAY_IDX(targets, i, const char *);
+
+              if (svn_path_is_url(target) || svn_dirent_is_absolute(target))
+                return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+                                         _("'%s' is not a relative path"),
+                                          target);
+
+              APR_ARRAY_PUSH(*relative_targets, const char *) =
+                apr_pstrdup(result_pool, target);
+            }
+        }
+      else
+        {
+          /* If we have a single URL, then the session will be rooted at
+             it, so just send an empty string for the paths we are
+             interested in. */
+          APR_ARRAY_PUSH(*relative_targets, const char *) = "";
+        }
+
+      /* Remember that our targets are URLs. */
+      url_targets = TRUE;
     }
+  else /* WC path target. */
+    {
+      const char *target;
+      const char *target_abspath;
 
-  /* Make a copy of PEG_REVISION, we may need to change it to a
-     default value. */
-  peg_rev = *peg_revision;
+      url_targets = FALSE;
+      if (targets->nelts > 1)
+        return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
+                                _("When specifying working copy paths, only "
+                                  "one target may be given"));
 
-  /* Use the passed URL, if there is one.  */
-  url_or_path = APR_ARRAY_IDX(targets, 0, const char *);
-  session_opt_rev.kind = svn_opt_revision_unspecified;
+      /* An unspecified PEG_REVISION for a working copy path defaults
+         to svn_opt_revision_working. */
+      if (peg_revision->kind == svn_opt_revision_unspecified)
+          (*peg_revision).kind = svn_opt_revision_working;
 
-  for (i = 0; i < revision_ranges->nelts; i++)
+      /* Get URLs for each target */
+      target = APR_ARRAY_IDX(targets, 0, const char *);
+
+      SVN_ERR(svn_dirent_get_absolute(&target_abspath, target, scratch_pool));
+      SVN_ERR(svn_wc__node_get_url(&url_or_path, ctx->wc_ctx, target_abspath,
+                                   scratch_pool, scratch_pool));
+      APR_ARRAY_PUSH(*relative_targets, const char *) = "";
+    }
+
+  /* If this is a revision type that requires access to the working copy,
+   * we use our initial target path to figure out where to root the RA
+   * session, otherwise we use our URL. */
+  if (SVN_CLIENT__REVKIND_NEEDS_WC(peg_revision->kind))
+    {
+      if (url_targets)
+        return svn_error_create(SVN_ERR_CLIENT_BAD_REVISION, NULL,
+                                _("PREV, BASE, or COMMITTED revision "
+                                  "keywords are invalid for URL"));
+
+      else
+        SVN_ERR(svn_dirent_get_absolute(
+          ra_target, APR_ARRAY_IDX(targets, 0, const char *), result_pool));
+    }
+  else
+    {
+      *ra_target = apr_pstrdup(result_pool, url_or_path);
+    }
+
+  return SVN_NO_ERROR;
+}
+
+/* Keep track of oldest and youngest opt revs found.
+
+   If REV is younger than *YOUNGEST_REV, or *YOUNGEST_REV is
+   svn_opt_revision_unspecified, then set *YOUNGEST_REV equal to REV.
+
+   If REV is older than *OLDEST_REV, or *OLDEST_REV is
+   svn_opt_revision_unspecified, then set *OLDEST_REV equal to REV. */
+static void
+find_youngest_and_oldest_revs(svn_revnum_t *youngest_rev,
+                              svn_revnum_t *oldest_rev,
+                              svn_revnum_t rev)
+{
+  /* Is REV younger than YOUNGEST_REV? */
+  if (! SVN_IS_VALID_REVNUM(*youngest_rev)
+      || rev > *youngest_rev)
+    *youngest_rev = rev;
+
+  if (! SVN_IS_VALID_REVNUM(*oldest_rev)
+      || rev < *oldest_rev)
+    *oldest_rev = rev;
+
+  return;
+}
+
+/* If OPT_REV is of a revision kind that is dependent on the WC then query
+   WC to determine the equivalent svn_revnum_t and set *OPT_REV to a
+   svn_opt_revision_number kind reflecting that revnum. */
+static svn_error_t *
+resolve_wc_opt_revs(svn_opt_revision_t *opt_rev,
+                    const char *abspath,
+                    svn_client_ctx_t *ctx,
+                    apr_pool_t *scratch_pool)
+{
+  if (SVN_CLIENT__REVKIND_NEEDS_WC(opt_rev->kind))
+    {
+      svn_revnum_t range_start_as_revnum;// = 0;
+
+      SVN_ERR(svn_client__get_revision_number(&range_start_as_revnum,
+                                              NULL, ctx->wc_ctx, abspath,
+                                              NULL,/* No session needed */
+                                              opt_rev, scratch_pool));
+      opt_rev->kind = svn_opt_revision_number;
+      opt_rev->value.number = range_start_as_revnum;
+    }
+
+  return SVN_NO_ERROR;
+}
+
+/* Resolve the head revision.
+
+   If OPT_REV is not of svn_opt_revision_head kind, then do nothing.
+
+   Otherwise if *YOUNGEST_REV is valid, then set set *OPT_REV to a
+   svn_opt_revision_number kind reflecting the value of *YOUNGEST_REV.
+   Else contact the repository to determine the head svn_revnum_t and
+   set *OPT_REV to a svn_opt_revision_number kind reflecting that revnum.
+
+   RA_SESSION, RESOLVED_LOC_P, URL_OR_ABSPATH, and PEG_REV are all as per the
+   parameters of the same name in convert_opt_rev_array_to_rev_range_array().
+*/
+static svn_error_t *
+resolve_wc_head_revs(svn_opt_revision_t *opt_rev,
+                     svn_ra_session_t **ra_session,
+                     svn_client__pathrev_t **resolved_loc_p,
+                     svn_revnum_t *head_rev,
+                     const char *url_or_abspath,
+                     const svn_opt_revision_t *peg_rev,
+                     svn_client_ctx_t *ctx,
+                     apr_pool_t *result_pool,
+                     apr_pool_t *scratch_pool)
+{
+  if (opt_rev->kind == svn_opt_revision_head)
+    {
+      svn_opt_revision_t head_opt_rev;
+
+      head_opt_rev.kind = svn_opt_revision_head;
+
+      if (*ra_session == NULL)
+        SVN_ERR(svn_client__ra_session_from_path2(ra_session,
+                                                  resolved_loc_p,
+                                                  url_or_abspath, NULL,
+                                                  peg_rev, &head_opt_rev,
+                                                  ctx, result_pool));
+
+      /* Find HEAD rev, but only do it once! */
+      if (!SVN_IS_VALID_REVNUM(*head_rev))
+        SVN_ERR(svn_ra_get_latest_revnum(*ra_session, head_rev,
+                                         scratch_pool));
+
+      opt_rev->kind = svn_opt_revision_number;
+      opt_rev->value.number = *head_rev;
+    }
+
+  return SVN_NO_ERROR;
+}
+
+/* If OPT_REV is of svn_opt_revision_date kind, then contact the repository
+   to determine the equivalent svn_revnum_t and set *OPT_REV to a
+   svn_opt_revision_number kind reflecting that revnum.
+
+   RA_SESSION, RESOLVED_LOC_P, URL_OR_ABSPATH, and PEG_REV are all as per the
+   parameters of the same name in convert_opt_rev_array_to_rev_range_array().
+*/
+static svn_error_t *
+resolve_wc_date_revs(svn_opt_revision_t *opt_rev,
+                     svn_ra_session_t **ra_session,
+                     svn_client__pathrev_t **resolved_loc_p,
+                     const char *url_or_abspath,
+                     const svn_opt_revision_t *peg_rev,
+                     svn_client_ctx_t *ctx,
+                     apr_pool_t *result_pool,
+                     apr_pool_t *scratch_pool)
+{
+  if (opt_rev->kind == svn_opt_revision_date)
+    {
+      svn_revnum_t date_as_revnum;
+
+      if (*ra_session == NULL)
+        SVN_ERR(svn_client__ra_session_from_path2(ra_session,
+                                                  resolved_loc_p,
+                                                  url_or_abspath, NULL,
+                                                  peg_rev, opt_rev,
+                                                  ctx, result_pool));
+
+      SVN_ERR(svn_ra_get_dated_revision(*ra_session, &date_as_revnum,
+                                        opt_rev->value.date, scratch_pool));
+      opt_rev->kind = svn_opt_revision_number;
+      opt_rev->value.number = date_as_revnum;
+    }
+
+  return SVN_NO_ERROR;
+}
+
+typedef struct rev_range_t
+{
+  svn_revnum_t range_start;
+  svn_revnum_t range_end;
+} rev_range_t;
+
+/* Convert array of svn_opt_revision_t ranges to an array of svn_revnum_t
+   ranges.
+
+   Given a log target URL_OR_ABSPATH@PEG_REV and an array of
+   svn_opt_revision_range_t's OPT_REV_RANGES, resolve the opt revs in
+   OPT_REV_RANGES to svn_revnum_t's and return these in *REVISION_RANGES, an
+   array of rev_range_t *.
+
+   Set *YOUNGEST_REV and *OLDEST_REV to the youngest and oldest revisions
+   found in *REVISION_RANGES.
+
+   If the repository needs to be contacted to resolve svn_opt_revision_date or
+   svn_opt_revision_head revisions, then the session used to do this is
+   returned in RA_SESSION and the resolved location used to open the session
+   is returned in *RESOLVED_LOC_P (see parameter of the same name in
+   svn_client__ra_session_from_path2).  If the repository is not contacted,
+   then set *RA_SESSION and *RESOLVED_LOC_P to NULL. */
+static svn_error_t*
+convert_opt_rev_array_to_rev_range_array(
+  apr_array_header_t **revision_ranges,
+  svn_ra_session_t **ra_session,
+  svn_client__pathrev_t **resolved_loc_p,
+  svn_revnum_t *youngest_rev,
+  svn_revnum_t *oldest_rev,
+  const char *url_or_abspath,
+  const apr_array_header_t *opt_rev_ranges,
+  const svn_opt_revision_t *peg_rev,
+  svn_client_ctx_t *ctx,
+  apr_pool_t *result_pool,
+  apr_pool_t *scratch_pool)
+{
+  int i;
+
+  /* Initialize the input/output parameters. */
+  *youngest_rev = *oldest_rev = SVN_INVALID_REVNUM;
+  *ra_session = NULL;
+  *resolved_loc_p = NULL;
+
+  /* Convert OPT_REV_RANGES to an array of rev_range_t and find the youngest
+     and oldest revision range that spans all of OPT_REV_RANGES. */
+  *revision_ranges = apr_array_make(result_pool, opt_rev_ranges->nelts,
+                                    sizeof(rev_range_t *));
+
+  for (i = 0; i < opt_rev_ranges->nelts; i++)
     {
       svn_opt_revision_range_t *range;
+      rev_range_t *rev_range;
+      svn_boolean_t start_same_as_end = FALSE;
 
-      range = APR_ARRAY_IDX(revision_ranges, i, svn_opt_revision_range_t *);
+      range = APR_ARRAY_IDX(opt_rev_ranges, i, svn_opt_revision_range_t *);
 
+      /* Right now RANGE can be any valid pair of svn_opt_revision_t's.  We
+         will now convert all RANGEs in place to the corresponding
+         svn_opt_revision_number kind. */
       if ((range->start.kind != svn_opt_revision_unspecified)
           && (range->end.kind == svn_opt_revision_unspecified))
         {
@@ -329,15 +609,15 @@ svn_client_log5(const apr_array_header_t
           /* Default to any specified peg revision.  Otherwise, if the
            * first target is a URL, then we default to HEAD:0.  Lastly,
            * the default is BASE:0 since WC@HEAD may not exist. */
-          if (peg_rev.kind == svn_opt_revision_unspecified)
+          if (peg_rev->kind == svn_opt_revision_unspecified)
             {
-              if (svn_path_is_url(url_or_path))
+              if (svn_path_is_url(url_or_abspath))
                 range->start.kind = svn_opt_revision_head;
               else
                 range->start.kind = svn_opt_revision_base;
             }
           else
-            range->start = peg_rev;
+            range->start = *peg_rev;
 
           if (range->end.kind == svn_opt_revision_unspecified)
             {
@@ -354,163 +634,128 @@ svn_client_log5(const apr_array_header_t
              _("Missing required revision specification"));
         }
 
-      /* Determine the revision to open the RA session to. */
-      if (session_opt_rev.kind == svn_opt_revision_unspecified)
-        {
-          if (range->start.kind == svn_opt_revision_number &&
-              range->end.kind == svn_opt_revision_number)
-            {
-              session_opt_rev =
-                  (range->start.value.number > range->end.value.number ?
-                   range->start : range->end);
-            }
-          else if (range->start.kind == svn_opt_revision_head ||
-                   range->end.kind == svn_opt_revision_head)
-            {
-              session_opt_rev.kind = svn_opt_revision_head;
-            }
-          else if (range->start.kind == svn_opt_revision_date &&
-                   range->end.kind == svn_opt_revision_date)
-            {
-              session_opt_rev =
-                  (range->start.value.date > range->end.value.date ?
-                   range->start : range->end);
-            }
-        }
-    }
-
-  /* Use the passed URL, if there is one.  */
-  if (svn_path_is_url(url_or_path))
-    {
-      /* Initialize this array, since we'll be building it below */
-      condensed_targets = apr_array_make(pool, 1, sizeof(const char *));
-
-      /* The logic here is this: If we get passed one argument, we assume
-         it is the full URL to a file/dir we want log info for. If we get
-         a URL plus some paths, then we assume that the URL is the base,
-         and that the paths passed are relative to it.  */
-      if (targets->nelts > 1)
-        {
-          /* We have some paths, let's use them. Start after the URL.  */
-          for (i = 1; i < targets->nelts; i++)
-            {
-              const char *target;
-
-              target = APR_ARRAY_IDX(targets, i, const char *);
-
-              if (svn_path_is_url(target) || svn_dirent_is_absolute(target))
-                return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
-                                         _("'%s' is not a relative path"),
-                                          target);
-
-              APR_ARRAY_PUSH(condensed_targets, const char *) = target;
-            }
-        }
+      if (memcmp(&(range->start), &(range->end),
+                 sizeof(svn_opt_revision_t)) == 0)
+        start_same_as_end = TRUE;
+
+      /* If the caller asked for COMMITTED, PREVIOUS, BASE, or WORKING then
+         resolve those to svn_opt_revision_number opt revs. */
+      SVN_ERR(resolve_wc_opt_revs(&(range->start), url_or_abspath, ctx,
+                                  scratch_pool));
+      if (start_same_as_end)
+        range->end = range->start;
       else
-        {
-          /* If we have a single URL, then the session will be rooted at
-             it, so just send an empty string for the paths we are
-             interested in. */
-          APR_ARRAY_PUSH(condensed_targets, const char *) = "";
-        }
+        SVN_ERR(resolve_wc_opt_revs(&(range->end), url_or_abspath, ctx,
+                                    scratch_pool));
 
-      /* Remember that our targets are URLs. */
-      url_targets = TRUE;
+      /* If the caller asked about the head revision we need to resolve that
+         to a revision number and possibly open a RA session. */
+      SVN_ERR(resolve_wc_head_revs(&(range->start), ra_session,
+                                   resolved_loc_p, youngest_rev,
+                                   url_or_abspath, peg_rev, ctx,
+                                   result_pool, scratch_pool));
+      if (start_same_as_end)
+        range->end = range->start;
+      else
+        SVN_ERR(resolve_wc_head_revs(&(range->end), ra_session,
+                                     resolved_loc_p, youngest_rev,
+                                     url_or_abspath, peg_rev, ctx,
+                                     result_pool, scratch_pool));
+
+      /* If the caller gave us dates we need to resolve them to revision
+         numbers and possibly open a RA session. */
+      SVN_ERR(resolve_wc_date_revs(&(range->start), ra_session,
+                                   resolved_loc_p, url_or_abspath, peg_rev,
+                                   ctx, result_pool, scratch_pool));
+      if (start_same_as_end)
+        range->end = range->start;
+      else
+        SVN_ERR(resolve_wc_date_revs(&(range->end), ra_session,
+                                     resolved_loc_p, url_or_abspath, peg_rev,
+                                     ctx, result_pool, scratch_pool));
+
+      /* Possibly update the oldest and youngest revisions requested. */
+      find_youngest_and_oldest_revs(youngest_rev,
+                                    oldest_rev,
+                                    range->start.value.number);
+      find_youngest_and_oldest_revs(youngest_rev,
+                                    oldest_rev,
+                                    range->end.value.number);
+      rev_range = apr_palloc(result_pool, sizeof(*rev_range));
+      rev_range->range_start = range->start.value.number;
+      rev_range->range_end = range->end.value.number;
+      APR_ARRAY_PUSH(*revision_ranges, rev_range_t *) = rev_range;
     }
-  else
-    {
-      apr_array_header_t *target_urls;
-      apr_array_header_t *real_targets;
 
-      /* See FIXME about multiple wc targets, below. */
-      if (targets->nelts > 1)
-        return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
-                                _("When specifying working copy paths, only "
-                                  "one target may be given"));
+  return SVN_NO_ERROR;
+}
 
-      /* An unspecified PEG_REVISION for a working copy path defaults
-         to svn_opt_revision_working. */
-      if (peg_rev.kind == svn_opt_revision_unspecified)
-          peg_rev.kind = svn_opt_revision_working;
+static int
+compare_rev_to_segment(const void *key_p,
+                       const void *element_p)
+{
+  svn_revnum_t rev =
+    * (svn_revnum_t *)key_p;
+  const svn_location_segment_t *segment =
+    *((const svn_location_segment_t * const *) element_p);
+
+  if (rev < segment->range_start)
+    return -1;
+  else if (rev > segment->range_end)
+    return 1;
+  else
+    return 0;
+}
 
-      /* Get URLs for each target */
-      target_urls = apr_array_make(pool, 1, sizeof(const char *));
-      real_targets = apr_array_make(pool, 1, sizeof(const char *));
-      iterpool = svn_pool_create(pool);
-      for (i = 0; i < targets->nelts; i++)
-        {
-          const char *url;
-          const char *target = APR_ARRAY_IDX(targets, i, const char *);
-          const char *target_abspath;
-
-          svn_pool_clear(iterpool);
-          SVN_ERR(svn_dirent_get_absolute(&target_abspath, target, iterpool));
-          SVN_ERR(svn_wc__node_get_url(&url, ctx->wc_ctx, target_abspath,
-                                       pool, iterpool));
-
-          if (! url)
-            return svn_error_createf
-              (SVN_ERR_ENTRY_MISSING_URL, NULL,
-               _("Entry '%s' has no URL"),
-               svn_dirent_local_style(target, pool));
-
-          APR_ARRAY_PUSH(target_urls, const char *) = url;
-          APR_ARRAY_PUSH(real_targets, const char *) = target;
-        }
-
-      /* if we have no valid target_urls, just exit. */
-      if (target_urls->nelts == 0)
-        return SVN_NO_ERROR;
-
-      /* Find the base URL and condensed targets relative to it. */
-      SVN_ERR(svn_uri_condense_targets(&url_or_path, &condensed_targets,
-                                       target_urls, TRUE, pool, iterpool));
-
-      if (condensed_targets->nelts == 0)
-        APR_ARRAY_PUSH(condensed_targets, const char *) = "";
-
-      /* 'targets' now becomes 'real_targets', which has bogus,
-         unversioned things removed from it. */
-      targets = real_targets;
-      svn_pool_destroy(iterpool);
-    }
-
-
-  {
-    svn_client__pathrev_t *actual_loc;
-
-    /* If this is a revision type that requires access to the working copy,
-     * we use our initial target path to figure out where to root the RA
-     * session, otherwise we use our URL. */
-    if (SVN_CLIENT__REVKIND_NEEDS_WC(peg_rev.kind))
-      {
-        if (url_targets)
-          SVN_ERR(svn_uri_condense_targets(&ra_target, NULL, targets,
-                                           TRUE, pool, pool));
-        else
-          SVN_ERR(svn_dirent_condense_targets(&ra_target, NULL, targets,
-                                              TRUE, pool, pool));
-      }
-    else
-      ra_target = url_or_path;
-
-    SVN_ERR(svn_client__ra_session_from_path2(&ra_session, &actual_loc,
-                                             ra_target, NULL,
-                                             &peg_rev, &session_opt_rev,
-                                             ctx, pool));
+/* Run svn_ra_get_log2 for PATHS, one or more paths relative to RA_SESSION's
+   common parent, for each revision in REVISION_RANGES, an array of
+   rev_range_t.
+
+   RA_SESSION is an open session pointing to ACTUAL_LOC.
+
+   LOG_SEGMENTS is an array of svn_location_segment_t * items representing the
+   history of PATHS from the oldest to youngest revisions found in
+   REVISION_RANGES.
+
+   The TARGETS, LIMIT, DISCOVER_CHANGED_PATHS, STRICT_NODE_HISTORY,
+   INCLUDE_MERGED_REVISIONS, REVPROPS, REAL_RECEIVER, and REAL_RECEIVER_BATON
+   parameters are all as per the svn_client_log5 API. */
+static svn_error_t *
+run_ra_get_log(apr_array_header_t *revision_ranges,
+               apr_array_header_t *paths,
+               apr_array_header_t *log_segments,
+               svn_client__pathrev_t *actual_loc,
+               svn_ra_session_t *ra_session,
+               /* The following are as per svn_client_log5. */ 
+               const apr_array_header_t *targets,
+               int limit,
+               svn_boolean_t discover_changed_paths,
+               svn_boolean_t strict_node_history,
+               svn_boolean_t include_merged_revisions,
+               const apr_array_header_t *revprops,
+               svn_log_entry_receiver_t real_receiver,
+               void *real_receiver_baton,
+               svn_client_ctx_t *ctx,
+               apr_pool_t *scratch_pool)
+{
+  int i;
+  pre_15_receiver_baton_t rb = {0};
+  apr_pool_t *iterpool;
+  svn_boolean_t has_log_revprops;
 
-    SVN_ERR(svn_ra_has_capability(ra_session, &has_log_revprops,
-                                  SVN_RA_CAPABILITY_LOG_REVPROPS, pool));
+  SVN_ERR(svn_ra_has_capability(ra_session, &has_log_revprops,
+                                SVN_RA_CAPABILITY_LOG_REVPROPS,
+                                scratch_pool));
 
-    if (!has_log_revprops) {
+  if (!has_log_revprops)
+    {
       /* See above pre-1.5 notes. */
       rb.ctx = ctx;
 
       /* Create ra session on first use */
-      rb.ra_session_pool = pool;
+      rb.ra_session_pool = scratch_pool;
       rb.ra_session_url = actual_loc->url;
     }
-  }
 
   /* It's a bit complex to correctly handle the special revision words
    * such as "BASE", "COMMITTED", and "PREV".  For example, if the
@@ -559,35 +804,55 @@ svn_client_log5(const apr_array_header_t
    * epg wonders if the repository could send a unified stream of log
    * entries if the paths and revisions were passed down.
    */
-  iterpool = svn_pool_create(pool);
+  iterpool = svn_pool_create(scratch_pool);
   for (i = 0; i < revision_ranges->nelts; i++)
     {
-      svn_revnum_t start_revnum, end_revnum, youngest_rev = SVN_INVALID_REVNUM;
+      const char *old_session_url;
       const char *path = APR_ARRAY_IDX(targets, 0, const char *);
       const char *local_abspath_or_url;
-      svn_opt_revision_range_t *range;
+      const char *segment_url = actual_loc->url;
+      rev_range_t *range;
       limit_receiver_baton_t lb;
       svn_log_entry_receiver_t passed_receiver;
       void *passed_receiver_baton;
       const apr_array_header_t *passed_receiver_revprops;
+      svn_location_segment_t **matching_segment;
+      svn_revnum_t younger_rev;
 
       svn_pool_clear(iterpool);
 
       if (!svn_path_is_url(path))
-        SVN_ERR(svn_dirent_get_absolute(&local_abspath_or_url, path, iterpool));
+        SVN_ERR(svn_dirent_get_absolute(&local_abspath_or_url, path,
+                                        iterpool));
       else
         local_abspath_or_url = path;
 
-      range = APR_ARRAY_IDX(revision_ranges, i, svn_opt_revision_range_t *);
+      range = APR_ARRAY_IDX(revision_ranges, i, rev_range_t *);
 
-      SVN_ERR(svn_client__get_revision_number(&start_revnum, &youngest_rev,
-                                              ctx->wc_ctx, local_abspath_or_url,
-                                              ra_session, &range->start,
-                                              iterpool));
-      SVN_ERR(svn_client__get_revision_number(&end_revnum, &youngest_rev,
-                                              ctx->wc_ctx, local_abspath_or_url,
-                                              ra_session, &range->end,
-                                              iterpool));
+      /* Issue #4355: Account for renames spanning requested
+         revision ranges. */
+      younger_rev = MAX(range->range_start, range->range_end);
+      matching_segment = bsearch(&younger_rev, log_segments->elts,
+                                 log_segments->nelts, log_segments->elt_size,
+                                 compare_rev_to_segment);
+      SVN_ERR_ASSERT(*matching_segment);
+      
+      /* A segment with a NULL path means there is gap in the history.
+         We'll just proceed and let svn_ra_get_log2 fail with a useful
+         error...*/
+      if ((*matching_segment)->path != NULL)
+        {
+          /* ...but if there is history, then we must account for issue
+             #4355 and make sure our RA session is pointing at the correct
+             location. */
+          segment_url = svn_path_url_add_component2(
+            actual_loc->repos_root_url, (*matching_segment)->path,
+            scratch_pool);
+          SVN_ERR(svn_client__ensure_ra_session_url(&old_session_url,
+                                                    ra_session,
+                                                    segment_url,
+                                                    scratch_pool));
+        }
 
       if (has_log_revprops)
         {
@@ -617,9 +882,9 @@ svn_client_log5(const apr_array_header_t
         }
 
       SVN_ERR(svn_ra_get_log2(ra_session,
-                              condensed_targets,
-                              start_revnum,
-                              end_revnum,
+                              paths,
+                              range->range_start,
+                              range->range_end,
                               limit,
                               discover_changed_paths,
                               strict_node_history,
@@ -642,3 +907,98 @@ svn_client_log5(const apr_array_header_t
 
   return SVN_NO_ERROR;
 }
+
+/*** Public Interface. ***/
+
+svn_error_t *
+svn_client_log5(const apr_array_header_t *targets,
+                const svn_opt_revision_t *peg_revision,
+                const apr_array_header_t *opt_rev_ranges,
+                int limit,
+                svn_boolean_t discover_changed_paths,
+                svn_boolean_t strict_node_history,
+                svn_boolean_t include_merged_revisions,
+                const apr_array_header_t *revprops,
+                svn_log_entry_receiver_t real_receiver,
+                void *real_receiver_baton,
+                svn_client_ctx_t *ctx,
+                apr_pool_t *pool)
+{
+  svn_ra_session_t *ra_session;
+  const char *url_or_path;
+  const char *old_session_url;
+  const char *ra_target;
+  svn_opt_revision_t youngest_opt_rev;
+  svn_revnum_t youngest_rev;
+  svn_revnum_t oldest_rev;
+  svn_opt_revision_t peg_rev;
+  svn_client__pathrev_t *actual_loc;
+  apr_array_header_t *log_segments;
+  apr_array_header_t *revision_ranges;
+  apr_array_header_t *relative_targets;
+
+  if (opt_rev_ranges->nelts == 0)
+    {
+      return svn_error_create
+        (SVN_ERR_CLIENT_BAD_REVISION, NULL,
+         _("Missing required revision specification"));
+    }
+
+  /* Make a copy of PEG_REVISION, we may need to change it to a
+     default value. */
+  peg_rev = *peg_revision;
+
+  SVN_ERR(resolve_log_targets(&relative_targets, &ra_target, &peg_rev,
+                              targets, ctx, pool, pool));
+
+  /* Use the passed URL, if there is one.  */
+  url_or_path = APR_ARRAY_IDX(targets, 0, const char *);
+
+  /* Convert OPT_REV_RANGES to an array of rev_range_t and find the youngest
+     and oldest revision range that spans all of OPT_REV_RANGES. */
+  SVN_ERR(convert_opt_rev_array_to_rev_range_array(&revision_ranges,
+                                                   &ra_session, &actual_loc,
+                                                   &youngest_rev,
+                                                   &oldest_rev,
+                                                   ra_target,
+                                                   opt_rev_ranges, &peg_rev,
+                                                   ctx, pool,  pool));
+  youngest_opt_rev.kind = svn_opt_revision_number;
+  youngest_opt_rev.value.number = youngest_rev;
+
+  /* If we didn't already open a RA session above, do so now. */
+  if (!ra_session)
+    {
+      SVN_ERR(svn_client__ra_session_from_path2(&ra_session, &actual_loc,
+                                                ra_target, NULL,
+                                                &peg_rev, &youngest_opt_rev,
+                                                ctx, pool));
+    }
+  else
+    {
+      /* We already opened a session above to resolve dates or head to
+         revision numbers.  So just make sure RA_SESSION points to the
+         right URL. */
+      SVN_ERR(svn_client__resolve_rev_and_url(&actual_loc, ra_session,
+                                              ra_target, &peg_rev,
+                                              &youngest_opt_rev, ctx, pool));
+      SVN_ERR(svn_client__ensure_ra_session_url(&old_session_url, ra_session,
+                                                actual_loc->url, pool));
+    }
+
+  /* Get the svn_location_segment_t's representing the requested log ranges. */
+  SVN_ERR(svn_client__repos_location_segments(&log_segments, ra_session,
+                                              actual_loc->url,
+                                              actual_loc->rev, /* peg */
+                                              actual_loc->rev, /* start */
+                                              oldest_rev,      /* end */
+                                              ctx, pool));
+
+  SVN_ERR(run_ra_get_log(revision_ranges, relative_targets, log_segments,
+                         actual_loc, ra_session, targets, limit,
+                         discover_changed_paths, strict_node_history,
+                         include_merged_revisions, revprops, real_receiver,
+                         real_receiver_baton, ctx, pool));
+
+  return SVN_NO_ERROR;
+}

Modified: subversion/trunk/subversion/libsvn_client/ra.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/ra.c?rev=1478220&r1=1478219&r2=1478220&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/ra.c (original)
+++ subversion/trunk/subversion/libsvn_client/ra.c Wed May  1 22:27:31 2013
@@ -447,39 +447,14 @@ svn_client_open_ra_session2(svn_ra_sessi
                                                   scratch_pool));
 }
 
-
-
-
-/* Given PATH_OR_URL, which contains either a working copy path or an
-   absolute URL, a peg revision PEG_REVISION, and a desired revision
-   REVISION, find the path at which that object exists in REVISION,
-   following copy history if necessary.  If REVISION is younger than
-   PEG_REVISION, then check that PATH_OR_URL is the same node in both
-   PEG_REVISION and REVISION, and return @c
-   SVN_ERR_CLIENT_UNRELATED_RESOURCES if it is not the same node.
-
-   If PEG_REVISION->kind is 'unspecified', the peg revision is 'head'
-   for a URL or 'working' for a WC path.  If REVISION->kind is
-   'unspecified', the operative revision is the peg revision.
-
-   Store the actual location of the object in *RESOLVED_LOC_P.
-
-   RA_SESSION should be an open RA session pointing at the URL of
-   PATH_OR_URL, or NULL, in which case this function will open its own
-   temporary session.
-
-   Use authentication baton cached in CTX to authenticate against the
-   repository.
-
-   Use POOL for all allocations. */
-static svn_error_t *
-resolve_rev_and_url(svn_client__pathrev_t **resolved_loc_p,
-                    svn_ra_session_t *ra_session,
-                    const char *path_or_url,
-                    const svn_opt_revision_t *peg_revision,
-                    const svn_opt_revision_t *revision,
-                    svn_client_ctx_t *ctx,
-                    apr_pool_t *pool)
+svn_error_t *
+svn_client__resolve_rev_and_url(svn_client__pathrev_t **resolved_loc_p,
+                                svn_ra_session_t *ra_session,
+                                const char *path_or_url,
+                                const svn_opt_revision_t *peg_revision,
+                                const svn_opt_revision_t *revision,
+                                svn_client_ctx_t *ctx,
+                                apr_pool_t *pool)
 {
   svn_opt_revision_t peg_rev = *peg_revision;
   svn_opt_revision_t start_rev = *revision;
@@ -545,9 +520,9 @@ svn_client__ra_session_from_path2(svn_ra
   if (corrected_url && svn_path_is_url(path_or_url))
     path_or_url = corrected_url;
 
-  SVN_ERR(resolve_rev_and_url(&resolved_loc, ra_session,
-                              path_or_url, peg_revision, revision,
-                              ctx, pool));
+  SVN_ERR(svn_client__resolve_rev_and_url(&resolved_loc, ra_session,
+                                          path_or_url, peg_revision, revision,
+                                          ctx, pool));
 
   /* Make the session point to the real URL. */
   SVN_ERR(svn_ra_reparent(ra_session, resolved_loc->url, pool));
@@ -1009,9 +984,9 @@ svn_client__youngest_common_ancestor(con
                                             path_or_url1, NULL,
                                             revision1, revision1,
                                             ctx, sesspool));
-  SVN_ERR(resolve_rev_and_url(&loc2, session,
-                              path_or_url2, revision2, revision2,
-                              ctx, scratch_pool));
+  SVN_ERR(svn_client__resolve_rev_and_url(&loc2, session,
+                                          path_or_url2, revision2, revision2,
+                                          ctx, scratch_pool));
 
   SVN_ERR(svn_client__get_youngest_common_ancestor(
             &ancestor, loc1, loc2, session, ctx, result_pool, scratch_pool));

Modified: subversion/trunk/subversion/tests/cmdline/authz_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/authz_tests.py?rev=1478220&r1=1478219&r2=1478220&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/authz_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/authz_tests.py Wed May  1 22:27:31 2013
@@ -576,7 +576,8 @@ def authz_log_and_tracing_test(sbox):
   if sbox.repo_url.startswith('http'):
     expected_err2 = expected_err
   else:
-    expected_err2 = ".*svn: E220001: Item is not readable.*"
+    expected_err2 = ".*svn: E220001: Unreadable path encountered; " \
+                    "access denied.*"
 
   # if we do the same thing directly on the unreadable file, we get:
   # svn: Item is not readable

Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/log_tests.py?rev=1478220&r1=1478219&r2=1478220&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/log_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/log_tests.py Wed May  1 22:27:31 2013
@@ -2369,7 +2369,6 @@ def merge_sensitive_log_with_search(sbox
 # Test for issue #4355 'svn_client_log5 broken with multiple revisions
 # which span a rename'.
 @Issue(4355)
-@XFail()
 @SkipUnless(server_has_mergeinfo)
 def log_multiple_revs_spanning_rename(sbox):
   "log for multiple revs which span a rename"
@@ -2403,6 +2402,7 @@ def log_multiple_revs_spanning_rename(sb
   svntest.main.file_write(msg_file, msg)
   svntest.main.file_append(mu_path2, "4")
   svntest.main.run_svn(None, 'ci', '-F', msg_file, wc_dir)
+  svntest.main.run_svn(None, 'up', wc_dir)
 
   # Check that log can handle a revision range that spans a rename.
   exit_code, output, err = svntest.actions.run_and_verify_svn(
@@ -2456,7 +2456,8 @@ def log_multiple_revs_spanning_rename(sb
   log_chain = parse_log_output(output)
   check_log_chain(log_chain, [1,4,3,2])
 
-  # As above, but revision ranges from younger to older revs fail:
+  # As above, but revision ranges from younger to older.  Previously this
+  # failed with:
   #
   #  >svn log ^/trunk -r1:1 -r2:4
   #  ------------------------------------------------------------------------



Mime
View raw message