summaryrefslogtreecommitdiff
path: root/run-command.c
AgeCommit message (Collapse)AuthorFilesLines
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-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-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>
2010-01-01run-command: add "use shell" optionLibravatar Jeff King1-0/+47
Many callsites run "sh -c $CMD" to run $CMD. We can make it a little simpler for them by factoring out the munging of argv. For simple cases with no arguments, this doesn't help much, but: 1. For cases with arguments, we save the caller from having to build the appropriate shell snippet. 2. We can later optimize to avoid the shell when there are no metacharacters in the program. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-09-18Test for WIN32 instead of __MINGW32_Libravatar Frank Li1-4/+4
The code which is conditional on MinGW32 is actually conditional on Windows. Use the WIN32 symbol, which is defined by the MINGW32 and MSVC environments, but not by Cygwin. Define SNPRINTF_SIZE_CORR=1 for MSVC too, as its vsnprintf function does not add NUL at the end of the buffer if the result fits the buffer size exactly. Signed-off-by: Frank Li <lznuaa@gmail.com> Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com> Acked-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-09-18Fix __stdcall placement and function prototypeLibravatar Frank Li1-1/+1
MSVC requires __stdcall to be between the functions return value and the function name, and that the function pointer type is in the form of return_type (WINAPI *function_name)(arguments...) Signed-off-by: Frank Li <lznuaa@gmail.com> Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com> Acked-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-09-18Avoid declaration after statementLibravatar Frank Li1-0/+2
MSVC does not understand this C99 style. Signed-off-by: Frank Li <lznuaa@gmail.com> Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com> Acked-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-09-11start_command: do not clobber cmd->env on Windows code pathLibravatar Johannes Sixt1-5/+2
Previously, it would not be possible to call start_command twice for the same struct child_process that has env set. The fix is achieved by moving the loop that modifies the environment block into a helper function. This also allows us to make two other helper functions static. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-08-10Merge branch 'js/run-command-updates'Libravatar Junio C Hamano1-46/+59
* js/run-command-updates: api-run-command.txt: describe error behavior of run_command functions run-command.c: squelch a "use before assignment" warning receive-pack: remove unnecessary run_status report run_command: report failure to execute the program, but optionally don't run_command: encode deadly signal number in the return value run_command: report system call errors instead of returning error codes run_command: return exit code as positive value MinGW: simplify waitpid() emulation macros
2009-08-04run-command.c: squelch a "use before assignment" warningLibravatar David Soria Parra1-1/+1
i686-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5490) compiler (and probably others) mistakenly thinks variable failed_errno is used before assigned. Work it around by giving it a fake initialization. Signed-off-by: David Soria Parra <dsp@php.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-07-06run_command: report failure to execute the program, but optionally don'tLibravatar Johannes Sixt1-4/+8
In the case where a program was not found, it was still the task of the caller to report an error to the user. Usually, this is an interesting case but only few callers actually reported a specific error (though many call sites report a generic error message regardless of the cause). With this change the error is reported by run_command, but since there is one call site in git.c that does not want that, an option is added to struct child_process, which is used to turn the error off. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-07-06run_command: encode deadly signal number in the return valueLibravatar Johannes Sixt1-1/+8
We now write the signal number in the error message if the program terminated by a signal. The negative return value is constructed such that after truncation to 8 bits it looks like a POSIX shell's $?: $ echo 0000 | { git upload-pack .; echo $? >&2; } | : error: git-upload-pack died of signal 13 141 Previously, the exit code was 255 instead of 141. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-07-06run_command: report system call errors instead of returning error codesLibravatar Johannes Sixt1-40/+49
The motivation for this change is that system call failures are serious errors that should be reported to the user, but only few callers took the burden to decode the error codes that the functions returned into error messages. If at all, then only an unspecific error message was given. A prominent example is this: $ git upload-pack . | : fatal: unable to run 'git-upload-pack' In this example, git-upload-pack, the external command invoked through the git wrapper, dies due to SIGPIPE, but the git wrapper does not bother to report the real cause. In fact, this very error message is copied to the syslog if git-daemon's client aborts the connection early. With this change, system call failures are reported immediately after the failure and only a generic failure code is returned to the caller. In the above example the error is now to the point: $ git upload-pack . | : error: git-upload-pack died of signal Note that there is no error report if the invoked program terminated with a non-zero exit code, because it is reasonable to expect that the invoked program has already reported an error. (But many run_command call sites nevertheless write a generic error message.) There was one special return code that was used to identify the case where run_command failed because the requested program could not be exec'd. This special case is now treated like a system call failure with errno set to ENOENT. No error is reported in this case, because the call site in git.c expects this as a normal result. Therefore, the callers that carefully decoded the return value still check for this condition. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>