Hi list,
Recently I had some headache with darcs not reporting the correct error
when I had set DARCS_APPLY_HTTP wrong, so I decided to do something about
the error reporting. I have attached a patch.
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.
/ Magnus Jonsson
New patches:
[Made exec_ throw an exception when a command failed to execute.
Magnus Jonsson <[EMAIL PROTECTED]>**20060924210338
Before, exec_ just returned an exit code of 1 if it failed to run a
program. The user would get strange error messages that didn't point
to the real problem. For example if the user had set DARCS_APPLY_HTTP
to some executable that didn't exist, darcs wouldn't tell the user
the correct reason for the failure. This patch adds detection of execvp
failure by employing a tradition unix solution involving pipes. When a
failure occurs, a new ExecFailure exception is thrown and caught at the
toplevel where an error message is printed.
] {
hunk ./Exec.lhs 18
-{-# OPTIONS -fffi #-}
-module Exec ( exec, exec_, exec_interactive
+{-# OPTIONS -fffi -fglasgow-exts #-}
+module Exec ( exec, exec_, exec_interactive, ExecException (ExecFailure)
hunk ./Exec.lhs 26
+import Control.Exception (throwDyn)
+import Data.Typeable (Typeable)
+
hunk ./Exec.lhs 33
-import Monad ( liftM )
hunk ./Exec.lhs 40
+-- This exception is thrown if an exec can't do it's job,
+-- for example because fork/pipe/execvp failed or because there was
+-- an error setting up the file redirecs.
+data ExecException = ExecFailure String [String] (Maybe FilePath) (Maybe
FilePath) (Maybe FilePath)
+ deriving (Show,Typeable)
+
hunk ./Exec.lhs 54
+-- currently on win32 there's no way to discern whether the
+-- execution of c failed or whether c returned an error code,
+-- so we assume that the command was run.
hunk ./Exec.lhs 75
-exec_ c args minp mout merr = do
- fval <- c_fork
- let -- set up stdin redirection if needed
- withStdin job =
- case minp of
- Nothing -> job
- Just inp -> withCString inp $ \in_c -> do
- fdin <- open_read in_c
- c_dup2 fdin 0
- job
- -- set up stdout/stderr redirection if needed
- withStdout job =
- case mout of
- -- no stdout redirection
- Nothing ->
- case merr of
- Nothing -> job
- Just e -> withCString e $ \err_c -> do
- fderr <- open_write err_c
- c_dup2 fderr 2
- job
- -- stdout redirection
- Just out -> withCString out $ \out_c -> do
- fdout <- open_write out_c
- c_dup2 fdout 1
- case merr of
- Nothing -> do c_dup2 fdout 2 -- stderr to stdout
- job
- Just e -> withCString e $ \err_c -> do
- fderr <- open_write err_c
- c_dup2 fderr 2
- job
- -- fork and go
- case fval of
- -1 -> return $ ExitFailure $ 1
- 0 -> withStdin $ withStdout $
- withCString c $ \c_c ->
- withCStrings (c:args) $ \c_args -> do
- -- execvp only returns if there is an error:
- ExitFailure `liftM` execvp_no_vtalarm c_c c_args
- pid -> do ecode <- smart_wait pid
- if ecode == 0 then return ExitSuccess
- else return $ ExitFailure ecode
+exec_ cmd args minp mout merr = do
+ pid <- withCString cmd $ \ c_cmd ->
+ withCStrings (cmd:args) $ \ c_args ->
+ withCString (maybe "" id minp) $ \ c_minp ->
+ withCString (maybe "" id mout) $ \ c_mout ->
+ withCString (maybe "" id merr) $ \ c_merr ->
+ exec_extern c_cmd c_args c_minp c_mout c_merr
+ if pid `elem` [(-1),(-2),(-3)]
+ then throwDyn (ExecFailure cmd args minp mout merr)
+ else do ecode <- smart_wait pid
+ case ecode of
+ 0 -> return ExitSuccess
+ _ -> return (ExitFailure ecode)
hunk ./Exec.lhs 89
-foreign import ccall unsafe "static unistd.h dup2" c_dup2
- :: CInt -> CInt -> IO CInt
hunk ./Exec.lhs 91
-foreign import ccall unsafe "static compat.h open_read" open_read
- :: CString -> IO CInt
-foreign import ccall unsafe "static compat.h open_write" open_write
- :: CString -> IO CInt
-foreign import ccall unsafe "static unistd.h fork" c_fork
- :: IO Int
-foreign import ccall unsafe "static compat.h execvp_no_vtalarm"
execvp_no_vtalarm
- :: CString -> Ptr CString -> IO Int
+foreign import ccall unsafe "static compat.h exec_extern" exec_extern
+ :: CString -> Ptr CString -> CString -> CString -> CString -> IO Int
hunk ./Exec.lhs 110
+
hunk ./c_compat.c 20
-int open_read(const char *fname) {
+static int open_read(const char *fname) {
hunk ./c_compat.c 24
-int open_write(const char *fname) {
+static int open_write(const char *fname) {
hunk ./c_compat.c 60
-int execvp_no_vtalarm(const char *file, char *const argv[]) {
+static int execvp_no_vtalarm(const char *file, char *const argv[]) {
hunk ./c_compat.c 75
+
+static int is_some(const char* string) {
+ return string != NULL && string[0] != '\0';
+}
+
+/*
+ * exec_extern(file,argv) tries to execute
+ * returns -1 if fork() failed
+ * returns -2 if pipe() failed
+ * returns -3 if execvp_no_vtalarm() failed
+ * returns the pid of the child otherwise
+ */
+int exec_extern(const char* file, char *const argv[], const char* minp, const
char* mout, const char* merr) {
+ int pipefd[2] = { 0, 0 };
+ int fval = 0;
+ char buf[1] = { 0 };
+
+ /* this pipe is used to signal from the
+ child process in case of an error */
+ if(pipe(pipefd) != 0)
+ return -2;
+
+ /* pipefd[0] is now a fd for reading from the pipe
+ pipefd[1] is now a fd for writing to the pipe */
+
+ fval = fork();
+ switch(fval) {
+ case -1:
+ {
+ /* the fork failed */
+ close(pipefd[0]);
+ close(pipefd[1]);
+ return -1;
+ }
+ case 0:
+ {
+ /* we're inside the child process */
+ close(pipefd[0]);
+
+ /* redirect inputs and outputs */
+
+ if(is_some(minp)) {
+ int fdinp = open_read(minp);
+ if(fdinp == -1)
+ goto error;
+ dup2(fdinp,0);
+ }
+ if(is_some(mout)) {
+ /* stdout redirection */
+ int fdout = open_write(mout);
+ if (fdout == -1)
+ goto error;
+ dup2(fdout,1);
+ if(is_some(merr)) {
+ int fderr = open_write(merr);
+ if(fderr == -1)
+ goto error;
+ dup2(fderr,2);
+ }
+ else {
+ /* stderr to stdout */
+ dup2(fdout,2);
+ }
+ }
+ else {
+ /* no stdout redirection */
+ if(is_some(merr)) {
+ int fderr = open_write(merr);
+ if (fderr == -1)
+ goto error;
+ dup2(open_write(merr),2);
+ }
+ }
+
+ /* execvp only returns if there is an error */
+ execvp_no_vtalarm(file,argv);
+
+ error:
+ /* execvp failed or one of the redirects failed.*/
+
+ /* signal to the parent process that we got a problem */
+ write(pipefd[1],buf,1);
+
+ close(pipefd[1]);
+
+ exit(127);
+ }
+ default:
+ {
+ int const pid = fval;
+ /* we're inside the parent process and fval is the pid of the child */
+ ssize_t len=0;
+
+ close(pipefd[1]);
+
+ do {
+ len = read(pipefd[0],buf,1);
+ } while(len == -1 && errno == EINTR);
+
+ close(pipefd[0]);
+
+ if(len == 0) {
+ /* EOF. The child process closed the pipe without writing
+ anything to it, indicating that everything went fine. */
+ return pid;
+ }
+ else {
+ /* There was an error with the redirects or the execvp call. */
+ smart_wait(pid);
+ return -3;
+ }
+ }
+ };
+}
hunk ./compat.h 6
-int open_read(const char *fname);
-int open_write(const char *fname);
hunk ./compat.h 8
-int execvp_no_vtalarm(const char *file, char *const argv[]);
+int exec_extern(const char *file, char *const argv[],
+ const char *minp,
+ const char *mout,
+ const char *merr);
hunk ./darcs.lhs 34
-import Control.Exception ( Exception( AssertionFailed ), handleJust, )
+import Control.Exception ( Exception( AssertionFailed ), handleJust, catchDyn )
+import Exec ( ExecException ( ExecFailure ) )
hunk ./darcs.lhs 658
-main = with_atexit $ withSignalsHandled $ handleJust assertions bug $ do
+main = with_atexit $ withSignalsHandled $ handleJust assertions bug $
+ flip catchDyn (\ (ExecFailure cmd args _ _ _) ->
+ bug $ "Failed to execute external command: "
+ ++ unwords (cmd:args)
+ ++ "\nHere are some possible reasons:\n"
+ ++ "- The command doesn't exist.\n"
+ ++ "- The file permissions for the command
are wrong.\n"
+ ++ "- The command is not in your path.\n")
+ $ do
}
Context:
[whatsnew --look-for-adds doesn't read unadded files (fix for issue79)
Jason Dagit <[EMAIL PROTECTED]>**20060910193803
The default mode for whatsnew --look-for-adds is summary mode. In summary
mode full patches are not needed. This fix changes whatsnew
--look-for-adds to stop computing the full patch for a file when the
file is not managed by darcs.
]
[Correct canonical email for Kirill Smelkov
Kirill Smelkov <[EMAIL PROTECTED]>**20060912080004]
[Be explicit about timezone handling (issue220); assume local by default.
Eric Kow <[EMAIL PROTECTED]>**20060812102034
Except for the local timezone in the user interface, this patch is not
expected to change darcs's behaviour. It merely makes current practice
explicit:
- Assume local timezone when parsing date strings from the user
interface (previous behaviour was assuming UTC).
- Assume UTC timezone when parsing date strings from PatchInfo.
Newer patch date strings do *not* specify the timezone, so it
would be prudent to treat these as UTC.
- Disregard timezone information altogether when reading patch
dates (issue220). Note that this bug was not caused by assuming local
timezone, because legacy patch date strings explicitly tell you what
the timezone to use. The bug was caused by a patch that fixed
issue173 by using timezone information correctly. To preserve
backwards-compatability, we deliberatly replicate the incorrect
behaviour of overriding the timezone with UTC.
(PatchInfo.make_filename)
]
[Account for timezone offset in cleanDate (Fixes issue173).
Eric Kow <[EMAIL PROTECTED]>**20060610193049
]
[move test for tabs from makefile to haskell_policy test
Tommy Pettersson <[EMAIL PROTECTED]>**20060730122348]
[add test for haskell policy
Tommy Pettersson <[EMAIL PROTECTED]>**20060730121404]
[ratify some uses of readFile and hGetContents
Tommy Pettersson <[EMAIL PROTECTED]>**20060730121158]
[Remove direct dependency to mapi32.dll; Improve MAPI compatibility.
Esa Ilari Vuokko <[EMAIL PROTECTED]>**20051130000915]
[Canonize Kirill Smelkov and Anders Hockersten.
Eric Kow <[EMAIL PROTECTED]>**20060910052541]
[Correct 'one one' in web page.
Eric Kow <[EMAIL PROTECTED]>**20060908191241]
[Fix merge conflicts.
Juliusz Chroboczek <[EMAIL PROTECTED]>**20060906191317]
[fix bug in pristine handling when dealing with multiple patches.
David Roundy <[EMAIL PROTECTED]>**20060731111404]
[fix ordering of operations to call pull_first_middles properly.
David Roundy <[EMAIL PROTECTED]>**20060730111409]
[make amend-record.pl test a bit pickier.
David Roundy <[EMAIL PROTECTED]>**20060730103854]
[simplify code a tad in get.
David Roundy <[EMAIL PROTECTED]>**20060726121737]
[fix bug in refactoring of get.
David Roundy <[EMAIL PROTECTED]>**20060726121655]
[refactor Population.
David Roundy <[EMAIL PROTECTED]>**20060716034837]
[add TODO for refactoring get_markedup_file.
David Roundy <[EMAIL PROTECTED]>**20060716034339]
[partial refactoring in annotate.
David Roundy <[EMAIL PROTECTED]>**20060716034319]
[don't use DarcsRepo in list_authors.
David Roundy <[EMAIL PROTECTED]>**20060716033450]
[I've now eliminated need to export DarcsRepo.write_patch.
David Roundy <[EMAIL PROTECTED]>**20060716033109]
[partially refactor Optimize.
David Roundy <[EMAIL PROTECTED]>**20060716032934]
[partial refactoring of Get.
David Roundy <[EMAIL PROTECTED]>**20060716031605]
[refactor amend-record.
David Roundy <[EMAIL PROTECTED]>**20060716021003]
[add TODO to refactor unrevert handling.
David Roundy <[EMAIL PROTECTED]>**20060716020247]
[refactor Unrecord, adding tentativelyRemovePatches.
David Roundy <[EMAIL PROTECTED]>**20060716015150]
[refactor tag.
David Roundy <[EMAIL PROTECTED]>**20060716011853]
[refactor Repository to allow truly atomic updates.
David Roundy <[EMAIL PROTECTED]>**20060716011245]
[Do not redirect to or from /dev/null when calling ssh.
Eric Kow <[EMAIL PROTECTED]>**20060903214831
Redirection of stdin and stdout breaks putty, which uses these to
interact with the user. Quiet mode, and redirecting stderr are good
enough for making ssh silent.
]
[Exec improvements : Windows redirection, and more redirection control.
Eric Kow <[EMAIL PROTECTED]>**20060707054134
- Implement ability to redirect to /dev/null under Windows
(eivuokko on #darcs points out that it is NUL under Windows)
- Add exec_ function, which does the same thing as exec,
but allows redirection on stderr, and also allows us
to NOT redirect stdin/stderr
]
[Ignore .git if _darcs found.
Juliusz Chroboczek <[EMAIL PROTECTED]>**20060831231933]
[overhaul the darcs.net front page.
Mark Stosberg <[EMAIL PROTECTED]>**20060820191415
The themes to this change are:
- Focus on the key benefits of darcs:
Distributed. Interactive. Smart.
- Recognize that the wiki is the central resource,
and remove some information that is duplicated here
and reference the wik instead.
I can post a demo of this HTML for easy comparison if you'd like.
Mark
]
[Reimplement --disable-ssh-cm flag (issue239).
Eric Kow <[EMAIL PROTECTED]>**20060812134856
My patch to "Only launch SSH control master on demand" accidentally
removed the ability to disable use of SSH ControlMaster. Also, the
way it was implemented is not compatible with launching on demand.
This implementation relies on a notion of global variables using
unsafe IORefs.
]
[Compile Global.lhs in place of AtExit.lhs.
Eric Kow <[EMAIL PROTECTED]>**20060812121943]
[Rename AtExit module to Global.
Eric Kow <[EMAIL PROTECTED]>**20060812121925
The goal is to capture some broad "global" notions like exit handlers
and global variables. Note the GPL header thrown in for good measure.
]
[Raise exception if unable to open logfile (issue142).
Zachary P. Landau <[EMAIL PROTECTED]>**20060810034035]
[Make the pull 'permission test' work when run as root
Jon Olsson <[EMAIL PROTECTED]>**20060831193834]
[TAG darcs-unstable-20060831
Juliusz Chroboczek <[EMAIL PROTECTED]>**20060831191554]
Patch bundle hash:
67e240b70a618a2147e78583c3f356622018ef23
_______________________________________________
darcs-devel mailing list
[email protected]
http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
