Issue 2431 darcs leaves empty files in repo

Title darcs leaves empty files in repo
Priority Status resolved
Milestone 2.10.0 Resolved in 2.10.0
Superseder Nosy List bf
Assigned To

Created on 2015-01-31.20:22:34 by bf, last changed 2015-02-26.19:28:18 by noreply.

msg17970 (view) Author: bf Date: 2015-01-31.20:22:33
ben@sarun[1]: .../darcs/screened > darcs whatsnew -l                   
a ./darcs2426
a ./darcs2434
a ./darcs2481
a ./darcs2521

I haven't yet investigated which command is responsible but since a
while I always have these files lying around in my clone of the darcs repo.

> darcs --version
2.9.9 (+ 226 patches)
msg18196 (view) Author: bf Date: 2015-02-20.19:28:34
It seems these are created by 'darcs send'. They appear even if nothing
is actually sent.
msg18198 (view) Author: bf Date: 2015-02-20.22:53:10
Something is definitely not right here. I added a bit of debugging to
Darcs.Repository.Lock with the result that 'darcs send --dryrun' now

Creating patch to "http://darcs.net/screened"...
withTemp: created ./darcs5612
withTemp: created ./darcs5612
withTemp: created ./darcs5612
withTemp: created ./darcs5612
withTemp: created ./darcs5613
Patch bundle would  be sent to: patches@darcs.net
[...etc etc...]

Note that the file darcs5612 gets created 4 times. And indeed, it is
this file that is left:

drwxrwxr-x 7 ben ben   4096 Feb 20 20:28 _darcs
-rw------- 1 ben ben      0 Feb 20 23:45 darcs5612
-rw-rw-r-- 1 ben ben  26755 Feb 19 02:20 darcs.cabal

Does this mean openBinaryTempFile is broken? Why is withTemp even called
so often? Or do we perhaps call withTemp recursively?
msg18199 (view) Author: bf Date: 2015-02-20.23:48:36
We are using openBinaryTempFile in an unsafe manner here: the file gets
closed immediately inside withTemp, then, in the user specified closure
we reopen it and close it again (by calling copyFileOrUrl on the
filename). I guess openBinaryTempFile operates under the assumption that
after the file handle it returns is closed the caller is done with it
and the name can be re-used. I can reproduce this with a simple test
program where I get the same file name 20 times in a row.

Fixing this will require some refactoring.
msg18201 (view) Author: bf Date: 2015-02-21.20:17:04
The fact that openBinaryTempFile may return the same file name on
repeated calls was a red herring.

The real problem is the use of the CatchT transformer on top of IO. Note
the documentation says:

  "Do not mix CatchT with IO. Choose one or the other for the bottom
   of your transformer stack!"

and later:

  "This should /never/ be used in combination with IO."

I won't go into the details of how exactly using CatchT broke withTemp
(which actually used bracket to make sure the temporary file gets
deleted). On a side note, I have myself to blame for this: I failed to
notice this problem when I reviewed the recent changes that improved the
error messages that Darcs issues when it fails to identify a remote

I will send a patch that fixes this issue by rolling back most of the
generalized monad stuff and especially all uses of CatchT.
msg18233 (view) Author: noreply Date: 2015-02-26.19:28:17
The following patch sent by Ben Franksen <benjamin.franksen@helmholtz-berlin.de> updated issue issue2431 with
status=resolved;resolvedin=2.10.0 HEAD

* resolve issue2431: rollback Control.Monad.Catch generalizations 
Ignore-this: f60a8b6b7b70d9d17b21289e89120ce6

I even went so far as to remove the exceptions package from the cabal file
to make sure this stuff doesn't creep back in. For details of why this use
of Control.Monad.Catch is unsafe, please refer to the documentation of this
Date User Action Args
2015-01-31 20:22:34bfcreate
2015-01-31 20:23:21bfsetmilestone: 2.10.0
2015-02-20 19:28:35bfsetmessages: + msg18196
2015-02-20 22:53:11bfsetmessages: + msg18198
2015-02-20 23:48:38bfsetmessages: + msg18199
2015-02-21 20:17:07bfsetmessages: + msg18201
2015-02-26 19:28:18noreplysetstatus: unknown -> resolved
messages: + msg18233
resolvedin: 2.10.0