Hi,
Last week, I was working on this issue: "Whenever the pre-unlock hook
fails, Subversion server sends the error message "Unlock blocked by
pre-unlock hook (exit code 1)...." to Apache. But Apache's mod_dav is
not handling the error and it is not sending any error messages to
client side, instead "500 Internal Server Error" is displayed to the user."
Actually, that is not a bug with subversion. Apache's mod_dav have to
handle the error properly and send it to the user. I have filed one
issue in issues.apache.org and uploaded my patch there.[1]
While working on this issue, I came across another bug: "whenever the
pre-unlock hook exits with non-zero return code and if the connection is
made via ra_neon, the unlock operation ends with "<filename> unlocked.\n
Aborted". The file is actually unlocked in the working copy not in the
repository". The repository is still having the lock due to pre-unlock
blocking the unlock action. Here, Working copy should not lose the lock
on the file, right?
If the connection is made via ra_serf, then the error message is "svn:
E020014: Unspecified error message: 500 Internal Server Error". (It
should not display Internal server error. The mod_dav patch at [1] will
fix it.)
I have attached a testcase and fix for this issue.
ra_neon:
This testcase aborts with the following exception as there are unhandled
errors(error with 500 http status code) during unlock operation and they
didn't get cleared from the pool.
<snip>
EXCEPTION: SVNProcessTerminatedBySignal
Traceback (most recent call last):
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py",
line 1288, in run
rc = self.pred.run(sandbox)
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/testcase.py",
line 176, in run
return self.func(sandbox)
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/lock_tests.py",
line 1718, in block_unlock_if_pre_unlock_hook_fails
svntest.actions.run_and_verify_svn2(None, None, "", 1, 'unlock',
pi_path)
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/actions.py",
line 306, in run_and_verify_svn2
exit_code, out, err = main.run_svn(want_err, *varargs)
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py",
line 591, in run_svn
*(_with_auth(_with_config_dir(varargs))))
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py",
line 347, in run_command
None, *varargs)
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py",
line 509, in run_command_stdin
*varargs)
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py",
line 479, in spawn_process
stdout_lines, stderr_lines, exit_code = wait_on_pipe(kid, binary_mode)
File
"/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py",
line 445, in wait_on_pipe
raise SVNProcessTerminatedBySignal
SVNProcessTerminatedBySignal
</snip>
At the end of the testcase, I thought of checking the status of the
working copy with expected status as "writelocked=K". But I couldn't do
it as the test raises exception in between, i.e., during unlock
operation itself.
ra_serf:
The test passes with ra_serf as all kind of errors has been handled here.
Please review the patch and respond.
Thanks & Regards,
Vijayaguru
[1] https://issues.apache.org/bugzilla/show_bug.cgi?id=51297
P.S: I will be very happy if someone from Apache's mod_dav development
team can take a look at the patch in [1]. Please let me know if we can
handle it in some other way. Eagerly awaiting for your response.
Index: subversion/libsvn_ra_neon/lock.c
===================================================================
--- subversion/libsvn_ra_neon/lock.c (revision 1130003)
+++ subversion/libsvn_ra_neon/lock.c (working copy)
@@ -460,7 +460,7 @@
_("No lock on path '%s'"
" (%d Bad Request)"), path, code);
default:
- break; /* Handle as error */
+ SVN_ERR(err);
}
}
else
Index: subversion/tests/cmdline/lock_tests.py
===================================================================
--- subversion/tests/cmdline/lock_tests.py (revision 1130003)
+++ subversion/tests/cmdline/lock_tests.py (working copy)
@@ -1693,6 +1693,31 @@
None, expected_status)
+#----------------------------------------------------------------------
+def block_unlock_if_pre_unlock_hook_fails(sbox):
+ "block unlock operation if pre-unlock hook fails"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+ repo_dir = sbox.repo_dir
+
+ svntest.actions.create_failing_hook(repo_dir, "pre-unlock", "")
+
+ # lock a file.
+ pi_path = os.path.join(wc_dir, 'A', 'D', 'G', 'pi')
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/D/G/pi', writelocked='K')
+
+ svntest.actions.run_and_verify_svn(None, ".*locked by user", [], 'lock',
+ '-m', '', pi_path)
+
+ svntest.actions.run_and_verify_status(wc_dir, expected_status)
+
+ # Now, unlock the file via ra_dav. This unlock operation should fail as
+ # pre-unlock hook blocks it.
+ svntest.actions.run_and_verify_svn2(None, None, "", 1, 'unlock', pi_path)
+
+
########################################################################
# Run the tests
@@ -1739,6 +1764,7 @@
replace_and_propset_locked_path,
cp_isnt_ro,
update_locked_deleted,
+ block_unlock_if_pre_unlock_hook_fails,
]
if __name__ == '__main__':
with ra_neon, Unlocking a file succeeds in client side, though pre-unlock hook
exits with
non-zero return code.
* subversion/libsvn_ra_neon/lock.c
(do_unlock): Handle the error by default.
* subversion/tests/cmdline/lock_tests.py
(block_unlock_if_pre_unlock_hook_fails): Add a test case.
Patch by: Vijayaguru G <vijay{_AT_}collab.net>
