[PATCH] Issue 838 merge should copy-with-history

Hello

Issue #838

http://subversion.tigris.org/issues/show_bug.cgi?id=838

concerns merge adding new files and directories. This should result
in a copy-with-history not a simple add, otherwise merging back
gets tricky.

Now the "correct" way to do this involves support from the wc layer,
so that when a newly copied directory hierarchy is merged it gets
converted into a copy-with-history. This is possible, but is not
exactly straightforward. The "quick-n-dirty" solution is to change
merge's use of svn_client_add into svn_client_copy. This does the
right thing as far as copy-with-history is concerned, but it will open
new RA sessions to do each copy, retrieving the same data that the
merge itself is going to retrieve.

Here is the simple svn_client_copy using patch. What do people think?

Copy-with-history when adding new files and directories during a merge.

* subversion/libsvn_client/diff.c
  (struct merge_cmd_baton): Add target, path, revision and auth_baton
  members.
  (merge_file_added, merge_dir_added): Use svn_client_copy instead of
  svn_client_add.
  (svn_client_merge): Set new members in merge_cmd_baton.

* subversion/tests/clients/cmdline/merge_tests.py
  (add_with_history): New test. Half the Python doesn't work :-(

Index: ./subversion/libsvn_client/diff.c
===================================================================
--- ./subversion/libsvn_client/diff.c
+++ ./subversion/libsvn_client/diff.c Wed Jul 31 22:02:13 2002
@@ -279,6 +279,10 @@
 
 struct merge_cmd_baton {
   svn_boolean_t force;
+ const char *target;
+ const char *path;
+ const svn_client_revision_t *revision;
+ svn_client_auth_baton_t *auth_baton;
   apr_pool_t *pool;
 };
 
@@ -347,13 +351,20 @@
   struct merge_cmd_baton *merge_b = baton;
   apr_pool_t *subpool = svn_pool_create (merge_b->pool);
   enum svn_node_kind kind;
+ const char *copyfrom_url;
+ const char *child;
 
   SVN_ERR (svn_io_check_path (mine, &kind, subpool));
   switch (kind)
     {
     case svn_node_none:
- SVN_ERR (svn_io_copy_file (yours, mine, TRUE, subpool));
- SVN_ERR (svn_client_add (mine, FALSE, NULL, NULL, subpool));
+ child = svn_path_is_child(merge_b->target, mine, merge_b->pool);
+ assert (child != NULL);
+ copyfrom_url = svn_path_join (merge_b->path, child, merge_b->pool);
+ /* ### FIXME: This will get the file again! */
+ SVN_ERR (svn_client_copy (NULL, copyfrom_url, merge_b->revision, mine,
+ merge_b->auth_baton, NULL, NULL, NULL, NULL,
+ merge_b->pool));
       break;
     case svn_node_dir:
       /* ### create a .drej conflict or something someday? */
@@ -365,32 +376,23 @@
       {
         /* file already exists, is it under version control? */
         svn_wc_entry_t *entry;
+ svn_error_t *err;
         SVN_ERR (svn_wc_entry (&entry, mine, FALSE, subpool));
- if (entry)
- {
- if (entry->schedule == svn_wc_schedule_delete)
- {
- /* If already scheduled for deletion, then carry out
- an add, which is really a (R)eplacement. */
- SVN_ERR (svn_io_copy_file (yours, mine, TRUE, subpool));
- SVN_ERR (svn_client_add (mine, FALSE, NULL, NULL, subpool));
- }
- else
- {
- svn_error_t *err;
- err = svn_wc_merge (older, yours, mine,
- ".older", ".yours", ".working", /* ###? */
- subpool);
- if (err && (err->apr_err != SVN_ERR_WC_CONFLICT))
- return err;
- }
- }
- else
+
+ /* If it's an unversioned file, don't touch it. If its scheduled
+ for deletion, then rm removed it from the working copy and the
+ user must have recreated it, don't touch it */
+ if (!entry || entry->schedule == svn_wc_schedule_delete)
           return svn_error_createf (SVN_ERR_WC_OBSTRUCTED_UPDATE, 0, NULL,
                                     subpool,
                                     "Cannot create file '%s' for addition, "
                                     "because an unversioned file by that name "
                                     "already exists.", mine);
+ err = svn_wc_merge (older, yours, mine,
+ ".older", ".yours", ".working", /* ###? */
+ subpool);
+ if (err && (err->apr_err != SVN_ERR_WC_CONFLICT))
+ return err;
         break;
       }
     default:
@@ -443,20 +445,29 @@
   apr_pool_t *subpool = svn_pool_create (merge_b->pool);
   enum svn_node_kind kind;
   svn_wc_entry_t *entry;
+ const char *copyfrom_url, *child;
+
+ child = svn_path_is_child (merge_b->target, path, subpool);
+ assert (child != NULL);
+ copyfrom_url = svn_path_join (merge_b->path, child, subpool);
 
   SVN_ERR (svn_io_check_path (path, &kind, subpool));
   switch (kind)
     {
     case svn_node_none:
- SVN_ERR (svn_client_mkdir (NULL, path, NULL, NULL, NULL,
- NULL, NULL, subpool));
+ /* ### FIXME: This will get the directory tree again! */
+ SVN_ERR (svn_client_copy (NULL, copyfrom_url, merge_b->revision, path,
+ merge_b->auth_baton, NULL, NULL, NULL, NULL,
+ subpool));
       break;
     case svn_node_dir:
- /* ### should unversioned directories generate 'obstructed update'
- errors? */
+ /* Adding an unversioned directory doesn't destroy data */
       SVN_ERR (svn_wc_entry (&entry, path, TRUE, subpool));
       if (! entry || (entry && entry->schedule == svn_wc_schedule_delete))
- SVN_ERR (svn_client_add (path, FALSE, NULL, NULL, subpool));
+ /* ### FIXME: This will get the directory tree again! */
+ SVN_ERR (svn_client_copy (NULL, copyfrom_url, merge_b->revision, path,
+ merge_b->auth_baton, NULL, NULL, NULL, NULL,
+ subpool));
       break;
     case svn_node_file:
       /* ### create a .drej conflict or something someday? */
@@ -1254,6 +1265,10 @@
     {
       struct merge_cmd_baton merge_cmd_baton;
       merge_cmd_baton.force = force;
+ merge_cmd_baton.target = target_wcpath;
+ merge_cmd_baton.path = path2;
+ merge_cmd_baton.revision = revision2;
+ merge_cmd_baton.auth_baton = auth_baton;
       merge_cmd_baton.pool = pool;
 
       SVN_ERR (do_merge (notify_func,
Index: ./subversion/tests/clients/cmdline/merge_tests.py
===================================================================
--- ./subversion/tests/clients/cmdline/merge_tests.py
+++ ./subversion/tests/clients/cmdline/merge_tests.py Wed Jul 31 21:57:24 2002
@@ -366,6 +366,120 @@
 
 #----------------------------------------------------------------------
 
+# Merge should copy-with-history when adding files or directories
+
+def add_with_history(sbox):
+ "merge and add new files/dirs with history"
+
+ if sbox.build():
+ return 1
+
+ wc_dir = sbox.wc_dir
+
+ C_path = os.path.join(wc_dir, 'A', 'C')
+ F_path = os.path.join(wc_dir, 'A', 'B', 'F')
+ F_url = os.path.join(svntest.main.current_repo_url, 'A', 'B', 'F')
+ Q_path = os.path.join(F_path, 'Q')
+ svntest.main.run_svn(None, 'mkdir', Q_path)
+ foo_path = os.path.join(F_path, 'foo')
+ svntest.main.file_append(foo_path, "foo")
+ svntest.main.run_svn(None, 'add', foo_path)
+ bar_path = os.path.join(Q_path, 'bar')
+ svntest.main.file_append(bar_path, "bar")
+ svntest.main.run_svn(None, 'add', bar_path)
+
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/B/F/Q' : Item(verb='Adding'),
+ 'A/B/F/Q/bar' : Item(verb='Adding'),
+ 'A/B/F/foo' : Item(verb='Adding'),
+ })
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
+ expected_status.tweak(wc_rev=1)
+ expected_status.add({
+ 'A/B/F/Q' : Item(status='_ ', wc_rev=2, repos_rev=2),
+ 'A/B/F/Q/bar' : Item(status='_ ', wc_rev=2, repos_rev=2),
+ 'A/B/F/foo' : Item(status='_ ', wc_rev=2, repos_rev=2),
+ })
+ if svntest.actions.run_and_verify_commit(wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None,
+ None, None,
+ wc_dir):
+ print "commit failed"
+ return 1
+
+### I give up, what is the magic incantation to get this to work?
+#
+# expected_output = wc.State(C_path, {'Q' : Item(status='A '),
+# 'Q/bar' : Item(status='A '),
+# 'foo' : Item(status='A '),
+# })
+#
+# expected_disk = wc.State(C_path, {'Q' : wc.StateItem(),
+# 'Q/bar' : wc.StateItem("bar"),
+# 'foo' : wc.StateItem("foo"),
+# })
+#
+# expected_status= wc.State(C_path, {
+# 'Q' : Item(status='A ', wc_rev='-', copied='+', repos_rev=2),
+# 'foo' : Item(status='A ', wc_rev='-', copied='+', repos_rev=2),
+# })
+#
+# if svntest.actions.run_and_verify_merge(C_path, '1', '2', F_url,
+# expected_output,
+# expected_disk,
+# expected_status):
+# print "merge failed"
+# return 1
+
+ out,err = svntest.main.run_svn(None, 'merge', '-r1:2', F_url, C_path)
+ if (not re.match("^A.*/A/C/Q$", out[0])
+ or not re.match("^A.*/A/C/Q/bar$", out[1])
+ or not re.match("^A.*/A/C/foo$", out[2])):
+ print "merge failed"
+ return 1
+
+### Can't get this to work either.
+#
+# expected_status.add({
+# 'A/C/Q' : Item(status='A ', wc_rev='-', copied='+', repos_rev='2'),
+# 'A/C/Q/bar' : Item(status='A ', wc_rev='-', copied='+', repos_rev='2'),
+# 'A/C/foo' : Item(status='A ', wc_rev='-', copied='+', repos_rev='2'),
+# })
+# if svntest.actions.run_and_verify_status(wc_dir, expected_status):
+# print "status failed"
+# return 1
+
+ # Althogh the merge command produces three lines of output, the
+ # status output is only two lines. The file Q/foo does not appear in
+ # the status because it is simply a child of a copied directory.
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/C/Q' : Item(verb='Adding'),
+ 'A/C/foo' : Item(verb='Adding'),
+ })
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 3)
+ expected_status.tweak(wc_rev=1)
+ expected_status.add({
+ 'A/B/F/Q' : Item(status='_ ', wc_rev=2, repos_rev=3),
+ 'A/B/F/Q/bar' : Item(status='_ ', wc_rev=2, repos_rev=3),
+ 'A/B/F/foo' : Item(status='_ ', wc_rev=2, repos_rev=3),
+ 'A/C/Q' : Item(status='_ ', wc_rev=3, repos_rev=3),
+ 'A/C/Q/bar' : Item(status='_ ', wc_rev=3, repos_rev=3),
+ 'A/C/foo' : Item(status='_ ', wc_rev=3, repos_rev=3),
+ })
+ if svntest.actions.run_and_verify_commit(wc_dir,
+ expected_output,
+ expected_status,
+ None,
+ None, None,
+ None, None,
+ wc_dir):
+ print "commit of merge failed"
+ return 1
+
+#----------------------------------------------------------------------
 
 ########################################################################
 # Run the tests
@@ -374,6 +488,7 @@
 # list all tests here, starting with None:
 test_list = [ None,
               textual_merges_galore,
+ add_with_history,
               # property_merges_galore, # Would be nice to have this.
               # tree_merges_galore, # Would be nice to have this.
               # various_merges_galore, # Would be nice to have this.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org