[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH]Use svn_path_is_ancestor instead of custom incomplete code

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2007-02-27 16:47:55 CET

Hi All,
Peter Lundblad and Daniel Rall have suggested to use
'svn_path_is_ancestor' in deciding whether to call (no-diff)set_path on
a subtree.

Attached patch has the testcase and the fix.

With regards
Kamesh Jayachandran

[[[

Use time proven 'svn_path_is_ancestor' rather than custom incomplete code.
Testcase to showcase the defect.

* subversion/libsvn_client/diff.c
  (do_merge):
    Use time proven 'svn_path_is_ancestor' rather than custom incomplete code.

* subversion/tests/cmdline/merge_tests.py
  (create_deep_tree): Renamed from create_deep_trees. To match the fact that
   it creates one deep tree.(/A/B/F/E).
  (avoid_repeated_merge_using_inherited_merge_info,
   avoid_repeated_merge_on_subtree_with_merge_info):
   Adjust for the above change.
  (create_deep_trees): New function, creates 2 deep trees (/A/B/F/E
   and /A/B/F/E1).
  (call_set_path_only_on_subtree_while_doing_subtree_merges): New testcase.
  (test_list): Add 'call_set_path_only_on_subtree_while_doing_subtree_merges'.

Patch by: kameshj
Found by: plundblad
          dlr
]]]

Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 23505)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -2397,18 +2397,14 @@
                                                       j,
                                                       svn_sort__item_t);
               child_wcpath = item->key;
- if (item->klen < target_wcpath_len ||
- strcmp(child_wcpath, target_wcpath) == 0 ||
- strncmp(child_wcpath, target_wcpath, target_wcpath_len) != 0)
+ if (svn_path_is_ancestor(target_wcpath, child_wcpath) &&
+ strcmp(child_wcpath, target_wcpath) != 0)
                 {
- /* Path is not a child of the WC's merge target. */
- continue;
+ child_repos_path = child_wcpath + target_wcpath_len + 1;
+ SVN_ERR(reporter->set_path(report_baton, child_repos_path,
+ is_revert ? r->end - 1 : r->end,
+ FALSE, NULL, pool));
                 }
-
- child_repos_path = child_wcpath + target_wcpath_len + 1;
- SVN_ERR(reporter->set_path(report_baton, child_repos_path,
- is_revert ? r->end - 1 : r->end,
- FALSE, NULL, pool));
             }
         }
 
Index: subversion/tests/cmdline/merge_tests.py
===================================================================
--- subversion/tests/cmdline/merge_tests.py (revision 23505)
+++ subversion/tests/cmdline/merge_tests.py (working copy)
@@ -4061,7 +4061,7 @@
                                        expected_backup_status,
                                        expected_backup_skip)
 
-def create_deep_trees(wc_dir):
+def create_deep_tree(wc_dir):
   """Create A/B/F/E by moving A/B/E to A/B/F/E, copy A/B to
   A/copy-of-B, and return the expected status."""
 
@@ -4113,6 +4113,82 @@
 
   return expected_status
 
+def create_deep_trees(wc_dir):
+ """Create A/B/F/E by moving A/B/E to A/B/F/E.
+ Copy A/B/F/E to A/B/F/E1.
+ Copy A/B to A/copy-of-B, and return the expected status.
+ At the end of this function WC would be at r4"""
+
+ A_path = os.path.join(wc_dir, 'A')
+ A_B_path = os.path.join(A_path, 'B')
+ A_B_E_path = os.path.join(A_B_path, 'E')
+ A_B_F_path = os.path.join(A_B_path, 'F')
+ A_B_F_E_path = os.path.join(A_B_F_path, 'E')
+ A_B_F_E1_path = os.path.join(A_B_F_path, 'E1')
+
+ # Deepen the directory structure we're working with by moving E to
+ # underneath F and committing, creating revision 2.
+ svntest.main.run_svn(None, 'mv', A_B_E_path, A_B_F_path)
+ expected_output = wc.State(wc_dir, {
+ 'A/B/E' : Item(verb='Deleting'),
+ 'A/B/F/E' : Item(verb='Adding')
+ })
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.remove('A/B/E', 'A/B/E/alpha', 'A/B/E/beta')
+ expected_status.add({
+ 'A/B/F/E' : Item(status=' ', wc_rev=2),
+ 'A/B/F/E/alpha' : Item(status=' ', wc_rev=2),
+ 'A/B/F/E/beta' : Item(status=' ', wc_rev=2),
+ })
+ svntest.actions.run_and_verify_commit(wc_dir, expected_output,
+ expected_status, None,
+ None, None, None, None, wc_dir)
+
+
+ svntest.main.run_svn(None, 'cp', A_B_F_E_path, A_B_F_E1_path)
+ expected_output = wc.State(wc_dir, {
+ 'A/B/F/E1' : Item(verb='Adding')
+ })
+ expected_status.add({
+ 'A/B/F/E1' : Item(status=' ', wc_rev=3),
+ 'A/B/F/E1/alpha' : Item(status=' ', wc_rev=3),
+ 'A/B/F/E1/beta' : Item(status=' ', wc_rev=3),
+ })
+ svntest.actions.run_and_verify_commit(wc_dir, expected_output,
+ expected_status, None,
+ None, None, None, None, wc_dir)
+
+ # Bring the entire WC up to date with rev 3.
+ svntest.actions.run_and_verify_svn(None, None, [], 'update', wc_dir)
+ expected_status.tweak(wc_rev=3)
+
+ # Copy B and commit, creating revision 4.
+ copy_of_B_path = os.path.join(A_path, 'copy-of-B')
+ svntest.main.run_svn(None, "cp", A_B_path, copy_of_B_path)
+
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/copy-of-B' : Item(verb='Adding'),
+ })
+ expected_status.add({
+ 'A/copy-of-B' : Item(status=' ', wc_rev=4),
+ 'A/copy-of-B/F' : Item(status=' ', wc_rev=4),
+ 'A/copy-of-B/F/E' : Item(status=' ', wc_rev=4),
+ 'A/copy-of-B/F/E/alpha' : Item(status=' ', wc_rev=4),
+ 'A/copy-of-B/F/E/beta' : Item(status=' ', wc_rev=4),
+ 'A/copy-of-B/F/E1' : Item(status=' ', wc_rev=4),
+ 'A/copy-of-B/F/E1/alpha' : Item(status=' ', wc_rev=4),
+ 'A/copy-of-B/F/E1/beta' : Item(status=' ', wc_rev=4),
+ 'A/copy-of-B/lambda' : Item(status=' ', wc_rev=4),
+ })
+ svntest.actions.run_and_verify_commit(wc_dir, expected_output,
+ expected_status, None,
+ None, None, None, None, wc_dir)
+
+ # Bring the entire WC up to date with rev 4.
+ svntest.actions.run_and_verify_svn(None, None, [], 'update', wc_dir)
+ expected_status.tweak(wc_rev=4)
+ return expected_status
+
 def avoid_repeated_merge_using_inherited_merge_info(sbox):
   "use inherited merge info to avoid repeated merge"
 
@@ -4126,7 +4202,7 @@
   copy_of_B_path = os.path.join(A_path, 'copy-of-B')
 
   # Create a deeper directory structure.
- expected_status = create_deep_trees(wc_dir)
+ expected_status = create_deep_tree(wc_dir)
 
   # Edit alpha and commit it, creating revision 4.
   alpha_path = os.path.join(A_B_F_path, 'E', 'alpha')
@@ -4250,7 +4326,7 @@
   copy_of_B_path = os.path.join(A_path, 'copy-of-B')
 
   # Create a deeper directory structure.
- expected_status = create_deep_trees(wc_dir)
+ expected_status = create_deep_tree(wc_dir)
 
   # Edit alpha and commit it, creating revision 4.
   alpha_path = os.path.join(A_B_F_E_path, 'alpha')
@@ -4563,6 +4639,161 @@
   finally:
     os.chdir(saved_cwd)
 
+def call_set_path_only_on_subtree_while_doing_subtree_merges(sbox):
+ "call set_path only on subtree path"
+ # Create deep trees A/B/F/E and A/B/F/E1 and copy A/B to A/copy-of-B
+ # with the help of 'create_deep_trees'
+ # As /A/copy-of-B/F/E1 is not a child of /A/copy-of-B/F/E,
+ # set_path should not be called on /A/copy-of-B/F/E1 while
+ # doing a implicit subtree merge on /A/copy-of-B/F/E.
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ A_path = os.path.join(wc_dir, 'A')
+ A_B_path = os.path.join(A_path, 'B')
+ A_B_E_path = os.path.join(A_B_path, 'E')
+ A_B_F_path = os.path.join(A_B_path, 'F')
+ A_B_F_E_path = os.path.join(A_B_F_path, 'E')
+ copy_of_B_path = os.path.join(A_path, 'copy-of-B')
+
+ # Create a deeper directory structure.
+ expected_status = create_deep_trees(wc_dir)
+
+ # Edit alpha and commit it, creating revision 5.
+ alpha_path = os.path.join(A_B_F_E_path, 'alpha')
+ new_content_for_alpha1 = 'new content to alpha\n'
+ svntest.main.file_write(alpha_path, new_content_for_alpha1)
+
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/B/F/E/alpha' : Item(verb='Sending'),
+ })
+ expected_status.tweak('A/B/F/E/alpha', wc_rev=5)
+ svntest.actions.run_and_verify_commit(wc_dir, expected_output,
+ expected_status, None,
+ None, None, None, None, wc_dir)
+
+
+ for path in ('E', 'E1'):
+ path_name = os.path.join(copy_of_B_path, 'F', path)
+ # Search for the comment entitled "The Merge Kluge" elsewhere in
+ # this file, to understand why we shorten and chdir() below.
+ short_path_name = shorten_path_kludge(path_name)
+
+ # Merge r5 to path_name.
+ expected_output = wc.State(short_path_name, {
+ 'alpha' : Item(status='U '),
+ })
+ expected_status = wc.State(short_path_name, {
+ '' : Item(status=' M', wc_rev=4),
+ 'alpha' : Item(status='M ', wc_rev=4),
+ 'beta' : Item(status=' ', wc_rev=4),
+ })
+ expected_disk = wc.State('', {
+ '' : Item(props={SVN_PROP_MERGE_INFO : '/A/B/F/E:5'}),
+ 'alpha' : Item(new_content_for_alpha1),
+ 'beta' : Item("This is the file 'beta'.\n"),
+ })
+ expected_skip = wc.State(short_path_name, { })
+ saved_cwd = os.getcwd()
+ try:
+ os.chdir(svntest.main.work_dir)
+ svntest.actions.run_and_verify_merge(short_path_name, '4', '5',
+ sbox.repo_url + '/A/B/F/E',
+ expected_output,
+ expected_disk,
+ expected_status,
+ expected_skip,
+ None,
+ None,
+ None,
+ None,
+ None, 1)
+ finally:
+ os.chdir(saved_cwd)
+
+ # Commit the result of the merge, creating new revision.
+ expected_output = svntest.wc.State(path_name, {
+ '' : Item(verb='Sending'),
+ 'alpha' : Item(verb='Sending'),
+ })
+ svntest.actions.run_and_verify_commit(short_path_name,
+ expected_output, None, None,
+ None, None, None, None, wc_dir)
+
+ # Edit A/B/F/E/alpha and commit it, creating revision 8.
+ new_content_for_alpha = 'new content to alpha\none more line\n'
+ svntest.main.file_write(alpha_path, new_content_for_alpha)
+
+ expected_output = svntest.wc.State(A_B_F_E_path, {
+ 'alpha' : Item(verb='Sending'),
+ })
+ expected_status = wc.State(A_B_F_E_path, {
+ '' : Item(status=' ', wc_rev=4),
+ 'alpha' : Item(status=' ', wc_rev=8),
+ 'beta' : Item(status=' ', wc_rev=4),
+ })
+ svntest.actions.run_and_verify_commit(A_B_F_E_path, expected_output,
+ expected_status, None,
+ None, None, None, None, wc_dir)
+
+ # Search for the comment entitled "The Merge Kluge" elsewhere in
+ # this file, to understand why we shorten and chdir() below.
+ short_copy_of_B_path = shorten_path_kludge(copy_of_B_path)
+
+ # Update the WC to bring /A/copy_of_B to rev 8.
+ # Without this update expected_status tree would be cumbersome to
+ # understand.
+ svntest.actions.run_and_verify_svn(None, None, [], 'update', wc_dir)
+
+ # Merge changes from rev 4:8 of A/B into A/copy_of_B.
+ expected_output = wc.State(short_copy_of_B_path, {
+ 'F/E/alpha' : Item(status='U ')
+ })
+ expected_status = wc.State(short_copy_of_B_path, {
+ # When we merge multiple sub-targets, we record merge info on each
+ # child.
+ '' : Item(status=' M', wc_rev=8),
+ 'F/E' : Item(status=' M', wc_rev=8),
+ 'F/E/alpha' : Item(status='M ', wc_rev=8),
+ 'F/E/beta' : Item(status=' ', wc_rev=8),
+ 'F/E1' : Item(status=' M', wc_rev=8),
+ 'F/E1/alpha' : Item(status=' ', wc_rev=8),
+ 'F/E1/beta' : Item(status=' ', wc_rev=8),
+ 'lambda' : Item(status=' ', wc_rev=8),
+ 'F' : Item(status=' ', wc_rev=8)
+ })
+ expected_disk = wc.State('', {
+ '' : Item(props={SVN_PROP_MERGE_INFO : '/A/B:5-8'}),
+ 'F/E' : Item(props={SVN_PROP_MERGE_INFO : '/A/B/F/E:5-8'}),
+ 'F/E/alpha' : Item(new_content_for_alpha),
+ 'F/E/beta' : Item("This is the file 'beta'.\n"),
+ 'F' : Item(),
+ 'F/E1' : Item(props={SVN_PROP_MERGE_INFO :
+ '/A/B/F/E:5\n/A/B/F/E1:5-8\n'}),
+ 'F/E1/alpha' : Item(new_content_for_alpha1),
+ 'F/E1/beta' : Item("This is the file 'beta'.\n"),
+ 'lambda' : Item("This is the file 'lambda'.\n")
+ })
+ expected_skip = wc.State(short_copy_of_B_path, { })
+ saved_cwd = os.getcwd()
+ try:
+ os.chdir(svntest.main.work_dir)
+ svntest.actions.run_and_verify_merge(short_copy_of_B_path, '4', '8',
+ sbox.repo_url + '/A/B',
+ expected_output,
+ expected_disk,
+ expected_status,
+ expected_skip,
+ None,
+ None,
+ None,
+ None,
+ None, 1)
+ finally:
+ os.chdir(saved_cwd)
+
+
+
 ########################################################################
 # Run the tests
 
@@ -4611,6 +4842,7 @@
               avoid_repeated_merge_using_inherited_merge_info,
               avoid_repeated_merge_on_subtree_with_merge_info,
               obey_reporter_api_semantics_while_doing_subtree_merges,
+ call_set_path_only_on_subtree_while_doing_subtree_merges,
              ]
 
 if __name__ == '__main__':

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 27 16:47:29 2007

This is an archived mail posted to the Subversion Dev mailing list.