darcs

Issue 73 Improve recovery advice on error apply patch to working

Title Improve recovery advice on error apply patch to working
Priority wishlist Status resolved
Milestone Resolved in
Superseder Automatically record a backup bundle of working dir changes before pull, removing a non-empty directory is catastrophically painful
View: 274, 154
Nosy List darcs-devel, dmitry.kurochkin, jch, kowey, markstos, thorkilnaur, tommy, zooko
Assigned To
Topics ProbablyEasy

Created on 2005-12-24.01:42:03 by zooko, last changed 2009-08-27.14:02:02 by admin.

Messages
msg267 (view) Author: zooko Date: 2005-12-24.01:42:02
I don't think that the string "user error" ought to appear in there...

I had many unrecorded changes, and I pulled a patch that removed all files in a
directory.  Here is the screen shot.

The advice on manual recovery isn't that good because actually I had many
unrecorded changes.  However fortunately I have another copy of them so I can
regenerate them after doing this recovery..

Regards,

Zooko

HACL yumyum:~/work/newnewtailorfrom770/tailorizer/trunk$ time darcs pull
Pulling from "/home/zooko/work/newnewtailorfrom770/dw"...

Thu Dec  8 14:58:11 AST 2005  zooko@zooko.com
  * remove old tests
Shall I pull this patch? (1/40) [ynWvpxqadjk], or ? for help: x
    R ./trunk/amdlib/storage/test/old/Makefile
    R ./trunk/amdlib/storage/test/old/storage_testdata.py
    R ./trunk/amdlib/storage/test/old/storage_unittest.py
    M! ./trunk/amdlib/storage/test/old/test_blockdb.py -539 r4
    M! ./trunk/amdlib/storage/test/old/test_localblockstore.py -177 r1
    M! ./trunk/amdlib/storage/test/old/test_storecollection.py -787 r2
    R ./trunk/amdlib/storage/test/old/test_storefile.py

Thu Dec  8 14:58:11 AST 2005  zooko@zooko.com
  * remove old tests
Shall I pull this patch? (1/40) [ynWvpxqadjk], or ? for help: y

Thu Dec  8 15:31:23 AST 2005  zooko@zooko.com
  * remove old tests
Shall I pull this patch? (2/40) [ynWvpxqadjk], or ? for help: d
We have conflicts in the following files:
./trunk/amdlib/storage/test/old ./trunk/amdlib/storage/test/old/test_blockdb.py ./trunk/amdlib/storage/test/old/test_localblockstore.py ./trunk/amdlib/storage/test/old/test_storecollection.py

darcs failed:  user error (Error applying patch to working dir:
./trunk/amdlib/storage/test/old: 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.

real    0m38.185s
user    0m30.020s
sys     0m0.060s
msg268 (view) Author: zooko Date: 2005-12-24.01:45:07
Setting priority to "bug".  This was with darcs-1.0.5.

It is because of incidents like this that I do not yet widely recommend darcs to 
people who are interested only in revision control software that is mature and 
stable.

However, incidents like this have become somewhat rarer in the last 6 months or 
so, so I remain hopeful that darcs 1.0.6 or 1.0.7 can be considered really 
mature (with of course the caveat of the merger lockup problem).
msg269 (view) Author: zooko Date: 2005-12-24.01:53:19
P.S.  Sorry to gripe about it.  It was disconcerting to have some of my code sort 
of being "juggled" -- i.e. unrecorded and in the middle of revision control 
changes -- and then have darcs tell me that I have to revert in order to recover 
consistency...
msg270 (view) Author: droundy Date: 2005-12-24.16:23:17
Thanks for the bug report! (and on Christmas Eve, no less!)

On Sat, Dec 24, 2005 at 01:42:03AM +0000, Zooko wrote:
> I don't think that the string "user error" ought to appear in there...

Indeed, this is three bugs in one!  This "user error" message snuck through
due to some "improved" error reporting (the message suggesting that you
could revert).

The trouble is that ghc considers programmers to be "users", so when one
calls the "error" function, it is reported as a user error.  At the top
level, we catch these and report them appropriately, but this
error-message-massaging code needs to report "user errors" specially (to
remove the "user error" bit).

> The advice on manual recovery isn't that good because actually I had many
> unrecorded changes.  However fortunately I have another copy of them so I can
> regenerate them after doing this recovery..

I guess maybe we should modify this advice to suggest that if you had no
unrecorded changes, then you should probably use revert to repair the
situation.

...
> darcs failed:  user error (Error applying patch to working dir:
> ./trunk/amdlib/storage/test/old: 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.

Okay, this is the third bug.  We are failing when applying a rmdir patch to
a non-empty working directory.  Instead we should simply not remove the
directory.  I know we used to do this properly, and am not sure what
changed.  We used to handle this by *always* silently ignoring non-empty
directories when applying patches, but that isn't really a good approach
(since it makes corruption-reporting in darcs check less accurate).  It
would be nice to have a separate mode for applying patches to the working
directory, which is more lax.

All three of these bugs could be handled by moderately new darcs hackers,
of anyone's interested.
-- 
David Roundy
http://www.darcs.net
msg407 (view) Author: zooko Date: 2006-01-20.17:34:09
So the current darcs-unstable behavior looks like this:

DARC yumyum:~/playground/darcs/unstable/test$ mkdir d
DARC yumyum:~/playground/darcs/unstable/test$ darcs init
DARC yumyum:~/playground/darcs/unstable/test$ darcs add d
DARC yumyum:~/playground/darcs/unstable/test$ darcs record --all -m'd'
Finished recording patch 'd'
DARC yumyum:~/playground/darcs/unstable/test$ touch d/beans
DARC yumyum:~/playground/darcs/unstable/test$ darcs add d/beans
DARC yumyum:~/playground/darcs/unstable/test$ darcs record --all -m'beans. 
Mmmmmm...'
Finished recording patch 'beans.  Mmmmmm...'
DARC yumyum:~/playground/darcs/unstable/test$ touch d/carrots
DARC yumyum:~/playground/darcs/unstable/test$ darcs oblit

Fri Jan 20 13:32:02 AST 2006  "Zooko O'Whielacronx <zooko@zooko.com>"
  * beans.  Mmmmmm...
Shall I obliterate this patch? (1/2)  [ynWvpxqadjk], or ? for help: w

Fri Jan 20 13:31:49 AST 2006  "Zooko O'Whielacronx <zooko@zooko.com>"
  * d
Shall I obliterate this patch? (2/2)  [ynWvpxqadjk], or ? for help: y

darcs failed:  Couldn't undo patch in working dir.
./d: removeDirectory: unsatisified constraints (Directory not empty)
DARC yumyum:~/playground/darcs/unstable/test$ darcs changes
DARC yumyum:~/playground/darcs/unstable/test$ 
-------

I still find this error message unsatisfactory, since it did in fact unrecord
the patch and remove the b/beans file.  How about something like "Warning:
couldn't remove some directories as they were non-empty.", or even print out
which directories?  This is what utilities like GNU stow and dpkg do in such a case.
msg606 (view) Author: jch Date: 2006-04-07.23:29:14
Looks to me like this is the same as 154.  Superseding, please revert if you
disagree.
msg1852 (view) Author: kowey Date: 2007-07-17.06:01:18
The three issues:

1. error applying patch to working dir (fixed in 1.0.9 with tolerant application)
2. blaming things on user (will submit a patch to use PrettyException)
3. not-so-good recovery advice (see below)

We might as well implement that feature which auto records a backup bundle, and
then modify the advice to take that into account.
msg1901 (view) Author: kowey Date: 2007-07-23.06:44:37
I think the second issue (blaming the user) is now resolved in unstable.  What
now remains is to (a) determine if better advice is moot because of tolerant
application and (b) if not, give better advice.  Downgrading.

Tue Jul 17 07:06:13 CEST 2007  Eric Kow <eric.kow@loria.fr>
  * Use Control.Exception.catch in Darcs.Utils
  
  This affects helper functions like catchall and clarify_errors.

Tue Jul 17 07:07:32 CEST 2007  Eric Kow <eric.kow@loria.fr>
  * Use prettyException in clarify_errors (issue73).
  
  This solves one of three bugs in issue73, namely that we blame the user
  for darcs's own errors.
msg2300 (view) Author: markstos Date: 2008-01-01.17:18:00
I used zooko's test script to test against 2.0.0pre3.  It does not fail there,
but reports:

Warning: Not deleting ./d because it is non-empty.
Finished obliterating.

It seems perhaps providing better advice for failure is moot then, so I'm
marking it as "resolved-in-unstable"
History
Date User Action Args
2005-12-24 01:42:03zookocreate
2005-12-24 01:45:08zookosetstatus: unread -> unknown
nosy: droundy, tommy, zooko
messages: + msg268
2005-12-24 01:53:20zookosetnosy: droundy, tommy, zooko
messages: + msg269
2005-12-24 16:23:18droundysetnosy: droundy, tommy, zooko
messages: + msg270
title: darcs failed: user error (Error applying patch to working dir: -> darcs failed: user error (Error applying patch to working dir:
2006-01-20 17:34:10zookosetnosy: droundy, tommy, zooko
messages: + msg407
2006-04-07 23:29:16jchsetnosy: + jch
superseder: + removing a non-empty directory is catastrophically painful
messages: + msg606
2007-07-17 06:01:21koweysetnosy: + kowey, beschmi
superseder: + Automatically record a backup bundle of working dir changes before pull
messages: + msg1852
2007-07-23 06:44:39koweysettopic: + ProbablyEasy
priority: bug -> wishlist
title: darcs failed: user error (Error applying patch to working dir: -> Improve recovery advice on error apply patch to working
messages: + msg1901
assignedto: kowey ->
2008-01-01 17:18:01markstossetstatus: unknown -> resolved-in-unstable
nosy: + markstos
messages: + msg2300
2008-08-05 23:28:33koweysetstatus: resolved-in-unstable -> resolved
nosy: + dagit
2009-08-06 17:46:45adminsetnosy: + jast, Serware, dmitry.kurochkin, darcs-devel, mornfall, simon, thorkilnaur, - droundy, jch
2009-08-06 20:42:29adminsetnosy: - beschmi
2009-08-10 22:17:40adminsetnosy: + jch, - darcs-devel, jast, Serware, mornfall
2009-08-11 00:08:30adminsetnosy: - dagit
2009-08-25 17:58:32adminsetnosy: + darcs-devel, - simon
2009-08-27 14:02:02adminsetnosy: jch, tommy, kowey, markstos, darcs-devel, zooko, thorkilnaur, dmitry.kurochkin