On Thu, 2011-07-14 at 14:54 +0200, Bert Huijben wrote:
>
> > -----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.
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
Received on 2011-07-14 15:03:16 CEST