You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2012/12/22 11:47:43 UTC

svn commit: r1425271 - in /subversion/trunk/subversion: libsvn_client/merge.c libsvn_client/repos_diff.c tests/cmdline/merge_reintegrate_tests.py tests/cmdline/merge_tests.py tests/cmdline/merge_tree_conflict_tests.py tests/cmdline/tree_conflict_tests.py

Author: rhuijben
Date: Sat Dec 22 10:47:43 2012
New Revision: 1425271

URL: http://svn.apache.org/viewvc?rev=1425271&view=rev
Log:
Resolve the regressions caused by the recent diff improvements by stopping the
global skip of descendants of obstructions in merges. This improves the skip
notifications by reporting the actual skipped descendant instead of just the
opened directory. This makes the merge code properly store non inherited
mergeinfo for these skipped subtrees.

But as usual tweaking the merge processing changes more than the issue I'm
trying to fix: in this case some missing files scenarios start working
unexpectedly.

* subversion/libsvn_client/merge.c
  (merge_dir_opened): Don't skip descendants assuming some other code will
    do the right thing, as that will remove required knowledge for the other
    code.

* subversion/libsvn_client/repos_diff.c
  (change_file_prop,
   change_dir_prop): Don't report dav properties as changes at all. They are
     an implementation detail that shouldn't show up in higher layers.

* subversion/tests/cmdline/merge_reintegrate_tests.py
  (reintegrate_on_shallow_wc): Remove XFail marker. Expect skip on the truely
    skipped target instead of its obstructed parent.

* subversion/tests/cmdline/merge_tests.py
  (merge_to_sparse_directories): Remove XFail marker. Expect skip on the
    targets instead of obstructed parents. Expect more skips.

* subversion/tests/cmdline/merge_tree_conflict_tests.py
  (tree_conflicts_merge_edit_onto_missing): Update skip expectations, but
    keep as XFail as some operations start succeeding, which need further
    tweak.
  (tree_conflicts_merge_del_onto_missing): Update skip expectations and
    remove XFail marker.

* subversion/tests/cmdline/tree_conflict_tests.py
  (at_directory_external): Mark XFail. Most likely we had a bug in 1.7 here,
    which was invisible because we didn't verify the result

Modified:
    subversion/trunk/subversion/libsvn_client/merge.c
    subversion/trunk/subversion/libsvn_client/repos_diff.c
    subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py
    subversion/trunk/subversion/tests/cmdline/merge_tests.py
    subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
    subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py

Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1425271&r1=1425270&r2=1425271&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Sat Dec 22 10:47:43 2012
@@ -2723,9 +2723,7 @@ merge_dir_opened(svn_boolean_t *tree_con
 
   if (obstr_state != svn_wc_notify_state_inapplicable)
     {
-      if (skip_children)
-        *skip_children = TRUE;
-      /* But don't skip THIS, to allow a skip notification */
+      /* In Subversion <= 1.7 we skipped descendants here */
       return SVN_NO_ERROR;
     }
 
@@ -2747,8 +2745,7 @@ merge_dir_opened(svn_boolean_t *tree_con
           if (parent_depth != svn_depth_unknown &&
               parent_depth < svn_depth_immediates)
             {
-              if (skip_children)
-                *skip_children = TRUE;
+              /* In Subversion <= 1.7 we skipped descendants here */
               return SVN_NO_ERROR;
             }
         }

Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=1425271&r1=1425270&r2=1425271&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/repos_diff.c Sat Dec 22 10:47:43 2012
@@ -1221,12 +1221,16 @@ change_file_prop(void *file_baton,
 {
   struct file_baton *fb = file_baton;
   svn_prop_t *propchange;
+  svn_prop_kind_t propkind;
 
   /* Skip *everything* within a newly tree-conflicted directory. */
   if (fb->skip)
     return SVN_NO_ERROR;
 
-  if (!fb->has_propchange && svn_property_kind2(name) == svn_prop_regular_kind)
+  propkind = svn_property_kind2(name);
+  if (propkind == svn_prop_wc_kind)
+    return SVN_NO_ERROR;
+  else if (propkind == svn_prop_regular_kind)
     fb->has_propchange = TRUE;
 
   propchange = apr_array_push(fb->propchanges);
@@ -1247,12 +1251,16 @@ change_dir_prop(void *dir_baton,
 {
   struct dir_baton *db = dir_baton;
   svn_prop_t *propchange;
+  svn_prop_kind_t propkind;
 
   /* Skip *everything* within a newly tree-conflicted directory. */
   if (db->skip)
     return SVN_NO_ERROR;
 
-  if (!db->has_propchange && svn_property_kind2(name) == svn_prop_regular_kind)
+  propkind = svn_property_kind2(name);
+  if (propkind == svn_prop_wc_kind)
+    return SVN_NO_ERROR;
+  else if (propkind == svn_prop_regular_kind)
     db->has_propchange = TRUE;
 
   propchange = apr_array_push(db->propchanges);

Modified: subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py?rev=1425271&r1=1425270&r2=1425271&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py Sat Dec 22 10:47:43 2012
@@ -740,7 +740,6 @@ def reintegrate_fail_on_switched_wc(sbox
 # Test for issue #3603 'allow reintegrate merges into WCs with
 # missing subtrees'.
 @Issue(3603)
-@XFail()
 def reintegrate_on_shallow_wc(sbox):
   "merge --reintegrate in shallow wc"
 
@@ -833,7 +832,7 @@ def reintegrate_on_shallow_wc(sbox):
   expected_A_disk.tweak('D', props={SVN_PROP_MERGEINFO : '/A_COPY/D:2-4*'})
   # ... a depth-restricted item is skipped ...
   expected_A_skip.add({
-      'D/H' : Item()
+      'D/H/psi' : Item()
   })
   # Currently this fails due to r1424469.  For a full explanation see
   # http://svn.haxx.se/dev/archive-2012-12/0472.shtml

Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1425271&r1=1425270&r2=1425271&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Sat Dec 22 10:47:43 2012
@@ -7731,7 +7731,6 @@ def merge_away_subtrees_noninheritable_r
 # Handle merge info for sparsely-populated directories
 @Issue(2827)
 @SkipUnless(server_has_mergeinfo)
-@XFail()
 def merge_to_sparse_directories(sbox):
   "merge to sparse directories"
 
@@ -7850,10 +7849,8 @@ def merge_to_sparse_directories(sbox):
                               "prop:name" : "propval"}),
     })
   expected_skip = svntest.wc.State(immediates_dir, {
-    'D/H'               : Item(),
-    'B/E'               : Item(),
-    })
-  expected_skip = svntest.wc.State(immediates_dir, {
+    'D/H/omega'         : Item(),
+    'B/E/beta'          : Item(),
     })
   svntest.actions.run_and_verify_merge(immediates_dir, '4', '9',
                                        sbox.repo_url + '/A', None,
@@ -7906,7 +7903,8 @@ def merge_to_sparse_directories(sbox):
                        props={SVN_PROP_MERGEINFO : '/A/mu:5-9'}),
     })
   expected_skip = svntest.wc.State(files_dir, {
-    'D'               : Item(),
+    'D/H/omega'       : Item(),
+    'B/E/beta'        : Item(),
     })
   svntest.actions.run_and_verify_merge(files_dir, '4', '9',
                                        sbox.repo_url + '/A', None,
@@ -7949,7 +7947,8 @@ def merge_to_sparse_directories(sbox):
     })
   expected_skip = svntest.wc.State(empty_dir, {
     'mu'               : Item(),
-    'D'               : Item(),
+    'D/H/omega'        : Item(),
+    'B/E/beta'         : Item(),
     })
   svntest.actions.run_and_verify_merge(empty_dir, '4', '9',
                                        sbox.repo_url + '/A', None,

Modified: subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py?rev=1425271&r1=1425270&r2=1425271&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py Sat Dec 22 10:47:43 2012
@@ -1330,23 +1330,21 @@ def tree_conflicts_merge_edit_onto_missi
     expected_status.tweak('DDD/D1/D2',       wc_rev=3, status='! ')
     expected_status.tweak('DDD/D1/D2/D3',    wc_rev=3, status='! ')
 
-  # Currently this fails due to r1424469, we are not showing the roots of
-  # missing subtrees as skipped even though there are operative changes
-  # within those subtrees.
-  # See http://svn.haxx.se/dev/archive-2012-12/0472.shtml
-  # and http://svn.haxx.se/dev/archive-2012-12/0475.shtml
   expected_skip = svntest.wc.State('', {
     'F/alpha'           : Item(),
-    # BH: After fixing several issues in the obstruction handling
-    #     I get the following Skip notifications. Please review!
-    'D/D1'              : Item(),
-    'DD/D1'             : Item(),
-    'DF/D1'             : Item(),
-    'DDD/D1'            : Item(),
-    'DDF/D1'            : Item(),
+    # Obstruction handling improvements in 1.7 and 1.8 added
+    'DDF/D1/D2/gamma'   : Item(),
+    'DF/D1/beta'        : Item(),
+    # BH: At some point we got reported skips at or below
+    # these, which I could and cannot explain. These might be cases
+    # of issue #2910
+    #'D/D1'              : Item(),
+    #'DD/D1'             : Item(),
+    #'DDD/D1'            : Item(),
     })
 
-
+  # Currently this test fails because some parts of the merge
+  # start succeeding. 
   svntest.actions.deep_trees_run_tests_scheme_for_merge(sbox,
     [ DeepTreesTestCase(
                "local_tree_missing_incoming_leaf_edit",
@@ -1359,7 +1357,6 @@ def tree_conflicts_merge_edit_onto_missi
              ) ], False, do_commit_conflicts=False, ignore_ancestry=True)
 
 #----------------------------------------------------------------------
-@XFail()
 def tree_conflicts_merge_del_onto_missing(sbox):
   "tree conflicts: tree missing, leaf del"
 
@@ -1418,21 +1415,15 @@ def tree_conflicts_merge_del_onto_missin
     expected_status.tweak('DDD/D1/D2',       wc_rev=3, status='! ')
     expected_status.tweak('DDD/D1/D2/D3',    wc_rev=3, status='! ')
 
-  # Currently this fails due to r1424469, we are not showing the roots of
-  # missing subtrees as skipped even though there are operative changes
-  # within those subtrees.
-  # See http://svn.haxx.se/dev/archive-2012-12/0472.shtml
-  # and http://svn.haxx.se/dev/archive-2012-12/0475.shtml
   expected_skip = svntest.wc.State('', {
     'F/alpha'           : Item(),
     'D/D1'              : Item(),
-    # BH: After fixing several issues in the obstruction handling
-    #     I get the following Skip notifications. Please review!
+    # Obstruction handling improvements in 1.7 and 1.8 added
     'D/D1'              : Item(),
-    'DD/D1'             : Item(),
-    'DF/D1'             : Item(),
-    'DDD/D1'            : Item(),
-    'DDF/D1'            : Item(),
+    'DD/D1/D2'          : Item(),
+    'DF/D1/beta'        : Item(),
+    'DDD/D1/D2/D3'      : Item(),
+    'DDF/D1/D2/gamma'   : Item(),
     })
 
   svntest.actions.deep_trees_run_tests_scheme_for_merge(sbox,

Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1425271&r1=1425270&r2=1425271&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Sat Dec 22 10:47:43 2012
@@ -1068,6 +1068,7 @@ def lock_update_only(sbox):
 
 #----------------------------------------------------------------------
 @Issue(3469)
+@XFail()
 def at_directory_external(sbox):
   "tree conflict at directory external"