On Thu, 2011-07-14 at 14:54 +0200, Bert Huijben wrote: > > > -----Original Message----- > > From: Julian Foad [mailto:julian.f...@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
