You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2011/12/11 17:36:32 UTC

svn commit: r1213018 - in /subversion/trunk/subversion: libsvn_client/client.h libsvn_client/merge.c libsvn_client/ra.c libsvn_client/switch.c tests/libsvn_client/client-test.c

Author: julianfoad
Date: Sun Dec 11 16:36:32 2011
New Revision: 1213018

URL: http://svn.apache.org/viewvc?rev=1213018&view=rev
Log:
Fix a bug in svn_client__get_youngest_common_ancestor() that returned an
invalid relpath ("/" instead of "") if the youngest common ancestor was the
root of the repository in revision 0.  This may occur when the source of a
merge is the repository root at two different revisions.  I do not know
whether the error would have affected the operation of such a merge.

* subversion/libsvn_client/client.h,
  subversion/libsvn_client/ra.c
  (svn_client__get_youngest_common_ancestor): Fix the r0 special case to
    return ''. Rename the path output parameter to end in '_relpath' for
    clarity.

* subversion/tests/libsvn_client/client-test.c
  (test_youngest_common_ancestor): New test, testing both an ordinary case
    and the special case.
  (test_funcs): Add the new test.

* subversion/libsvn_client/merge.c
  (merge_locked, calculate_left_hand_side, merge_reintegrate_locked): Rename
    the youngest common ancestor path variable to end in '_relpath' for
    clarity.

* subversion/libsvn_client/switch.c
  (switch_internal): Same.

Modified:
    subversion/trunk/subversion/libsvn_client/client.h
    subversion/trunk/subversion/libsvn_client/merge.c
    subversion/trunk/subversion/libsvn_client/ra.c
    subversion/trunk/subversion/libsvn_client/switch.c
    subversion/trunk/subversion/tests/libsvn_client/client-test.c

Modified: subversion/trunk/subversion/libsvn_client/client.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/client.h?rev=1213018&r1=1213017&r2=1213018&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/client.h (original)
+++ subversion/trunk/subversion/libsvn_client/client.h Sun Dec 11 16:36:32 2011
@@ -185,7 +185,7 @@ svn_client__repos_location_segments(apr_
                                     apr_pool_t *pool);
 
 
-/* Set *ANCESTOR_PATH and *ANCESTOR_REVISION to the youngest common
+/* Set *ANCESTOR_RELPATH and *ANCESTOR_REVISION to the youngest common
    ancestor path (a path relative to the root of the repository) and
    revision, respectively, of the two locations identified as
    PATH_OR_URL1@REV1 and PATH_OR_URL2@REV2.  Use the authentication
@@ -193,7 +193,7 @@ svn_client__repos_location_segments(apr_
    This function assumes that PATH_OR_URL1@REV1 and PATH_OR_URL2@REV2
    both refer to the same repository.  Use POOL for all allocations. */
 svn_error_t *
-svn_client__get_youngest_common_ancestor(const char **ancestor_path,
+svn_client__get_youngest_common_ancestor(const char **ancestor_relpath,
                                          svn_revnum_t *ancestor_revision,
                                          const char *path_or_url1,
                                          svn_revnum_t rev1,

Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1213018&r1=1213017&r2=1213018&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Sun Dec 11 16:36:32 2011
@@ -9356,7 +9356,7 @@ merge_locked(const char *source1,
   apr_array_header_t *merge_sources;
   svn_error_t *err;
   svn_boolean_t use_sleep = FALSE;
-  const char *yc_path = NULL;
+  const char *yc_relpath = NULL;
   svn_revnum_t yc_rev = SVN_INVALID_REVNUM;
   apr_pool_t *sesspool;
   svn_boolean_t same_repos;
@@ -9412,7 +9412,7 @@ merge_locked(const char *source1,
 
   /* Unless we're ignoring ancestry, see if the two sources are related.  */
   if (! ignore_ancestry)
-    SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_path, &yc_rev,
+    SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_relpath, &yc_rev,
                                                      source.url1, source.rev1,
                                                      source.url2, source.rev2,
                                                      ctx, scratch_pool));
@@ -9434,18 +9434,18 @@ merge_locked(const char *source1,
                       merge recording, then record-only two merges:
                       from A to C, and from C to B
   */
-  if (yc_path && SVN_IS_VALID_REVNUM(yc_rev))
+  if (yc_relpath && SVN_IS_VALID_REVNUM(yc_rev))
     {
+      /* Make YC_RELPATH into a full URL. */
+      const char *yc_url = svn_path_url_add_component2(source_repos_root.url,
+                                                       yc_url, scratch_pool);
+
       /* Note that our merge sources are related. */
       related = TRUE;
 
-      /* Make YC_PATH into a full URL. */
-      yc_path = svn_path_url_add_component2(source_repos_root.url, yc_path,
-                                            scratch_pool);
-
       /* If the common ancestor matches the right side of our merge,
          then we only need to reverse-merge the left side. */
-      if ((strcmp(yc_path, source.url2) == 0) && (yc_rev == source.rev2))
+      if ((strcmp(yc_url, source.url2) == 0) && (yc_rev == source.rev2))
         {
           ancestral = TRUE;
           SVN_ERR(normalize_merge_sources_internal(
@@ -9455,7 +9455,7 @@ merge_locked(const char *source1,
         }
       /* If the common ancestor matches the left side of our merge,
          then we only need to merge the right side. */
-      else if ((strcmp(yc_path, source.url1) == 0) && (yc_rev == source.rev1))
+      else if ((strcmp(yc_url, source.url1) == 0) && (yc_rev == source.rev1))
         {
           ancestral = TRUE;
           SVN_ERR(normalize_merge_sources_internal(
@@ -10303,7 +10303,7 @@ calculate_left_hand_side(const char **ur
   apr_hash_t *segments_hash = apr_hash_make(scratch_pool);
   svn_boolean_t never_synced;
   svn_revnum_t youngest_merged_rev;
-  const char *yc_ancestor_path;
+  const char *yc_ancestor_relpath;
   svn_revnum_t yc_ancestor_rev;
   const char *source_url;
   const char *target_url;
@@ -10362,12 +10362,12 @@ calculate_left_hand_side(const char **ur
   target_url = svn_path_url_add_component2(source_repos_root,
                                            target_repos_rel_path,
                                            iterpool);
-  SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_ancestor_path,
+  SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_ancestor_relpath,
                                                    &yc_ancestor_rev,
                                                    source_url, source_rev,
                                                    target_url, target_rev,
                                                    ctx, iterpool));
-  if (!(yc_ancestor_path && SVN_IS_VALID_REVNUM(yc_ancestor_rev)))
+  if (!(yc_ancestor_relpath && SVN_IS_VALID_REVNUM(yc_ancestor_rev)))
     return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
                              _("'%s@%ld' must be ancestrally related to "
                                "'%s@%ld'"), source_url, source_rev,
@@ -10430,7 +10430,7 @@ calculate_left_hand_side(const char **ur
     {
       /* We never merged to the source.  Just return the branch point. */
       *url_left = svn_path_url_add_component2(source_repos_root,
-                                              yc_ancestor_path, result_pool);
+                                              yc_ancestor_relpath, result_pool);
       *rev_left = yc_ancestor_rev;
     }
   else
@@ -10462,7 +10462,7 @@ merge_reintegrate_locked(const char *sou
   svn_ra_session_t *target_ra_session;
   svn_ra_session_t *source_ra_session;
   const char *source_repos_rel_path, *target_repos_rel_path;
-  const char *yc_ancestor_path;
+  const char *yc_ancestor_relpath;
   svn_revnum_t yc_ancestor_rev;
   merge_source_t source;
   svn_mergeinfo_t unmerged_to_source_mergeinfo_catalog;
@@ -10577,13 +10577,13 @@ merge_reintegrate_locked(const char *sou
   if (strcmp(source.url1, target_url))
     SVN_ERR(svn_ra_reparent(target_ra_session, source.url1, scratch_pool));
 
-  SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_ancestor_path,
+  SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_ancestor_relpath,
                                                    &yc_ancestor_rev,
                                                    source.url2, source.rev2,
                                                    source.url1, source.rev1,
                                                    ctx, scratch_pool));
 
-  if (!(yc_ancestor_path && SVN_IS_VALID_REVNUM(yc_ancestor_rev)))
+  if (!(yc_ancestor_relpath && SVN_IS_VALID_REVNUM(yc_ancestor_rev)))
     return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
                              _("'%s@%ld' must be ancestrally related to "
                                "'%s@%ld'"),
@@ -10598,7 +10598,7 @@ merge_reintegrate_locked(const char *sou
       svn_mergeinfo_t final_unmerged_catalog = apr_hash_make(scratch_pool);
 
       SVN_ERR(find_unsynced_ranges(source_repos_rel_path,
-                                   yc_ancestor_path,
+                                   yc_ancestor_relpath,
                                    unmerged_to_source_mergeinfo_catalog,
                                    merged_to_source_mergeinfo_catalog,
                                    final_unmerged_catalog,

Modified: subversion/trunk/subversion/libsvn_client/ra.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/ra.c?rev=1213018&r1=1213017&r2=1213018&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/ra.c (original)
+++ subversion/trunk/subversion/libsvn_client/ra.c Sun Dec 11 16:36:32 2011
@@ -800,7 +800,7 @@ svn_client__repos_locations(const char *
 
 
 svn_error_t *
-svn_client__get_youngest_common_ancestor(const char **ancestor_path,
+svn_client__get_youngest_common_ancestor(const char **ancestor_relpath,
                                          svn_revnum_t *ancestor_revision,
                                          const char *path_or_url1,
                                          svn_revnum_t rev1,
@@ -814,7 +814,7 @@ svn_client__get_youngest_common_ancestor
   apr_hash_t *history1, *history2;
   apr_hash_index_t *hi;
   svn_revnum_t yc_revision = SVN_INVALID_REVNUM;
-  const char *yc_path = NULL;
+  const char *yc_relpath = NULL;
   svn_opt_revision_t revision1, revision2;
   svn_boolean_t has_rev_zero_history1;
   svn_boolean_t has_rev_zero_history2;
@@ -878,7 +878,7 @@ svn_client__get_youngest_common_ancestor
                   || (yc_range->end > yc_revision))
                 {
                   yc_revision = yc_range->end;
-                  yc_path = path + 1;
+                  yc_relpath = path + 1;
                 }
             }
         }
@@ -886,13 +886,13 @@ svn_client__get_youngest_common_ancestor
 
   /* It's possible that PATH_OR_URL1 and PATH_OR_URL2's only common
      history is revision 0. */
-  if (!yc_path && has_rev_zero_history1 && has_rev_zero_history2)
+  if (!yc_relpath && has_rev_zero_history1 && has_rev_zero_history2)
     {
-      yc_path = "/";
+      yc_relpath = "";
       yc_revision = 0;
     }
 
-  *ancestor_path = yc_path;
+  *ancestor_relpath = yc_relpath;
   *ancestor_revision = yc_revision;
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_client/switch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/switch.c?rev=1213018&r1=1213017&r2=1213018&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/switch.c (original)
+++ subversion/trunk/subversion/libsvn_client/switch.c Sun Dec 11 16:36:32 2011
@@ -197,7 +197,7 @@ switch_internal(svn_revnum_t *result_rev
      ### okay? */
   if (! ignore_ancestry)
     {
-      const char *target_url, *yc_path;
+      const char *target_url, *yc_relpath;
       svn_revnum_t target_rev, yc_rev;
 
       SVN_ERR(svn_wc__node_get_url(&target_url, ctx->wc_ctx, local_abspath,
@@ -206,11 +206,11 @@ switch_internal(svn_revnum_t *result_rev
                                         local_abspath, pool));
       /* ### It would be nice if this function could reuse the existing
              ra session instead of opening two for its own use. */
-      SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_path, &yc_rev,
+      SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_relpath, &yc_rev,
                                                        switch_rev_url, revnum,
                                                        target_url, target_rev,
                                                        ctx, pool));
-      if (! (yc_path && SVN_IS_VALID_REVNUM(yc_rev)))
+      if (! (yc_relpath && SVN_IS_VALID_REVNUM(yc_rev)))
         return svn_error_createf(SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
                                  _("'%s' shares no common ancestry with '%s'"),
                                  switch_url, local_abspath);

Modified: subversion/trunk/subversion/tests/libsvn_client/client-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/client-test.c?rev=1213018&r1=1213017&r2=1213018&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_client/client-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_client/client-test.c Sun Dec 11 16:36:32 2011
@@ -28,6 +28,7 @@
 #include <limits.h>
 #include "svn_mergeinfo.h"
 #include "../../libsvn_client/mergeinfo.h"
+#include "../../libsvn_client/client.h"
 #include "svn_pools.h"
 #include "svn_client.h"
 #include "svn_repos.h"
@@ -650,6 +651,71 @@ test_16k_add(const svn_test_opts_t *opts
 }
 #endif
 
+static svn_error_t *
+test_youngest_common_ancestor(const svn_test_opts_t *opts,
+                              apr_pool_t *pool)
+{
+  const char *repos_url;
+  svn_client_ctx_t *ctx;
+  svn_opt_revision_t head_rev = { svn_opt_revision_head, { 0 } };
+  svn_opt_revision_t zero_rev = { svn_opt_revision_number, { 0 } };
+  svn_client_copy_source_t source;
+  apr_array_header_t *sources;
+  const char *dest;
+  const char *yc_ancestor_relpath;
+  svn_revnum_t yc_ancestor_rev;
+
+  /* Create a filesytem and repository containing the Greek tree. */
+  SVN_ERR(create_greek_repos(&repos_url, "test-youngest-common-ancestor", opts, pool));
+
+  svn_client_create_context(&ctx, pool);
+
+  /* Copy a file into dir 'A', keeping its own basename. */
+  sources = apr_array_make(pool, 1, sizeof(svn_client_copy_source_t *));
+  source.path = svn_path_url_add_component2(repos_url, "iota", pool);
+  source.peg_revision = &head_rev;
+  source.revision = &head_rev;
+  APR_ARRAY_PUSH(sources, svn_client_copy_source_t *) = &source;
+  dest = svn_path_url_add_component2(repos_url, "A", pool);
+  SVN_ERR(svn_client_copy6(sources, dest, TRUE /* copy_as_child */,
+                           FALSE /* make_parents */,
+                           FALSE /* ignore_externals */,
+                           NULL, NULL, NULL, ctx, pool));
+
+  /* Test: YCA(iota@2, A/iota@2) is iota@1. */
+  SVN_ERR(svn_client__get_youngest_common_ancestor(
+            &yc_ancestor_relpath, &yc_ancestor_rev,
+            svn_path_url_add_component2(repos_url, "iota", pool), 2,
+            svn_path_url_add_component2(repos_url, "A/iota", pool), 2,
+            ctx, pool));
+  SVN_TEST_STRING_ASSERT(yc_ancestor_relpath, "iota");
+  SVN_TEST_ASSERT(yc_ancestor_rev == 1);
+
+  /* Copy the root directory (at revision 0) into A as 'ROOT'. */
+  sources = apr_array_make(pool, 1, sizeof(svn_client_copy_source_t *));
+  source.path = repos_url;
+  source.peg_revision = &zero_rev;
+  source.revision = &zero_rev;
+  APR_ARRAY_PUSH(sources, svn_client_copy_source_t *) = &source;
+  dest = svn_path_url_add_component2(repos_url, "A/ROOT", pool);
+  SVN_ERR(svn_client_copy6(sources, dest, FALSE /* copy_as_child */,
+                           FALSE /* make_parents */,
+                           FALSE /* ignore_externals */,
+                           NULL, NULL, NULL, ctx, pool));
+
+  /* Test: YCA(''@0, A/ROOT@3) is ''@0 (handled as a special case). */
+  SVN_ERR(svn_client__get_youngest_common_ancestor(
+            &yc_ancestor_relpath, &yc_ancestor_rev,
+            svn_path_url_add_component2(repos_url, "", pool), 0,
+            svn_path_url_add_component2(repos_url, "A/ROOT", pool), 3,
+            ctx, pool));
+  SVN_TEST_STRING_ASSERT(yc_ancestor_relpath, "");
+  SVN_TEST_ASSERT(yc_ancestor_rev == 0);
+
+  return SVN_NO_ERROR;
+}
+
+
 /* ========================================================================== */
 
 struct svn_test_descriptor_t test_funcs[] =
@@ -665,5 +731,6 @@ struct svn_test_descriptor_t test_funcs[
 #ifdef TEST16K_ADD
     SVN_TEST_OPTS_PASS(test_16k_add, "test adding 16k files"),
 #endif
+    SVN_TEST_OPTS_PASS(test_youngest_common_ancestor, "test youngest_common_ancestor"),
     SVN_TEST_NULL
   };