[[[ When a Subversion client tries to commit a deletion, a property modification, or a text modification, and the item no longer exists in the repository, the commit should fail with an "out of date" error. The error message should be consistent for all remote access methods (v. issue #2740). * subversion/libsvn_client/commit_util.c (out_of_date): New function. (do_item_commit): To give the user an "out of date" error instead of "not found", replace SVN_ERR() with manual error handling when asking the repo to delete dirs or files, to open files, or to receive property deltas during commit. * subversion/libsvn_client/commit.c (svn_client_commit4): Give the user an "out of date" error instead of "not found", if the RA editor can't be initialized. * subversion/libsvn_ra_serf/commit.c (delete_entry): Treat the response '404 Not Found' as an error. * subversion/libsvn_ra_neon/commit.c (commit_delete_entry): Treat the response '404 Not Found' as an error. * subversion/libsvn_repos/commit.c (out_of_date): Extend for case where we don't know the node type because the node does not exist. (delete_entry): If a path is not found, return an error. * subversion/tests/cmdline/commit_tests.py (commit_out_of_date_deletions): Extend test cases to include all relevant combinations of deletion vs (property-change | text-change | deletion). Use svntest.actions.run_and_verify_commit consistently. ]]] Index: subversion/libsvn_client/commit_util.c =================================================================== --- subversion/libsvn_client/commit_util.c (revision 29632) +++ subversion/libsvn_client/commit_util.c (working copy) @@ -44,6 +44,16 @@ #define SVN_CLIENT_COMMIT_DEBUG */ +/* Create and return a generic out-of-dateness error. */ +static svn_error_t * +out_of_date(const char *path, svn_node_kind_t kind) +{ + return svn_error_createf(SVN_ERR_WC_NOT_UP_TO_DATE, NULL, + (kind == svn_node_dir + ? _("Directory '%s' is out of date") + : _("File '%s' is out of date")), + path); +} /*** Harvesting Commit Candidates ***/ @@ -1082,6 +1092,7 @@ do_item_commit(void **dir_baton, apr_hash_t *file_mods = cb_baton->file_mods; const char *notify_path_prefix = cb_baton->notify_path_prefix; svn_client_ctx_t *ctx = cb_baton->ctx; + svn_error_t *err = SVN_NO_ERROR; /* Do some initializations. */ *dir_baton = NULL; @@ -1191,8 +1202,17 @@ do_item_commit(void **dir_baton, if (item->state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE) { assert(parent_baton); - SVN_ERR(editor->delete_entry(path, item->revision, - parent_baton, pool)); + err = editor->delete_entry(path, item->revision, parent_baton, pool); + if (err) + { + if (err->apr_err == SVN_ERR_FS_NOT_FOUND + || err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND) + { + svn_error_clear(err); + return out_of_date(path, kind); + } + return err; + } } /* If this item is supposed to be added, do so. */ @@ -1248,9 +1268,19 @@ do_item_commit(void **dir_baton, if (! file_baton) { assert(parent_baton); - SVN_ERR(editor->open_file(path, parent_baton, - item->revision, - file_pool, &file_baton)); + err = editor->open_file(path, parent_baton, + item->revision, + file_pool, &file_baton); + if (err) + { + if (err->apr_err == SVN_ERR_FS_NOT_FOUND + || err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND)+ { + svn_error_clear(err); + return out_of_date(path, kind); + } + return err; + } + } } else @@ -1259,24 +1290,38 @@ do_item_commit(void **dir_baton, { if (! parent_baton) { - SVN_ERR(editor->open_root - (cb_baton->edit_baton, item->revision, - pool, dir_baton)); + SVN_ERR(editor->open_root(cb_baton->edit_baton, + item->revision, pool, + dir_baton)); } else { - SVN_ERR(editor->open_directory - (path, parent_baton, item->revision, - pool, dir_baton)); + SVN_ERR(editor->open_directory(path, parent_baton, + item->revision, pool, + dir_baton)); } } } SVN_ERR(svn_wc_entry(&tmp_entry, item->path, adm_access, TRUE, pool)); - SVN_ERR(svn_wc_transmit_prop_deltas - (item->path, adm_access, tmp_entry, editor, - (kind == svn_node_dir) ? *dir_baton : file_baton, NULL, pool)); + /* When committing a dir that no longer exists in the repo, the + "not found" error appears during the delta transmisssion. */ + err = svn_wc_transmit_prop_deltas + (item->path, adm_access, tmp_entry, editor, + (kind == svn_node_dir) ? *dir_baton : file_baton, NULL, pool); + + if (err) + { + if (err->apr_err == SVN_ERR_FS_NOT_FOUND + || err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND) + { + svn_error_clear(err); + return out_of_date(path, kind); + } + return err; + } + /* Make any additional client -> repository prop changes. */ if (item->outgoing_prop_changes) { @@ -1312,9 +1357,19 @@ do_item_commit(void **dir_baton, if (! file_baton) { assert(parent_baton); - SVN_ERR(editor->open_file(path, parent_baton, - item->revision, - file_pool, &file_baton)); + err = editor->open_file(path, parent_baton, + item->revision, + file_pool, &file_baton); + if (err) + { + if (err->apr_err == SVN_ERR_FS_NOT_FOUND + || err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND) + { + svn_error_clear(err); + return out_of_date(path, svn_node_file); + } + return err; + } } /* Add this file mod to the FILE_MODS hash. */ Index: subversion/libsvn_client/commit.c =================================================================== --- subversion/libsvn_client/commit.c (revision 29632) +++ subversion/libsvn_client/commit.c (working copy) @@ -1689,8 +1689,18 @@ svn_client_commit4(svn_commit_info_t **commit_info_p, base_url, base_dir, base_dir_access, log_msg, commit_items, commit_info_p, TRUE, lock_tokens, keep_locks, pool))) - goto cleanup; - + { + /* For consistency, re-raise a "not found" error as "out of date". */ + if (cmt_err->apr_err == SVN_ERR_FS_NOT_FOUND + || cmt_err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND) + { + svn_error_clear(cmt_err); + cmt_err = svn_error_createf(SVN_ERR_WC_NOT_UP_TO_DATE, NULL, + _("Directory '%s' is out of date"), + base_dir); + } + goto cleanup; + } /* Make a note that we have a commit-in-progress. */ commit_in_progress = TRUE; Index: subversion/libsvn_ra_serf/commit.c =================================================================== --- subversion/libsvn_ra_serf/commit.c (revision 29632) +++ subversion/libsvn_ra_serf/commit.c (working copy) @@ -1287,11 +1287,8 @@ delete_entry(const char *path, return err; } - /* 204 No Content: item successfully deleted - 404 Not found: ignored, the item might have been deleted in this - transaction. */ - if (delete_ctx->progress.status != 204 && - delete_ctx->progress.status != 404) + /* 204 No Content: item successfully deleted */ + if (delete_ctx->progress.status != 204) { return return_response_err(handler, &delete_ctx->progress); } Index: subversion/libsvn_ra_neon/commit.c =================================================================== --- subversion/libsvn_ra_neon/commit.c (revision 29632) +++ subversion/libsvn_ra_neon/commit.c (working copy) @@ -751,15 +751,11 @@ static svn_error_t * commit_delete_entry(const char *p APR_HASH_KEY_STRING, SVN_DAV_OPTION_KEEP_LOCKS); } - /* 404 is ignored, because mod_dav_svn is effectively merging - against the HEAD revision on-the-fly. In such a universe, a - failed deletion (because it's already missing) is OK; deletion - is an idempotent merge operation. */ serr = svn_ra_neon__simple_request(&code, parent->cc->ras, "DELETE", child, extra_headers, NULL, - 204 /* Created */, - 404 /* Not Found */, pool); + 204 /* No Content */, + NULL, pool); /* A locking-related error most likely means we were deleting a directory rather than a file, and didn't send all of the Index: subversion/tests/cmdline/commit_tests.py =================================================================== --- subversion/tests/cmdline/commit_tests.py (revision 29632) +++ subversion/tests/cmdline/commit_tests.py (working copy) @@ -1608,62 +1608,100 @@ def commit_nonrecursive(sbox): #---------------------------------------------------------------------- # Regression for #1017: ra_neon was allowing the deletion of out-of-date # files or dirs, which majorly violates Subversion's semantics. +# An out-of-date error should be raised if the object to be committed has +# already been deleted or modified in the repo. - def commit_out_of_date_deletions(sbox): "commit deletion of out-of-date file or dir" + # Path Repo WC backup + # =========== ==== ========= + # A/C pset del + # A/I del pset + # A/B/F del del + # A/D/H/omega text del + # A/B/E/alpha pset del + # A/D/H/chi del text + # A/B/E/beta del pset + # A/D/H/psi del del + sbox.build() wc_dir = sbox.wc_dir + + # Need another empty dir + I_path = os.path.join(wc_dir, 'A', 'I') + os.mkdir(I_path) + svntest.main.run_svn(None, 'add', I_path) + svntest.main.run_svn(None, 'ci', '-m', 'prep', wc_dir) + svntest.main.run_svn(None, 'up', wc_dir) # Make a backup copy of the working copy wc_backup = sbox.add_wc_path('backup') svntest.actions.duplicate_dir(wc_dir, wc_backup) - # Change omega's text, and make a propchange to A/C directory - omega_path = os.path.join(wc_dir, 'A', 'D', 'H', 'omega') + # Edits in wc 1 C_path = os.path.join(wc_dir, 'A', 'C') - svntest.main.file_append(omega_path, 'appended omega text') + omega_path = os.path.join(wc_dir, 'A', 'D', 'H', 'omega') + alpha_path = os.path.join(wc_dir, 'A', 'B', 'E', 'alpha') svntest.main.run_svn(None, 'propset', 'fooprop', 'foopropval', C_path) + svntest.main.file_append(omega_path, 'appended omega text') + svntest.main.run_svn(None, 'propset', 'fooprop', 'foopropval', alpha_path) - # Commit revision 2. + # Deletions in wc 1 + I_path = os.path.join(wc_dir, 'A', 'I') + F_path = os.path.join(wc_dir, 'A', 'B', 'F') + chi_path = os.path.join(wc_dir, 'A', 'D', 'H', 'chi') + beta_path = os.path.join(wc_dir, 'A', 'B', 'E', 'beta') + psi_path = os.path.join(wc_dir, 'A', 'D', 'H', 'psi') + svntest.main.run_svn(None, 'rm', I_path, F_path, chi_path, beta_path, + psi_path) + + # Commit in wc 1 expected_output = svntest.wc.State(wc_dir, { - 'A/D/H/omega' : Item(verb='Sending'), - 'A/C' : Item(verb='Sending'), - }) - expected_status = svntest.actions.get_virginal_state(wc_dir, 1) - expected_status.tweak('A/D/H/omega', 'A/C', wc_rev=2, status=' ') + 'A/C' : Item(verb='Sending'), + 'A/I' : Item(verb='Deleting'), + 'A/B/F' : Item(verb='Deleting'), + 'A/D/H/omega' : Item(verb='Sending'), + 'A/B/E/alpha' : Item(verb='Sending'), + 'A/D/H/chi' : Item(verb='Deleting'), + 'A/B/E/beta' : Item(verb='Deleting'), + 'A/D/H/psi' : Item(verb='Deleting'), + }) + expected_status = svntest.actions.get_virginal_state(wc_dir, 2) + expected_status.tweak('A/C', 'A/D/H/omega', 'A/B/E/alpha', wc_rev=3, + status=' ') + expected_status.remove('A/B/F', 'A/D/H/chi', 'A/B/E/beta', 'A/D/H/psi') + commit = svntest.actions.run_and_verify_commit + commit(wc_dir, expected_output, expected_status, None, wc_dir) - svntest.actions.run_and_verify_commit(wc_dir, - expected_output, - expected_status, - None, - wc_dir) - - # Now, in the second working copy, schedule both omega and C for deletion. - omega_path = os.path.join(wc_backup, 'A', 'D', 'H', 'omega') + # Edits in wc backup + I_path = os.path.join(wc_backup, 'A', 'I') + chi_path = os.path.join(wc_backup, 'A', 'D', 'H', 'chi') + beta_path = os.path.join(wc_backup, 'A', 'B', 'E','beta') + svntest.main.run_svn(None, 'propset', 'fooprop', 'foopropval', I_path) + svntest.main.file_append(chi_path, 'appended chi text') + svntest.main.run_svn(None, 'propset', 'fooprop', 'foopropval', beta_path) + + # Deletions in wc backup C_path = os.path.join(wc_backup, 'A', 'C') - svntest.main.run_svn(None, 'rm', omega_path, C_path) + F_path = os.path.join(wc_backup, 'A', 'B', 'F') + omega_path = os.path.join(wc_backup, 'A', 'D', 'H', 'omega') + alpha_path = os.path.join(wc_backup, 'A', 'B', 'E', 'alpha') + psi_path = os.path.join(wc_backup, 'A', 'D', 'H', 'psi') + svntest.main.run_svn(None, 'rm', C_path, F_path, omega_path, alpha_path, + psi_path) - # Attempt to delete omega. This should return an (expected) - # out-of-dateness error. - outlines, errlines = svntest.main.run_svn(1, 'commit', '-m', 'blah', - omega_path) - for line in errlines: - if re.match(".*[Oo]ut.of.date.*", line): - break - else: - raise svntest.Failure - - # Attempt to delete directory C. This should return an (expected) - # out-of-dateness error. - outlines, errlines = svntest.main.run_svn(1, 'commit', '-m', 'blah', C_path) - for line in errlines: - if re.match(".*[Oo]ut.of.date.*", line): - break - else: - raise svntest.Failure - + # A commit of any one of these files or dirs should fail + error_re = "out of date" + commit(wc_backup, None, None, error_re, C_path) + commit(wc_backup, None, None, error_re, I_path) + commit(wc_backup, None, None, error_re, F_path) + commit(wc_backup, None, None, error_re, omega_path) + commit(wc_backup, None, None, error_re, alpha_path) + commit(wc_backup, None, None, error_re, chi_path) + commit(wc_backup, None, None, error_re, beta_path) + commit(wc_backup, None, None, error_re, psi_path) + def commit_with_bad_log_message(sbox): "commit with a log message containing bad data" Index: subversion/libsvn_repos/commit.c =================================================================== --- subversion/libsvn_repos/commit.c (revision 29632) +++ subversion/libsvn_repos/commit.c (working copy) @@ -125,8 +125,10 @@ out_of_date(const char *path, svn_node_kind_t kind) return svn_error_createf(SVN_ERR_FS_TXN_OUT_OF_DATE, NULL, (kind == svn_node_dir ? _("Directory '%s' is out of date") - : _("File '%s' is out of date")), - path); + : kind == svn_node_file + ? _("File '%s' is out of date") + : _("File or directory '%s' is out of date")), + path); } @@ -237,10 +239,9 @@ delete_entry(const char *path, SVN_ERR(check_authz(eb, parent->path, eb->txn_root, svn_authz_write, pool)); - /* If PATH doesn't exist in the txn, that's fine (merge - allows this). */ + /* If PATH doesn't exist in the txn, the working copy is out of date. */ if (kind == svn_node_none) - return SVN_NO_ERROR; + return out_of_date(full_path, kind); /* Now, make sure we're deleting the node we *think* we're deleting, else return an out-of-dateness error. */