You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/03/04 16:17:49 UTC

svn commit: r1664035 - in /subversion/trunk/subversion: libsvn_wc/externals.c libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c tests/cmdline/externals_tests.py tests/libsvn_wc/op-depth-test.c

Author: rhuijben
Date: Wed Mar  4 15:17:49 2015
New Revision: 1664035

URL: http://svn.apache.org/r1664035
Log:
Don't loose files that replace file externals, when they are replaced
by actual file. The regression test for this issue uncovered the
commit fix in r1663991, but the change to db_base_remove() in this
revision resolves the problem in a different way.

* subversion/libsvn_wc/externals.c
  (svn_wc__external_remove): When deleting a file external, request
    addition of a not-present marker in its place.

* subversion/libsvn_wc/wc-queries.sql
  (STMT_DELETE_BASE_RECURSIVE): Include node itself.

* subversion/libsvn_wc/wc_db.c
  (db_base_remove): Detect if a deleted file external really needs
    to be replaced by a marker. Update caller.

* subversion/tests/cmdline/externals_tests.py
  (file_external_to_normal_file): New test.
  (test_list): Add file_external_to_normal_file.

* subversion/tests/libsvn_wc/op-depth-test.c
  (check_db_rows): Don't use print_row before all values are set
    in the row struct to avoid segfaults.
  (revert_file_externals): Expect not_present rows to appear.

Modified:
    subversion/trunk/subversion/libsvn_wc/externals.c
    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/tests/cmdline/externals_tests.py
    subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c

Modified: subversion/trunk/subversion/libsvn_wc/externals.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/externals.c?rev=1664035&r1=1664034&r2=1664035&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/externals.c (original)
+++ subversion/trunk/subversion/libsvn_wc/externals.c Wed Mar  4 15:17:49 2015
@@ -1496,8 +1496,8 @@ svn_wc__external_remove(svn_wc_context_t
   else
     {
       SVN_ERR(svn_wc__db_base_remove(wc_ctx->db, local_abspath,
-                                     FALSE, FALSE, FALSE,
-                                     SVN_INVALID_REVNUM,
+                                     FALSE, TRUE, FALSE,
+                                     0,
                                      NULL, NULL, scratch_pool));
       SVN_ERR(svn_wc__wq_run(wc_ctx->db, local_abspath,
                              cancel_func, cancel_baton,

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1664035&r1=1664034&r2=1664035&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Mar  4 15:17:49 2015
@@ -249,7 +249,8 @@ WHERE wc_id = ?1 AND IS_STRICT_DESCENDAN
 
 -- STMT_DELETE_BASE_RECURSIVE
 DELETE FROM nodes
-WHERE wc_id = ?1 AND IS_STRICT_DESCENDANT_OF(local_relpath, ?2)
+WHERE wc_id = ?1 AND (local_relpath = ?2 
+                      OR IS_STRICT_DESCENDANT_OF(local_relpath, ?2))
   AND op_depth = 0
 
 -- STMT_DELETE_WORKING_OP_DEPTH

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1664035&r1=1664034&r2=1664035&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Mar  4 15:17:49 2015
@@ -2125,11 +2125,13 @@ db_base_remove(svn_wc__db_wcroot_t *wcro
   int op_depth;
   svn_node_kind_t wrk_kind;
   svn_boolean_t no_delete_wc = FALSE;
+  svn_boolean_t file_external;
 
   SVN_ERR(svn_wc__db_base_get_info_internal(&status, &kind, &revision,
                                             &repos_relpath, &repos_id,
                                             NULL, NULL, NULL, NULL, NULL,
-                                            NULL, NULL, NULL, NULL, NULL,
+                                            NULL, NULL, NULL, NULL,
+                                            &file_external,
                                             wcroot, local_relpath,
                                             scratch_pool, scratch_pool));
 
@@ -2360,35 +2362,69 @@ db_base_remove(svn_wc__db_wcroot_t *wcro
   SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
   SVN_ERR(svn_sqlite__step_done(stmt));
 
-  /* Step 5: handle the BASE node itself */
-  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
-                                    STMT_DELETE_BASE_NODE));
-  SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
-  SVN_ERR(svn_sqlite__step_done(stmt));
-
   SVN_ERR(db_retract_parent_delete(wcroot, local_relpath, 0, scratch_pool));
 
   if (mark_not_present || mark_excluded)
     {
       struct insert_base_baton_t ibb;
-      blank_ibb(&ibb);
+      svn_boolean_t no_marker = FALSE;
 
-      ibb.repos_id = repos_id;
-      ibb.status = mark_excluded ? svn_wc__db_status_excluded
-                                 : svn_wc__db_status_not_present;
-      ibb.kind = kind;
-      ibb.repos_relpath = repos_relpath;
-      ibb.revision = SVN_IS_VALID_REVNUM(marker_revision)
-                        ? marker_revision
-                        : revision;
-
-      /* Depending upon KIND, any of these might get used. */
-      ibb.children = NULL;
-      ibb.depth = svn_depth_unknown;
-      ibb.checksum = NULL;
-      ibb.target = NULL;
+      if (file_external)
+        {
+          const char *parent_local_relpath;
+          const char *name;
+          svn_error_t *err;
 
-      SVN_ERR(insert_base_node(&ibb, wcroot, local_relpath, scratch_pool));
+          /* For file externals we only want to place a not present marker
+             if there is a BASE parent */
+          
+          svn_relpath_split(&parent_local_relpath, &name, local_relpath,
+                            scratch_pool);
+
+          err = svn_wc__db_base_get_info_internal(NULL, NULL, NULL,
+                                                  &repos_relpath, &repos_id,
+                                                  NULL, NULL, NULL, NULL, NULL,
+                                                  NULL, NULL, NULL, NULL, NULL,
+                                                  wcroot, parent_local_relpath,
+                                                  scratch_pool, scratch_pool);
+
+          if (err && err->apr_err != SVN_ERR_WC_PATH_NOT_FOUND)
+            return svn_error_trace(err);
+          else if (err)
+            {
+              svn_error_clear(err);
+              no_marker = TRUE;
+            }
+          else
+            {
+              /* Replace the repos_relpath with something more expected than
+                 the unrelated old file external repository relpath, which
+                 one day may come from a different repository */
+              repos_relpath = svn_relpath_join(repos_relpath, name, scratch_pool);
+            }
+        }
+
+      if (!no_marker)
+        {
+          blank_ibb(&ibb);
+
+          ibb.repos_id = repos_id;
+          ibb.status = mark_excluded ? svn_wc__db_status_excluded
+                                     : svn_wc__db_status_not_present;
+          ibb.kind = kind;
+          ibb.repos_relpath = repos_relpath;
+          ibb.revision = SVN_IS_VALID_REVNUM(marker_revision)
+                            ? marker_revision
+                            : revision;
+
+          /* Depending upon KIND, any of these might get used. */
+          ibb.children = NULL;
+          ibb.depth = svn_depth_unknown;
+          ibb.checksum = NULL;
+          ibb.target = NULL;
+
+          SVN_ERR(insert_base_node(&ibb, wcroot, local_relpath, scratch_pool));
+        }
     }
 
   SVN_ERR(add_work_items(wcroot->sdb, work_items, scratch_pool));
@@ -3480,11 +3516,18 @@ db_external_remove(const svn_skel_t *wor
                    apr_pool_t *scratch_pool)
 {
   svn_sqlite__stmt_t *stmt;
+  int affected_rows;
 
   SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
                                     STMT_DELETE_EXTERNAL));
   SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
-  SVN_ERR(svn_sqlite__step_done(stmt));
+  SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
+
+  if (!affected_rows)
+    return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
+                             _("The node '%s' is not an external."),
+                             path_for_error_message(wcroot, local_relpath,
+                                                    scratch_pool));
 
   SVN_ERR(add_work_items(wcroot->sdb, work_items, scratch_pool));
 

Modified: subversion/trunk/subversion/tests/cmdline/externals_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/externals_tests.py?rev=1664035&r1=1664034&r2=1664035&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/externals_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/externals_tests.py Wed Mar  4 15:17:49 2015
@@ -4086,6 +4086,75 @@ def nested_notification(sbox):
   svntest.actions.run_and_verify_svn(expected_output, [],
                                      'update', sbox.ospath('D1'))
 
+def file_external_to_normal_file(sbox):
+  "change a file external to a normal file"
+
+  sbox.build()
+  wc_dir = sbox.wc_dir
+  sbox.simple_propset('svn:externals', '^/iota iota', 'A')
+  sbox.simple_commit()
+
+  expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
+  expected_status.add({
+    'A/iota'            : Item(status='  ', wc_rev='2', switched='X'),
+  })
+  expected_output = svntest.wc.State(wc_dir, {
+    'A/iota'            : Item(status='A '),
+  })
+
+  svntest.actions.run_and_verify_update(wc_dir, expected_output, None,
+                                        expected_status)
+
+  # Create second working copy in this state
+  sbox2 = sbox.clone_dependent(copy_wc=True)
+
+  sbox.simple_propdel('svn:externals', 'A')
+
+  expected_output = svntest.wc.State(wc_dir, {
+      'A/iota'            : Item(verb='Removed external'),
+  })
+  expected_status.remove('A/iota')
+  expected_status.tweak('A', status=' M')
+  svntest.actions.run_and_verify_update(wc_dir, expected_output, None,
+                                        expected_status)
+
+  sbox.simple_copy('iota', 'A/iota')
+  sbox.simple_commit()
+
+  expected_output = svntest.wc.State(wc_dir, {
+  })
+  expected_status.tweak(wc_rev=3)
+  expected_status.tweak('A', status='  ')
+  expected_status.add({
+    # This case used to triggered a switched status in 1.8.x before this
+    # test (and the fix for this problem) where added.
+    'A/iota'            : Item(status='  ', wc_rev='3'),
+  })
+  svntest.actions.run_and_verify_update(wc_dir, expected_output, None,
+                                        expected_status)
+
+
+  wc_dir = sbox2.wc_dir
+
+  expected_status = svntest.actions.get_virginal_state(wc_dir, 3)
+  expected_output = svntest.wc.State(wc_dir, {
+    'A'                 : Item(status=' U'),
+    'A/iota'            : Item(verb='Removed external', prev_verb='Skipped'),
+  })
+  # This reports an obstruction and removes the file external
+  svntest.actions.run_and_verify_update(wc_dir, expected_output, None,
+                                        expected_status)
+
+  expected_status.add({
+    'A/iota'            : Item(status='  ', wc_rev='3'),
+  })
+  expected_output = svntest.wc.State(wc_dir, {
+    'A/iota'            : Item(status='A '),
+  })
+  # This should bring in the new file
+  svntest.actions.run_and_verify_update(wc_dir, expected_output, None,
+                                        expected_status)
+
 
 ########################################################################
 # Run the tests
@@ -4159,6 +4228,7 @@ test_list = [ None,
               copy_pin_externals_wc_mixed_revisions,
               copy_pin_externals_whitepace_dir,
               nested_notification,
+              file_external_to_normal_file,
              ]
 
 if __name__ == '__main__':

Modified: subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c?rev=1664035&r1=1664034&r2=1664035&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Wed Mar  4 15:17:49 2015
@@ -310,16 +310,17 @@ check_db_rows(svn_test__sandbox_t *b,
       row->repo_revnum = svn_sqlite__column_revnum(stmt, 3);
       row->repo_relpath = svn_sqlite__column_text(stmt, 4, b->pool);
       row->file_external = !svn_sqlite__column_is_null(stmt, 5);
-      if (row->file_external && svn_sqlite__column_is_null(stmt, 6))
-        comparison_baton.errors
-          = svn_error_createf(SVN_ERR_TEST_FAILED, comparison_baton.errors,
-                              "incomplete {%s}", print_row(row, b->pool));
       row->moved_to = svn_sqlite__column_text(stmt, 7, b->pool);
       row->moved_here = svn_sqlite__column_boolean(stmt, 8);
       SVN_ERR(svn_sqlite__column_properties(&props_hash, stmt, 9,
                                             b->pool, b->pool));
       row->props = props_hash_to_text(props_hash, b->pool);
 
+      if (row->file_external && svn_sqlite__column_is_null(stmt, 6))
+        comparison_baton.errors
+          = svn_error_createf(SVN_ERR_TEST_FAILED, comparison_baton.errors,
+                              "incomplete {%s}", print_row(row, b->pool));
+
       key = apr_psprintf(b->pool, "%d %s", row->op_depth, row->local_relpath);
       apr_hash_set(found_hash, key, APR_HASH_KEY_STRING, row);
 
@@ -3582,11 +3583,13 @@ revert_file_externals(const svn_test_opt
   SVN_ERR(sbox_wc_update(&b, "", 1));
   {
     nodes_row_t rows[] = {
-      { 0, "",    "normal", 1, "" },
-      { 0, "f",   "normal", 1, "f" },
-      { 1, "A",   "normal", NO_COPY_FROM },
-      { 0, "h",   "normal", 1, "f", TRUE },
-      { 0, "A/g", "normal", 1, "f", TRUE },
+      { 0, "",    "normal",      1, "" },
+      { 0, "f",   "normal",      1, "f" },
+      { 1, "A",   "normal",      NO_COPY_FROM },
+      { 0, "h",   "normal",      1, "f", TRUE },
+      { 0, "A/g", "normal",      1, "f", TRUE },
+
+      { 0, "g",   "not-present", 0, "g"},
       { 0 }
     };
     SVN_ERR(check_db_rows(&b, "", rows));
@@ -3595,10 +3598,12 @@ revert_file_externals(const svn_test_opt
   SVN_ERR(sbox_wc_revert(&b, "", svn_depth_infinity));
   {
     nodes_row_t rows[] = {
-      { 0, "",    "normal", 1, "" },
-      { 0, "f",   "normal", 1, "f" },
-      { 0, "h",   "normal", 1, "f", TRUE },
-      { 0, "A/g", "normal", 1, "f", TRUE },
+      { 0, "",    "normal",      1, "" },
+      { 0, "f",   "normal",      1, "f" },
+      { 0, "h",   "normal",      1, "f", TRUE },
+      { 0, "A/g", "normal",      1, "f", TRUE },
+
+      { 0, "g",   "not-present", 0, "g"},
       { 0 }
     };
     SVN_ERR(check_db_rows(&b, "", rows));
@@ -3607,9 +3612,11 @@ revert_file_externals(const svn_test_opt
   SVN_ERR(sbox_wc_update(&b, "", 1));
   {
     nodes_row_t rows[] = {
-      { 0, "",    "normal", 1, "" },
-      { 0, "f",   "normal", 1, "f" },
-      { 0, "g",   "normal", 1, "f", TRUE },
+      { 0, "",    "normal",      1, "" },
+      { 0, "f",   "normal",      1, "f" },
+      { 0, "g",   "normal",      1, "f", TRUE },
+
+      { 0, "h",   "not-present", 0, "h"},
       { 0 }
     };
     SVN_ERR(check_db_rows(&b, "", rows));