You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2010/03/16 17:40:28 UTC

svn commit: r923857 - in /subversion/branches/1.6.x: ./ CHANGES STATUS subversion/libsvn_client/merge.c subversion/tests/cmdline/merge_tests.py

Author: hwright
Date: Tue Mar 16 16:40:27 2010
New Revision: 923857

URL: http://svn.apache.org/viewvc?rev=923857&view=rev
Log:
Reintegrate the 1.6.x-r892050 branch:

 * r892050, 892085
   Fix a reintegrate bug which can occur when the merge source has mergeinfo
   that explicitly describes common history with the reintegrate target.
   Justification:
     Reintegrate merges may not work if the reintegrate source has self-
     referential mergeinfo that is also self-referential to the reintegrate
     target.  This occured in our own repository, see
     http://svn.haxx.se/dev/archive-2009-12/0338.shtml.
   Branch:
     ^/subversion/branches/1.6.x-r892050
   Votes:
     +1: pburba, rhuijben, cmpilato

Modified:
    subversion/branches/1.6.x/   (props changed)
    subversion/branches/1.6.x/CHANGES   (props changed)
    subversion/branches/1.6.x/STATUS
    subversion/branches/1.6.x/subversion/libsvn_client/merge.c
    subversion/branches/1.6.x/subversion/tests/cmdline/merge_tests.py

Propchange: subversion/branches/1.6.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Mar 16 16:40:27 2010
@@ -27,6 +27,7 @@
 /subversion/branches/1.6.x-r40452:880530-890996
 /subversion/branches/1.6.x-r889840:889888-890974
 /subversion/branches/1.6.x-r891672:891676-923748
+/subversion/branches/1.6.x-r892050:923839-923856
 /subversion/branches/1.6.x-r896522:896528-897866
 /subversion/branches/1.6.x-r898963:899874-915098
 /subversion/branches/1.6.x-r905326:905545-923537
@@ -61,4 +62,4 @@
 /subversion/branches/tc_url_rev:870696-870828
 /subversion/branches/tree-conflicts:864636-869499
 /subversion/branches/tree-conflicts-notify:870271-870353
-/subversion/trunk:875976,875980-875981,876054-876056,876092,876175,876299,876306,876427,876440,876450,876507,876571,879093,879688,880274-880275,880370,880450,880474,880525-880526,880552,881905,884842,886164,886197,888715,888979,889081,889840,891672,895514,895653,896522,898963,899826,899828,900797,901752,904301,904394,904594,905303,905326,906256,906305,917640,918211,922516
+/subversion/trunk:875976,875980-875981,876054-876056,876092,876175,876299,876306,876427,876440,876450,876507,876571,879093,879688,880274-880275,880370,880450,880474,880525-880526,880552,881905,884842,886164,886197,888715,888979,889081,889840,891672,892050,892085,895514,895653,896522,898963,899826,899828,900797,901752,904301,904394,904594,905303,905326,906256,906305,917640,918211,922516

Propchange: subversion/branches/1.6.x/CHANGES
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Mar 16 16:40:27 2010
@@ -27,6 +27,7 @@
 /subversion/branches/1.6.x-r40452/CHANGES:880530-890996
 /subversion/branches/1.6.x-r889840/CHANGES:889888-890974
 /subversion/branches/1.6.x-r891672/CHANGES:891676-923748
+/subversion/branches/1.6.x-r892050/CHANGES:923839-923856
 /subversion/branches/1.6.x-r896522/CHANGES:896528-897866
 /subversion/branches/1.6.x-r898963/CHANGES:899874-915098
 /subversion/branches/1.6.x-r905326/CHANGES:905545-923537

Modified: subversion/branches/1.6.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=923857&r1=923856&r2=923857&view=diff
==============================================================================
--- subversion/branches/1.6.x/STATUS (original)
+++ subversion/branches/1.6.x/STATUS Tue Mar 16 16:40:27 2010
@@ -216,17 +216,3 @@ Veto-blocked changes:
 
 Approved changes:
 =================
-
- * r892050, 892085
-   Fix a reintegrate bug which can occur when the merge source has mergeinfo
-   that explicitly describes common history with the reintegrate target.
-   Justification:
-     Reintegrate merges may not work if the reintegrate source has self-
-     referential mergeinfo that is also self-referential to the reintegrate
-     target.  This occured in our own repository, see
-     http://svn.haxx.se/dev/archive-2009-12/0338.shtml.
-   Branch:
-     ^/subversion/branches/1.6.x-r892050
-   Votes:
-     +1: pburba, rhuijben, cmpilato
-

Modified: subversion/branches/1.6.x/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_client/merge.c?rev=923857&r1=923856&r2=923857&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_client/merge.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_client/merge.c Tue Mar 16 16:40:27 2010
@@ -7663,6 +7663,9 @@ ensure_all_missing_ranges_are_phantoms(s
    path@TARGET_REV.  Effectively this is the mergeinfo catalog on the
    reintegrate target.
 
+   YC_ANCESTOR_REV is the revision of the youngest common ancestor of the
+   reintegrate source and the reintegrate target.
+
    SOURCE_REPOS_REL_PATH is the path of the reintegrate source relative to
    the root of the repository.  TARGET_REPOS_REL_PATH is the path of the
    reintegrate target relative to the root of the repository.
@@ -7696,6 +7699,7 @@ static svn_error_t *
 find_unmerged_mergeinfo(svn_mergeinfo_catalog_t *unmerged_to_source_catalog,
                         svn_boolean_t *never_synched,
                         svn_revnum_t *youngest_merged_rev,
+                        svn_revnum_t yc_ancestor_rev,
                         svn_mergeinfo_catalog_t source_catalog,
                         apr_hash_t *target_segments_hash,
                         const char *source_repos_rel_path,
@@ -7745,6 +7749,16 @@ find_unmerged_mergeinfo(svn_mergeinfo_ca
                                                   segments,
                                                   iterpool));
 
+      /* Remove any target history that is also part of the source's history,
+         i.e. their common ancestry.  By definition this has already been
+         "merged" from the target to the source.  If the source has explict
+         self referential mergeinfo it would intersect with the target's
+         history below, making it appear that some merges had been done from
+         the target to the source, when this might not actually be the case. */
+      SVN_ERR(svn_mergeinfo__filter_mergeinfo_by_ranges(
+        &target_history_as_mergeinfo, target_history_as_mergeinfo,
+        source_rev, yc_ancestor_rev, iterpool));
+
       /* Look for any explicit mergeinfo on the source path corresponding to
          the target path.  If we find any remove that from SOURCE_CATALOG.
          When this iteration over TARGET_SEGMENTS_HASH is complete all that
@@ -8034,6 +8048,9 @@ calculate_left_hand_side(const char **ur
   apr_hash_t *segments_hash = apr_hash_make(pool);
   svn_boolean_t never_synced;
   svn_revnum_t youngest_merged_rev;
+  const char *yc_ancestor_path;
+  const char *source_url;
+  const char *target_url;
 
   /* Get the history (segments) for the target and any of its subtrees
      with explicit mergeinfo. */
@@ -8058,6 +8075,26 @@ calculate_left_hand_side(const char **ur
                    APR_HASH_KEY_STRING, segments);
     }
 
+  /* Check that SOURCE_URL@SOURCE_REV and TARGET_URL@TARGET_REV are
+     actually related, we can't reintegrate if they are not.  Also
+     get an initial value for *REV_LEFT. */
+  source_url = svn_path_url_add_component2(source_repos_root,
+                                           source_repos_rel_path,
+                                           subpool),
+  target_url = svn_path_url_add_component2(source_repos_root,
+                                           target_repos_rel_path,
+                                           subpool);
+  SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_ancestor_path,
+                                                   rev_left,
+                                                   source_url, source_rev,
+                                                   target_url, target_rev,
+                                                   ctx, subpool));
+  if (!(yc_ancestor_path && SVN_IS_VALID_REVNUM(*rev_left)))
+    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,
+                             target_url, target_rev);
+
   /* Get the mergeinfo from the source, including its descendants
      with differing explicit mergeinfo. */
   APR_ARRAY_PUSH(source_repos_rel_path_as_array, const char *)
@@ -8075,6 +8112,7 @@ calculate_left_hand_side(const char **ur
   SVN_ERR(find_unmerged_mergeinfo(&unmerged_catalog,
                                   &never_synced,
                                   &youngest_merged_rev,
+                                  *rev_left,
                                   mergeinfo_catalog,
                                   segments_hash,
                                   source_repos_rel_path,
@@ -8094,24 +8132,6 @@ calculate_left_hand_side(const char **ur
   if (never_synced)
     {
       /* We never merged to the source.  Just return the branch point. */
-      const char *yc_ancestor_path,
-        *source_url = svn_path_url_add_component2(source_repos_root,
-                                                  source_repos_rel_path,
-                                                  subpool),
-        *target_url = svn_path_url_add_component2(source_repos_root,
-                                                  target_repos_rel_path,
-                                                  subpool);
-
-      SVN_ERR(svn_client__get_youngest_common_ancestor(&yc_ancestor_path,
-                                                       rev_left,
-                                                       source_url, source_rev,
-                                                       target_url, target_rev,
-                                                       ctx, subpool));
-      if (!(yc_ancestor_path && SVN_IS_VALID_REVNUM(*rev_left)))
-        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,
-                                 target_url, target_rev);
       *url_left = svn_path_url_add_component2(source_repos_root,
                                               yc_ancestor_path, pool);
     }

Modified: subversion/branches/1.6.x/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/tests/cmdline/merge_tests.py?rev=923857&r1=923856&r2=923857&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/branches/1.6.x/subversion/tests/cmdline/merge_tests.py Tue Mar 16 16:40:27 2010
@@ -16121,6 +16121,129 @@ def merge_replace_causes_tree_conflict(s
 
   actions.run_and_verify_status(wc_dir, expected_status)
 
+# Test for a reintegrate bug which can occur when the merge source
+# has mergeinfo that explicitly describes common history with the reintegrate
+# target, see http://mail-archives.apache.org/mod_mbox/subversion-dev/
+# 200912.mbox/%3C6cfe18eb0912161438wfb5234bj118aacdff7ffb25f@mail.gmail.com%3E
+def reintegrate_with_self_referential_mergeinfo(sbox):
+  "source has target's history as explicit mergeinfo"
+
+  sbox.build()
+  wc_dir = sbox.wc_dir
+
+  # Make some changes under 'A' in r2-5.
+  wc_disk, wc_status = set_up_branch(sbox, nbr_of_branches=0)
+
+  # Some paths we'll care about
+  A_path       = os.path.join(wc_dir, "A")
+  A2_path      = os.path.join(wc_dir, "A2")
+  A2_B_path    = os.path.join(wc_dir, "A2", "B")
+  A2_1_path    = os.path.join(wc_dir, "A2.1")
+  A2_1_mu_path = os.path.join(wc_dir, "A2.1", "mu")
+  
+  # r6 Copy A to A2 and then manually set some self-referential mergeinfo on
+  # A2/B and A2.
+  svntest.actions.run_and_verify_svn(None, ["At revision 5.\n"], [],
+                                     'up', wc_dir)
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'copy', A_path, A2_path)
+  # /A:3 describes A2's natural history, a.k.a. it's implicit mergeinfo, so
+  # it is self-referential.  Same for /A/B:4 and A2/B.  Normally this is
+  # redundant but not harmful.
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'ps', 'svn:mergeinfo', '/A:3', A2_path)
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'ps', 'svn:mergeinfo', '/A/B:4', A2_B_path)
+  svntest.actions.run_and_verify_svn(
+    None, None, [], 'ci', '-m',
+    'copy A to A2 and set some self-referential mergeinfo on the latter.',
+    wc_dir)
+
+  # r7 Copy A2 to A2.1
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'copy', A2_path, A2_1_path)
+  svntest.actions.run_and_verify_svn(None, None, [], 'ci',
+                                     '-m', 'copy A2to A2.1.', wc_dir)
+
+  # r8 Make a change on A2.1/mu
+  svntest.main.file_write(A2_1_mu_path, 'New A2.1 stuff')
+  svntest.actions.run_and_verify_svn(None, None, [], 'ci',
+                                     '-m', 'Work done on the A2.1 branch.',
+                                     wc_dir)
+  
+  # Update to uniform revision and reintegrated A2.1 back to A2.
+  svntest.actions.run_and_verify_svn(None, ["At revision 8.\n"], [],
+                                     'up', wc_dir)
+
+  # Now merge all available revisions from A to A_COPY:
+  expected_output = wc.State(A2_path, {
+    'mu' : Item(status='U '),
+    })
+  expected_status = wc.State(A2_path, {
+    ''          : Item(status=' M'),
+    'B'         : Item(status=' M'),
+    'mu'        : Item(status='M '),
+    'B/E'       : Item(status='  '),
+    'B/E/alpha' : Item(status='  '),
+    'B/E/beta'  : Item(status='  '),
+    'B/lambda'  : Item(status='  '),
+    'B/F'       : Item(status='  '),
+    'C'         : Item(status='  '),
+    'D'         : Item(status='  '),
+    'D/G'       : Item(status='  '),
+    'D/G/pi'    : Item(status='  '),
+    'D/G/rho'   : Item(status='  '),
+    'D/G/tau'   : Item(status='  '),
+    'D/gamma'   : Item(status='  '),
+    'D/H'       : Item(status='  '),
+    'D/H/chi'   : Item(status='  '),
+    'D/H/psi'   : Item(status='  '),
+    'D/H/omega' : Item(status='  '),
+    })
+  expected_status.tweak(wc_rev=8)
+  expected_disk = wc.State('', {
+    ''          : Item(props={SVN_PROP_MERGEINFO : '/A:3\n/A2.1:7-8'}),
+    'B'         : Item(props={SVN_PROP_MERGEINFO : '/A/B:4\n/A2.1/B:7-8'}),
+    'mu'        : Item("New A2.1 stuff"),
+    'B/E'       : Item(),
+    'B/E/alpha' : Item("This is the file 'alpha'.\n"),
+    'B/E/beta'  : Item("New content"),
+    'B/lambda'  : Item("This is the file 'lambda'.\n"),
+    'B/F'       : Item(),
+    'C'         : Item(),
+    'D'         : Item(),
+    'D/G'       : Item(),
+    'D/G/pi'    : Item("This is the file 'pi'.\n"),
+    'D/G/rho'   : Item("New content"),
+    'D/G/tau'   : Item("This is the file 'tau'.\n"),
+    'D/gamma'   : Item("This is the file 'gamma'.\n"),
+    'D/H'       : Item(),
+    'D/H/chi'   : Item("This is the file 'chi'.\n"),
+    'D/H/psi'   : Item("New content"),
+    'D/H/omega' : Item("New content"),
+    })
+  expected_skip = wc.State(A2_path, { })
+  # Previously failed with this error:
+  #
+  #   svn merge ^/A2.1" A2 --reintegrate
+  #  ..\..\..\subversion\svn\merge-cmd.c:349: (apr_err=160013)
+  #  ..\..\..\subversion\libsvn_client\merge.c:9219: (apr_err=160013)
+  #  ..\..\..\subversion\libsvn_client\ra.c:728: (apr_err=160013)
+  #  ..\..\..\subversion\libsvn_client\mergeinfo.c:733: (apr_err=160013)
+  #  ..\..\..\subversion\libsvn_client\ra.c:526: (apr_err=160013)
+  #  ..\..\..\subversion\libsvn_repos\rev_hunt.c:908: (apr_err=160013)
+  #  ..\..\..\subversion\libsvn_repos\rev_hunt.c:607: (apr_err=160013)
+  #  ..\..\..\subversion\libsvn_fs_fs\tree.c:2886: (apr_err=160013)
+  #  ..\..\..\subversion\libsvn_fs_fs\tree.c:669: (apr_err=160013)
+  #  svn: File not found: revision 4, path '/A2'
+  svntest.actions.run_and_verify_merge(A2_path, None, None,
+                                       sbox.repo_url + '/A2.1',
+                                       expected_output,
+                                       expected_disk,
+                                       expected_status,
+                                       expected_skip,
+                                       None, None, None, None,
+                                       None, 1, 0, '--reintegrate')
 
 ########################################################################
 # Run the tests
@@ -16342,6 +16465,7 @@ test_list = [ None,
               # ra_serf causes duplicate notifications with this test:
               Skip(merge_replace_causes_tree_conflict,
                    svntest.main.is_ra_type_dav_serf),
+              reintegrate_with_self_referential_mergeinfo,
              ]
 
 if __name__ == '__main__':