[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: [PATCH] Improve reporting WC is locked/busy ('run cleanup')

From: Bert Huijben <bert_at_qqmail.nl>
Date: Thu, 14 Jul 2011 14:54:51 +0200

> -----Original Message-----
> From: Julian Foad [mailto:julian.foad_at_wandisco.com]
> Sent: donderdag 14 juli 2011 13:58
> To: dev_at_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
Received on 2011-07-14 14:55:30 CEST

This is an archived mail posted to the Subversion Dev mailing list.