subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rhuij...@apache.org
Subject svn commit: r1424469 - in /subversion/trunk/subversion: libsvn_client/repos_diff.c tests/cmdline/merge_reintegrate_tests.py tests/cmdline/merge_tests.py tests/cmdline/merge_tree_conflict_tests.py
Date Thu, 20 Dec 2012 14:01:37 GMT
Author: rhuijben
Date: Thu Dec 20 14:01:37 2012
New Revision: 1424469

URL: http://svn.apache.org/viewvc?rev=1424469&view=rev
Log:
In the repository-repository diff handler: Don't retrieve pristine properties
when we already know that there are no differences to report on them.

By checking whether we really need the properties we avoid about 1000 ra calls
over running our test suite (ra local). This also resolves many spurious merge
changes that just change entry props.

On top of that stop reporting entry prop only changes as a file/directory
change to avoid doing unneeded work in the merge and diff handling.

This fixes some issues identified when the merge code was updated to do a
better obstruction detection, as originally we just skipped these non-changes
there in the merge code.

It is possible that this obscures some tree conflicts which were identified by
entry prop changes that should have been detected by the real change down the
tree (which might have been skipped).

* subversion/libsvn_client/repos_diff.c
  (dir_baton,
   file_baton): Add has_propchange boolean.
  (close_file,
   close_directory): Only retrieve the pristine properties if we may be going
     to use them in a callback.

  (change_file_prop,
   change_dir_prop): Detect if we have a real property change and if we have
     store that information.

* subversion/tests/cmdline/merge_reintegrate_tests.py
  (reintegrate_on_shallow_wc): Don't assume to get spurious change event.

* subversion/tests/cmdline/merge_tests.py
  (merge_to_sparse_directories): Don't expect entry prop change only skips.

* subversion/tests/cmdline/merge_tree_conflict_tests.py
  (tree_conflicts_merge_edit_onto_missing,
   tree_conflicts_merge_del_onto_missing):
    Remove some unexpected change notifications.

Modified:
    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

Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=1424469&r1=1424468&r2=1424469&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/repos_diff.c Thu Dec 20 14:01:37 2012
@@ -143,6 +143,9 @@ struct dir_baton {
   /* A cache of any property changes (svn_prop_t) received for this dir. */
   apr_array_header_t *propchanges;
 
+  /* Boolean indicating whether a node property was changed */
+  svn_boolean_t has_propchange;
+
   /* The pool passed in by add_dir, open_dir, or open_root.
      Also, the pool this dir baton is allocated in. */
   apr_pool_t *pool;
@@ -199,6 +202,9 @@ struct file_baton {
   /* A cache of any property changes (svn_prop_t) received for this file. */
   apr_array_header_t *propchanges;
 
+  /* Boolean indicating whether a node property was changed */
+  svn_boolean_t has_propchange;
+
   /* The pool passed in by add_file or open_file.
      Also, the pool this file_baton is allocated in. */
   apr_pool_t *pool;
@@ -971,9 +977,11 @@ close_file(void *file_baton,
                                       fb->path));
     }
 
-  if (!fb->added && fb->propchanges->nelts > 0)
+  if (fb->path_end_revision || fb->has_propchange)
     {
-      if (!fb->pristine_props)
+      const char *mimetype1, *mimetype2;
+
+      if (!fb->added && !fb->pristine_props)
         {
           /* We didn't receive a text change, so we have no pristine props.
              Retrieve just the props now. */
@@ -981,11 +989,7 @@ close_file(void *file_baton,
         }
 
       remove_non_prop_changes(fb->pristine_props, fb->propchanges);
-    }
 
-  if (fb->path_end_revision || fb->propchanges->nelts > 0)
-    {
-      const char *mimetype1, *mimetype2;
       get_file_mime_types(&mimetype1, &mimetype2, fb);
 
 
@@ -1100,37 +1104,40 @@ close_directory(void *dir_baton,
 
   scratch_pool = db->pool;
 
-  if (db->added)
+  if (db->has_propchange)
     {
-      pristine_props = eb->empty_hash;
-    }
-  else
-    {
-      SVN_ERR(svn_ra_get_dir2(eb->ra_session, NULL, NULL, &pristine_props,
-                              db->path, db->base_revision, 0, scratch_pool));
-    }
-
-  if (db->propchanges->nelts > 0)
-    {
-      remove_non_prop_changes(pristine_props, db->propchanges);
-    }
+      if (db->added)
+        {
+          pristine_props = eb->empty_hash;
+        }
+      else
+        {
+          SVN_ERR(svn_ra_get_dir2(eb->ra_session, NULL, NULL, &pristine_props,
+                                  db->path, db->base_revision, 0, scratch_pool));
+        }
 
-  if (db->propchanges->nelts > 0)
-    {
-      svn_boolean_t tree_conflicted = FALSE;
-      SVN_ERR(eb->diff_callbacks->dir_props_changed(
-               &prop_state, &tree_conflicted,
-               db->path, db->added,
-               db->propchanges, pristine_props,
-               eb->diff_cmd_baton, scratch_pool));
-      if (tree_conflicted)
-        db->tree_conflicted = TRUE;
+      if (db->propchanges->nelts > 0)
+        {
+          remove_non_prop_changes(pristine_props, db->propchanges);
+        }
 
-      if (prop_state == svn_wc_notify_state_obstructed
-          || prop_state == svn_wc_notify_state_missing)
+      if (db->propchanges->nelts > 0)
         {
-          content_state = prop_state;
-          skipped = TRUE;
+          svn_boolean_t tree_conflicted = FALSE;
+          SVN_ERR(eb->diff_callbacks->dir_props_changed(
+                   &prop_state, &tree_conflicted,
+                   db->path, db->added,
+                   db->propchanges, pristine_props,
+                   eb->diff_cmd_baton, scratch_pool));
+          if (tree_conflicted)
+            db->tree_conflicted = TRUE;
+
+          if (prop_state == svn_wc_notify_state_obstructed
+              || prop_state == svn_wc_notify_state_missing)
+            {
+              content_state = prop_state;
+              skipped = TRUE;
+            }
         }
     }
 
@@ -1218,6 +1225,9 @@ change_file_prop(void *file_baton,
   if (fb->skip)
     return SVN_NO_ERROR;
 
+  if (!fb->has_propchange && svn_property_kind2(name) == svn_prop_regular_kind)
+    fb->has_propchange = TRUE;
+
   propchange = apr_array_push(fb->propchanges);
   propchange->name = apr_pstrdup(fb->pool, name);
   propchange->value = value ? svn_string_dup(value, fb->pool) : NULL;
@@ -1241,6 +1251,9 @@ change_dir_prop(void *dir_baton,
   if (db->skip)
     return SVN_NO_ERROR;
 
+  if (!db->has_propchange && svn_property_kind2(name) == svn_prop_regular_kind)
+    db->has_propchange = TRUE;
+
   propchange = apr_array_push(db->propchanges);
   propchange->name = apr_pstrdup(db->pool, name);
   propchange->value = value ? svn_string_dup(value, db->pool) : NULL;

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=1424469&r1=1424468&r2=1424469&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_reintegrate_tests.py Thu Dec 20 14:01:37
2012
@@ -825,15 +825,6 @@ def reintegrate_on_shallow_wc(sbox):
                        'Some more work on the A_COPY branch', wc_dir)
   # Reuse the same expectations as the prior merge, except that
   # non-inheritable mergeinfo is set on the root of the missing subtree...
-  expected_mergeinfo_output.add({
-      'D' : Item(status=' U')
-      })
-  expected_A_status.tweak('D', status=' M')
-  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()
-  })
   # ... and the mergeinfo on the target root includes the latest rev on the branch.
   expected_A_disk.tweak('', props={SVN_PROP_MERGEINFO : '/A_COPY:2-4'})
   svntest.actions.run_and_verify_merge(A_path, None, None,

Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1424469&r1=1424468&r2=1424469&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Thu Dec 20 14:01:37 2012
@@ -7812,8 +7812,8 @@ def merge_to_sparse_directories(sbox):
   # Merge r4:9 into the immediates WC.
   # The root of the immediates WC should get inheritable r4:9 as should
   # the one file present 'mu'.  The three directory children present, 'B',
-  # 'C', and 'D' are checked out at depth empty; the two of these affected
-  # by the merge, 'B' and 'D', get non-inheritable mergeinfo for r4:9.
+  # 'C', and 'D' are checked out at depth empty; the one of these affected
+  # by the merge, 'D', gets non-inheritable mergeinfo for r4:9.
   # The root and 'D' do should also get the changes
   # that affect them directly (the prop adds from r8 and r9).
   expected_output = wc.State(immediates_dir, {
@@ -7823,14 +7823,14 @@ def merge_to_sparse_directories(sbox):
     })
   expected_mergeinfo_output = wc.State(immediates_dir, {
     ''  : Item(status=' U'),
-    'B' : Item(status=' U'),
     'D' : Item(status=' U'),
     })
   expected_elision_output = wc.State(immediates_dir, {
+    'D' : Item(status=' U'),
     })
   expected_status = wc.State(immediates_dir, {
     ''          : Item(status=' M', wc_rev=9),
-    'B'         : Item(status=' M', wc_rev=9),
+    'B'         : Item(status='  ', wc_rev=9),
     'mu'        : Item(status='M ', wc_rev=9),
     'C'         : Item(status='  ', wc_rev=9),
     'D'         : Item(status=' M', wc_rev=9),
@@ -7838,15 +7838,12 @@ def merge_to_sparse_directories(sbox):
   expected_disk = wc.State('', {
     ''          : Item(props={SVN_PROP_MERGEINFO : '/A:5-9',
                               "prop:name" : "propval"}),
-    'B'         : Item(props={SVN_PROP_MERGEINFO : '/A/B:5-9*'}),
+    'B'         : Item(),
     'mu'        : Item("New content"),
     'C'         : Item(),
-    'D'         : Item(props={SVN_PROP_MERGEINFO : '/A/D:5-9*',
-                              "prop:name" : "propval"}),
+    'D'         : Item(props={"prop:name" : "propval"}),
     })
   expected_skip = svntest.wc.State(immediates_dir, {
-    'D/H'               : Item(),
-    'B/E'               : Item(),
     })
   svntest.actions.run_and_verify_merge(immediates_dir, '4', '9',
                                        sbox.repo_url + '/A', None,
@@ -7900,7 +7897,6 @@ def merge_to_sparse_directories(sbox):
     })
   expected_skip = svntest.wc.State(files_dir, {
     'D'               : Item(),
-    'B'               : Item(),
     })
   svntest.actions.run_and_verify_merge(files_dir, '4', '9',
                                        sbox.repo_url + '/A', None,
@@ -7944,7 +7940,6 @@ def merge_to_sparse_directories(sbox):
   expected_skip = svntest.wc.State(empty_dir, {
     'mu'               : Item(),
     'D'               : Item(),
-    'B'               : 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=1424469&r1=1424468&r2=1424469&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py Thu Dec 20 14:01:37
2012
@@ -1332,12 +1332,8 @@ def tree_conflicts_merge_edit_onto_missi
   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!
+    #     I get the following Skip notification. Please review!
     'D/D1'              : Item(),
-    'DD/D1'             : Item(),
-    'DF/D1'             : Item(),
-    'DDD/D1'            : Item(),
-    'DDF/D1'            : Item(),
     })
 
 
@@ -1414,13 +1410,6 @@ def tree_conflicts_merge_del_onto_missin
   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!
-    'D/D1'              : Item(),
-    'DD/D1'             : Item(),
-    'DF/D1'             : Item(),
-    'DDD/D1'            : Item(),
-    'DDF/D1'            : Item(),
     })
 
   svntest.actions.deep_trees_run_tests_scheme_for_merge(sbox,



Mime
View raw message