You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@wandisco.com> on 2011/07/14 13:58:24 UTC
[PATCH] Improve reporting WC is locked/busy ('run cleanup')
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
Re: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Jul 14, 2011 at 9:36 AM, Stefan Sperling <st...@elego.de> wrote:
> On Thu, Jul 14, 2011 at 12:58:24PM +0100, Julian Foad wrote:
>> 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)
>
> What always bothered me about this message is that it does not
> explain the most likely reason why the working copy may be locked.
> Namely that another client is currently accessing the working copy,
> such as some GUI client performing background operations.
>
> The message currently implies that the only course of action is
> to run cleanup, but this is precisely the wrong thing to do if another
> client is performing an operation that will take a bit of time.
In my experience (with WC-1) that is never the cause of the message.
9 times out of 10 the problem is that the .svn folder has been removed
by some build process and the other is that some previous process
crashed and left locks behind.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Jul 14, 2011 at 12:58:24PM +0100, Julian Foad wrote:
> 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)
What always bothered me about this message is that it does not
explain the most likely reason why the working copy may be locked.
Namely that another client is currently accessing the working copy,
such as some GUI client performing background operations.
The message currently implies that the only course of action is
to run cleanup, but this is precisely the wrong thing to do if another
client is performing an operation that will take a bit of time.
So I would like this to say, maybe:
svn: E155004: Working copy '/home/...' locked.
svn: E155004: '/home/...' is already locked.
svn: another Subversion client might be modifying the working copy at the moment; please try again later
svn: if there is no other client run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
Re: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Thu, Jul 14, 2011 at 12:58:24 +0100:
> 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"
The message says 'cleanup', not 'svn cleanup', and I believe last time
it was discussed the conclusion was to leave it that way and not cater
to clients that confuse people by deviating from the standard documented
names for operations.
(It's not very useful to have a library of consumers of that library
presents its APIs under different names in their UIs... like having two
pizzerias that each call "A person-sized pizza with peperonni" by
a different name; and then telling people that they can checkout[1]
their dinner in one and commit[1] it in another.)
[1] oops, the diner should SEGV on reading this word...
Re: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 07/14/2011 10:00 AM, kmradke@rockwellcollins.com wrote:
> Stefan Sperling <st...@elego.de> wrote on 07/14/2011 08:40:29 AM:
>> On Thu, Jul 14, 2011 at 08:17:07AM -0500, kmradke@rockwellcollins.com wrote:
>> > Julian Foad <ju...@wandisco.com> wrote on 07/14/2011 06:58:24 AM:
>> > > 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'.
>> >
>> > I'm all for removing the overloaded "lock" terminology. We see a lot
>> > of confused users who think they somehow incorrectly locked a file
>> > in the repository when they see the old messages...
>>
>> I agree. It should say something like "the working copy is busy" instead.
>> ("busy" because it not used elsewhere, and because it will be
>> understood even by non-technical users who don't know what a lock is
>> supposed to be in this context).
>
> Not that I can vote, but using "busy" instead of "lock" gets my +1...
+1, this is a good idea. Maybe I could rewrite the "The Three Meanings of
“Lock”" section of the book to be "The Two Meanings...". :-)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by km...@rockwellcollins.com.
Stefan Sperling <st...@elego.de> wrote on 07/14/2011 08:40:29 AM:
> On Thu, Jul 14, 2011 at 08:17:07AM -0500, kmradke@rockwellcollins.com
wrote:
> > Julian Foad <ju...@wandisco.com> wrote on 07/14/2011 06:58:24
AM:
> > > 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'.
> >
> > I'm all for removing the overloaded "lock" terminology. We see a lot
> > of confused users who think they somehow incorrectly locked a file
> > in the repository when they see the old messages...
>
> I agree. It should say something like "the working copy is busy"
instead.
> ("busy" because it not used elsewhere, and because it will be
> understood even by non-technical users who don't know what a lock is
> supposed to be in this context).
Not that I can vote, but using "busy" instead of "lock" gets my +1...
Kevin R.
Re: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Jul 14, 2011 at 08:17:07AM -0500, kmradke@rockwellcollins.com wrote:
> Julian Foad <ju...@wandisco.com> wrote on 07/14/2011 06:58:24 AM:
> > 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'.
>
> I'm all for removing the overloaded "lock" terminology. We see a lot
> of confused users who think they somehow incorrectly locked a file
> in the repository when they see the old messages...
I agree. It should say something like "the working copy is busy" instead.
("busy" because it not used elsewhere, and because it will be
understood even by non-technical users who don't know what a lock is
supposed to be in this context).
Re: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by km...@rockwellcollins.com.
Julian Foad <ju...@wandisco.com> wrote on 07/14/2011 06:58:24 AM:
> 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'.
I'm all for removing the overloaded "lock" terminology. We see a lot
of confused users who think they somehow incorrectly locked a file
in the repository when they see the old messages...
Kevin R.
> 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;
RE: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2011-07-14 at 14:54 +0200, Bert Huijben wrote:
>
> > -----Original Message-----
> > From: Julian Foad [mailto:julian.foad@wandisco.com]
> > Sent: donderdag 14 juli 2011 13:58
> > To: dev@subversion.apache.org
> > Subject: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
> >
> > 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.
>
> +1 on SVN_ERR_WC_BUSY
>
> > 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"
>
> This assumes that we only store a single working copy in wc.db, while we designed wc.db to allow storing data for multiple working copies.
> -0 on this suggestion.
OK.
> > 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.
>
> If this is handled by wrapping the error, you get this for free.
> (I'm not sure if that is what you suggested)
Yes, that's what I mean: just wrap the error so this gets printed in the
standard format automatically.
> > 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
>
> Hmmm....
>
> Maybe we have a different (much larger) problem here.
>
> We don't store the wc_id with the workingqueue items, but we do assume that we know the path when we run the wq items...
> (The wq items contain local_relpaths instead of local abspaths)
>
> Probably easy to fix when we actually start storing multiple working copies in the DB, but I think it is a bug in how we store the data.
Hmm...
- Julian
RE: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Julian Foad [mailto:julian.foad@wandisco.com]
> Sent: donderdag 14 juli 2011 13:58
> To: dev@subversion.apache.org
> Subject: [PATCH] Improve reporting WC is locked/busy ('run cleanup')
>
> 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.
+1 on SVN_ERR_WC_BUSY
> 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"
This assumes that we only store a single working copy in wc.db, while we designed wc.db to allow storing data for multiple working copies.
-0 on this suggestion.
> 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.
If this is handled by wrapping the error, you get this for free.
(I'm not sure if that is what you suggested)
> 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
Hmmm....
Maybe we have a different (much larger) problem here.
We don't store the wc_id with the workingqueue items, but we do assume that we know the path when we run the wq items...
(The wq items contain local_relpaths instead of local abspaths)
Probably easy to fix when we actually start storing multiple working copies in the DB, but I think it is a bug in how we store the data.
Bert