darcs

Issue 434 Repository corruption issue

Title Repository corruption issue
Priority urgent Status resolved
Milestone Resolved in
Superseder Nosy List WorldMaker, darcs-devel, dmitry.kurochkin, granth, jamesdsadler, kowey, quick, sam.falvo, simonmar, simonpj, thorkilnaur, tommy
Assigned To
Topics

Created on 2007-04-16.13:34:51 by simonpj, last changed 2009-10-24.09:10:01 by admin.

Files
File name Uploaded Type Edit Remove
DRAFT-tolerant-IO.dpatch kowey, 2007-04-16.20:16:43 text/x-darcs-patch
DRAFT-tolerant-IO2.dpatch kowey, 2007-04-17.20:42:19 text/x-darcs-patch
Messages
msg1588 (view) Author: simonpj Date: 2007-04-16.13:34:29
David

This message is about Issue 274, which I raised 6 months ago.

Perhaps it wasn't clear enough from my initial report, but

        this is a repository corruption error

>From my point of view it's very, very serious: I cannot rely on Darcs not to arbitrarily corrupt my working tree, which embodies many, many hours of work.  This is, I think, the third time it has happened, and I am now going to have to spend an unhappy few hours unravelling which are my changes and which are changes from unrelated patches.  (Details below.)

For a source-code control system I believe that fixing repo-corruption errors should be (and I believe is) the highest priority.  But nothing has happened, so far as I know.  I suspect that's because I wasn't clear enough before that it's a serious problem.  Hence this message.

I know that you and the other Darcs developers have a zillion other things to do; and I know from my experience with GHC that you cannot satisfy everyone.   But I do think this one is worth some serious attention.  Please!

Details below

Thanks

Simon

=========================

Here are the details

It's pretty simple.

* I have a Darcs tree.

* I have made quite significant changes, but I have not yet recorded them.

* I do a darcs pull -av (via a shell script called darcs-all), and I get

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.

* Now "darcs pull -av" says there are no patches to pull

* But "darcs what -s" shows zillions of changes that I have not made.  They presumably came from patches that got put in my repository, but not applied to my working directory.

The result is that my own changes (representing many hours of investment) are now in my working directory inextricably tangled up with all the changes that other people have made.

I can't do 'darcs revert' because that would lose all my changes.

I avoided doing an earlier 'darcs record' because that leads to conflicts, and conflicts cause Darcs to crash, so we go to great lengths to avoid them.

[Simon M tells me that I should do the following
        * Record my changes
        * Pull stuff from the repo
        * If there are conflicts, then unrecord my changes and
                do some other fixing up to avoid having a conflict
                in the HEAD

I didn't do that.  But still the repo should not be corrupted.]

================

My guess as to the problem. I *think* this might have happened because the 'pull' modifies the 'darcs-all' script, which is in the midst of being executed.  On Windows a shell-script file is locked during execution, so the 'pull' can't modify it.  That crashes the pull.

But rather than fixing this (which may be Windows-specific) I submit that there should be some end-to-end guarantee that my repo will not be corrupted, regardless of what happens in the middle.
msg1591 (view) Author: droundy Date: 2007-04-16.15:14:47
Indeed, reading over Issue 274, I didn't think it meant this.  You're right,
this is a corruption error (I wouldn't call is a *repository* corruption error,
since it won't propagate to anyone else's repo, but it's important nonetheless).
 The workarounds suggested (e.g. by Simon M) would work, but I hadn't considered
the permissions problem.

I am not sure what I was thinking.

In this case, the idea of storing a patch describing the local changes and using
that in case of working-directory corruption should be doable.  Of course, we
might not be able to reverse our changes, depending on what the permissions
problem is, but we'd at least have the information necessary.  We'd like to be
able to do a revert followed by applying that patch, but quite likely the revert
itself will fail also, in the scenario you describe.

I'm not sure when I'll have time to look at this myself, but I'll do my best to
encourage others to look into it, and give guidance where I can.  And it may be
a feature I can work into some of my refactorings in a pretty way.

Thanks for your patience (and bug reports)!

David

David
msg1593 (view) Author: granth Date: 2007-04-16.16:38:57
Simon Peyton-Jones wrote:
> I am now going to have to spend an unhappy few hours unravelling which
> are my changes and which are changes from unrelated patches.

I have a technique that takes the recovery time down from a few hours to 
a few minutes:

When things do go wrong in this manner, you want to unrecord the pulled 
patches, in the order that Darcs presents them, until you reach one that 
broke everything. Once you've done that, you'll find there are far fewer 
apparent local changes - the only changes left should be your own and 
maybe some corresponding to part of the patch that broke things.

The cleanup works because Darcs updates the pristine files and patch 
files first, for all patches, then applies the patches to the working 
files. When it fails, all patches from before the failure will have been 
applied to both working and pristine, but all patches from after the 
failure will have been applied only to pristine. Unrecording removes 
changes from pristine but leaves the working copy alone, which is 
exactly what you want.

Regards,
Grant Husbands.
msg1595 (view) Author: kowey Date: 2007-04-16.18:06:59
Daid and all,

On Mon, Apr 16, 2007 at 15:14:58 +0000, David Roundy wrote:
> Indeed, reading over Issue 274, I didn't think it meant this.  You're right,
> this is a corruption error (I wouldn't call is a *repository* corruption error,
> since it won't propagate to anyone else's repo, but it's important nonetheless).
>  The workarounds suggested (e.g. by Simon M) would work, but I hadn't considered
> the permissions problem.

This sounds like a variant of issue154

Perhaps in addition to the local bundle, we should be very forgiving to
file system failures when applying patches to the working directory.
In general, the rule should be that if something fails, we emit a
warning and the crucial part, *move on*.  Other behaviours we might want
to layer on this, for example, are that if moving a file fails, we
should try copying it.

Perhaps at some point, this forgiving behaviour will cause patches to
emit very many warnings, so maybe in the future, darcs could
even collect the warnings and try to display them in a more succint
manner.
msg1596 (view) Author: droundy Date: 2007-04-16.18:20:49
On Mon, Apr 16, 2007 at 08:06:11PM +0200, Eric Y. Kow wrote:
> On Mon, Apr 16, 2007 at 15:14:58 +0000, David Roundy wrote:
> > Indeed, reading over Issue 274, I didn't think it meant this.  You're
> > right, this is a corruption error (I wouldn't call is a *repository*
> > corruption error, since it won't propagate to anyone else's repo, but
> > it's important nonetheless).  The workarounds suggested (e.g. by Simon
> > M) would work, but I hadn't considered the permissions problem.
> 
> This sounds like a variant of issue154
> 
> Perhaps in addition to the local bundle, we should be very forgiving to
> file system failures when applying patches to the working directory.
> In general, the rule should be that if something fails, we emit a
> warning and the crucial part, *move on*.  Other behaviours we might want
> to layer on this, for example, are that if moving a file fails, we
> should try copying it.

That's a very good idea--and much easier to implement, I think.  That's
what we do when removing non-empty directories.  Any chance you can code
that up?  :)
-- 
David Roundy
Department of Physics
Oregon State University
msg1597 (view) Author: kowey Date: 2007-04-16.18:23:57
On Mon, Apr 16, 2007 at 18:20:58 +0000, David Roundy wrote:
> That's a very good idea--and much easier to implement, I think.  That's
> what we do when removing non-empty directories.  Any chance you can code
> that up?  :)

Working on it now, but no promises, thesis and all.

Here's the rough idea...

class WriteableDirectory m => TolerantWriteableDirectory m where
    tmSetFileExecutable :: FileName -> Bool -> m ()
    tmWriteBinFile :: FileName -> String -> m ()
    tmWriteFilePS  :: FileName -> PackedString -> m ()
    tmWriteFilePSs :: FileName -> [PackedString] -> m ()
    tmCreateDirectory :: FileName -> m ()
    tmRemoveDirectory :: FileName -> m ()
    tmWriteDoc :: FileName -> Doc -> m ()
    tmCreateFile :: FileName -> m ()
    tmRemoveFile :: FileName -> m ()
    tmRename :: FileName -> FileName -> m ()
    tmModifyFilePS :: FileName -> (PackedString -> m PackedString) -> m ()
    tmModifyFilePSs :: FileName -> ([PackedString] -> m [PackedString]) -> m ()
msg1598 (view) Author: droundy Date: 2007-04-16.18:41:15
On Mon, Apr 16, 2007 at 08:23:11PM +0200, Eric Y. Kow wrote:
> On Mon, Apr 16, 2007 at 18:20:58 +0000, David Roundy wrote:
> > That's a very good idea--and much easier to implement, I think.  That's
> > what we do when removing non-empty directories.  Any chance you can code
> > that up?  :)
> 
> Working on it now, but no promises, thesis and all.
> 
> Here's the rough idea...
> 
> class WriteableDirectory m => TolerantWriteableDirectory m where
>     tmSetFileExecutable :: FileName -> Bool -> m ()
>     tmWriteBinFile :: FileName -> String -> m ()
>     tmWriteFilePS  :: FileName -> PackedString -> m ()
>     tmWriteFilePSs :: FileName -> [PackedString] -> m ()
>     tmCreateDirectory :: FileName -> m ()
>     tmRemoveDirectory :: FileName -> m ()
>     tmWriteDoc :: FileName -> Doc -> m ()
>     tmCreateFile :: FileName -> m ()
>     tmRemoveFile :: FileName -> m ()
>     tmRename :: FileName -> FileName -> m ()
>     tmModifyFilePS :: FileName -> (PackedString -> m PackedString) -> m ()
>     tmModifyFilePSs :: FileName -> ([PackedString] -> m [PackedString]) -> m ()

I'm not really clear why you want a new class.  Couldn't we just have a new
instance

newtype TolerantIO a = TIO (IO a)
runTolerantly :: TolerantIO a -> IO a

tolerantly :: IO a -> IO a
tolerantly io = runTolerantly $ 

instance WriteableDirectory TolerantIO where
     setFileExecutable f e = TIO (warning $ setFileExecutable f e)
     ...

warning :: IO a -> IO a
warning io = io `catch` \e -> do putStrLn $ "verbose warning... " ++ prettyException e

And then we've already got a flag in apply so we can tolerantly run our
apply.
-- 
David Roundy
Department of Physics
Oregon State University
msg1599 (view) Author: kowey Date: 2007-04-16.18:58:04
> I'm not really clear why you want a new class.  Couldn't we just have a new
> instance

Yeah, unintentional silliness on my part.  Actually, I had given up on
the new class and just started adding functions to the current one.  My
idea was to separate out the tolerance.

One slight complication is that the tolerant functions actually need a
flag to suppress the warnings if it is in quiet mode.

To be honest, I wouldn't mind doing away with the Quiet flag.  It seems
like these are the kinds of errors you'd want to report to the user no
matter what.  The other benefit of this it would avoid us littering the
class with ...IfPossible functions

> newtype TolerantIO a = TIO (IO a)
> runTolerantly :: TolerantIO a -> IO a
> 
> tolerantly :: IO a -> IO a
> tolerantly io = runTolerantly $ 
> 
> instance WriteableDirectory TolerantIO where
>      setFileExecutable f e = TIO (warning $ setFileExecutable f e)
>      ...
> 
> warning :: IO a -> IO a
> warning io = io `catch` \e -> do putStrLn $ "verbose warning... " ++ prettyException e
msg1600 (view) Author: kowey Date: 2007-04-16.20:11:30
Alright, I'm heading to bed.

Attached is a draft, my current progress to get some initial comments.

To simplify the problem, I have done away with the 'Quiet' flag

This code might be outright wrong, because I don't think I know what I'm
doing with the TolerantIO stuff!  For example, I don't think I
understand what the 'tolerantly' function is for.  Also, this might be
the first time I've actually written 'instance Monad...' for something.
(I suddenly feel quite naked)

Note that this draft also includes my first attempt, the ...InProgress
stuff.  That should be ignored.

Also, I initially wanted to use runTolerantly within apply, but we can't
do that because apply wants to be more general than IO; it wants to work
in any WriteableDirectory.  My solution was to move all the
runTolerantlys to the commands (in progress).  We might consider having
an applyTolerantly function, come to think of it.  Note also that if we
do things this way, we can even do away with the extra isW flag we
pass to apply.
msg1601 (view) Author: kowey Date: 2007-04-16.20:16:44
Naturally, it would have helped if I had actually attached the patch.
Attachments
msg1606 (view) Author: droundy Date: 2007-04-16.22:38:30
Hi Eric,

Thanks for taking a shot at this.  I think we've got a design problem.
What we really want is to have individual patch applications run
atomically, but to keep going even if a patch fails to apply.  I think 

On Mon, Apr 16, 2007 at 08:16:45PM +0000, Eric Kow wrote:
> +newtype TolerantIO a = TIO { runTolerantly :: IO a }

There's a problem here, which is that runTolerantly is actually
intolerant of errors.  We really want something like

actuallyRunTolerantly (TIO io) = io `catch` \_ -> ...

This is tricky, because we aren't quite sure what ... should be.  If the
typ of io is not IO (), we can't continue with the computation, even though
we want to do so!

> +instance ReadableDirectory TolerantIO where
> +    mDoesDirectoryExist d = TIO $ mDoesDirectoryExist d
> +    mDoesFileExist f = TIO $ mDoesFileExist f
> +    mIsFileExecutable f = TIO $ mIsFileExecutable f
> +    mInCurrentDirectory i (TIO j) = TIO $ mInCurrentDirectory i j
> +    mGetDirectoryContents = TIO mGetDirectoryContents
> +    mReadBinFile f = TIO $ mReadBinFile f
> +    mReadFilePS f = TIO $ mReadFilePS f
> +
> +instance WriteableDirectory TolerantIO where
> +     mWithCurrentDirectory f (TIO m) = TIO $ mWithCurrentDirectory f m
> +     mSetFileExecutable f e = warning $ mSetFileExecutable f e
> +     mWriteBinFile f s = warning $ mWriteBinFile f s
> +     mWriteFilePS f s = warning $ mWriteFilePS f s
> +     mCreateDirectory d = warning $ mCreateDirectory d
> +     mRemoveFile f = warning $ mRemoveFile f
> +     mRemoveDirectory d = warning $ mRemoveDirectory d
> +     mRename a b = warning $ mRename a b

These look good.

> +warning :: IO () -> TolerantIO ()
> +warning io = TIO $ io `catch` \e -> do putStrLn $ "verbose warning... " ++ show e

I think this (show e) should be (prettyException e), so we can avoid
printing "user error" when it's not a user error.

*************

I think we actually would want to split apply into two functions, something
like apply and apply_internal.

maybe do something like:

apply_internal :: Patch -> m ()
apply_internal = same code as before, pretty much

apply :: WriteableDirectory m =>
         Bool -- ^ working directory - if True, this means some ...
      -> Patch -> m ()
apply False = apply_internal
apply True = runTolerantly . apply_internal

This works under the assumption that we get rid of the Quiet option.
Perhaps we could relatively easily keep the Quiet option by defining

data QuietType
data LoudType

newtype TolerantIO a = TIO { runTolerantly :: IO a }
newtype MeekIO a = MeekIO { runMeekly :: IO a }

class Stubborn m where
    tentatively :: m a -> IO a
    stubbornly :: m () -> IO ()
    liftS :: IO a -> m a

instance Stubborn TolerantIO where
    tentatively = TIO
    stubbornly io = TIO $ io `catch` \e -> putStrLn ...
    liftS = runTolerantly

instance Stubborn MeekIO where
    tentatively = MeekIO
    stubbornly io = MeekIO $ io `catch` \e -> return ()
    liftS = runMeekly

instance Stubborn m => WriteableDirectory m where
  mWithCurrentDirectory f m = tentatively $ mWithCurrentDirectory f $ liftS m
  mSetFileExecutable f e = stubbornly $ mSetFileExecutable f e
  ...

where I'm basically just translating your code (which looks correct to
me) in this class.  Then we'd have something like 

apply opts True | Quiet `elem` opts = runMeekly . apply_internal
                | otherwise = runTolerantly . apply_internal

(And yes, I am obviously obsessed with adverbs as higher-order function
names...)

David

P.S. I suppose I might have written this myself in this amount of time, but
I probably wouldn't have done so good a job.  I think I really am better at
reviewing code and suggesting improvements than I am at writing code.
msg1607 (view) Author: droundy Date: 2007-04-16.22:48:48
On Mon, Apr 16, 2007 at 08:11:31PM +0000, Eric Kow wrote:
> Also, I initially wanted to use runTolerantly within apply, but we can't
> do that because apply wants to be more general than IO; it wants to work
> in any WriteableDirectory.  My solution was to move all the
> runTolerantlys to the commands (in progress).  We might consider having
> an applyTolerantly function, come to think of it.  Note also that if we
> do things this way, we can even do away with the extra isW flag we
> pass to apply.

See my other email for a response to this.  Except that the other email is
wrong, but I think an applyTolerantly function is definitely the way to
go.  Then applyTolerantly could perhaps just get the [DarcsFlag] while
apply would be the same as my apply_internal in the other email, and would
have only the patch argument.
-- 
David Roundy
Department of Physics
Oregon State University
msg1609 (view) Author: simonpj Date: 2007-04-17.09:05:23
| > I am now going to have to spend an unhappy few hours unravelling which
| > are my changes and which are changes from unrelated patches.
|
| I have a technique that takes the recovery time down from a few hours to
| a few minutes:
|
| When things do go wrong in this manner, you want to unrecord the pulled
| patches, in the order that Darcs presents them, until you reach one that
| broke everything. Once you've done that, you'll find there are far fewer
| apparent local changes - the only changes left should be your own and
| maybe some corresponding to part of the patch that broke things.

Excellent point thank you.  How do I ask Darcs to unrecord in the "right" order? In principle it could present patches in any topologically-sorted order?  Are you relying on an undocumented feature? (That's ok with me if so!)

Thanks again

S
msg1610 (view) Author: granth Date: 2007-04-17.10:03:27
Simon Peyton-Jones wrote:
> How do I ask Darcs to unrecord in the "right" order?

I've always seen Darcs pull in patch-sequence order and unrecord in the 
reverse, so it appears to be predictable.

> In principle it could present patches in any topologically-sorted order?
> Are you relying on an undocumented feature? (That's ok with me if so!)

I'd say I'm relying on an implementation detail. I hold out hope that 
the repository-state corruption will be fixed before the apparent patch 
order for pull or unrecord is altered. Given the rapid progress that 
Eric and David are making against this bug, it looks like it will be.

> Thanks again

I'm glad I could help.

Regards,
Grant.
msg1618 (view) Author: kowey Date: 2007-04-17.20:42:30
(latest version attached)  More progress to be made...

> > +newtype TolerantIO a = TIO { runTolerantly :: IO a }
> 
> There's a problem here, which is that runTolerantly is actually
> intolerant of errors.  We really want something like

I wonder if it's just an issue of how we name things.  Maybe what I call
runTolerantly should be something else, lower ourselves back into plain
old IO.

What we really mean here is not runTolerantly, but runThatTolerantThing,
where thatTolerantThing is expected to catch its own errors.

Or did I just confuse myself?

> actuallyRunTolerantly (TIO io) = io `catch` \_ -> ...

Yeah, tricky.  But maybe we can do without?

> > +warning :: IO () -> TolerantIO ()
> > +warning io = TIO $ io `catch` \e -> do putStrLn $ "verbose warning... " ++ show e
> 
> I think this (show e) should be (prettyException e), so we can avoid
> printing "user error" when it's not a user error.

Yes.  I had used show because I didn't realise that prettyException had
already made it to stable (forgot to pull into one of my local
branches).  Was happy to have cherry picking, though.  Before I realised
it was already in stable, I pulled it in from unstable, and it went in
fine.

> data QuietType
> data LoudType
> 
> newtype TolerantIO a = TIO { runTolerantly :: IO a }
> newtype MeekIO a = MeekIO { runMeekly :: IO a }

Perhaps another way is to put the meekness flag into TolerantIO (it
wouldn't be a newtype anymore).  We could use something like a ReaderT
monad... or maybe even StateT since we will one day want to accumulate
error messages (for example, if you notice you can't delete a bunch of
files because you can't delete the underlying directory, then just
report the directory).

I'm a bit hazy on this... will have to work out if a FooT IO transformer
is really what we want.

> P.S. I suppose I might have written this myself in this amount of time, but
> I probably wouldn't have done so good a job.  I think I really am better at
> reviewing code and suggesting improvements than I am at writing code.

Well... delegating tasks does improve our bus factor somewhat, borrowing
from svn folklore.  Speaking of which, is there anybody willing to write
the tests for this?
Attachments
msg1619 (view) Author: droundy Date: 2007-04-17.21:27:20
On Tue, Apr 17, 2007 at 08:42:32PM +0000, Eric Kow wrote:
> > > +newtype TolerantIO a = TIO { runTolerantly :: IO a }
> > 
> > There's a problem here, which is that runTolerantly is actually
> > intolerant of errors.  We really want something like
> 
> I wonder if it's just an issue of how we name things.  Maybe what I call
> runTolerantly should be something else, lower ourselves back into plain
> old IO.
> 
> What we really mean here is not runTolerantly, but runThatTolerantThing,
> where thatTolerantThing is expected to catch its own errors.

This is actually correct.  The "catch" is already living inside that IO
thing, so this really does run tolerantly.  I caught this misunderstanding
on my part after writing this, but didn't get it fixed.  Your code looks
good.

> > > +warning :: IO () -> TolerantIO ()
> > > +warning io = TIO $ io `catch` \e -> do putStrLn $ "verbose warning... " ++ show e
> > 
> > I think this (show e) should be (prettyException e), so we can avoid
> > printing "user error" when it's not a user error.
> 
> Yes.  I had used show because I didn't realise that prettyException had
> already made it to stable (forgot to pull into one of my local
> branches).  Was happy to have cherry picking, though.  Before I realised
> it was already in stable, I pulled it in from unstable, and it went in
> fine.

I'm thinking that we'd like to extend this to give a more verbose and
situation-specific warning.  This could also be used to handle the meekness
scenario.  I'm imagining something like 

data TolerantIO a = TIO { runTolerantly :: String -> IO a }

warning :: IO () -> TolerantIO ()
warning io = TIO runit
    where runit "" = io `catch` $ \_ -> return ()
          runit msg = io `catch` $ \e -> putStrLn $ msg ++ prettyException e

Then we can have custom warning messages when running tolerantly, and no
messages when running quietly.

> > data QuietType
> > data LoudType
> > 
> > newtype TolerantIO a = TIO { runTolerantly :: IO a }
> > newtype MeekIO a = MeekIO { runMeekly :: IO a }
> 
> Perhaps another way is to put the meekness flag into TolerantIO (it
> wouldn't be a newtype anymore).  We could use something like a ReaderT
> monad... or maybe even StateT since we will one day want to accumulate
> error messages (for example, if you notice you can't delete a bunch of
> files because you can't delete the underlying directory, then just
> report the directory).
> 
> I'm a bit hazy on this... will have to work out if a FooT IO transformer
> is really what we want.

Monad transformers scare me.

> hunk ./Repository.lhs 241
> +applyTolerantly :: Repository -> [DarcsFlag] -> Patch -> IO ()
> +applyTolerantly (Repo r _ (DarcsRepository _)) opts patch =
> +    withCurrentDirectory r $ runTolerantly $ apply opts True patch
> +applyTolerantly (Repo _ _ GitRepository) _ _ = return ()

I'd call this applyToWorking.
-- 
David Roundy
Department of Physics
Oregon State University
msg1620 (view) Author: droundy Date: 2007-04-17.21:31:06
On Tue, Apr 17, 2007 at 09:27:34PM +0000, David Roundy wrote:
> > hunk ./Repository.lhs 241
> > +applyTolerantly :: Repository -> [DarcsFlag] -> Patch -> IO ()
> > +applyTolerantly (Repo r _ (DarcsRepository _)) opts patch =
> > +    withCurrentDirectory r $ runTolerantly $ apply opts True patch
> > +applyTolerantly (Repo _ _ GitRepository) _ _ = return ()
> 
> I'd call this applyToWorking.

Oh yes, and in unstable Repository has a [DarcsFlag], so we'll need one
less flag to applyToWorking.
-- 
David Roundy
Department of Physics
Oregon State University
msg1627 (view) Author: simonpj Date: 2007-04-18.11:02:01
Oh DARN.

Your technique would work fine if it weren't for a *different* show-stopper bug in Darcs:

sh-2.04$ darcs unrecord
...
darcs.exe: failed to read patch in get_extra:
Fri Dec 15 21:44:30 GMT Standard Time 2006  Ian Lynagh <igloo@earth.li>
  * Free more things that we allocate
Perhaps this is a 'partial' repository?

Full log below.  ('darcs check' is happy with the repo, incidentally.)

I have reported this before.  Indeed I have reported cases where

        I cannot unrecord even IMMEDIATELY after recording.

It's terribly frustrating.  I think that's why I'd mentally blanked out even the possibility of unrecording.

I suppose I should simply stop using partial repositories.  I know that helps; whether it guarantees to avoid the get_extra bug I do not know.  I just have not gotten around to it, which I guess is my fault.

Darcsers: please treat this as a separate bug report, not Issue 274.  I'm not sure I can locate my previous report.  Again, I don't know where this bug lives on your priority list, but it seems a serious one to me.

So I'm back to manually untangling my repo.

Simon

| -----Original Message-----
| From: Grant Husbands [mailto:bugs@darcs.net]
| Sent: 16 April 2007 17:39
| To: beschmi@cloaked.de; droundy@darcs.net; eric.kow@gmail.com; grant.husbands@businesswebsoftware.com;
| igloo@earth.li; ptp@lysator.liu.se; Simon Marlow; Simon Peyton-Jones
| Subject: [issue434] Darcs grief: Issue 274
|
|
| Grant Husbands <grant.husbands@businesswebsoftware.com> added the comment:
|
| Simon Peyton-Jones wrote:
| > I am now going to have to spend an unhappy few hours unravelling which
| > are my changes and which are changes from unrelated patches.
|
| I have a technique that takes the recovery time down from a few hours to
| a few minutes:
|
| When things do go wrong in this manner, you want to unrecord the pulled
| patches, in the order that Darcs presents them, until you reach one that
| broke everything. Once you've done that, you'll find there are far fewer
| apparent local changes - the only changes left should be your own and
| maybe some corresponding to part of the patch that broke things.
|
| The cleanup works because Darcs updates the pristine files and patch
| files first, for all patches, then applies the patches to the working
| files. When it fails, all patches from before the failure will have been
| applied to both working and pristine, but all patches from after the
| failure will have been applied only to pristine. Unrecording removes
| changes from pristine but leaves the working copy alone, which is
| exactly what you want.
|
| Simon Peyton-Jones wrote:
| > How do I ask Darcs to unrecord in the "right" order?
|
| I've always seen Darcs pull in patch-sequence order and unrecord in the
| reverse, so it appears to be predictable.
|
| > In principle it could present patches in any topologically-sorted order?
| > Are you relying on an undocumented feature? (That's ok with me if so!)
|
| I'd say I'm relying on an implementation detail. I hold out hope that
| the repository-state corruption will be fixed before the apparent patch
| order for pull or unrecord is altered. Given the rapid progress that
| Eric and David are making against this bug, it looks like it will be.
|
|
| Regards,
| Grant Husbands.
|
| ----------
| nosy: +grant.husbands
| title: permissions error in working directory corrupts working directory -> Darcs grief: Issue 274
|
| ____________________________________
| Darcs issue tracker <bugs@darcs.net>
| <http://bugs.darcs.net/issue434>
| ____________________________________

sh-2.04$ darcs --version
1.0.7 (release)

sh-2.04$ darcs unrecord
plink: unknown option "-O"

Sun Apr 15 17:27:17 GMT Standard Time 2007  Ian Lynagh <igloo@earth.li>
  * Don't try to install docs stuff if NO_HADDOCK_DOCS=YES
Shall I unrecord this patch? (1/429)  [ynWvpxqadjk], or ? for help: y

Sun Apr 15 17:23:15 GMT Standard Time 2007  Ian Lynagh <igloo@earth.li>
  * Implement ifBuildable
  ifBuildable only runs a command it is given if the library in . either
  must be built (core-package - readline) or is buildable.
Shall I unrecord this patch? (2/429)  [ynWvpxqadjk], or ? for help: y

Sat Apr 14 22:40:27 GMT Standard Time 2007  wolfgang.thaller@gmx.net
  * make opt_HiVersion an Integer instead of Int to prevent overflow

  When a snapshot version number is included, opt_HiVersion tends to exceed the range of a 32bit Int.

  MERGE TO STABLE
Shall I unrecord this patch? (3/429)  [ynWvpxqadjk], or ? for help: y

Sat Apr 14 00:05:55 GMT Standard Time 2007  Ian Lynagh <igloo@earth.li>
  * Allow ProjectTags to be specified in mk/build.mk
  ProjectTags is expected to be of the form -tag1-tag2 (i.e. the same as
  Cabal tags). These are appended to the GHC version number.
Shall I unrecord this patch? (4/429)  [ynWvpxqadjk], or ? for help: d

darcs.exe: failed to read patch in get_extra:
Fri Dec 15 21:44:30 GMT Standard Time 2006  Ian Lynagh <igloo@earth.li>
  * Free more things that we allocate
Perhaps this is a 'partial' repository?
msg1628 (view) Author: kowey Date: 2007-04-18.11:18:36
[forwarding; meant to sent to bugtracker]

Simon,

> sh-2.04$ darcs unrecord
> ...
> darcs.exe: failed to read patch in get_extra:

That sounds pretty serious.  I think it's one of our urgent bugs
http://bugs.darcs.net/issue317
which Ian has also experienced.

Another idea you might try for easing the untangling is to darcs get a
fresh copy of the repository (partial even if you want) without the
offending patches and then do something like a graphical
diff, or even brutally cp your working directory (i.e. all but
_darcs).  That might be equivalent to what Grant suggested.
msg1629 (view) Author: granth Date: 2007-04-18.11:51:42
Simon Peyton-Jones wrote:
> Your technique would work fine if it weren't for a *different* show-stopper bug in Darcs:
> 
> sh-2.04$ darcs unrecord
> ...
> darcs.exe: failed to read patch in get_extra:
> 
> So I'm back to manually untangling my repo.

Before I discovered how useful unrecord was, in this situation, I got 
used to copying files around behind Darcs' back. I believe doing so will 
help you here.

Essentially, you want to get a complete (not partial) repo containing 
all of the patches Darcs believes are in your broken repo. It doesn't 
need to contain your working changes. Then, you want to copy your 
working folder from the broken repo over the working folder in the new 
one. Now that you have a non-partial repo in the same broken state as 
the partial one, you should be able to successfully follow the recovery 
steps I previously gave.

Regards,
Grant.
msg1630 (view) Author: droundy Date: 2007-04-18.14:14:13
...
> sh-2.04$ darcs unrecord
> ...
> darcs.exe: failed to read patch in get_extra:
> Fri Dec 15 21:44:30 GMT Standard Time 2006  Ian Lynagh <igloo@earth.li>
>   * Free more things that we allocate
> Perhaps this is a 'partial' repository?
...
> I suppose I should simply stop using partial repositories.  I know that
> helps; whether it guarantees to avoid the get_extra bug I do not know.  I
> just have not gotten around to it, which I guess is my fault.

Yes, stopping using partial repositories should definitely fix this bug.

There's code in the works that will replace partial repositories (but
require a switch to a new repository format), which will allow them to be
incrementally made less partial.  This should "fix" this issue.  Although
we really need to look into why get_extra is trying to look at old patches,
and just haven't had the time (and it comes after conflicts in priority).

You can (in a copy!) unrecord the latest patches by hand by simply deleting
the lines in _darcs/inventory describing the patches you pulled (which
should all be at the end of the file) and then running darcs repair.  It's
ugly, but it might help you.

David
msg1631 (view) Author: droundy Date: 2007-04-18.14:17:41
On Tue, Apr 17, 2007 at 10:21:02PM -0700, Kevin Quick wrote:
> I am a bit concerned that it was a darcs pull that encountered this
> problem for you.  In my case, it was a push from a user that didn't
> have proper access rights to some of the files in my repo (and did on
> others).  I wouldn't have expected this in the pull case, however.
> Are you certain it was a pull, and in either case, have you been
> able to reproduce it?

It's a windows-specific case, which I believe we understand.  You can't
delete an open file on windows, and darcs always modifies files by writing
a new copy and then replacing the old with the new.  But Simon is executing
a script in the working directory that darcs wants to modify, and therefore
it can't, which causes the whole trouble.  It wouldn't happen with POSIX
file system semantics, which is why it hasn't bothered people before.  :(

David
msg1632 (view) Author: quick Date: 2007-04-18.14:18:55
Simon,

This sounds like a variation of the permissions issue I wrote about
recently.  I'll be departing in a couple of days for a 2 wk vacation,
but I am in agreement on this issue and am willing to work with
folks to help resolve this upon my return.

I am a bit concerned that it was a darcs pull that encountered this
problem for you.  In my case, it was a push from a user that didn't
have proper access rights to some of the files in my repo (and did on
others).  I wouldn't have expected this in the pull case, however.
Are you certain it was a pull, and in either case, have you been
able to reproduce it?

-KQ

On Mon, 16 Apr 2007 13:34:51 +0000, Simon Peyton-Jones <bugs@darcs.net>
wrote:

>
> New submission from Simon Peyton-Jones <simonpj@microsoft.com>:
>
> David
>
> This message is about Issue 274, which I raised 6 months ago.
>
> Perhaps it wasn't clear enough from my initial report, but
>
>         this is a repository corruption error
>
> >From my point of view it's very, very serious: I cannot rely on Darcs not to arbitrarily corrupt my working tree, which embodies many, many hours of work.  This is, I think, the third time it has happened, and I am now going to have to spend an unhappy few hours unravelling which are my changes and which are changes from unrelated patches.  (Details below.)
>
> For a source-code control system I believe that fixing repo-corruption errors should be (and I believe is) the highest priority.  But nothing has happened, so far as I know.  I suspect that's because I wasn't clear enough before that it's a serious problem.  Hence this message.
>
> I know that you and the other Darcs developers have a zillion other things to do; and I know from my experience with GHC that you cannot satisfy everyone.   But I do think this one is worth some serious attention.  Please!
>
> Details below
>
> Thanks
>
> Simon
>
> =========================
>
> Here are the details
>
> It's pretty simple.
>
> * I have a Darcs tree.
>
> * I have made quite significant changes, but I have not yet recorded them.
>
> * I do a darcs pull -av (via a shell script called darcs-all), and I get
>
> 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.
>
> * Now "darcs pull -av" says there are no patches to pull
>
> * But "darcs what -s" shows zillions of changes that I have not made.  They presumably came from patches that got put in my repository, but not applied to my working directory.
>
> The result is that my own changes (representing many hours of investment) are now in my working directory inextricably tangled up with all the changes that other people have made.
>
> I can't do 'darcs revert' because that would lose all my changes.
>
> I avoided doing an earlier 'darcs record' because that leads to conflicts, and conflicts cause Darcs to crash, so we go to great lengths to avoid them.
>
> [Simon M tells me that I should do the following
>         * Record my changes
>         * Pull stuff from the repo
>         * If there are conflicts, then unrecord my changes and
>                 do some other fixing up to avoid having a conflict
>                 in the HEAD
>
> I didn't do that.  But still the repo should not be corrupted.]
>
> ================
>
> My guess as to the problem. I *think* this might have happened because the 'pull' modifies the 'darcs-all' script, which is in the midst of being executed.  On Windows a shell-script file is locked during execution, so the 'pull' can't modify it.  That crashes the pull.
>
> But rather than fixing this (which may be Windows-specific) I submit that there should be some end-to-end guarantee that my repo will not be corrupted, regardless of what happens in the middle.
>
> ----------
> messages: 1588
> nosy: EricKow, beschmi, droundy, igloo, simonmar, simonpj, tommy
> status: unread
> title: Darcs grief: Issue 274
>
> ____________________________________
> Darcs issue tracker <bugs@darcs.net>
> <http://bugs.darcs.net/issue434>
> ____________________________________
> _______________________________________________
> darcs-devel mailing list
> darcs-devel@darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-devel
msg1634 (view) Author: simonpj Date: 2007-04-18.14:52:55
| You can (in a copy!) unrecord the latest patches by hand by simply deleting
| the lines in _darcs/inventory describing the patches you pulled (which
| should all be at the end of the file) and then running darcs repair.  It's
| ugly, but it might help you.

I was not brave enough to undertake such an enterprise.  What I did instead was to 'darcs revert' file by file, making sure I reverted everything *except* my own changes which I wanted to keep.

I think I've gotten a good approximation to the right answer.  But things like the ability to unrecord (always) and not to get repo corruption are good things for Darcs to focus on.

Thanks for your help, and the recent flurry of activity

Simon
msg1639 (view) Author: quick Date: 2007-04-18.19:51:07
Quoting David Roundy <droundy@darcs.net>:

> On Tue, Apr 17, 2007 at 10:21:02PM -0700, Kevin Quick wrote:
> > I am a bit concerned that it was a darcs pull that encountered this
> > problem for you.  In my case, it was a push from a user that didn't
> > have proper access rights to some of the files in my repo (and did on
> > others).  I wouldn't have expected this in the pull case, however.
> > Are you certain it was a pull, and in either case, have you been
> > able to reproduce it?
> 
> It's a windows-specific case, which I believe we understand.  You can't
> delete an open file on windows, and darcs always modifies files by writing
> a new copy and then replacing the old with the new.  But Simon is executing
> a script in the working directory that darcs wants to modify, and therefore
> it can't, which causes the whole trouble.  It wouldn't happen with POSIX
> file system semantics, which is why it hasn't bothered people before.  :(

Ah, OK.  Different root problem, but same results though: the patches are
not applied but the working directory is modified.

I'm thinking there are two elements here:
  1) Fix (if appropriate/possible) the root cause of both issues (Simon's
     above and mine with the pusher privilege problems).
  2) In the event that any pull/push/apply fails due to this type of
     unexpected issue, it should return the entire current tree to its
     original form (as much as possible).  This might be done by doing
     all modifications on a copy and then swapping in that copy (expensive
     spacewise) or reverting all operations (similar to applying the
     inverse patch to the working tree, but not entirely).

-KQ

-------------------------------------------------
This mail sent through IMP: http://horde.org/imp/
msg1641 (view) Author: kowey Date: 2007-04-18.20:10:43
Nothing to show tonight.  I hope nobody minds the daily progress report;
the idea is really to get this thing out the door as soon as we can, so
I'm just reporting back everytime I have something that compiles,
getting feedback early.  Kind of frustrating to be working on something
only an hour or so every day, though.

> warning :: IO () -> TolerantIO ()
> warning io = TIO runit
>     where runit "" = io `catch` $ \_ -> return ()
>           runit msg = io `catch` $ \e -> putStrLn $ msg ++ prettyException e

Hmm... I think I'll leave that for somebody else to implement.  I think
I'd be happy to get something minimal working and a few tests, just do
what we need to minimise 'corruption' of the working directory.

> > +applyTolerantly (Repo _ _ GitRepository) _ _ = return ()
> 
> I'd call this applyToWorking.

Absolutely.

...

Also, this copyDirectory function is getting rather annoying to
implement.  Any reason I shouldn't just use a slurpy?  I'm a bit scared
of the lazy aspect.
msg1642 (view) Author: droundy Date: 2007-04-18.20:21:15
On Wed, Apr 18, 2007 at 02:50:13PM -0500, quick@sparq.org wrote:
> I'm thinking there are two elements here:
>   1) Fix (if appropriate/possible) the root cause of both issues (Simon's
>      above and mine with the pusher privilege problems).
>   2) In the event that any pull/push/apply fails due to this type of
>      unexpected issue, it should return the entire current tree to its
>      original form (as much as possible).  This might be done by doing
>      all modifications on a copy and then swapping in that copy (expensive
>      spacewise) or reverting all operations (similar to applying the
>      inverse patch to the working tree, but not entirely).

Indeed.  I think that #2 is the "real" solution we've discussed here, and
what we thought would be best would be to store a patch bundle holding the
current changes (output of darcs whatsnew), as if you had recorded before
pulling.  Then we'd be able to use that + the pristine cache to recreate
the initial state (minus unmanaged files).  Whether we want to do that
automatically, I'm not sure.  We could also use this information (if we
don't delete it) to make unpull remove the conflict markers for conflicts
with unrecorded changes (once certain that there were no local changes made
since the pull).

This should be almost free in terms of computational time (as we have to
compute the whatsnew anyhow), and not very expensive in terms of disk space
either.  But it could be expensive in terms of the complexity of the user
interface, if we try to give user access to it.
-- 
David Roundy
Department of Physics
Oregon State University
msg1643 (view) Author: droundy Date: 2007-04-18.20:46:27
On Wed, Apr 18, 2007 at 08:10:52PM +0000, Eric Kow wrote:
> Also, this copyDirectory function is getting rather annoying to
> implement.  Any reason I shouldn't just use a slurpy?  I'm a bit scared
> of the lazy aspect.

I presume you're referring to this:

+   mRename a b = TIO $ catchJust ioErrors
+           (mRename a b)
+           (\e -> when (isPermissionError e) $
+           putStrLn $ "Warning: Could not rename " ++ fn2fp a ++ " to " ++ fn2fp b
+                        ++ ".  Copying it instead [FIXME: NOT YET IMPLEMENTED]")
+                       -- need a copyDirectoryRecursively

Is there some reason why we can't do something like:

explain :: String -> TolerantIO ()
explain s = TIO $ putStrLn s

mRename o n =
  do isd <- mDoesDirectoryExist n
     isf <- mDoesFileExist n
     if isd || isf
       then ... (cleverly rename n to something unique)
       else TIO $ mRename o n `catch`
            \e -> case e of
                  ... handle existing case of mRename
                  ... do when (isPermissionError e) $
                             runTolerantly (renameRecursively o n)

-- The following to be called only when mRename failed
renameRecursively :: String -> String -> TolerantIO ()
renameRecursively o n = do
  isd <- mDoesDirectoryExist o
  if not isd
    then mReadFilePS o >>= mWriteFilePS n
    else do mCreateDirectory n
            explain "Couldn't rename directory o to n so I am copying it..."
            mWithCurrentDirectory o $
              do fs <- mGetDirectoryContents
                 mapM_ ((flip mRename) n) fs

Note that there isn't a TIO here, this is all done within the tolerant
monad, which I think does what we want.

Incidentally, we should add a mDoesFileOrDirectoryExist to the DarcsIO
monad, since it's faster to check both at the same time in the IO monad,
and we can define a default for it, so we wouldn't need to complicate the
writing of instances.
-- 
David Roundy
Department of Physics
Oregon State University
msg1644 (view) Author: sam.falvo Date: 2007-04-19.16:19:37
On 4/16/07, Grant Husbands <grant.husbands@businesswebsoftware.com> wrote:
> Simon Peyton-Jones wrote:
> > I am now going to have to spend an unhappy few hours unravelling which
> > are my changes and which are changes from unrelated patches.

I'm curious to learn how difficult it would be to implement
transactional semantics to Darcs?  This would prevent any kind of
corruption in the event of any kind of error at all.
msg1645 (view) Author: droundy Date: 2007-04-19.17:09:34
On Thu, Apr 19, 2007 at 08:56:57AM -0700, Samuel A. Falvo II wrote:
> I'm curious to learn how difficult it would be to implement
> transactional semantics to Darcs?  This would prevent any kind of
> corruption in the event of any kind of error at all.

Pretty easy (almost) with the new hashed inventory format.  We don't want
truly atomic behavior on the pristine cache, as that'd slow darcs down too
much (and I don't see any way of avoiding this slowdown, short of
implementing our own journalling filesystem, which would definitely be a
nice option, but not so easy).
-- 
David Roundy
Department of Physics
Oregon State University
msg1646 (view) Author: droundy Date: 2007-04-19.17:16:12
On Thu, Apr 19, 2007 at 10:08:45AM -0700, David Roundy wrote:
> On Thu, Apr 19, 2007 at 08:56:57AM -0700, Samuel A. Falvo II wrote:
> > I'm curious to learn how difficult it would be to implement
> > transactional semantics to Darcs?  This would prevent any kind of
> > corruption in the event of any kind of error at all.
> 
> Pretty easy (almost) with the new hashed inventory format.  We don't want
> truly atomic behavior on the pristine cache, as that'd slow darcs down too
> much (and I don't see any way of avoiding this slowdown, short of
> implementing our own journalling filesystem, which would definitely be a
> nice option, but not so easy).

I should mention that transactional semantics in the working directory is
impossible to implement with any guarantees, because there's always the
possibility that halfway through an update someone will change permissions
of some of the working-directory files, or that disk space will be used up,
etc.  I don't believe anything bigger than a single file can be updated
atomically.  I suppose you could create a new directory and rename it over
an existing one, but you wouldn't want that behavior, since it'd mess up
any running programs with that directory as their working directory.
-- 
David Roundy
Department of Physics
Oregon State University
msg1647 (view) Author: sam.falvo Date: 2007-04-19.17:24:49
On 4/19/07, David Roundy <droundy@darcs.net> wrote:
> I should mention that transactional semantics in the working directory is
> impossible to implement with any guarantees, because there's always the

I meant within the context of darcs only.  If another application is
running that is tweaking the filesystem out from underneath darcs,
well, we obviously can't control that, and the user gets what he
deserves.  ;D
msg1648 (view) Author: droundy Date: 2007-04-19.17:38:20
On Thu, Apr 19, 2007 at 10:23:53AM -0700, Samuel A. Falvo II wrote:
> On 4/19/07, David Roundy <droundy@darcs.net> wrote:
> >I should mention that transactional semantics in the working directory is
> >impossible to implement with any guarantees, because there's always the
> 
> I meant within the context of darcs only.  If another application is
> running that is tweaking the filesystem out from underneath darcs,
> well, we obviously can't control that, and the user gets what he
> deserves.  ;D

I disagree, in the case of the working directory.  We need to code
pessimistically, since it's perfectly fair for the user to do things in the
working directory while darcs is running.  Admittedly, they may not get the
effect they want, but they shouldn't be able to corrupt darcs by so doing
(with the more restrictive definition which says that messing up the
working directory doesn't count as corrupting the repository).

The problem is that when we're talking about transactional semantics, we're
talking precisely about exceptional circumstances--meaning error cases,
things that aren't supposed to happen, but really do happen in real life.
And that's something we just need to deal with.  On windows, we can't
modify any file that any other program has open.  This can change at any
time, and there's nothing we can do about it, and telling users that if
they have automated backup software, using darcs while that software is
running could corrupt their repository, and it's their own fault because
they shouldn't be doing that, that's no good.  On posix systems, we still
have many filesystems that don't obey posix semantics, and that's something
we have to live with.

With posix filesystem semantics, the *only* atomic operations that
exist--that I'm aware of--operate on a single file.  On windows we don't
even have that... or at least, noone has told me how to do anything
atomically, and we don't.

How would you implement transactional semantics for the working directory,
even in the best of circumstances? I'll admit, this is certainly a subject
I've never actually studied, so there may be some sort of a technique that
I haven't thought of.  We could certainly implement transactional semantics
on the pristine cache by writing a fresh copy for each change, and only
deleting the old one when we're finished, but that sort of approach on the
working directory just won't work.
-- 
David Roundy
Department of Physics
Oregon State University
msg1653 (view) Author: kowey Date: 2007-04-19.20:22:00
Note the 'final' and very minimal version of the patch on darcs-devel.

When I first tried to implement the copy stuff last night, I got the
impression that I had wandered into a thicket of annoying corner cases
and gotchas, but now cannot remember what had annoyed me so or why I
was thinking of the slurpies in the first place.

>      isf <- mDoesFileExist n
>      if isd || isf
>        then ... (cleverly rename n to something unique)

I had been thinking about this.  On the one hand, renaming away the
user's stuff might be slightly rude.  On the other hand, doing so would
help us to apply as much of the patch as possible.  I suppose it's
ok if we just point out that we had done it.

>               do fs <- mGetDirectoryContents
>                  mapM_ ((flip mRename) n) fs

Do we need to filter out '.' and '..', or is that handled magically
somewhere? No big deal either way, I guess

> Note that there isn't a TIO here, this is all done within the tolerant
> monad, which I think does what we want.

If we wrap everything in warning, we're ok because exceptions get caught
somewhere.  We should probably not implement any of the m...  functions
without starting by warning.  Typically, we should probably also not
try to print out own error messages; just throw a new exception and let
the generic handler take care of the formatting.
msg1654 (view) Author: droundy Date: 2007-04-19.20:38:15
On Thu, Apr 19, 2007 at 08:22:10PM +0000, Eric Kow wrote:
> Note the 'final' and very minimal version of the patch on darcs-devel.

Thanks! I may take a look at doing this myself.

> When I first tried to implement the copy stuff last night, I got the
> impression that I had wandered into a thicket of annoying corner cases
> and gotchas, but now cannot remember what had annoyed me so or why I
> was thinking of the slurpies in the first place.
> 
> >      isf <- mDoesFileExist n
> >      if isd || isf
> >        then ... (cleverly rename n to something unique)
> 
> I had been thinking about this.  On the one hand, renaming away the
> user's stuff might be slightly rude.  On the other hand, doing so would
> help us to apply as much of the patch as possible.  I suppose it's
> ok if we just point out that we had done it.

Yeah, I think it's probably the best way to get around a bad
situation--which is also a pretty rare situation.

> >               do fs <- mGetDirectoryContents
> >                  mapM_ ((flip mRename) n) fs
> 
> Do we need to filter out '.' and '..', or is that handled magically
> somewhere? No big deal either way, I guess

Yeah, that's a "safety feature" of the DarcsIO monad, that it doesn't let
you escape from a withCurrentDirectory.

> > Note that there isn't a TIO here, this is all done within the tolerant
> > monad, which I think does what we want.
> 
> If we wrap everything in warning, we're ok because exceptions get caught
> somewhere.  We should probably not implement any of the m...  functions
> without starting by warning.  Typically, we should probably also not
> try to print out own error messages; just throw a new exception and let
> the generic handler take care of the formatting.

Yeah, I suspect the worst that'll happen is that we'll end up reporting
many redundant warnings.  (e.g. couldn't create directory foo/, couldn't
create file foo/bar, couldn't create file foo/bar2...)
-- 
David Roundy
Department of Physics
Oregon State University
msg1655 (view) Author: jamesdsadler Date: 2007-04-20.09:59:11
A crazy idea (that would work on OSes that support FUSE (or something
equivalent).

 - The user creates a filesystem (in a file) in their home directory.
 - They run a 'darcs-daemon' to mount the filesystem as part of the OS's
   real filesystem.  This makes the user-mode filesystem
   indistinguishable from the 'real' filesystem to all processes.
 - When a pull/record/whatever runs, darcs effectively locks the
   filesystem from access by other processes until darcs completes.  It
   can do this, because the darcs-daemon is running the show.
 - Darcs is the only process making changes: guaranteed!  We now have
   the power to enforce transactional semantics including rollback.
 - The actual filesystem can be any freely available filesystem - it
   doesn't need to be written specifically for darcs. All we need to do
   is leverage FUSE to act as the gatekeeper.

Something to think about anyway!

Best Regards,

James.

On Thu, Apr 19, 2007 at 10:15:23AM -0700, David Roundy wrote:
> On Thu, Apr 19, 2007 at 10:08:45AM -0700, David Roundy wrote:
> > On Thu, Apr 19, 2007 at 08:56:57AM -0700, Samuel A. Falvo II wrote:
> > > I'm curious to learn how difficult it would be to implement
> > > transactional semantics to Darcs?  This would prevent any kind of
> > > corruption in the event of any kind of error at all.
> > 
> > Pretty easy (almost) with the new hashed inventory format.  We don't want
> > truly atomic behavior on the pristine cache, as that'd slow darcs down too
> > much (and I don't see any way of avoiding this slowdown, short of
> > implementing our own journalling filesystem, which would definitely be a
> > nice option, but not so easy).
> 
> I should mention that transactional semantics in the working directory is
> impossible to implement with any guarantees, because there's always the
> possibility that halfway through an update someone will change permissions
> of some of the working-directory files, or that disk space will be used up,
> etc.  I don't believe anything bigger than a single file can be updated
> atomically.  I suppose you could create a new directory and rename it over
> an existing one, but you wouldn't want that behavior, since it'd mess up
> any running programs with that directory as their working directory.
> -- 
> David Roundy
> Department of Physics
> Oregon State University
> _______________________________________________
> darcs-devel mailing list
> darcs-devel@darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-devel
msg1658 (view) Author: WorldMaker Date: 2007-04-22.14:42:25
David Roundy wrote:
> I should mention that transactional semantics in the working directory is
> impossible to implement with any guarantees, because there's always the
> possibility that halfway through an update someone will change permissions
> of some of the working-directory files, or that disk space will be used up,
> etc.  I don't believe anything bigger than a single file can be updated
> atomically.  I suppose you could create a new directory and rename it over
> an existing one, but you wouldn't want that behavior, since it'd mess up
> any running programs with that directory as their working directory.

In some cases you can use local Operating System components to deal with 
that.  For instance, NTFS on Windows Vista supports transactions. 
Having to optimize for every potential file system and operating system 
could be a hassle.

Interestingly enough, Vista betas had a command-line tool named 
"transaction" that allowed you to start/stop/rollback transactions 
yourself, but they apparently removed it in the final version.  If it 
were still around you could manually do:

transaction /start
darcs pull -a
...  Some error occurs ...
transaction /rollback
msg1659 (view) Author: droundy Date: 2007-04-22.14:46:11
On Sat, Apr 21, 2007 at 05:56:41PM -0400, Max Battcher wrote:
> In some cases you can use local Operating System components to deal with
> that.  For instance, NTFS on Windows Vista supports transactions.  Having
> to optimize for every potential file system and operating system could be
> a hassle.

This would definitely be an option if someone were to write a library to
abstract over transactional filesystems.  Reiser4 also has transactions.
But it doesn't seem worth putting much effort into transactions when such a
small minority of users will be affected.  Although, if someone wanted to
create such a module, it would definitely be appreciated... although we'd
want to be sure not to use it as a bandaid over bugs.
-- 
David Roundy
http://www.darcs.net
msg1806 (view) Author: kowey Date: 2007-07-12.06:44:06
Resolved in 1.0.9 with tolerant application of patches (I hope!)
Please continue this discussion on darcs-devel if needed or re-open if we've
still got this source of grief
History
Date User Action Args
2007-04-16 13:34:51simonpjcreate
2007-04-16 15:14:58droundysetstatus: unread -> unknown
nosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj
messages: + msg1591
2007-04-16 15:45:33droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj
title: Darcs grief: Issue 274 -> permissions error in working directory corrupts working directory
2007-04-16 16:39:12grant.husbandssetnosy: + grant.husbands
messages: + msg1593
title: permissions error in working directory corrupts working directory -> Darcs grief: Issue 274
2007-04-16 18:07:02koweysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1595
2007-04-16 18:20:58droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1596
2007-04-16 18:23:58koweysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1597
2007-04-16 18:41:17droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1598
2007-04-16 18:58:06koweysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1599
2007-04-16 20:11:31koweysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1600
2007-04-16 20:16:45koweysetfiles: + DRAFT-tolerant-IO.dpatch
nosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1601
2007-04-16 22:38:40droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1606
2007-04-16 22:48:57droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1607
2007-04-17 09:05:26simonpjsetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, grant.husbands
messages: + msg1609
2007-04-17 10:03:35darcsdevelsetnosy: + darcsdevel
messages: + msg1610
2007-04-17 17:13:54simonsetnosy: + simon
2007-04-17 20:42:32koweysetfiles: + DRAFT-tolerant-IO2.dpatch
nosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel
messages: + msg1618
2007-04-17 21:27:34droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel
messages: + msg1619
2007-04-17 21:31:17droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel
messages: + msg1620
2007-04-18 11:02:08simonpjsetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel
messages: + msg1627
2007-04-18 11:18:39koweysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel
messages: + msg1628
2007-04-18 11:51:48darcsdevelsetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel
messages: + msg1629
2007-04-18 14:14:26droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel
messages: + msg1630
2007-04-18 14:17:50droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel
messages: + msg1631
2007-04-18 14:19:06quicksetnosy: + quick
messages: + msg1632
2007-04-18 14:53:05simonpjsetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick
messages: + msg1634
2007-04-18 19:51:09quicksetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick
messages: + msg1639
2007-04-18 20:10:52koweysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick
messages: + msg1641
2007-04-18 20:21:25droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick
messages: + msg1642
2007-04-18 20:46:30droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick
messages: + msg1643
2007-04-19 16:19:44sam.falvosetnosy: + sam.falvo
messages: + msg1644
2007-04-19 17:09:48droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick, sam.falvo
messages: + msg1645
2007-04-19 17:16:23droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick, sam.falvo
messages: + msg1646
2007-04-19 17:24:58sam.falvosetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick, sam.falvo
messages: + msg1647
2007-04-19 17:38:35droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick, sam.falvo
messages: + msg1648
2007-04-19 20:22:10koweysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick, sam.falvo
messages: + msg1653
2007-04-19 20:38:27droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, simon, grant.husbands, darcsdevel, quick, sam.falvo
messages: + msg1654
2007-04-19 21:10:03simonsetnosy: - simon
2007-04-20 09:59:21jamesdsadlersetnosy: + jamesdsadler
messages: + msg1655
2007-04-22 14:42:30admin1setnosy: + admin1
messages: + msg1658
2007-04-22 14:46:13droundysetnosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, jamesdsadler, grant.husbands, darcsdevel, quick, sam.falvo, admin1
messages: + msg1659
2007-07-12 06:44:08koweysetstatus: unknown -> resolved
nosy: droundy, tommy, beschmi, kowey, igloo, simonmar, simonpj, jamesdsadler, grant.husbands, darcsdevel, quick, sam.falvo, admin1
messages: + msg1806
2008-01-08 01:31:09markstossettitle: Darcs grief: Issue 274 -> Repository corruption issue
2009-08-06 17:34:27adminsetnosy: + markstos, jast, Serware, dmitry.kurochkin, darcs-devel, zooko, dagit, mornfall, simon, thorkilnaur, - droundy, igloo, simonmar, simonpj, jamesdsadler, grant.husbands, darcsdevel, quick, sam.falvo, admin1
2009-08-06 20:31:45adminsetnosy: - beschmi
2009-08-10 22:02:57adminsetnosy: + jamesdsadler, grant.husbands, igloo, simonmar, darcsdevel, simonpj, quick, sam.falvo, admin1, - markstos, darcs-devel, zooko, jast, dagit, Serware, mornfall
2009-08-25 17:19:22adminsetnosy: + darcs-devel, - igloo
2009-08-25 17:49:02adminsetnosy: - simon
2009-08-27 13:52:27adminsetnosy: tommy, kowey, darcs-devel, simonmar, simonpj, jamesdsadler, grant.husbands, darcsdevel, quick, sam.falvo, admin1, thorkilnaur, dmitry.kurochkin
2009-10-23 22:38:04adminsetnosy: + marlowsd, - simonmar
2009-10-23 23:36:35adminsetnosy: + simonmar, - marlowsd
2009-10-24 00:33:53adminsetnosy: + granth, - grant.husbands
2009-10-24 08:40:35adminsetnosy: - darcsdevel
2009-10-24 09:10:01adminsetnosy: + WorldMaker, - admin1