The attached patch makes some improvements to how we report that the WC
is locked or busy (the 'run cleanup' messages). I need a sanity check
to make sure I've understood the relationship between how we want to
handle 'the work queue is busy' and 'there is a write lock on a WC
directory'.
When the WC is locked we may (depending on the timing) see:
$ svn up -q & svn up -q
svn: E155004: Working copy '/home/...' locked.
svn: E155004: '/home/...' is already locked.
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for
details)
(or, depending on the timing, there may be two 'database is locked'
lines instead of the 'is already locked' line), or
$ svn up & svn commit
svn: E155004: Commit failed (details follow):
svn: E155004: Working copy '/home/...' locked.
svn: E155004: '/home/...' is already locked.
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for
details)
When the work queue is not empty, we see:
$ svn status
svn: E155037: Previous operation has not finished; run 'cleanup' if it
was interrupted
My recommendations, mainly for the WQ-not-empty case:
1. The 'E155037' message shown above comes from libsvn_wc. But the
part about running 'cleanup' should be added by 'svn', since in other
clients it may have a different name or may not be applicable at all,
especially if the client can find out that in fact the other operation
is still running. So libsvn_wc would say just:
"A previous operation on the working copy has not finished"
and 'svn' would wrap that in:
"Run 'svn cleanup' if the previous operation was interrupted"
See also point (2).
2. Consider wrapping this SVN_ERR_WC_CLEANUP_REQUIRED (E155037) error
in a SVN_ERR_WC_LOCKED (E155004) error to preserve API compatibility
w.r.t. this kind of situation. Inside libsvn_wc, of course the
condition being reported here is not the same as a 'lock', but to an
outside caller the net effect is basically that it's a lock.
If we don't wrap this in SVN_ERR_WC_LOCKED, then in order to support
(1), 'svn' should learn to recognize the new error code as well, at the
point where it determines whether to suggest the user should run
'cleanup'.
3. The error code SVN_ERR_WC_CLEANUP_REQUIRED is misnamed, since
libsvn_wc does not know whether cleanup is required. Rename it to
SVN_ERR_WC_BUSY or SVN_ERR_WC_WORK_QUEUE_NOT_EMPTY.
4. libsvn_wc should include the WC root in the error message:
"A previous operation on the working copy at '<wcroot_abspath>' has
not finished"
5. 'svn' should print its message about running 'cleanup' via the
standard error message mechanism instead of using cmdline_fputs(), so
that the message appears in the standard (new) format with the
'Ennnnnn:' prefix. This suggestion is independent of the others so I'll
commit it separately, but it's included in this patch for you to see.
With this patch, typical results are:
$ svn up -q & svn up -q
svn: E155004: Run 'svn cleanup' to remove locks (type 'svn help
cleanup' for details)
svn: E155004: Working copy '/home/...' locked.
svn: E155004: '/home/...' is already locked.
$ svn up -q & svn commit
svn: E155004: Run 'svn cleanup' to remove locks (type 'svn help
cleanup' for details)
svn: E155004: Commit failed (details follow):
svn: E155004: Working copy '/home/...' locked.
svn: E155004: '/home/...' is already locked.
and for the more interesting, WQ-not-empty, case:
$ svn status
svn: E155037: Run 'svn cleanup' to remove locks (type 'svn help
cleanup' for details)
svn: E155037: A previous operation on the working copy at '/home/...'
has not finished
I would propose all of this for 1.7.0 back-port.
Concerns?
- Julian
Improve how we report that the WC is locked or busy (the 'run cleanup'
messages).
* subversion/include/svn_error_codes.h
(SVN_ERR_WC_CLEANUP_REQUIRED): Rename to SVN_ERR_WC_BUSY and remove the
words about running 'cleanup', since libsvn_wc doesn't know whether
cleanup is required.
* subversion/libsvn_wc/wc_db_wcroot.c
(verify_no_work): Take the WC root path and include that in the error
message. Add a doc string.
(svn_wc__db_pdh_create_wcroot): Adjust accordingly.
* subversion/svn/main.c
(main): Print the message about running 'cleanup' when the subcommand
returned SVN_ERR_WC_BUSY as well as when it returned SVN_ERR_WC_LOCKED.
Print the message about running 'cleanup' using the standard mechanism
for error messages (so, for example, the error code will be displayed).
--This line, and those below, will be ignored--
Index: subversion/include/svn_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h (revision 1145998)
+++ subversion/include/svn_error_codes.h (working copy)
@@ -520,10 +520,9 @@ SVN_ERROR_START
"The working copy needs to be upgraded")
/** @since New in 1.7. */
- SVN_ERRDEF(SVN_ERR_WC_CLEANUP_REQUIRED,
+ SVN_ERRDEF(SVN_ERR_WC_BUSY,
SVN_ERR_WC_CATEGORY_START + 37,
- "Previous operation has not finished; "
- "run 'cleanup' if it was interrupted")
+ "A previous operation on the working copy has not finished")
/** @since New in 1.7. */
SVN_ERRDEF(SVN_ERR_WC_INVALID_OPERATION_DEPTH,
Index: subversion/libsvn_wc/wc_db_wcroot.c
===================================================================
--- subversion/libsvn_wc/wc_db_wcroot.c (revision 1145998)
+++ subversion/libsvn_wc/wc_db_wcroot.c (working copy)
@@ -139,9 +139,12 @@ get_path_kind(svn_wc__db_t *db,
}
-/* */
+/* Return an error if the work queue in SDB is non-empty. WCROOT_ABSPATH
+ * is used in the error message. */
static svn_error_t *
-verify_no_work(svn_sqlite__db_t *sdb)
+verify_no_work(svn_sqlite__db_t *sdb,
+ const char *wcroot_abspath,
+ apr_pool_t *scratch_pool)
{
svn_sqlite__stmt_t *stmt;
svn_boolean_t have_row;
@@ -151,8 +154,11 @@ verify_no_work(svn_sqlite__db_t *sdb)
SVN_ERR(svn_sqlite__reset(stmt));
if (have_row)
- return svn_error_create(SVN_ERR_WC_CLEANUP_REQUIRED, NULL,
- NULL /* nothing to add. */);
+ return svn_error_createf(SVN_ERR_WC_BUSY, NULL,
+ _("A previous operation on the working "
+ "copy at '%s' has not finished"),
+ svn_dirent_local_style(wcroot_abspath,
+ scratch_pool));
return SVN_NO_ERROR;
}
@@ -275,12 +281,12 @@ svn_wc__db_pdh_create_wcroot(svn_wc__db_
if (format >= SVN_WC__HAS_WORK_QUEUE
&& (enforce_empty_wq || (format < SVN_WC__VERSION && auto_upgrade)))
{
- svn_error_t *err = verify_no_work(sdb);
+ svn_error_t *err = verify_no_work(sdb, wcroot_abspath, scratch_pool);
if (err)
{
/* Special message for attempts to upgrade a 1.7-dev wc with
outstanding workqueue items. */
- if (err->apr_err == SVN_ERR_WC_CLEANUP_REQUIRED
+ if (err->apr_err == SVN_ERR_WC_BUSY
&& format < SVN_WC__VERSION && auto_upgrade)
err = svn_error_quick_wrap(err, _("Cleanup with an older 1.7 "
"client before upgrading with "
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 1145998)
+++ subversion/svn/main.c (working copy)
@@ -2596,6 +2596,14 @@ main(int argc, const char *argv[])
_("Please see the 'svn upgrade' command"));
}
+ /* Tell the user about 'svn cleanup' if any error on the stack
+ was about locked working copies. */
+ if (svn_error_find_cause(err, SVN_ERR_WC_LOCKED)
+ || svn_error_find_cause(err, SVN_ERR_WC_BUSY))
+ err = svn_error_quick_wrap(err,
+ _("Run 'svn cleanup' to remove locks "
+ "(type 'svn help cleanup' for details)"));
+
/* Issue #3014:
* Don't print anything on broken pipes. The pipe was likely
* closed by the process at the other end. We expect that
@@ -2606,14 +2614,6 @@ main(int argc, const char *argv[])
if (err->apr_err != SVN_ERR_IO_PIPE_WRITE_ERROR)
svn_handle_error2(err, stderr, FALSE, "svn: ");
- /* Tell the user about 'svn cleanup' if any error on the stack
- was about locked working copies. */
- if (svn_error_find_cause(err, SVN_ERR_WC_LOCKED))
- svn_error_clear(svn_cmdline_fputs(_("svn: run 'svn cleanup' to "
- "remove locks (type 'svn help "
- "cleanup' for details)\n"),
- stderr, pool));
-
svn_error_clear(err);
svn_pool_destroy(pool);
return EXIT_FAILURE;
