darcs

Issue 255 deal more gracefully with pull errors

Title deal more gracefully with pull errors
Priority wishlist Status resolved
Milestone Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, jch, kowey, simonpj, thorkilnaur, tommy
Assigned To
Topics

Created on 2006-09-01.10:38:42 by simonpj, last changed 2009-08-27.13:50:08 by admin.

Messages
msg949 (view) Author: simonpj Date: 2006-09-01.10:38:38
Twice in the last couple of days I've gotten this: (darcs 1.0.8)

	darcs failed:  user error (Error applying patch to working dir:
	./darcs-all-0: renameFile: permission denied (Permission
denied))
	This may have left your working directory an inconsistent
	but recoverable state. If you had no un-recorded changes
	by using 'darcs revert' you should be able to make your
	working directory consistent again.

This isn't very helpful!  At the very least it'd help to say *which*
file is permission denied for.  And "user error" sounds like my fault,
but I don't know what I did wrong.

And much worse, while the message helpfully says what to do if you have
no changes in your tree, it leaves you high and dry if you do!  Do I
just have to abandon my work?  Fortunately in this case I think I am in
the no-change state --- but it doesn't look as if I have any way to find
out whether or not I am in that state.  Alas.

(The trouble with a revision-control system is that it's holding the
crown jewels, so your users are more than usually sensitive about bugs!)

Simon
msg952 (view) Author: jch Date: 2006-09-01.15:13:54
Hi Simon,

> 	darcs failed:  user error (Error applying patch to working dir:
> 	./darcs-all-0: renameFile: permission denied (Permission
> denied))

> This isn't very helpful!  At the very least it'd help to say *which*
> file is permission denied for.

> And "user error" sounds like my fault, but I don't know what I did
> wrong.

Yes, while we're careful with error handling, our error reporting is
quite horrible.  (It's improved a lot, by the way -- it used to
consist entirely of things like ``impossible case at...'', which
cannot be decoded without the source.)

I guess I'm going to have to spend some time with the hierarchichal
libraries in order to try to understand the Ghc condition system.  But
don't hold your breath -- understanding the Common Lisp condition
system took me a decade, and that one is documented.

> And much worse, while the message helpfully says what to do if you have
> no changes in your tree, it leaves you high and dry if you do!  Do I
> just have to abandon my work?

Well, it's only your working dir, after all.  It's in an undefined but
probably not too horribly broken state right now (a patch bundle has
been applied half-way to it), so you type ``darcs what | more'' and
work out which files you want to revert, and which ones you want to
keep.

The message does sound okay to me, but I'm useless at user interfaces,
and the English, she is difficult.  If anyone has a better suggestion
for the message, I'm listening.

> (The trouble with a revision-control system is that it's holding the
> crown jewels, so your users are more than usually sensitive about bugs!)

Your crown jewels are safe under _darcs/patches, and the above did not
do anything to them -- we're very, very careful about the committed
data.  (If you find any situation short of a system crash where we
actually corrupt your committed data, that's a /very/ serious bug.
And even a system crash we should survive in most cases, if your
filesystem can handle it -- I hear NTFS is journalled nowadays.)

For obvious efficiency reasons, we're not even trying to be atomic
when accessing the working dir.  In this case, for example, an action
failed half-way through the working dir -- some of the files are in
the old, and some in the new state.

We are reasonably careful with the working dir, though -- we will
always warn you before we allow you to do something destructive to the
working dir, and in one case (darcs revert) we'll even save enough
data to allow you to undo it (darcs unrevert).

                                        Juliusz
msg957 (view) Author: simonpj Date: 2006-09-04.08:45:52
| > (The trouble with a revision-control system is that it's holding the
| > crown jewels, so your users are more than usually sensitive about bugs!)
| 
| Your crown jewels are safe under _darcs/patches, and the above did not
| do anything to them -- we're very, very careful about the committed
| data.  (If you find any situation short of a system crash where we
| actually corrupt your committed data, that's a /very/ serious bug.
| And even a system crash we should survive in most cases, if your
| filesystem can handle it -- I hear NTFS is journalled nowadays.)

The difficulty is with un-committed (un-recorded) data.  

I can't record until I've pulled.  (Why?  Because then I get conflicts, and Darcs crashes, under circumstances I can't predict, when there are conflicts.  So I have learned to avoid conflicts.)  So I must pull before I record.

So when I pull I may have extremely valuable un-recorded data.  

| The message does sound okay to me, but I'm useless at user interfaces,
| and the English, she is difficult.  If anyone has a better suggestion
| for the message, I'm listening.

The message says what to do if you have no un-recorded data.  By omission, it suggested to me that if you do have un-recorded data, it might be irretrievably lost.  That's not so, but it sounded like it to me.

My suggestion for the error message would be to say
	a) how to find out if you have un-recorded data
	b) how to recover it

You may think that it's stupid not to know the answer to (a), but I usually have four or five trees on the go at any moment, and I don’t always remember which ones have unrecorded work in progress.

| For obvious efficiency reasons, we're not even trying to be atomic
| when accessing the working dir.  In this case, for example, an action
| failed half-way through the working dir -- some of the files are in
| the old, and some in the new state.

But you know which patches you applied, and which you did not.  So you could just un-apply them all, couldn't you?   

Thanks.  As you'll gather I'm a particularly un-educated Darcs user.

Simon
msg962 (view) Author: droundy Date: 2006-09-05.20:08:36
On Fri, Sep 01, 2006 at 10:38:42AM +0000, simonpj wrote:
> Twice in the last couple of days I've gotten this: (darcs 1.0.8)
> 
> 	darcs failed:  user error (Error applying patch to working dir:
> 	./darcs-all-0: renameFile: permission denied (Permission
> denied))
> 	This may have left your working directory an inconsistent
> 	but recoverable state. If you had no un-recorded changes
> 	by using 'darcs revert' you should be able to make your
> 	working directory consistent again.
> 
> This isn't very helpful!  At the very least it'd help to say *which*
> file is permission denied for.  And "user error" sounds like my fault,
> but I don't know what I did wrong.

The file that's causing trouble is ./darcs-all.  This is a
windows-specific issue, because in windows you can't delete a file
that's currently being executed (or even is open).  I can see how with
darcs-all this would be a severe issue, but don't see an easy way
around it.  The message (unfortunately) only mentions the name of the
temporary file that we create and then rename to the final filename.
Unfortunately, Windows file system symantics are rather confusing (to
me anyways), and it's hard to see how one can make a reliable system
using them.  :( In this case, I can't see how we could work around
this.  I suppose you'd just have to rename darcs-all to something else
before running it, but I don't see what darcs could do!

I believe the "user" error shows up as a bug introduced when the
change was made that added the file information to the error message.
When showing the error, we need to special case "UserError" (or
whatever it's called) to avoid this, but I'm not sure where this needs
to be done.  :(

I don't suppose it's any consolation to hear that it may really be
your fault (in a twisted sense) as one of the creators of the Haskell
language, and ghc in particular, since the error that is triggered by
"fail" in the IO monad, when caught and shown (which is what's
happening now), created this "user error" message.
-- 
David Roundy
msg964 (view) Author: droundy Date: 2006-09-05.20:26:51
On Mon, Sep 04, 2006 at 09:45:36AM +0100, Simon Peyton-Jones wrote:
> The difficulty is with un-committed (un-recorded) data.  
> 
> I can't record until I've pulled.  (Why?  Because then I get
> conflicts, and Darcs crashes, under circumstances I can't predict,
> when there are conflicts.  So I have learned to avoid conflicts.)
> So I must pull before I record.

A workaround is to record, pull and then either unrecord or
amend-record.

[...]
> | For obvious efficiency reasons, we're not even trying to be atomic
> | when accessing the working dir.  In this case, for example, an action
> | failed half-way through the working dir -- some of the files are in
> | the old, and some in the new state.
> 
> But you know which patches you applied, and which you did not.  So
> you could just un-apply them all, couldn't you?

The trouble is that we'd then need to hold onto the contents of all
applied patches, which is something we try very hard to never do (in
order to save memory).  It'd also take a fair amound of bookkeeping.
Actually, with clever enough bookkeeping, we'd not need to hold onto
the patches themselves, now that I think about it...
-- 
David Roundy
msg966 (view) Author: simonpj Date: 2006-09-06.07:39:09
| > | For obvious efficiency reasons, we're not even trying to be atomic
| > | when accessing the working dir.  In this case, for example, an action
| > | failed half-way through the working dir -- some of the files are in
| > | the old, and some in the new state.
| >
| > But you know which patches you applied, and which you did not.  So
| > you could just un-apply them all, couldn't you?
| 
| The trouble is that we'd then need to hold onto the contents of all
| applied patches, which is something we try very hard to never do (in
| order to save memory).  It'd also take a fair amound of bookkeeping.
| Actually, with clever enough bookkeeping, we'd not need to hold onto
| the patches themselves, now that I think about it...

That'd be great.  At the moment I'm faced with a situation where Darcs tells me that, if I have unrecorded changes, my working directory has been corrupted in an un-specified way.

Now, I think that I can repair it by *selectively* un-reverting (though if so, could the message please say so?).  Which is pretty delicate but perhaps do-able.  (What would happen if there were conflict markers in the files?)

But a related trouble is that it's hard to *detect* the problem.  Darcs is slow (or the network is slow) -- so I often say (./darcs-all pull -av; make), and go do something else for a while.  Now, if this problem, whatever it is, happens, it may have scrolled off the top.  And now the difficulty is that "darcs pull" says "nothing to pull", so I think that I've gotten everything.  But I haven't!  Last night I spent a while puzzling over why the change I was expecting to be in my working directory wasn’t, even though the patches had apparently been pulled!

Eventually I try 'darcs what', and get a huge list of changes that are in my working directory but are apparently not recorded.  Of course, (most of) these are the ones I should revert, but there's nothing to tell me that, and it's confusing and alarming to the naïve.

You should say that I should watch all of Darcs's output like a hawk, but it would be much preferable if the *state* of my tree told me what state it is in, rather than its *history*.

I'm not saying this is an easy problem to solve, or that Darcs is bad. I'm just reporting a user experience.

Simon
msg971 (view) Author: jch Date: 2006-09-06.17:12:25
> The trouble is that we'd then need to hold onto the contents of all
> applied patches, which is something we try very hard to never do (in
> order to save memory).  It'd also take a fair amound of bookkeeping.
> Actually, with clever enough bookkeeping, we'd not need to hold onto
> the patches themselves, now that I think about it...

Well, bringing the recorded state to where it was before the failure
is easy -- actually, unless I'm missing something, it'll be trivial
with your latest changes, we'll just need to delay the call to
finalize_whatever until after we've munged the working dir, with a
nice exception handler for the rollback.

But I don't see how doing this would solve Simon's problem: although
the patches will not have been pulled, the workig dir will still be in
a half-applied state, which I agree is a pain.

I can see two solutions.  One would be to get Simon to get a student
to implement an IO monad that implements commit and rollback (with the
assumption that nothing else touches the filesystem at the same time,
of course).

The second is a Windows-specific hack.  We could modify the code that
applies patches to the working directory to first open all the files
that we intend to modify with delete intent (yes, Windows has such a
notion), and only then start performing the modifications.  Now the
Microsoft docs are not very clear about it, but I understand that the
a sucessful open guarantees that the unlink cannot fail.

For the record, opening with delete intent is done with CreateFile.
No kidding.

I'm not quite sure what that'd do to performance, though, and I'm
afraid we'd run out of file desc^W^Wfile handles.

                                        Juliusz
msg972 (view) Author: jch Date: 2006-09-06.17:22:36
> The file that's causing trouble is ./darcs-all.  This is a
> windows-specific issue, because in windows you can't delete a file
> that's currently being executed (or even is open).

Actually, under 9x, you can, but I don't know what the effect is.  The
docs say ``To prevent loss of data, close files before you attempt to
delete them,'' which is very kind advice.

> since the error that is triggered by "fail" in the IO monad, when
> caught and shown (which is what's happening now), created this "user
> error" message.

I'm not clear about this -- who threw the error, us or the Ghc
libraries?  If it's us, we're clearly throwing the wrong error.

Simon, I can't find how to create new kinds of IOErrors.  Do I need to
wrap IO in my own monad?

                                        Juliusz
msg973 (view) Author: jch Date: 2006-09-06.17:47:08
> My suggestion for the error message would be to say
> 	a) how to find out if you have un-recorded data
> 	b) how to recover it

Okay.

> You may think that it's stupid not to know the answer to (a), but I
> usually have four or five trees on the go at any moment, and I don’t
> always remember which ones have unrecorded work in progress.

My suggestion would be to record more often, you can always unrecord
or amend-record later on.  Just make sure you don't push unfinished
work, tracking down all the copies of a patch you want to get rid of
might turn out to be a pain.

(David -- shall we implement unpushable patches?)

> | For obvious efficiency reasons, we're not even trying to be atomic
> | when accessing the working dir.  In this case, for example, an action
> | failed half-way through the working dir -- some of the files are in
> | the old, and some in the new state.

> But you know which patches you applied, and which you did not.  So
> you could just un-apply them all, couldn't you?

Ah-ha.  You mean to the working tree.

Sorry for being so slow.  Yes, that's definitely an excellent idea,
we'll just have to make sure we handle sensibly the case when applying
the inverse patches fails (think filesystem full).

                                        Juliusz
msg977 (view) Author: simonpj Date: 2006-09-07.09:11:31
| > But you know which patches you applied, and which you did not.  So
| > you could just un-apply them all, couldn't you?
| 
| Ah-ha.  You mean to the working tree.
| 
| Sorry for being so slow.  Yes, that's definitely an excellent idea,
| we'll just have to make sure we handle sensibly the case when applying
| the inverse patches fails (think filesystem full).

Right: just the working tree!  That’s the one that contains my un-recorded changes.  In short, the proposal is that a failed pull is a no-op, which is WAY easier for someone like me to understand.  That would be a big improvement.

| My suggestion would be to record more often, you can always unrecord
| or amend-record later on.  Just make sure you don't push unfinished
| work, tracking down all the copies of a patch you want to get rid of
| might turn out to be a pain.

I don't tend to record half-tested stuff, in case I accidentally push it and screw up everyone else.  Pushing is so slow that I usually do 'darcs push -a', and go do something else meanwhile.

Simon
msg978 (view) Author: simonpj Date: 2006-09-07.12:31:34
| darcs failed:  user error (Error applying patch to working dir:
| ./Text/Regex: removeDirectory: unsatisified constraints (Directory not empty))
| This may have left your working directory an inconsistent
| but recoverable state. If you had no un-recorded changes
| by using 'darcs revert' you should be able to make your
| working directory consistent again.

Further to this thread, the original problem occurred because 

- the patch I was pulling deleted a directory
- the working tree contained files in that directory that are not in the repository
(namely object files etc)

So the pull failed because it couldn’t delete the directory because it wasn't empty.  See above

But this does not seem like a good reason to abandon pulling, and emit an error message.  Leaving the directory there seems fine to me!

So couldn't the 'pull' simply continue instead (perhaps emitting a warning)?

Simon
msg1535 (view) Author: kowey Date: 2007-03-10.08:11:23
> Further to this thread, the original problem occurred because 
> 
> - the patch I was pulling deleted a directory
> - the working tree contained files in that directory that are not in the >
repository

Simon, is there any chance at all that this was with an older version of darcs,
say 1.0.7?

The reason I ask is that I recall having fixed just that issue in
http://bugs.darcs.net/issue154

The larger issues still remain to be solved, of course.  I just wanted to make
sure there wasn't a new bug on this particular point.

Thanks,
msg1541 (view) Author: simonpj Date: 2007-03-12.11:40:24
Glark.  I can log into bugs.darcs.net, but I can't see how to add a comment, so I'll reply by email.

The original message for issue 255 (msg949) clearly states darcs 1.0.8.

However, my message msg 978 seems to report a different error to the original one; and does nto give a darcs version.  So it's not clear to me if the two bugs are the same. Perhaps msg978 is for darcs 1.0.7, and is fixed.  I'm not sure how 978 ended up in the issue255 thread.  I doubt I put it there.

Sorry for not reporting the version number in the 978 bug report.  Stupid.

Simon

| -----Original Message-----
| From: Eric Kow [mailto:bugs@darcs.net]
| Sent: 10 March 2007 08:12
| To: beschmi@cloaked.de; droundy@darcs.net; eric.kow@gmail.com; jch@pps.jussieu.fr; ptp@lysator.liu.se;
| Simon Peyton-Jones
| Subject: [issue255] deal more gracefully with pull errors
|
|
| Eric Kow <eric.kow@gmail.com> added the comment:
|
| > Further to this thread, the original problem occurred because
| >
| > - the patch I was pulling deleted a directory
| > - the working tree contained files in that directory that are not in the >
| repository
|
| Simon, is there any chance at all that this was with an older version of darcs,
| say 1.0.7?
|
| The reason I ask is that I recall having fixed just that issue in
| http://bugs.darcs.net/issue154
|
| The larger issues still remain to be solved, of course.  I just wanted to make
| sure there wasn't a new bug on this particular point.
|
| Thanks,
|
| ----------
| nosy: +EricKow, beschmi
| priority:  -> wishlist
| title: User error? -> deal more gracefully with pull errors
|
| ____________________________________
| Darcs issue tracker <bugs@darcs.net>
| <http://bugs.darcs.net/issue255>
| ____________________________________
msg1849 (view) Author: kowey Date: 2007-07-17.04:44:28
This looks like it has been fixed in 1.0.9 with tolerant application of patches
in the working directory.
History
Date User Action Args
2006-09-01 10:38:42simonpjcreate
2006-09-01 15:14:23jchsetstatus: unread -> unknown
nosy: + jch
messages: + msg952
2006-09-04 08:45:56simonpjsetnosy: droundy, jch, tommy, simonpj
messages: + msg957
2006-09-05 20:08:43droundysetnosy: droundy, jch, tommy, simonpj
messages: + msg962
2006-09-05 20:26:58droundysetnosy: droundy, jch, tommy, simonpj
messages: + msg964
2006-09-06 07:39:15simonpjsetnosy: droundy, jch, tommy, simonpj
messages: + msg966
2006-09-06 17:12:31jchsetnosy: droundy, jch, tommy, simonpj
messages: + msg971
2006-09-06 17:22:47jchsetnosy: droundy, jch, tommy, simonpj
messages: + msg972
2006-09-06 17:47:15jchsetnosy: droundy, jch, tommy, simonpj
messages: + msg973
2006-09-07 09:11:38simonpjsetnosy: droundy, jch, tommy, simonpj
messages: + msg977
2006-09-07 12:31:44simonpjsetnosy: droundy, jch, tommy, simonpj
messages: + msg978
2007-03-10 08:11:37koweysetnosy: + kowey, beschmi
messages: + msg1535
title: User error? -> deal more gracefully with pull errors
2007-03-12 11:40:33simonpjsetnosy: droundy, jch, tommy, beschmi, kowey, simonpj
messages: + msg1541
2007-07-17 04:44:30koweysetstatus: unknown -> resolved
nosy: droundy, jch, tommy, beschmi, kowey, simonpj
messages: + msg1849
2009-08-06 17:35:27adminsetnosy: + markstos, jast, Serware, dmitry.kurochkin, darcs-devel, zooko, dagit, mornfall, simon, thorkilnaur, - droundy, jch, simonpj
2009-08-06 20:32:34adminsetnosy: - beschmi
2009-08-10 21:55:05adminsetnosy: + jch, simonpj, - markstos, darcs-devel, zooko, jast, dagit, Serware, mornfall
2009-08-25 17:49:43adminsetnosy: + darcs-devel, - simon
2009-08-27 13:50:08adminsetnosy: jch, tommy, kowey, darcs-devel, simonpj, thorkilnaur, dmitry.kurochkin