Philip Martin <philip@codematters.co.uk> writes:
> *shrug*  it's what I prefer.
Consider it done, sir.
> > Especially considering there is considerable documentation on
> > the issue that you need to look at anyway.
> 
> I was expecting your elegant patch and lucid log message to be
> sufficient.
Ben's elegant patch and lucid log message follow:
--------------------8-<-------cut-here---------8-<-----------------------
Stop checking the URL and the revision of the existing working copy dir when
attempting to replace the directory with a new directory that has history.
Fixes Issue #2144.
* subversion/includes/svn_wc.h
  (svn_wc_ensure_adm): Adjust the documentation about how we handle
    scheduled deleted directories.
* subversion/libsvn_wc/adm_files.c
  (check_adm_exists): Do not check the URL or the revision of a directory
    scheduled for deletion.
* subversion/tests/clients/cmdline/merge_tests.py
  (merge_dir_replace): New test for testing merging a directory replacement.
  (test_list): Add merge_dir_replace to the list of tests to run.
Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h	(revision 12095)
+++ subversion/include/svn_wc.h	(working copy)
@@ -1145,10 +1145,10 @@
  * initialized to an unlocked state.
  *
  * If the administrative area already exists then the given @a url
- * must match the URL in the administrative area or an error will be
- * returned. The given @a revision must also match except for the
- * special case of adding a directory that has a name matching one
- * scheduled for deletion, in which case @a revision must be zero.
+ * must match the URL in the administrative area and the given
+ * @a revision must match the BASE of the working copy dir unless
+ * the admin directory is scheduled for deletion or the
+ * SVN_ERR_WC_OBSTRUCTED_UPDATE error will be returned.
  *
  * @a uuid may be @c NULL.
 
Index: subversion/libsvn_wc/adm_files.c
===================================================================
--- subversion/libsvn_wc/adm_files.c	(revision 12095)
+++ subversion/libsvn_wc/adm_files.c	(working copy)
@@ -892,27 +892,27 @@
                                   _("No entry for '%s'"),
                                   svn_path_local_style (path, pool));
 
-      /* The revisions must match except when adding a directory with a
-         name that matches a directory scheduled for deletion. That's
-         because the deleted directory's administrative dir will still be
-         in place but will have an arbitrary revision. */
-      if (entry->revision != revision
-          && !(entry->schedule == svn_wc_schedule_delete && revision == 0))
-        return
-          svn_error_createf
-          (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
-           _("Revision %ld doesn't match existing revision %ld in '%s'"),
-           revision, entry->revision,
-           svn_path_local_style (path, pool));
+      /* When the directory exists and is scheduled for deletion do not
+       * check the revision or the URL.  The revision can be any 
+       * arbitrary revision and the URL may differ if the add is
+       * being driven from a merge which will have a different URL. */
+      if (entry->schedule != svn_wc_schedule_delete)
+        {
+          if (entry->revision != revision)
+            return
+              svn_error_createf
+              (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
+               _("Revision %ld doesn't match existing revision %ld in '%s'"),
+               revision, entry->revision, path);
 
-      /** ### comparing URLs, should they be canonicalized first? */
-      if (strcmp (entry->url, url) != 0)
-        return
-          svn_error_createf
-          (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
-           _("URL '%s' doesn't match existing URL '%s' in '%s'"),
-           url, entry->url,
-           svn_path_local_style (path, pool));
+          /** ### comparing URLs, should they be canonicalized first? */
+          if (strcmp (entry->url, url) != 0)
+            return
+              svn_error_createf
+              (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
+               _("URL '%s' doesn't match existing URL '%s' in '%s'"),
+               url, entry->url, path);
+	}
     }
 
   *exists = wc_exists;
Index: subversion/tests/clients/cmdline/merge_tests.py
===================================================================
--- subversion/tests/clients/cmdline/merge_tests.py	(revision 12095)
+++ subversion/tests/clients/cmdline/merge_tests.py	(working copy)
@@ -2195,6 +2195,129 @@
     os.chdir(saved_cwd)
 
 
+#----------------------------------------------------------------------
+# A merge that replaces a directory
+# Tests for Issue #2144
+  
+def merge_dir_replace(sbox):
+  "merge a replacement of a directory"
+
+  sbox.build()
+  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 = svntest.main.current_repo_url + '/A/B/F'
+
+  foo_path = os.path.join(F_path, 'foo')
+
+  # Create foo in F
+  svntest.actions.run_and_verify_svn(None, None, [], 'mkdir', foo_path)
+
+  expected_output = wc.State(wc_dir, {
+    '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/foo'    : Item(status='  ', wc_rev=2, repos_rev=2),
+    })
+  svntest.actions.run_and_verify_commit(wc_dir,
+                                        expected_output,
+                                        expected_status,
+                                        None, None, None, None, None,
+                                        wc_dir)
+  
+  # Merge foo onto C
+  expected_output = wc.State(C_path, {
+    'foo' : Item(status='A '),
+    })
+  expected_disk = wc.State('', {
+    'foo' : Item(),
+    })
+  expected_status = wc.State(C_path, {
+    ''    : Item(status='  ', wc_rev=1, repos_rev=2),
+    'foo' : Item(status='A ', wc_rev='-', copied='+', repos_rev=2),
+    })
+  expected_skip = wc.State(C_path, { })
+  svntest.actions.run_and_verify_merge(C_path, '1', '2', F_url,
+                                       expected_output,
+                                       expected_disk,
+                                       expected_status,
+                                       expected_skip)
+
+  # Commit merge of foo onto C
+  expected_output = svntest.wc.State(wc_dir, {
+    '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/foo'  : Item(status='  ', wc_rev=2, repos_rev=3),
+    'A/C/foo'    : Item(status='  ', wc_rev=3, repos_rev=3),
+    })
+  svntest.actions.run_and_verify_commit(wc_dir,
+                                        expected_output,
+                                        expected_status,
+                                        None, None, None, None, None,
+                                        wc_dir)
+
+  # Delete foo on F
+  svntest.actions.run_and_verify_svn(None, None, [], 'rm', foo_path)
+  expected_output = svntest.wc.State(wc_dir, {
+    'A/B/F/foo'   : Item(verb='Deleting'),
+    })
+  expected_status = svntest.actions.get_virginal_state(wc_dir, 4)
+  expected_status.tweak(wc_rev=1)
+  expected_status.add({
+    'A/C/foo'     : Item(status='  ', wc_rev=3, repos_rev=4),
+    })
+  svntest.actions.run_and_verify_commit(wc_dir,
+                                        expected_output,
+                                        expected_status,
+                                        None, None, None, None, None,
+                                        wc_dir)
+
+  # Recreate foo in F
+  svntest.actions.run_and_verify_svn(None, None, [], 'mkdir', foo_path)
+
+  expected_output = wc.State(wc_dir, {
+    'A/B/F/foo' : Item(verb='Adding'),
+    })
+  expected_status = svntest.actions.get_virginal_state(wc_dir, 5)
+  expected_status.tweak(wc_rev=1)
+  expected_status.add({
+    'A/B/F/foo'    : Item(status='  ', wc_rev=5, repos_rev=5),
+    'A/C/foo'     : Item(status='  ', wc_rev=3, repos_rev=5),
+    })
+  svntest.actions.run_and_verify_commit(wc_dir,
+                                        expected_output,
+                                        expected_status,
+                                        None, None, None, None, None,
+                                        wc_dir)
+  # Merge replacement of foo onto C
+  expected_output = wc.State(C_path, {
+    'foo' : Item(status='D '),
+    'foo' : Item(status='A '),
+    })
+  expected_disk = wc.State('', {
+    'foo' : Item(),
+    })
+  expected_status = wc.State(C_path, {
+    ''    : Item(status='  ', wc_rev=1, repos_rev=5),
+    'foo' : Item(status='R ', wc_rev='-', copied='+', repos_rev=5),
+    })
+  expected_skip = wc.State(C_path, { })
+  svntest.actions.run_and_verify_merge(C_path, '2', '5', F_url,
+                                       expected_output,
+                                       expected_disk,
+                                       expected_status,
+                                       expected_skip,
+                                       None, None, None, None, None,
+                                       0, # skip props
+                                       0) # don't do a dry-run the output differs
+
+  
 ########################################################################
 # Run the tests
 
@@ -2220,6 +2343,7 @@
               merge_funny_chars_on_path,
               merge_keyword_expansions,
               merge_prop_change_to_deleted_target,
+              merge_dir_replace,
               # 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.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Dec  1 18:09:26 2004