Author: stsp Date: Fri Aug 12 16:05:15 2011 New Revision: 1157172 URL: http://svn.apache.org/viewvc?rev=1157172&view=rev Log: Make 'svn revert' error out if only one side of a move is reverted.
For now, 'revert' and always refuses to incompletely revert a move. This will later be extended so that both sides of a move are reverted if no other local mods affect the moved nodes. * subversion/libsvn_client/revert.c (path_is_in_target_list): New helper. Figures out of a given node is within the revert target list. (check_moves): New helper. Checks for moved-away and moved-here nodes within the to-be-reverted subtree, and throws an error if only one half of a move is present. (svn_client_revert2): Run check_moves() if we're not reverting the entire WC. * subversion/tests/cmdline/copy_tests.py (mv_and_revert_directory): Revert both sides of move to avoid test failure. Modified: subversion/trunk/subversion/libsvn_client/revert.c subversion/trunk/subversion/tests/cmdline/copy_tests.py Modified: subversion/trunk/subversion/libsvn_client/revert.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/revert.c?rev=1157172&r1=1157171&r2=1157172&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_client/revert.c (original) +++ subversion/trunk/subversion/libsvn_client/revert.c Fri Aug 12 16:05:15 2011 @@ -109,6 +109,124 @@ revert(void *baton, apr_pool_t *result_p return SVN_NO_ERROR; } +/* Set *IS_IN_TARGET_LIST to TRUE if LOCAL_ABSPATH is an element + * or a child of an element in TARGET_LIST. Else return FALSE. */ +static svn_error_t * +path_is_in_target_list(svn_boolean_t *is_in_target_list, + const char *local_abspath, + const apr_array_header_t *target_list, + apr_pool_t *scratch_pool) +{ + int i; + + for (i = 0; i < target_list->nelts; i++) + { + const char *target = APR_ARRAY_IDX(target_list, i, const char *); + const char *target_abspath; + + SVN_ERR(svn_dirent_get_absolute(&target_abspath, target, scratch_pool)); + *is_in_target_list = (svn_dirent_skip_ancestor(target_abspath, + local_abspath) != NULL); + if (*is_in_target_list) + return SVN_NO_ERROR; + } + + return SVN_NO_ERROR; +} + +/* If any children of LOCAL_ABSPATH have been moved-here + * or moved-away, verify that both halfs of each move are + * in the revert target list. */ +static svn_error_t * +check_moves(const char *local_abspath, + const apr_array_header_t *target_list, + svn_wc_context_t *wc_ctx, + apr_pool_t *scratch_pool) +{ + const char *moved_to_abspath; + const char *copy_op_root_abspath; + const char *moved_from_abspath; + const char *delete_op_root_abspath; + svn_error_t *err; + + err = svn_wc__node_was_moved_away(&moved_to_abspath, + ©_op_root_abspath, + wc_ctx, local_abspath, + scratch_pool, scratch_pool); + if (err) + { + if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND) + { + svn_error_clear(err); + moved_to_abspath = NULL; + copy_op_root_abspath = NULL; + } + else + return svn_error_trace(err); + } + + if (moved_to_abspath && copy_op_root_abspath && + strcmp(moved_to_abspath, copy_op_root_abspath) == 0) + { + svn_boolean_t is_in_target_list; + + SVN_ERR(path_is_in_target_list(&is_in_target_list, + moved_to_abspath, target_list, + scratch_pool)); + if (!is_in_target_list) + { + /* TODO: If the moved-away node has no post-move mods + * we can just add it to the target list. */ + return svn_error_createf( + SVN_ERR_ILLEGAL_TARGET, NULL, + _("Cannot revert '%s' because it was moved to '%s' " + "which is not part of the revert; both sides of " + "the move must be reverted together"), + svn_dirent_local_style(local_abspath, scratch_pool), + svn_dirent_local_style(moved_to_abspath, scratch_pool)); + } + } + + err = svn_wc__node_was_moved_here(&moved_from_abspath, + &delete_op_root_abspath, + wc_ctx, local_abspath, + scratch_pool, scratch_pool); + if (err) + { + if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND) + { + svn_error_clear(err); + moved_from_abspath = NULL; + delete_op_root_abspath = NULL; + } + else + return svn_error_trace(err); + } + + if (moved_from_abspath && delete_op_root_abspath && + strcmp(moved_from_abspath, delete_op_root_abspath) == 0) + { + svn_boolean_t is_in_target_list; + + SVN_ERR(path_is_in_target_list(&is_in_target_list, + moved_from_abspath, target_list, + scratch_pool)); + if (!is_in_target_list) + { + /* TODO: If the moved-here node has no post-move mods + * we can just add it to the target list. */ + return svn_error_createf( + SVN_ERR_ILLEGAL_TARGET, NULL, + _("Cannot revert '%s' because it was moved from '%s' " + "which is not part of the revert; both sides of " + "the move must be reverted together"), + svn_dirent_local_style(local_abspath, scratch_pool), + svn_dirent_local_style(moved_from_abspath, scratch_pool)); + } + } + + return SVN_NO_ERROR; +} svn_error_t * svn_client_revert2(const apr_array_header_t *paths, @@ -170,6 +288,13 @@ svn_client_revert2(const apr_array_heade local_abspath, pool)); lock_target = wc_root ? local_abspath : svn_dirent_dirname(local_abspath, pool); + if (!wc_root) + { + err = check_moves(local_abspath, paths, ctx->wc_ctx, subpool); + if (err) + goto errorful; + } + err = svn_wc__call_with_write_lock(revert, &baton, ctx->wc_ctx, lock_target, FALSE, pool, pool); if (err) Modified: subversion/trunk/subversion/tests/cmdline/copy_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/copy_tests.py?rev=1157172&r1=1157171&r2=1157172&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/copy_tests.py (original) +++ subversion/trunk/subversion/tests/cmdline/copy_tests.py Fri Aug 12 16:05:15 2011 @@ -777,8 +777,9 @@ def mv_and_revert_directory(sbox): # Issue 932: revert failed to lock the parent directory svntest.actions.run_and_verify_svn(None, None, [], 'revert', '--recursive', - new_E_path) + E_path, new_E_path) expected_status.remove('A/B/F/E', 'A/B/F/E/alpha', 'A/B/F/E/beta') + expected_status.tweak('A/B/E', 'A/B/E/alpha', 'A/B/E/beta', status=' ') svntest.actions.run_and_verify_status(wc_dir, expected_status)