Author: rhuijben
Date: Wed Jul 13 12:28:17 2011
New Revision: 1145972

URL: http://svn.apache.org/viewvc?rev=1145972&view=rev
Log:
Make the 'populate target tree' code in wc_db.c return an error when it
receives an nonexisting node as (root-)target.
(This also resolves some remaining parts of issue #3779)

* subversion/libsvn_wc/wc-queries.sql
  (STMT_INSERT_TARGET_DEPTH_INFINITY,
   STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_INFINITY): Avoid the LIKE operator
     when we can use an index to obtain the same result.

* subversion/libsvn_wc/wc_db.c
  (populate_targets_tree): Don't pass/generate the like argument. Obtain the
    number of affected rows and check if the node actually exists if the number
    of affected rows is 0.

* subversion/svn/changelist-cmd.c
  (svn_cl__changelist): Return an error when an error occurred, just like
    the other svn commands that handle SVN_WC_PATH_NOT_FOUND.

* subversion/tests/cmdline/changelist_tests.py
  (add_remove_non_existent_target,
   add_remove_unversioned_target): New tests. Based on a patch by Noorul
     Islam K M, but tweaked for the different error handling.
  (test_list): Add new tests.

* subversion/tests/cmdline/tree_conflict_tests.py
  (actual_only_node_behaviour): Update expected result and remove review marker
    as we now produce a warning.

Found by: danielsh
          Noorul Islam K M <noorul{_AT_}collab.net>

Modified:
    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/svn/changelist-cmd.c
    subversion/trunk/subversion/tests/cmdline/changelist_tests.py
    subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1145972&r1=1145971&r2=1145972&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Jul 13 12:28:17 
2011
@@ -473,7 +473,10 @@ WHERE wc_id = ?1 AND (parent_relpath = ?
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
 SELECT wc_id, local_relpath, parent_relpath, kind
 FROM nodes_current
-WHERE wc_id = ?1 AND (local_relpath = ?2 OR local_relpath LIKE ?3 ESCAPE '#')
+WHERE wc_id = ?1
+       AND (?2 = ''
+            OR local_relpath = ?2 
+            OR (local_relpath > ?2 || '/' AND local_relpath < ?2 || '0'))
 
 -- STMT_INSERT_TARGET_WITH_CHANGELIST
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
@@ -503,8 +506,11 @@ INSERT INTO targets_list(wc_id, local_re
 SELECT N.wc_id, N.local_relpath, N.parent_relpath, N.kind
   FROM actual_node AS A JOIN nodes_current AS N
     ON A.wc_id = N.wc_id AND A.local_relpath = N.local_relpath
- WHERE N.wc_id = ?1 AND A.changelist = ?3
-       AND (N.local_relpath = ?2 OR N.local_relpath LIKE ?4 ESCAPE '#')
+ WHERE N.wc_id = ?1
+       AND (?2 = ''
+            OR N.local_relpath = ?2 
+            OR (N.local_relpath > ?2 || '/' AND N.local_relpath < ?2 || '0'))
+       AND A.changelist = ?3
 
 -- STMT_SELECT_TARGETS
 SELECT local_relpath, parent_relpath from targets_list

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1145972&r1=1145971&r2=1145972&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Jul 13 12:28:17 2011
@@ -4741,14 +4741,7 @@ populate_targets_tree(svn_wc__db_wcroot_
                       apr_pool_t *scratch_pool)
 {
   svn_sqlite__stmt_t *stmt;
-  const char *like_arg;
-
-  if (depth == svn_depth_infinity)
-    {
-      /* Calculate a value we're going to need later. */
-      like_arg = construct_like_arg(local_relpath, scratch_pool);
-    }
-
+  int affected_rows = 0;
   SVN_ERR(svn_sqlite__exec_statements(wcroot->sdb,
                                       STMT_CREATE_TARGETS_LIST));
 
@@ -4786,15 +4779,16 @@ populate_targets_tree(svn_wc__db_wcroot_
 
       for (i = 0; i < changelist_filter->nelts; i++)
         {
+          int sub_affected;
           const char *changelist = APR_ARRAY_IDX(changelist_filter, i,
                                                  const char *);
 
           SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
           SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
                                     local_relpath, changelist));
-          if (depth == svn_depth_infinity)
-            SVN_ERR(svn_sqlite__bind_text(stmt, 4, like_arg));
-          SVN_ERR(svn_sqlite__step_done(stmt));
+          SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
+
+          affected_rows += sub_affected;
         }
     }
   else /* No changelist filtering */
@@ -4827,9 +4821,21 @@ populate_targets_tree(svn_wc__db_wcroot_
 
       SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
       SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
-      if (depth == svn_depth_infinity)
-        SVN_ERR(svn_sqlite__bind_text(stmt, 3, like_arg));
-      SVN_ERR(svn_sqlite__step_done(stmt));
+      SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
+    }
+
+  /* Does the target exist? */
+  if (affected_rows == 0)
+    {
+      svn_boolean_t exists;
+      SVN_ERR(does_node_exist(&exists, wcroot, local_relpath));
+
+      if (!exists)
+        return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
+                                 _("The node '%s' was not found."),
+                                 path_for_error_message(wcroot,
+                                                        local_relpath,
+                                                        scratch_pool));
     }
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/svn/changelist-cmd.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/changelist-cmd.c?rev=1145972&r1=1145971&r2=1145972&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/changelist-cmd.c (original)
+++ subversion/trunk/subversion/svn/changelist-cmd.c Wed Jul 13 12:28:17 2011
@@ -45,6 +45,7 @@ svn_cl__changelist(apr_getopt_t *os,
   svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
   apr_array_header_t *targets;
   svn_depth_t depth = opt_state->depth;
+  svn_boolean_t success = TRUE;
 
   /* If we're not removing changelists, then our first argument should
      be the name of a changelist. */
@@ -98,24 +99,31 @@ svn_cl__changelist(apr_getopt_t *os,
 
   if (changelist_name)
     {
-      return svn_cl__try
-              (svn_client_add_to_changelist(targets, changelist_name,
+      SVN_ERR(svn_cl__try(
+               svn_client_add_to_changelist(targets, changelist_name,
                                             depth, opt_state->changelists,
                                             ctx, pool),
-               NULL, opt_state->quiet,
+               &success, opt_state->quiet,
                SVN_ERR_UNVERSIONED_RESOURCE,
                SVN_ERR_WC_PATH_NOT_FOUND,
-               SVN_NO_ERROR);
+               SVN_NO_ERROR));
     }
   else
     {
-      return svn_cl__try
-              (svn_client_remove_from_changelists(targets, depth,
+      SVN_ERR(svn_cl__try(
+               svn_client_remove_from_changelists(targets, depth,
                                                   opt_state->changelists,
                                                   ctx, pool),
-               NULL, opt_state->quiet,
+               &success, opt_state->quiet,
                SVN_ERR_UNVERSIONED_RESOURCE,
                SVN_ERR_WC_PATH_NOT_FOUND,
-               SVN_NO_ERROR);
+               SVN_NO_ERROR));
     }
+
+  if (!success)
+    return svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL,
+                            _("Could not display info for all targets because "
+                              "some targets don't exist"));
+  else
+    return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/tests/cmdline/changelist_tests.py
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/changelist_tests.py?rev=1145972&r1=1145971&r2=1145972&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/changelist_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/changelist_tests.py Wed Jul 13 
12:28:17 2011
@@ -1132,6 +1132,43 @@ def revert_deleted_in_changelist(sbox):
                                      'revert', '-R', sbox.ospath('A'))
   svntest.actions.run_and_verify_info(expected_infos, sbox.ospath('A/mu'))
 
+def add_remove_non_existent_target(sbox):
+  "add and remove non-existent target to changelist"
+
+  sbox.build(read_only = True)
+  wc_dir = sbox.wc_dir
+  bogus_path = os.path.join(wc_dir, 'A', 'bogus')
+
+  expected_err = "svn: warning: W155010: The node '" + \
+                 re.escape(os.path.abspath(bogus_path)) + \
+                 "' was not found"
+
+  svntest.actions.run_and_verify_svn(None, None, expected_err,
+                                     'changelist', 'testlist',
+                                     bogus_path)
+
+  svntest.actions.run_and_verify_svn(None, None, expected_err,
+                                     'changelist', bogus_path,
+                                      '--remove')
+
+def add_remove_unversioned_target(sbox):
+  "add and remove unversioned target to changelist"
+
+  sbox.build(read_only = True)
+  unversioned = sbox.ospath('unversioned')
+  svntest.main.file_write(unversioned, "dummy contents", 'w+')
+
+  expected_err = "svn: warning: W155010: The node '" + \
+                 re.escape(os.path.abspath(unversioned)) + \
+                 "' was not found"
+
+  svntest.actions.run_and_verify_svn(None, None, expected_err,
+                                     'changelist', 'testlist',
+                                     unversioned)
+
+  svntest.actions.run_and_verify_svn(None, None, expected_err,
+                                     'changelist', unversioned,
+                                      '--remove')
 
 
 ########################################################################
@@ -1153,6 +1190,8 @@ test_list = [ None,
               move_added_keeps_changelist,
               change_to_dir,
               revert_deleted_in_changelist,
+              add_remove_non_existent_target,
+              add_remove_unversioned_target,
              ]
 
 if __name__ == '__main__':

Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1145972&r1=1145971&r2=1145972&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Wed Jul 13 
12:28:17 2011
@@ -1164,10 +1164,8 @@ def actual_only_node_behaviour(sbox):
   run_and_verify_svn(None, expected_stdout, expected_stderr,
                      "cat", "-r", "BASE", foo_path)
   # changelist (cl)
-  ### this does not error out -- needs review
-  ### the item does not end up in the changelist so this is a cosmetic problem
   expected_stdout = None
-  expected_stderr = []
+  expected_stderr = ".*svn: warning: W155010: The node '.*foo' was not found."
   run_and_verify_svn(None, expected_stdout, expected_stderr,
                      "changelist", "my_changelist", foo_path)
 


Reply via email to