On Fri, 29 Sep 2006, Eric Y. Kow wrote:
Hi,
On Sun, Sep 24, 2006 at 22:34:54 -0400, Magnus Jonsson wrote:
What this patch does is, it introduces a new exception ExecFailed that
gets thrown if exec_ fails to at all execute the file. Because of the
design of unix, detectin this condition requires some hackery involving
pipes (njs suggested this to me on irc). I also added a handler at
toplevel main to report the ExecFailed exceptions to the user. Other than
this, exec_ and friends behave exactly as before.
Let me know what you think. I'm ready to do some revision if you guys
think it's needed.
The exec stuff has changed considerably (in the current batch of
accepted patches). Would you please rewrite your patch in light of the
new changes?
Yes, I'll have a look at the new changes and add my changes if needed.
I haven't had a deep look. Some questions:
1) It adds some new exit codes for internal usage, paraphrasing the comments,
-1 for fork() failure
-2 for pipe(),
-3 execvp_no_vtalarm()
Dut does this mean we're counting on the programs themselves never
exiting with these codes?
exec_extern returns process id of the newly launched child process, or one
of those error codes if it fails. There used to be no/little error
checking here.
You then use smart_wait_pid to get the return code, just like before. This
is the same as before.
2) These exit codes don't actually get returned by darcs, right? You
catch them and then just throw an ExitFailure if it's one of the
expected codes
ExecFailure only represents failure to start an external program. It
doesn't carry with it any exit code, because no program was run.
3) I'm a bit nervous about catching ExitFailure at main - it seems like
this is something we would want to bubble up. How do you know this
doesn't misreport ExitFailures thrown by darcs itself?
It is better than the current status I think - right now these failures
pass unseen and if here is a failure, the desired side effects never take
place. This way darcs realizes that there's a problem and bails out. Darcs
itself shouldn't throw any ExecFailure. Only Exec.hs should throw
ExecFailure.
4) Any reason the bulk of the code is written in C and not Haskell?
I started out writing it in Haskell, but then I realized I would need a
lot of FFI imports so I decided it would be more straight-forward to do it
directly in C and expose a minimal interface through the FFI. If you think
the C code is messy, that's not because it's written in C, but because of
all the hoops you need to go through to create a process and error-check
everything.
Best,
Magnus Jonsson
_______________________________________________
darcs-devel mailing list
[email protected]
http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
