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 2014/09/03 15:17:52 UTC
svn commit: r1622244 - in /subversion/trunk/subversion:
libsvn_ra_serf/lock.c mod_dav_svn/lock.c mod_dav_svn/util.c
tests/cmdline/lock_tests.py
Author: rhuijben
Date: Wed Sep 3 13:17:51 2014
New Revision: 1622244
URL: http://svn.apache.org/r1622244
Log:
Make the behavior of post lock and unlock hooks consistent between DAV and
the other ra implementations.
* subversion/libsvn_ra_serf/lock.c
(run_locks): Report unlocks as succeeded before reporting the lock failure.
* subversion/mod_dav_svn/lock.c
(append_locks,
remove_lock): Specialize the behavior for post hook failures.
* subversion/mod_dav_svn/util.c
(dav_svn__convert_err): Don't hide post unlock errors.
* subversion/tests/cmdline/lock_tests.py
(failing_post_hooks): Remove XFail marker for DAV. Update expected errors,
while keeping the working copy state checks.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/lock.c
subversion/trunk/subversion/mod_dav_svn/lock.c
subversion/trunk/subversion/mod_dav_svn/util.c
subversion/trunk/subversion/tests/cmdline/lock_tests.py
Modified: subversion/trunk/subversion/libsvn_ra_serf/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/lock.c?rev=1622244&r1=1622243&r2=1622244&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/lock.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/lock.c Wed Sep 3 13:17:51 2014
@@ -315,6 +315,19 @@ run_locks(svn_ra_serf__session_t *sess,
&& !SVN_ERR_IS_UNLOCK_ERROR(err)
&& !SVN_ERR_IS_LOCK_ERROR(err))
{
+ /* If the error that we are going to report is just about the
+ POST unlock hook, we should first report that the operation
+ succeeded, or the repository and working copy will be
+ out of sync... */
+
+ if (lock_func &&
+ err->apr_err == SVN_ERR_REPOS_POST_UNLOCK_HOOK_FAILED)
+ {
+ err = svn_error_compose_create(
+ err, lock_func(lock_baton, ctx->path, locking,
+ NULL, NULL, ctx->pool));
+ }
+
return svn_error_trace(err); /* Don't go through callbacks */
}
Modified: subversion/trunk/subversion/mod_dav_svn/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/lock.c?rev=1622244&r1=1622243&r2=1622244&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/lock.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/lock.c Wed Sep 3 13:17:51 2014
@@ -787,6 +787,30 @@ append_locks(dav_lockdb *lockdb,
DAV_ERR_LOCK_SAVE_LOCK,
"Anonymous lock creation is not allowed.");
}
+ else if (serr && serr->apr_err == SVN_ERR_REPOS_POST_LOCK_HOOK_FAILED)
+ {
+ /* The lock was created in the repository, so we should report the node
+ as locked to the client */
+
+ /* First log the hook failure, for diagnostics. This clears serr */
+ dav_svn__log_err(info->r,
+ dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "Post lock hook failure.",
+ resource->pool),
+ APLOG_WARNING);
+
+ /* How can we report the error to the client?
+
+ We can't return an error code, as that would make it impossible
+ to return the lock details?
+
+ Add yet another custom header?
+ Just an header doesn't handle a full error chain...
+
+ ### Current behavior: we don't report an error.
+ */
+
+ }
else if (serr && (svn_error_find_cause(serr, SVN_ERR_REPOS_HOOK_FAILURE) ||
serr->apr_err == SVN_ERR_FS_NO_SUCH_LOCK ||
serr->apr_err == SVN_ERR_FS_LOCK_EXPIRED ||
@@ -897,6 +921,22 @@ remove_lock(dav_lockdb *lockdb,
DAV_ERR_LOCK_SAVE_LOCK,
"Anonymous lock removal is not allowed.");
}
+ else if (serr && serr->apr_err == SVN_ERR_REPOS_POST_UNLOCK_HOOK_FAILED
+ && !resource->info->repos->is_svn_client)
+ {
+ /* Generic DAV clients don't understand the specific error code we
+ would produce here as being just a warning, so lets produce a
+ success result. We removed the lock anyway. */
+
+ /* First log the hook failure, for diagnostics. This clears serr */
+ dav_svn__log_err(info->r,
+ dav_svn__convert_err(serr,
+ HTTP_INTERNAL_SERVER_ERROR,
+ "Post unlock hook failure.",
+ resource->pool),
+ APLOG_WARNING);
+
+ }
else if (serr)
return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
"Failed to remove a lock.",
Modified: subversion/trunk/subversion/mod_dav_svn/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/util.c?rev=1622244&r1=1622243&r2=1622244&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/util.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/util.c Wed Sep 3 13:17:51 2014
@@ -152,7 +152,7 @@ dav_svn__convert_err(svn_error_t *serr,
derr = build_error_chain(pool, purged_serr, status);
if (message != NULL
- && purged_serr->apr_err != SVN_ERR_REPOS_HOOK_FAILURE)
+ && !svn_error_find_cause(purged_serr, SVN_ERR_REPOS_HOOK_FAILURE))
/* Don't hide hook failures; we might hide the error text */
derr = dav_push_error(pool, status, purged_serr->apr_err,
message, derr);
Modified: subversion/trunk/subversion/tests/cmdline/lock_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/lock_tests.py?rev=1622244&r1=1622243&r2=1622244&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/lock_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/lock_tests.py Wed Sep 3 13:17:51 2014
@@ -1964,7 +1964,6 @@ def lock_hook_messages(sbox):
expected_err, actual_stderr)
-@XFail(svntest.main.is_ra_type_dav)
def failing_post_hooks(sbox):
"locking with failing post-lock and post-unlock"
@@ -1978,20 +1977,25 @@ def failing_post_hooks(sbox):
pi_path = sbox.ospath('A/D/G/pi')
expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
expected_status.tweak('A/D/G/pi', writelocked='K')
- expected_fail_err_re = ".*error text"
-
+
+ if svntest.main.is_ra_type_dav():
+ expected_lock_err = []
+ expected_unlock_err = '.*svn: E165009: Unlock succeeded.*' #
+ else:
+ expected_unlock_err = expected_lock_err = ".*error text"
+
# Failing post-lock doesn't stop lock being created.
- svntest.actions.run_and_verify_svn2(None, "'pi' locked by user",
- expected_fail_err_re, 1,
- 'lock', '-m', '', pi_path)
+ svntest.actions.run_and_verify_svn(None, "'pi' locked by user",
+ expected_lock_err,
+ 'lock', '-m', '', pi_path)
svntest.actions.run_and_verify_status(wc_dir, expected_status)
expected_status.tweak('A/D/G/pi', writelocked=None)
# Failing post-unlock doesn't stop lock being removed.
- svntest.actions.run_and_verify_svn2(None, "'pi' unlocked",
- expected_fail_err_re, 1,
- 'unlock', pi_path)
+ svntest.actions.run_and_verify_svn(None, "'pi' unlocked",
+ expected_unlock_err,
+ 'unlock', pi_path)
svntest.actions.run_and_verify_status(wc_dir, expected_status)
@XFail()