[darcs-devel] [patch1932] fix doc comment for filterDirContents (and 13 more)

[Ben Franksen]

Ben Franksen <ben.franksen at online.de> added the comment:

>>   * fix build with -fcurl
> 
> Fine. Now we have the new HTTP downloading could we just drop -fcurl? I
> can't remember if we discussed that before.

This came up because I had a situation where I wanted to compare with a
darcs built against curl. I cannot remember the details, but I had a
serious performance regression when interacting with remote
repositories, related to the asynchronous download changes. As long as
it doesn't hurt us seriously I would like us to keep it.

>>   * fallback to simplePrinters if fancyPrinters throws exception
> 
> I really don't like the catchall. Did you have some specific error
> condition in mind?

It failed for me in a specific situation, but unfortunately I cannot
remember precisely which exception I got. It was related to the problem
with the race between cleanup of temporary directories and
asynchronously downloading files. I believe the problem was that this
could lead to exceptions thrown from an exception handler.

My justification for using catchall here was that we don't loose
anything significant, just coloring. I don't think the exeption in
question was an IO error because I would have used catchIOError in that
case.

>>    * do atexit actions in the opposite order of registration
> 
> I don't think this is correct. atExit puts the new action at the head of
> the list. So if you call atexit a; atexit b; atexit c, then the list
> will be c;b;a.

Oh my, thanks for catching this one. This change was made in haste when
I tried to fix the race conditions mentioned below. For whatever reasons
it improved the situation, so I thought it must be correct. But I wasn't
satisfied with that and wanted a more thorough fix. Afterwards I just
thought "why not?". Will rollback.

>>   * make working with temporary directories more robust
>>   
>>   Cleaning up after withTempDir or withDelayedDir fails if an asynchronous
>>   thread or an interleaved IO action continues to create files in this
>>   directory. We rather want those operations to fail, so we first rename the
>>   directory and then delete that.
> 
> Sounds good - as long as the windows tests don't throw up any problems!

Ahem, yes, we should check that. I keep forgetting that there are
problems with renaming directories in Windows.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1932>
__________________________________