rhuij...@apache.org writes: > 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"));
I think this should be something like this. Could not add all targets to changelist because some targets don't exist I will add test cases for handling multiple targets. > + 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" This is actually different from what 1.6 displays. Is this fine? Thanks and Regards Noorul
