[darcs-devel] Re: [darcs #201] "Fail: takeLock (Permission denied)" Causes Inconsistent State

[David Roundy]

On Sat, Feb 05, 2005 at 02:01:04AM +0000, Mark Stosberg wrote:
> On 2005-02-04, David Roundy <droundy at abridgegame.org> wrote:
> > On Thu, Feb 03, 2005 at 11:50:48AM -0500,  via RT wrote:
> >> While pulling, I got the following error:
> >> 
> >> Fail: takeLock foo.txt-0: permission denied (Permission denied)
> >> 
> >> because (upon later investigation) I didn't have sufficient permission to
> >> write to a directory into which darcs wanted to pull some newly-added
> >> files. Unfortunately, though, darcs did add the files into the
> >> _darcs/current/ tree, thus causing an inconsistency between the darcs
> >> current directory and my working directory.  Subsequent use of darcs
> >> check did not notice the inconsistency.
> >
> > I'm not quite clear which directory the problem was in, which would be
> > helpful.  This error message will be clearer in darcs 1.0.2 (which will be
> > released either today or tomorrow).
> >
> > Am I right that this was a subdirectory in your working directory to which
> > there were no write permissions? 
> 
> [ I'll try to answer for the requestor, who is a co-worker. ] 
> 
> Yes, you're right about this. 
> 
> > If that is the case, I'm not sure what
> > darcs should have done.  Darcs *could* (in principle) check whether it has
> > write permissions prior to making any modifications.  We'd only need to
> > check permissions on directorys... except under windows, where we'd also
> > need to check permissions on all files.  And there still would be no
> > guarantee, since the permissions could change after we checked them.
> 
> I think it would be nice to check for write permissions, even if they
> may change (that seems unlikely). 

True, but we'd still have other possible reasons for failure.  But yes, a
preliminary check would be kind.  Probably it would be best to disable this
(especially since I'd be likely to just check that we have write
permissions in the entire working directory, which could be expensive), so
people who need speed (read people converting the bkcvs repository) don't
have to traverse the entire working directory looking at permissions.

> > Or alternatively, in case of errors we could try to backtrack and undo the
> > changes we've already done.  That seems even more dangerous, since it's
> > quite possible (e.g. disk-full error) that the same thing that caused the
> > first failure will also interfere with the backtracking.
> >
> > So in short, I'm not sure (apart from fixing the error message) what could
> > or should be done in this situation.
> 
> Part of the improvement could be for the error message to suggest a
> solution to recover from the inconsistent state. Something like:
> 
>  darcs was not able to write to: /path/to/dir
>  This may have left your working directory an inconsistent but
>  recoverable state. Assuming you had no un-recorded changes
>  in the affected files, using 'darcs revert' you should
>  make your working directory  consistent again. 

I've added a clarification along these lines (which will show up if darcs
send ever succeeds).  Included in my patch is a new function
"clarify_error" which can be used to add an explanatory message to the
normal output from an exception being thrown.  I haven't actually tested
this, so if you (or someone else) could try this out, that would be great.

Also, if someone were to go through the other commands and add
clarifications to potentially serious or confusing errors in other
commands--particularly in the sections protected by "block", that would be
great.
-- 
David Roundy
http://www.darcs.net