You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/05/14 00:44:17 UTC

svn commit: r1482139 - in /subversion/branches/1.8.x: ./ STATUS subversion/include/private/svn_client_private.h subversion/libsvn_client/log.c subversion/libsvn_client/ra.c subversion/tests/cmdline/authz_tests.py subversion/tests/cmdline/log_tests.py

Author: breser
Date: Mon May 13 22:44:16 2013
New Revision: 1482139

URL: http://svn.apache.org/r1482139
Log:
Merge the r1469363 group from trunk:

 * r1469363, r1469645, r1469674, r1470037, r1470537, r1478220, r1478221,
   r1478465, r1478998, r1480723
   Fix issue #4355 'svn_client_log5 broken with multiple revisions which
   span a rename'.
   Justification:
     svn_client_log5 doesn't deliver what it promises, see
     http://subversion.tigris.org/issues/show_bug.cgi?id=4355#desc1 for
     a brief example of the problem.
   Notes:
     r1469363, r1469645, r1469674, r1470037, and r1470537 are the creation
     and subsequent tweaks of a single new log test for this issue.  r1478220
     is the fix.  r1478221, r1478465, r1478998, and r1480723 are (mostly)
     minor follow-ups to that fix.
   Votes:
     +1: pburba, julianfoad, breser

Modified:
    subversion/branches/1.8.x/   (props changed)
    subversion/branches/1.8.x/STATUS
    subversion/branches/1.8.x/subversion/include/private/svn_client_private.h
    subversion/branches/1.8.x/subversion/libsvn_client/log.c
    subversion/branches/1.8.x/subversion/libsvn_client/ra.c
    subversion/branches/1.8.x/subversion/tests/cmdline/authz_tests.py
    subversion/branches/1.8.x/subversion/tests/cmdline/log_tests.py

Propchange: subversion/branches/1.8.x/
------------------------------------------------------------------------------
  Merged /subversion/trunk:r1469363,1469645,1469674,1470037,1470537,1478220-1478221,1478465,1478998,1480723

Modified: subversion/branches/1.8.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/STATUS?rev=1482139&r1=1482138&r2=1482139&view=diff
==============================================================================
--- subversion/branches/1.8.x/STATUS (original)
+++ subversion/branches/1.8.x/STATUS Mon May 13 22:44:16 2013
@@ -124,20 +124,3 @@ Approved changes:
 # that would restart the soak should not be added unless they are resolving
 # blocking issues.  If in doubt see this link for details:
 # http://subversion.apache.org/docs/community-guide/releasing.html#release-stabilization
-
- * r1469363, r1469645, r1469674, r1470037, r1470537, r1478220, r1478221,
-   r1478465, r1478998, r1480723
-   Fix issue #4355 'svn_client_log5 broken with multiple revisions which
-   span a rename'.
-   Justification:
-     svn_client_log5 doesn't deliver what it promises, see
-     http://subversion.tigris.org/issues/show_bug.cgi?id=4355#desc1 for
-     a brief example of the problem.
-   Notes:
-     r1469363, r1469645, r1469674, r1470037, and r1470537 are the creation
-     and subsequent tweaks of a single new log test for this issue.  r1478220
-     is the fix.  r1478221, r1478465, r1478998, and r1480723 are (mostly)
-     minor follow-ups to that fix.
-   Votes:
-     +1: pburba, julianfoad, breser
-

Modified: subversion/branches/1.8.x/subversion/include/private/svn_client_private.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/include/private/svn_client_private.h?rev=1482139&r1=1482138&r2=1482139&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/include/private/svn_client_private.h (original)
+++ subversion/branches/1.8.x/subversion/include/private/svn_client_private.h Mon May 13 22:44:16 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/branches/1.8.x/subversion/libsvn_client/log.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/libsvn_client/log.c?rev=1482139&r1=1482138&r2=1482139&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/libsvn_client/log.c (original)
+++ subversion/branches/1.8.x/subversion/libsvn_client/log.c Mon May 13 22:44:16 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,225 @@ 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 *RELATIVE_TARGETS 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"));
+
+  /* Initialize the output array.  At a minimum, we need room for one
+     (possibly empty) relpath.  Otherwise, we have to hold a relpath
+     for every item in TARGETS except the first.  */
+  *relative_targets = apr_array_make(result_pool,
+                                     MAX(1, targets->nelts - 1),
+                                     sizeof(const char *));
 
-  if (revision_ranges->nelts == 0)
+  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;
+}
+
+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
+   RA_SESSION; it must be an open session to any URL in the right repository.
+*/
+static svn_error_t*
+convert_opt_rev_array_to_rev_range_array(
+  apr_array_header_t **revision_ranges,
+  svn_revnum_t *youngest_rev,
+  svn_revnum_t *oldest_rev,
+  svn_ra_session_t *ra_session,
+  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;
+  svn_revnum_t head_rev = SVN_INVALID_REVNUM;
+
+  /* Initialize the input/output parameters. */
+  *youngest_rev = *oldest_rev = SVN_INVALID_REVNUM;
+
+  /* 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 +498,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 +523,117 @@ 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)
+      /* Does RANGE describe a single svn_opt_revision_t? */
+      if (range->start.kind == range->end.kind)
         {
-          if (range->start.kind == svn_opt_revision_number &&
-              range->end.kind == svn_opt_revision_number)
+          if (range->start.kind == svn_opt_revision_number)
             {
-              session_opt_rev =
-                  (range->start.value.number > range->end.value.number ?
-                   range->start : range->end);
+              if (range->start.value.number == range->end.value.number)
+                start_same_as_end = TRUE;
             }
-          else if (range->start.kind == svn_opt_revision_head ||
-                   range->end.kind == svn_opt_revision_head)
+          else if (range->start.kind == svn_opt_revision_date)
             {
-              session_opt_rev.kind = svn_opt_revision_head;
+              if (range->start.value.date == range->end.value.date)
+                start_same_as_end = TRUE;
             }
-          else if (range->start.kind == svn_opt_revision_date &&
-                   range->end.kind == svn_opt_revision_date)
+          else
             {
-              session_opt_rev =
-                  (range->start.value.date > range->end.value.date ?
-                   range->start : range->end);
+              start_same_as_end = TRUE;
             }
         }
-    }
-
-  /* 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;
-            }
-        }
+      rev_range = apr_palloc(result_pool, sizeof(*rev_range));
+      SVN_ERR(svn_client__get_revision_number(
+                &rev_range->range_start, &head_rev,
+                ctx->wc_ctx, url_or_abspath, ra_session,
+                &range->start, scratch_pool));
+      if (start_same_as_end)
+        rev_range->range_end = rev_range->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 *) = "";
-        }
-
-      /* Remember that our targets are URLs. */
-      url_targets = TRUE;
+        SVN_ERR(svn_client__get_revision_number(
+                  &rev_range->range_end, &head_rev,
+                  ctx->wc_ctx, url_or_abspath, ra_session,
+                  &range->end, scratch_pool));
+
+      /* Possibly update the oldest and youngest revisions requested. */
+      find_youngest_and_oldest_revs(youngest_rev,
+                                    oldest_rev,
+                                    rev_range->range_start);
+      find_youngest_and_oldest_revs(youngest_rev,
+                                    oldest_rev,
+                                    rev_range->range_end);
+      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 +682,54 @@ 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;
+      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. */
+          const char *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 +759,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 +784,85 @@ 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 *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));
+
+  SVN_ERR(svn_client__ra_session_from_path2(&ra_session, &actual_loc,
+                                            ra_target, NULL, &peg_rev, &peg_rev,
+                                            ctx, pool));
+
+  /* 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,
+                                                   &youngest_rev,
+                                                   &oldest_rev,
+                                                   ra_session,
+                                                   ra_target,
+                                                   opt_rev_ranges, &peg_rev,
+                                                   ctx, pool,  pool));
+
+  /* Make ACTUAL_LOC and RA_SESSION point to the youngest operative rev. */
+  youngest_opt_rev.kind = svn_opt_revision_number;
+  youngest_opt_rev.value.number = youngest_rev;
+  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/branches/1.8.x/subversion/libsvn_client/ra.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/libsvn_client/ra.c?rev=1482139&r1=1482138&r2=1482139&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/libsvn_client/ra.c (original)
+++ subversion/branches/1.8.x/subversion/libsvn_client/ra.c Mon May 13 22:44:16 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/branches/1.8.x/subversion/tests/cmdline/authz_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/tests/cmdline/authz_tests.py?rev=1482139&r1=1482138&r2=1482139&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/tests/cmdline/authz_tests.py (original)
+++ subversion/branches/1.8.x/subversion/tests/cmdline/authz_tests.py Mon May 13 22:44:16 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/branches/1.8.x/subversion/tests/cmdline/log_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/tests/cmdline/log_tests.py?rev=1482139&r1=1482138&r2=1482139&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/tests/cmdline/log_tests.py (original)
+++ subversion/branches/1.8.x/subversion/tests/cmdline/log_tests.py Mon May 13 22:44:16 2013
@@ -2365,6 +2365,133 @@ def merge_sensitive_log_with_search(sbox
   }
   check_merge_results(log_chain, expected_merges)
 
+#----------------------------------------------------------------------
+# Test for issue #4355 'svn_client_log5 broken with multiple revisions
+# which span a rename'.
+@Issue(4355)
+@SkipUnless(server_has_mergeinfo)
+def log_multiple_revs_spanning_rename(sbox):
+  "log for multiple revs which span a rename"
+
+  sbox.build()
+  wc_dir = sbox.wc_dir
+  msg_file=os.path.join(sbox.repo_dir, 'log-msg')
+  msg_file=os.path.abspath(msg_file)
+  mu_path1 = os.path.join(wc_dir, 'A', 'mu')
+  mu_path2 = os.path.join(wc_dir, 'trunk', 'mu')
+  trunk_path = os.path.join(wc_dir, 'trunk')
+
+  # r2 - Change a file.
+  msg=""" Log message for revision 2
+  but with multiple lines
+  to test the code"""
+  svntest.main.file_write(msg_file, msg)
+  svntest.main.file_append(mu_path1, "2")
+  svntest.main.run_svn(None, 'ci', '-F', msg_file, wc_dir)
+
+  # r3 - Rename that file's parent.
+  svntest.main.run_svn(None, 'up', wc_dir)
+  sbox.simple_move('A', 'trunk')
+  svntest.main.run_svn(None, 'ci', '-m', "Log message for revision 3",
+                       wc_dir)
+
+  # r4 - Change the file again.
+  msg=""" Log message for revision 4
+  but with multiple lines
+  to test the code"""
+  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(
+    None, None, [], 'log', '-r2:4', sbox.repo_url + '/trunk/mu')
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [2,3,4])
+
+  # Check that log can handle discrete revisions that don't span a rename.
+  exit_code, output, err = svntest.actions.run_and_verify_svn(
+    None, None, [], 'log', '-c3,4', sbox.repo_url + '/trunk/mu')
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [3,4])
+
+  # Check that log can handle discrete revisions that span a rename.
+  #
+  # Currently this fails with:
+  #
+  #   >svn log ^/trunk -c2,3,1
+  #   ------------------------------------------------------------------------
+  #   r2 | jrandom | 2013-04-18 19:58:47 -0400 (Thu, 18 Apr 2013) | 3 lines
+  #
+  #    Log message for revision 2
+  #     but with multiple lines
+  #     to test the code
+  #   ------------------------------------------------------------------------
+  #   r3 | jrandom | 2013-04-18 19:58:47 -0400 (Thu, 18 Apr 2013) | 1 line
+  #
+  #   Log message for revision 3
+  #   ..\..\..\subversion\svn\log-cmd.c:868,
+  #   ..\..\..\subversion\libsvn_client\log.c:641,
+  #   ..\..\..\subversion\libsvn_repos\log.c:1931,
+  #   ..\..\..\subversion\libsvn_repos\log.c:1358,
+  #   ..\..\..\subversion\libsvn_fs\fs-loader.c:979,
+  #   ..\..\..\subversion\libsvn_fs_fs\tree.c:3205:
+  #     (apr_err=SVN_ERR_FS_NOT_FOUND)
+  #   svn: E160013: File not found: revision 1, path '/trunk'
+  exit_code, output, err = svntest.actions.run_and_verify_svn(
+    None, None, [], 'log', '-c2,3,1', sbox.repo_url + '/trunk/mu')
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [2,3,1])
+
+  # Should work with a WC target too.
+  exit_code, output, err = svntest.actions.run_and_verify_svn(
+    None, None, [], 'log', '-c2,3,1', mu_path2)
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [2,3,1])
+
+  # Discreet revision *ranges* which span a rename should work too.
+  exit_code, output, err = svntest.actions.run_and_verify_svn(
+    None, None, [], 'log', '-r1', '-r4:2', sbox.repo_url + '/trunk')
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [1,4,3,2])
+
+  # As above, but revision ranges from younger to older.  Previously this
+  # failed with:
+  #
+  #  >svn log ^/trunk -r1:1 -r2:4
+  #  ------------------------------------------------------------------------
+  #  r1 | jrandom | 2013-04-18 19:58:46 -0400 (Thu, 18 Apr 2013) | 1 line
+  #
+  #  Log message for revision 1.
+  #  ..\..\..\subversion\svn\log-cmd.c:868,
+  #  ..\..\..\subversion\libsvn_client\log.c:678,
+  #  ..\..\..\subversion\libsvn_repos\log.c:1931,
+  #  ..\..\..\subversion\libsvn_repos\log.c:1358,
+  #  ..\..\..\subversion\libsvn_fs\fs-loader.c:979,
+  #  ..\..\..\subversion\libsvn_fs_fs\tree.c:3205:
+  #    (apr_err=SVN_ERR_FS_NOT_FOUND)
+  #  svn: E160013: File not found: revision 4, path '/A'
+  exit_code, output, err = svntest.actions.run_and_verify_svn(
+    None, None, [], 'log', '-r1', '-r2:4', sbox.repo_url + '/trunk')
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [1,2,3,4])
+
+  # Discrete revs with WC-only opt revs shouldn't cause any problems.
+  exit_code, output, err = svntest.actions.run_and_verify_svn(
+    None, None, [], 'log', '-r1', '-rPREV', trunk_path)
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [1,3])
+
+  exit_code, output, err = svntest.actions.run_and_verify_svn(
+    None, None, [], 'log', '-r1', '-rCOMMITTED', trunk_path)
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [1,4])
+
+  exit_code, output, err = svntest.actions.run_and_verify_svn(
+    None, None, [], 'log', '-r1', '-rBASE', trunk_path)
+  log_chain = parse_log_output(output)
+  check_log_chain(log_chain, [1,4])
 
 ########################################################################
 # Run the tests
@@ -2411,6 +2538,7 @@ test_list = [ None,
               log_diff_moved,
               log_search,
               merge_sensitive_log_with_search,
+              log_multiple_revs_spanning_rename,
              ]
 
 if __name__ == '__main__':