Re: diff and merge behaviour [PATCH]

Ben Collins-Sussman <sussman@collab.net> writes:

> Branko ÄŒibej <brane@xbc.nu> writes:
>
> > 1. During the merge, several new files that had appeared on /trunk
> > were added to my WC. Well and good; _but_ they were added without
> > ancestry, creating a new node. I submit that "svn merge" should
> > actually perform a copy when adding or replacing files; this would
> > let diff avoid the weirdness I describe below.
>
> Wow, that's a really great idea. I can't believe we never thought of
> this before. Can you file an enhancement request for this?

How about this? It might not be very efficient, when using ra_dav
calling svn_client_copy will open another session, and but it works.
Is it worth commiting in its present form, in order to get the new
behaviour? As usual I had problems writing the Python test :-(

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.

Index: ./subversion/libsvn_client/diff.c
===================================================================
--- ./subversion/libsvn_client/diff.c
+++ ./subversion/libsvn_client/diff.c Fri Jul 26 00:10:58 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);
+ /* ### 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? */
@@ -443,13 +454,19 @@
   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;
 
   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));
+ child = svn_path_is_child (merge_b->target, path, merge_b->pool);
+ assert (child != NULL);
+ copyfrom_url = svn_path_join (merge_b->path, child, merge_b->pool);
+ /* ### 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,
+ merge_b->pool));
       break;
     case svn_node_dir:
       /* ### should unversioned directories generate 'obstructed update'
@@ -1254,6 +1271,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 Thu Jul 25 23:48:52 2002
@@ -366,6 +366,115 @@
 
 #----------------------------------------------------------------------
 
+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
+
+ 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 +483,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