summaryrefslogtreecommitdiff
path: root/run-command.c
AgeCommit message (Collapse)AuthorFilesLines
2013-03-21run-command: always set failed_errno in start_commandLibravatar Jeff King1-2/+3
When we fail to fork, we set the failed_errno variable to the value of errno so it is not clobbered by later syscalls. However, we do so in a conditional, and it is hard to see later under what conditions the variable has a valid value. Instead of setting it only when fork fails, let's just always set it after forking. This is more obvious for human readers (as we are no longer setting it as a side effect of a strerror call), and it is more obvious to gcc, which no longer generates a spurious -Wuninitialized warning. It also happens to match what the WIN32 half of the #ifdef does. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-07Merge branch 'sb/run-command-fd-error-reporting'Libravatar Junio C Hamano1-2/+6
* sb/run-command-fd-error-reporting: run-command: be more informative about what failed
2013-02-01run-command: be more informative about what failedLibravatar Stephen Boyd1-2/+6
While debugging an error with verify_signed_buffer() the error messages from run-command weren't very useful: error: cannot create pipe for gpg: Too many open files error: could not run gpg. because they didn't indicate *which* pipe couldn't be created. Print which pipe failed to be created in the error message so we can more easily debug similar problems in the future. For example, the above error now prints: error: cannot create standard error pipe for gpg: Too many open files error: could not run gpg. Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-14hooks: Add function to check if a hook existsLibravatar Aaron Schrab1-2/+13
Create find_hook() function to determine if a given hook exists and is executable. If it is, the path to the script will be returned, otherwise NULL is returned. This encapsulates the tests that are used to check for the existence of a hook in one place, making it easier to modify those checks if that is found to be necessary. This also makes it simple for places that can use a hook to check if a hook exists before doing, possibly lengthy, setup work which would be pointless if no such hook is present. The returned value is left as a static value from get_pathname() rather than a duplicate because it is anticipated that the return value will either be used as a boolean, immediately added to an argv_array list which would result in it being duplicated at that point, or used to actually run the command without much intervening work. Callers which need to hold onto the returned value for a longer time are expected to duplicate the return value themselves. Signed-off-by: Aaron Schrab <aaron@schrab.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-06run-command: encode signal death as a positive integerLibravatar Jeff King1-1/+1
When a sub-command dies due to a signal, we encode the signal number into the numeric exit status as "signal - 128". This is easy to identify (versus a regular positive error code), and when cast to an unsigned integer (e.g., by feeding it to exit), matches what a POSIX shell would return when reporting a signal death in $? or through its own exit code. So we have a negative value inside the code, but once it passes across an exit() barrier, it looks positive (and any code we receive from a sub-shell will have the positive form). E.g., death by SIGPIPE (signal 13) will look like -115 to us in inside git, but will end up as 141 when we call exit() with it. And a program killed by SIGPIPE but run via the shell will come to us with an exit code of 141. Unfortunately, this means that when the "use_shell" option is set, we need to be on the lookout for _both_ forms. We might or might not have actually invoked the shell (because we optimize out some useless shell calls). If we didn't invoke the shell, we will will see the sub-process's signal death directly, and run-command converts it into a negative value. But if we did invoke the shell, we will see the shell's 128+signal exit status. To be thorough, we would need to check both, or cast the value to an unsigned char (after checking that it is not -1, which is a magic error value). Fortunately, most callsites do not care at all whether the exit was from a code or from a signal; they merely check for a non-zero status, and sometimes propagate the error via exit(). But for the callers that do care, we can make life slightly easier by just using the consistent positive form. This actually fixes two minor bugs: 1. In launch_editor, we check whether the editor died from SIGINT or SIGQUIT. But we checked only the negative form, meaning that we would fail to notice a signal death exit code which was propagated through the shell. 2. In handle_alias, we assume that a negative return value from run_command means that errno tells us something interesting (like a fork failure, or ENOENT). Otherwise, we simply propagate the exit code. Negative signal death codes confuse us, and we print a useless "unable to run alias 'foo': Success" message. By encoding signal deaths using the positive form, the existing code just propagates it as it would a normal non-zero exit code. The downside is that callers of run_command can no longer differentiate between a signal received directly by the sub-process, and one propagated. However, no caller currently cares, and since we already optimize out some calls to the shell under the hood, that distinction is not something that should be relied upon by callers. Fix the same logic in t/test-terminal.perl for consistency [jc: raised by Jonathan in the discussion]. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Johannes Sixt <j6t@kdbg.org> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-05fix compilation with NO_PTHREADSLibravatar Jeff King1-1/+1
Commit 1327452 cleaned up an unused parameter from wait_or_whine, but forgot to update a caller that is inside "#ifdef NO_PTHREADS". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-12-02run-command: do not warn about child death from terminalLibravatar Jeff King1-1/+2
SIGINT and SIGQUIT are not generally interesting signals to the user, since they are typically caused by them hitting "^C" or otherwise telling their terminal to send the signal. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-12-02run-command: drop silent_exec_failure arg from wait_or_whineLibravatar Jeff King1-4/+3
We do not actually use this parameter; instead we complain from the child itself (for fork/exec) or from start_command (if we are using spawn on Windows). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-10-25Merge branch 'jk/no-more-pre-exec-callback'Libravatar Jeff King1-10/+0
Removes a workaround for buggy version of less older than version 406. * jk/no-more-pre-exec-callback: pager: drop "wait for output to run less" hack
2012-09-20Merge branch 'dg/run-command-child-cleanup' into maintLibravatar Junio C Hamano1-6/+7
* dg/run-command-child-cleanup: run-command.c: fix broken list iteration in clear_child_for_cleanup
2012-09-14Merge branch 'dg/run-command-child-cleanup'Libravatar Junio C Hamano1-6/+7
The code to wait for subprocess and remove it from our internal queue wasn't quite right. * dg/run-command-child-cleanup: run-command.c: fix broken list iteration in clear_child_for_cleanup
2012-09-11Merge branch 'jc/maint-sane-execvp-notdir' into maint-1.7.11Libravatar Junio C Hamano1-0/+2
* jc/maint-sane-execvp-notdir: sane_execvp(): ignore non-directory on $PATH
2012-09-11run-command.c: fix broken list iteration in clear_child_for_cleanupLibravatar David Gould1-6/+7
Iterate through children_to_clean using 'next' fields but with an extra level of indirection. This allows us to update the chain when we remove a child and saves us managing several variables around the loop mechanism. Signed-off-by: David Gould <david@optimisefitness.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-09-03Merge branch 'jc/maint-sane-execvp-notdir'Libravatar Junio C Hamano1-0/+2
"git foo" errored out with "Not a directory" when the user had a non directory on $PATH, and worse yet it masked an alias "foo" to run. * jc/maint-sane-execvp-notdir: sane_execvp(): ignore non-directory on $PATH
2012-07-31sane_execvp(): ignore non-directory on $PATHLibravatar Junio C Hamano1-0/+2
When you have a non-directory on your PATH, a funny thing happens: $ PATH=$PATH:/bin/sh git foo fatal: cannot exec 'git-foo': Not a directory? Worse yet, as real commands always take precedence over aliases, this behaviour interacts rather badly with them: $ PATH=$PATH:/bin/sh git -c alias.foo=show git foo -s fatal: cannot exec 'git-foo': Not a directory? This is because an ENOTDIR error from the underlying execvp(2) is reported back to the caller of our sane_execvp() wrapper as-is. Translating it to ENOENT, just like the case where we _might_ have the command in an unreadable directory, fixes it. Without an alias, we would get git: 'foo' is not a git command. See 'git --help'. and we use the 'foo' alias when it is available, of course. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-05pager: drop "wait for output to run less" hackLibravatar Jeff King1-10/+0
Commit 35ce862 (pager: Work around window resizing bug in 'less', 2007-01-24) causes git's pager sub-process to wait to receive input after forking but before exec-ing the pager. To handle this, run-command had to grow a "pre-exec callback" feature. Unfortunately, this feature does not work at all on Windows (where we do not fork), and interacts poorly with run-command's parent notification system. Its use should be discouraged. The bug in less was fixed in version 406, which was released in June 2007. It is probably safe at this point to remove our workaround. That lets us rip out the preexec_cb feature entirely. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-20Merge branch 'js/spawn-via-shell-path-fix'Libravatar Junio C Hamano1-0/+4
Mops up an unfortunate fallout from bw/spawn-via-shell-path topic. By Johannes Sixt * js/spawn-via-shell-path-fix: Do not use SHELL_PATH from build system in prepare_shell_cmd on Windows
2012-04-20Merge branch 'jk/run-command-eacces'Libravatar Junio C Hamano1-2/+64
When PATH contains an unreadable directory, alias expansion code did not kick in, and failed with an error that said "git-subcmd" was not found. By Jeff King (1) and Ramsay Jones (1) * jk/run-command-eacces: run-command: treat inaccessible directories as ENOENT compat/mingw.[ch]: Change return type of exec functions to int
2012-04-17Do not use SHELL_PATH from build system in prepare_shell_cmd on WindowsLibravatar Johannes Sixt1-0/+4
The recent change to use SHELL_PATH instead of "sh" to spawn shell commands is not suited for Windows: - The default setting, "/bin/sh", does not work when git has to run the shell because it is a POSIX style path, but not a proper Windows style path. - If it worked, it would hard-code a position in the files system where the shell is expected, making git (more precisely, the POSIX toolset that is needed alongside git) non-relocatable. But we cannot sacrifice relocatability on Windows. - Apart from that, even though the Makefile leaves SHELL_PATH set to "/bin/sh" for the Windows builds, the build system passes a mangled path to the compiler, and something like "D:/Src/msysgit/bin/sh" is used, which is doubly bad because it points to where /bin/sh resolves to on the system where git was built. - Finally, the system's CreateProcess() function that is used under mingw.c's hood does not work with forward slashes and cannot find the shell. Undo the earlier change on Windows. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-05run-command: treat inaccessible directories as ENOENTLibravatar Jeff King1-2/+64
When execvp reports EACCES, it can be one of two things: 1. We found a file to execute, but did not have permissions to do so. 2. We did not have permissions to look in some directory in the $PATH. In the former case, we want to consider this a permissions problem and report it to the user as such (since getting this for something like "git foo" is likely a configuration error). In the latter case, there is a good chance that the inaccessible directory does not contain anything of interest. Reporting "permission denied" is confusing to the user (and prevents our usual "did you mean...?" lookup). It also prevents git from trying alias lookup, since we do so only when an external command does not exist (not when it exists but has an error). This patch detects EACCES from execvp, checks whether we are in case (2), and if so converts errno to ENOENT. This behavior matches that of "bash" (but not of simpler shells that use execvp more directly, like "dash"). Test stolen from Junio. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-03Use SHELL_PATH from build system in run_command.c:prepare_shell_cmdLibravatar Ben Walton1-1/+5
During the testing of the 1.7.10 rc series on Solaris for OpenCSW, it was discovered that t7006-pager was failing due to finding a bad "sh" in PATH after a call to execvp("sh", ...). This call was setup by run_command.c:prepare_shell_cmd. The PATH in use at the time saw /opt/csw/bin given precedence to traditional Solaris paths such as /usr/bin and /usr/xpg4/bin. A package named schilyutils (Joerg Schilling's utilities) was installed on the build system and it delivered a modified version of the traditional Solaris /usr/bin/sh as /opt/csw/bin/sh. This version of sh suffers from many of the same problems as /usr/bin/sh. The command-specific pager test failed due to the broken "sh" handling ^ as a pipe character. It tried to fork two processes when it encountered "sed s/^/foo:/" as the pager command. This problem was entirely dependent on the PATH of the user at runtime. Possible fixes for this issue are: 1. Use the standard system() or popen() which both launch a POSIX shell on Solaris as long as _POSIX_SOURCE is defined. 2. The git wrapper could prepend SANE_TOOL_PATH to PATH thus forcing all unqualified commands run to use the known good tools on the system. 3. The run_command.c:prepare_shell_command() could use the same SHELL_PATH that is in the #! line of all all scripts and not rely on PATH to find the sh to run. Option 1 would preclude opening a bidirectional pipe to a filter script and would also break git for Windows as cmd.exe is spawned from system() (cf. v1.7.5-rc0~144^2, "alias: use run_command api to execute aliases, 2011-01-07). Option 2 is not friendly to users as it would negate their ability to use tools of their choice in many cases. Alternately, injecting SANE_TOOL_PATH such that it takes precedence over /bin and /usr/bin (and anything with lower precedence than those paths) as git-sh-setup.sh does would not solve the problem either as the user environment could still allow a bad sh to be found. (Many OpenCSW users will have /opt/csw/bin leading their PATH and some subset would have schilyutils installed.) Option 3 allows us to use a known good shell while still honouring the users' PATH for the utilities being run. Thus, it solves the problem while not negatively impacting either users or git's ability to run external commands in convenient ways. Essentially, the shell is a special case of tool that should not rely on SANE_TOOL_PATH and must be called explicitly. With this patch applied, any code path leading to run_command.c:prepare_shell_cmd can count on using the same sane shell that all shell scripts in the git suite use. Both the build system and run_command.c will default this shell to /bin/sh unless overridden. Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-01-08dashed externals: kill children on exitLibravatar Clemens Buchacher1-0/+1
Several git commands are so-called dashed externals, that is commands executed as a child process of the git wrapper command. If the git wrapper is killed by a signal, the child process will continue to run. This is different from internal commands, which always die with the git wrapper command. Enable the recently introduced cleanup mechanism for child processes in order to make dashed externals act more in line with internal commands. Signed-off-by: Clemens Buchacher <drizzd@aon.at> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-01-08run-command: optionally kill children on exitLibravatar Jeff King1-0/+68
When we spawn a helper process, it should generally be done and finish_command called before we exit. However, if we exit abnormally due to an early return or a signal, the helper may continue to run in our absence. In the best case, this may simply be wasted CPU cycles or a few stray messages on a terminal. But it could also mean a process that the user thought was aborted continues to run to completion (e.g., a push's pack-objects helper will complete the push, even though you killed the push process). This patch provides infrastructure for run-command to keep track of PIDs to be killed, and clean them on signal reception or input, just as we do with tempfiles. PIDs can be added in two ways: 1. If NO_PTHREADS is defined, async helper processes are automatically marked. By definition this code must be ready to die when the parent dies, since it may be implemented as a thread of the parent process. 2. If the run-command caller specifies the "clean_on_exit" option. This is not the default, as there are cases where it is OK for the child to outlive us (e.g., when spawning a pager). PIDs are cleared from the kill-list automatically during wait_or_whine, which is called from finish_command and finish_async. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Clemens Buchacher <drizzd@aon.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-10-05Merge branch 'jk/argv-array'Libravatar Junio C Hamano1-10/+8
* jk/argv-array: run_hook: use argv_array API checkout: use argv_array API bisect: use argv_array API quote: provide sq_dequote_to_argv_array refactor argv_array into generic code quote.h: fix bogus comment add sha1_array API docs
2011-09-14run_hook: use argv_array APILibravatar Jeff King1-10/+8
This was a pretty straightforward use, so it really doesn't save that many lines. Still, perhaps it's a little bit more readable. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-08-01notice error exit from pagerLibravatar Clemens Buchacher1-9/+6
If the pager fails to run, git produces no output, e.g.: $ GIT_PAGER=not-a-command git log The error reporting fails for two reasons: (1) start_command: There is a mechanism that detects errors during execvp introduced in 2b541bf8 (start_command: detect execvp failures early). The child writes one byte to a pipe only if execvp fails. The parent waits for either EOF, when the successful execvp automatically closes the pipe (see FD_CLOEXEC in fcntl(1)), or it reads a single byte, in which case it knows that the execvp failed. This mechanism is incompatible with the workaround introduced in 35ce8622 (pager: Work around window resizing bug in 'less'), which waits for input from the parent before the exec. Since both the parent and the child are waiting for input from each other, that would result in a deadlock. In order to avoid that, the mechanism is disabled by closing the child_notifier file descriptor. (2) finish_command: The parent correctly detects the 127 exit status from the child, but the error output goes nowhere, since by that time it is already being redirected to the child. No simple solution for (1) comes to mind. Number (2) can be solved by not sending error output to the pager. Not redirecting error output to the pager can result in the pager overwriting error output with standard output, however. Since there is no reliable way to handle error reporting in the parent, produce the output in the child instead. Signed-off-by: Clemens Buchacher <drizzd@aon.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-07-31error_routine: use parent's stderr if exec failsLibravatar Clemens Buchacher1-8/+7
The new process's error output may be redirected elsewhere, but if the exec fails, output should still go to the parent's stderr. This has already been done for the die_routine. Do the same for error_routine. Signed-off-by: Clemens Buchacher <drizzd@aon.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-04-20run-command: handle short writes and EINTR in die_childLibravatar Jonathan Nieder1-6/+9
If start_command fails after forking and before exec finishes, there is not much use in noticing an I/O error on top of that. finish_command will notice that the child exited with nonzero status anyway. So as noted in v1.7.0.3~20^2 (run-command.c: fix build warnings on Ubuntu, 2010-01-30) and v1.7.5-rc0~29^2 (2011-03-16), it is safe to ignore errors from write in this codepath. Even so, the result from write contains useful information: it tells us if the write was cancelled by a signal (EINTR) or was only partially completed (e.g., when writing to an almost-full pipe). Let's use write_in_full to loop until the desired number of bytes have been written (still ignoring errors if that fails). As a happy side effect, the assignment to a dummy variable to appease gcc -D_FORTIFY_SOURCE is no longer needed. xwrite and write_in_full check the return value from write(2). Noticed with gcc -Wunused-but-set-variable. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-04-18Revert "run-command: prettify -D_FORTIFY_SOURCE workaround"Libravatar Junio C Hamano1-11/+6
This reverts commit ebec842773932e6f853acac70c80f84209b5f83e, which somehow mistakenly thought that any non-zero return from write(2) is an error.
2011-03-17run-command: prettify -D_FORTIFY_SOURCE workaroundLibravatar Jonathan Nieder1-6/+11
Current gcc + glibc with -D_FORTIFY_SOURCE try very aggressively to protect against a programming style which uses write(...) without checking the return value for errors. Even the usual hint of casting to (void) does not suppress the warning. Sometimes when there is an output error, especially right before exit, there really is nothing to be done. The obvious solution, adopted in v1.7.0.3~20^2 (run-command.c: fix build warnings on Ubuntu, 2010-01-30), is to save the return value to a dummy variable: ssize_t dummy; dummy = write(...); But that (1) is ugly and (2) triggers -Wunused-but-set-variable warnings with gcc-4.6 -Wall, so we are not much better off than when we started. Instead, use an "if" statement with an empty body to make the intent clear. if (write(...)) ; /* yes, yes, there was an error. */ Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Improved-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-02-07start_command: flush buffers in the WIN32 code path as wellLibravatar Johannes Sixt1-1/+1
The POSIX code path did The Right Thing already, but we have to do the same on Windows. This bug caused failures in t5526-fetch-submodules, where the output of 'git fetch --recurse-submodules' was in the wrong order. Debugged-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-06-21Merge branch 'js/async-thread'Libravatar Junio C Hamano1-18/+57
* js/async-thread: fast-import: die_nicely() back to vsnprintf (reverts part of ebaa79f) Enable threaded async procedures whenever pthreads is available Dying in an async procedure should only exit the thread, not the process. Reimplement async procedures using pthreads Windows: more pthreads functions Fix signature of fcntl() compatibility dummy Make report() from usage.c public as vreportf() and use it. Modernize t5530-upload-pack-error. Conflicts: http-backend.c
2010-05-20start_command: close cmd->err descriptor when fork/spawn failsLibravatar bert Dvornik1-0/+2
Fix the problem where the cmd->err passed into start_command wasn't being properly closed when certain types of errors occurr. (Compare the affected code with the clean shutdown code later in the function.) On Windows, this problem would be triggered if mingw_spawnvpe() failed, which would happen if the command to be executed was malformed (e.g. a text file that didn't start with a #! line). If cmd->err was a pipe, the failure to close it could result in a hang while the other side was waiting (forever) for either input or pipe close, e.g. while trying to shove the output into the side band. On msysGit, this problem was causing a hang in t5516-fetch-push. [J6t: With a slight adjustment of the test case, the hang is also observed on Linux.] Signed-off-by: bert Dvornik <dvornik+git@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-04-11Merge branch 'jl/maint-submodule-gitfile-awareness'Libravatar Junio C Hamano1-3/+1
* jl/maint-submodule-gitfile-awareness: Windows: start_command: Support non-NULL dir in struct child_process
2010-04-11Windows: start_command: Support non-NULL dir in struct child_processLibravatar Johannes Sixt1-3/+1
A caller of start_command can set the member 'dir' to a directory to request that the child process starts with that directory as CWD. The first user of this feature was added recently in eee49b6 (Teach diff --submodule and status to handle .git files in submodules). On Windows, we have been lazy and had not implemented support for this feature, yet. This fixes the shortcoming. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-10Enable threaded async procedures whenever pthreads is availableLibravatar Johannes Sixt1-5/+5
Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-07Merge branch 'mw/maint-gcc-warns-unused-write'Libravatar Junio C Hamano1-4/+6
* mw/maint-gcc-warns-unused-write: run-command.c: fix build warnings on Ubuntu
2010-03-07Dying in an async procedure should only exit the thread, not the process.Libravatar Johannes Sixt1-0/+34
Async procedures are intended as helpers that perform a very restricted task, and the caller usually has to manage them in a larger context. Conceptually, the async procedure is not concerned with the "bigger picture" in whose context it is run. When it dies, it is not supposed to destroy this "bigger picture", but rather only its own limit view of the world. On POSIX, the async procedure is run in its own process, and exiting this process naturally had only these limited effects. On Windows (or when ASYNC_AS_THREAD is set), calling die() exited the whole process, destroying the caller (the "big picture") as well. This fixes it to exit only the thread. Without ASYNC_AS_THREAD, one particular effect of exiting the async procedure process is that it automatically closes file descriptors, most notably the writable end of the pipe that the async procedure writes to. The async API already requires that the async procedure closes the pipe ends when it exits normally. But for calls to die() no requirements are imposed. In the non-threaded case the pipe ends are closed implicitly by the exiting process, but in the threaded case, the die routine must take care of closing them. Now t5530-upload-pack-error.sh passes on Windows. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-07Reimplement async procedures using pthreadsLibravatar Johannes Sixt1-18/+23
On Windows, async procedures have always been run in threads, and the implementation used Windows specific APIs. Rewrite the code to use pthreads. A new configuration option is introduced so that the threaded implementation can also be used on POSIX systems. Since this option is intended only as playground on POSIX, but is mandatory on Windows, the option is not documented. One detail is that on POSIX it is necessary to set FD_CLOEXEC on the pipe handles. On Windows, this is not needed because pipe handles are not inherited to child processes, and the new calls to set_cloexec() are effectively no-ops. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-03run-command.c: fix build warnings on UbuntuLibravatar Michael Wookey1-4/+6
Building git on Ubuntu 9.10 warns that the return value of write(2) isn't checked. These warnings were introduced in commits: 2b541bf8 ("start_command: detect execvp failures early") a5487ddf ("start_command: report child process setup errors to the parent's stderr") GCC details: $ gcc --version gcc (Ubuntu 4.4.1-4ubuntu9) 4.4.1 Silence the warnings by reading (but not making use of) the return value of write(2). Signed-off-by: Michael Wookey <michaelwookey@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-05Merge branch 'sp/maint-push-sideband' into sp/push-sidebandLibravatar Junio C Hamano1-13/+77
* sp/maint-push-sideband: receive-pack: Send hook output over side band #2 receive-pack: Wrap status reports inside side-band-64k receive-pack: Refactor how capabilities are shown to the client send-pack: demultiplex a sideband stream with status data run-command: support custom fd-set in async run-command: Allow stderr to be a caller supplied pipe Update git fsck --full short description to mention packs Conflicts: run-command.c
2010-02-05run-command: support custom fd-set in asyncLibravatar Erik Faye-Lund1-13/+70
This patch adds the possibility to supply a set of non-0 file descriptors for async process communication instead of the default-created pipe. Additionally, we now support bi-directional communiction with the async procedure, by giving the async function both read and write file descriptors. To retain compatiblity and similar "API feel" with start_command, we require start_async callers to set .out = -1 to get a readable file descriptor. If either of .in or .out is 0, we supply no file descriptor to the async process. [sp: Note: Erik started this patch, and a huge bulk of it is his work. All bugs were introduced later by Shawn.] Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-05run-command: Allow stderr to be a caller supplied pipeLibravatar Shawn O. Pearce1-0/+8
Like .out, .err may now be set to a file descriptor > 0, which is a writable pipe/socket/file that the child's stderr will be redirected into. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-20Merge branch 'js/exec-error-report'Libravatar Junio C Hamano1-46/+131
* js/exec-error-report: Improve error message when a transport helper was not found start_command: detect execvp failures early run-command: move wait_or_whine earlier start_command: report child process setup errors to the parent's stderr Conflicts: Makefile
2010-01-18Merge branch 'js/windows'Libravatar Junio C Hamano1-40/+31
* js/windows: Do not use date.c:tm_to_time_t() from compat/mingw.c MSVC: Windows-native implementation for subset of Pthreads API MSVC: Fix an "incompatible pointer types" compiler warning Windows: avoid the "dup dance" when spawning a child process Windows: simplify the pipe(2) implementation Windows: boost startup by avoiding a static dependency on shell32.dll Windows: disable Python
2010-01-16Windows: avoid the "dup dance" when spawning a child processLibravatar Johannes Sixt1-40/+31
When stdin, stdout, or stderr must be redirected for a child process that on Windows is spawned using one of the spawn() functions of Microsoft's C runtime, then there is no choice other than to 1. make a backup copy of fd 0,1,2 with dup 2. dup2 the redirection source fd into 0,1,2 3. spawn 4. dup2 the backup back into 0,1,2 5. close the backup copy and the redirection source We used this idiom as well -- but we are not using the spawn() functions anymore! Instead, we have our own implementation. We had hardcoded that stdin, stdout, and stderr of the child process were inherited from the parent's fds 0, 1, and 2. But we can actually specify any fd. With this patch, the fds to inherit are passed from start_command()'s WIN32 section to our spawn implementation. This way, we can avoid the backup copies of the fds. The backup copies were a bug waiting to surface: The OS handles underlying the dup()ed fds were inherited by the child process (but were not associated with a file descriptor in the child). Consequently, the file or pipe represented by the OS handle remained open even after the backup copy was closed in the parent process until the child exited. Since our implementation of pipe() creates non-inheritable OS handles, we still dup() file descriptors in start_command() because dup() happens to create inheritable duplicates. (A nice side effect is that the fd cleanup in start_command is the same for Windows and Unix and remains unchanged.) Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-10start_command: detect execvp failures earlyLibravatar Johannes Sixt1-1/+46
Previously, failures during execvp could be detected only by finish_command. However, in some situations it is beneficial for the parent process to know earlier that the child process will not run. The idea to use a pipe to signal failures to the parent process and the test case were lifted from patches by Ilari Liusvaara. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-10run-command: move wait_or_whine earlierLibravatar Johannes Sixt1-42/+42
We want to reuse it from start_command. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-10start_command: report child process setup errors to the parent's stderrLibravatar Johannes Sixt1-3/+43
When the child process's environment is set up in start_command(), error messages were written to wherever the parent redirected the child's stderr channel. However, even if the parent redirected the child's stderr, errors during this setup process, including the exec itself, are usually an indication of a problem in the parent's environment. Therefore, the error messages should go to the parent's stderr. Redirection of the child's error messages is usually only used to redirect hook error messages during client-server exchanges. In these cases, hook setup errors could be regarded as information leak. This patch makes a copy of stderr if necessary and uses a special die routine that is used for all die() calls in the child that sends the errors messages to the parent's stderr. The trace call that reported a failed execvp is removed (because it writes to stderr) and replaced by die_errno() with special treatment of ENOENT. The improvement in the error message can be seen with this sequence: mkdir .git/hooks/pre-commit git commit Previously, the error message was error: cannot run .git/hooks/pre-commit: No such file or directory and now it is fatal: cannot exec '.git/hooks/pre-commit': Permission denied Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-05run-command: optimize out useless shell callsLibravatar Jeff King1-9/+11
If there are no metacharacters in the program to be run, we can just skip running the shell entirely and directly exec the program. The metacharacter test is pulled verbatim from launch_editor, which already implements this optimization. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>