summaryrefslogtreecommitdiff
path: root/run-command.c
AgeCommit message (Collapse)AuthorFilesLines
2015-03-25Merge branch 'jk/run-command-capture'Libravatar Junio C Hamano1-1/+22
The run-command interface was easy to abuse and make a pipe for us to read from the process, wait for the process to finish and then attempt to read its output, which is a pattern that lead to a deadlock. Fix such uses by introducing a helper to do this correctly (i.e. we need to read first and then wait the process to finish) and also add code to prevent such abuse in the run-command helper. * jk/run-command-capture: run-command: forbid using run_command with piped output trailer: use capture_command submodule: use capture_command wt-status: use capture_command run-command: introduce capture_command helper wt_status: fix signedness mismatch in strbuf_read call wt-status: don't flush before running "submodule status"
2015-03-22run-command: forbid using run_command with piped outputLibravatar Jeff King1-1/+6
Because run_command both spawns and wait()s for the command before returning control to the caller, any reads from the pipes we open must necessarily happen after wait() returns. This can lead to deadlock, as the child process may block on writing to us while we are blocked waiting for it to exit. Worse, it only happens when the child fills the pipe buffer, which means that the problem may come and go depending on the platform and the size of the output produced by the child. Let's detect and flag this dangerous construct so that we can catch potential bugs early in the test suite rather than having them happen in the field. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-22run-command: introduce capture_command helperLibravatar Jeff King1-0/+16
Something as simple as reading the stdout from a command turns out to be rather hard to do right. Doing: cmd.out = -1; run_command(&cmd); strbuf_read(&buf, cmd.out, 0); can result in deadlock if the child process produces a large amount of output. What happens is: 1. The parent spawns the child with its stdout connected to a pipe, of which the parent is the sole reader. 2. The parent calls wait(), blocking until the child exits. 3. The child writes to stdout. If it writes more data than the OS pipe buffer can hold, the write() call will block. This is a deadlock; the parent is waiting for the child to exit, and the child is waiting for the parent to call read(). So we might try instead: start_command(&cmd); strbuf_read(&buf, cmd.out, 0); finish_command(&cmd); But that is not quite right either. We are examining cmd.out and running finish_command whether start_command succeeded or not, which is wrong. Moreover, these snippets do not do any error handling. If our read() fails, we must make sure to still call finish_command (to reap the child process). And both snippets failed to close the cmd.out descriptor, which they must do (provided start_command succeeded). Let's introduce a run-command helper that can make this a bit simpler for callers to get right. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-10git-compat-util.h: move SHELL_PATH default into headerLibravatar Kyle J. McKay1-4/+0
If SHELL_PATH is not defined we use "/bin/sh". However, run-command.c is not the only file that needs to use the default value so move it into a common header. Signed-off-by: Kyle J. McKay <mackyle@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-12-22Merge branch 'jc/hook-cleanup'Libravatar Junio C Hamano1-17/+0
Remove unused code. * jc/hook-cleanup: run-command.c: retire unused run_hook_with_custom_index()
2014-12-01run-command.c: retire unused run_hook_with_custom_index()Libravatar Junio C Hamano1-17/+0
This was originally meant to be used to rewrite run_commit_hook() that only special cases the GIT_INDEX_FILE environment, but the run_hook_ve() refactoring done earlier made the implementation of run_commit_hook() thin and clean enough. Nobody uses this, so retire it as an unfinished clean-up made unnecessary. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-11-10run-command: use void to declare that functions take no parametersLibravatar René Scharfe1-2/+2
Explicitly declare that git_atexit_dispatch() and git_atexit_clear() take no parameters instead of leaving their parameter list empty and thus unspecified. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-24Merge branch 'eb/no-pthreads'Libravatar Junio C Hamano1-0/+40
Allow us build with NO_PTHREADS=NoThanks compilation option. * eb/no-pthreads: Handle atexit list internaly for unthreaded builds pack-objects: set number of threads before checking and warning index-pack: fix compilation with NO_PTHREADS
2014-10-19Handle atexit list internaly for unthreaded buildsLibravatar Etienne Buira1-0/+40
Wrap atexit()s calls on unthreaded builds to handle callback list internally. This is needed because on unthreaded builds, asyncs inherits parent's atexit() list, that gets run as soon as the async exit()s (and again at the end of async's parent process). That led to remove temporary files too early. Also remove a by-atexit-callback guard against this kind of issue in clone.c, as this patch makes it redundant. Fixes test 5537 (temporary shallow file vanished before unpack-objects could open it) BTW remove an unused variable in shallow.c. Helped-by: Duy Nguyen <pclouds@gmail.com> Helped-by: Andreas Schwab <schwab@linux-m68k.org> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Etienne Buira <etienne.buira@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-19run-command: add env_array, an optional argv_array for envLibravatar René Scharfe1-0/+6
Similar to args, add a struct argv_array member to struct child_process that simplifies specifying the environment for children. It is freed automatically by finish_command() or if start_command() encounters an error. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-20run-command: inline prepare_run_command_v_opt()Libravatar René Scharfe1-16/+8
Merge prepare_run_command_v_opt() and its only caller. This removes a pointer indirection and allows to initialize the struct child_process using CHILD_PROCESS_INIT. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-20run-command: call run_command_v_opt_cd_env() instead of duplicating itLibravatar René Scharfe1-3/+1
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-20run-command: introduce child_process_init()Libravatar René Scharfe1-0/+6
Add a helper function for initializing those struct child_process variables for which the macro CHILD_PROCESS_INIT can't be used. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-20run-command: introduce CHILD_PROCESS_INITLibravatar René Scharfe1-2/+1
Most struct child_process variables are cleared using memset first after declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to initialize them statically instead. That's shorter, doesn't require a function call and is slightly more readable (especially given that we already have STRBUF_INIT, ARGV_ARRAY_INIT etc.). Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-30Merge branch 'sk/mingw-uni-fix-more'Libravatar Junio C Hamano1-8/+2
Most of these are battle-tested in msysgit and are needed to complete what has been merged to 'master' already. * sk/mingw-uni-fix-more: Win32: enable color output in Windows cmd.exe Win32: patch Windows environment on startup Win32: keep the environment sorted Win32: use low-level memory allocation during initialization Win32: reduce environment array reallocations Win32: don't copy the environment twice when spawning child processes Win32: factor out environment block creation Win32: unify environment function names Win32: unify environment case-sensitivity Win32: fix environment memory leaks Win32: Unicode environment (incoming) Win32: Unicode environment (outgoing) Revert "Windows: teach getenv to do a case-sensitive search" tests: do not pass iso8859-1 encoded parameter
2014-07-21Win32: don't copy the environment twice when spawning child processesLibravatar Karsten Blees1-8/+2
When spawning child processes via start_command(), the environment and all environment entries are copied twice. First by make_augmented_environ / copy_environ to merge with child_process.env. Then a second time by make_environment_block to create a sorted environment block string as required by CreateProcess. Move the merge logic to make_environment_block so that we only need to copy the environment once. This changes semantics of the env parameter: it now expects a delta (such as child_process.env) rather than a full environment. This is not a problem as the parameter is only used by start_command() (all other callers previously passed char **environ, and now pass NULL). The merge logic no longer xstrdup()s the environment strings, so do_putenv must not free them. Add a parameter to distinguish this from normal putenv. Remove the now unused make_augmented_environ / free_environ API. Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Stepan Kasal <kasal@ucw.cz> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-17run-command: use internal argv_array of struct child_process in run_hook_ve()Libravatar René Scharfe1-11/+4
Use the existing argv_array member instead of providing our own. This way we don't have to initialize or clean it up explicitly. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-05-15run-command: store an optional argv_arrayLibravatar Jeff King1-1/+8
All child_process structs need to point to an argv. For flexibility, we do not mandate the use of a dynamic argv_array. However, because the child_process does not own the memory, this can make memory management with a separate argv_array difficult. For example, if a function calls start_command but not finish_command, the argv memory must persist. The code needs to arrange to clean up the argv_array separately after finish_command runs. As a result, some of our code in this situation just leaks the memory. To help such cases, this patch adds a built-in argv_array to the child_process, which gets cleaned up automatically (both in finish_command and when start_command fails). Callers may use it if they choose, but can continue to use the raw argv if they wish. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-03-18commit: fix patch hunk editing with "commit -p -m"Libravatar Benoit Pierre1-12/+32
Don't change git environment: move the GIT_EDITOR=":" override to the hook command subprocess, like it's already done for GIT_INDEX_FILE. Signed-off-by: Benoit Pierre <benoit.pierre@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-10-31run-command: trivial style fixesLibravatar Felipe Contreras1-8/+5
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-22Merge branch 'tr/fd-gotcha-fixes'Libravatar Junio C Hamano1-1/+4
Two places we did not check return value (expected to be a file descriptor) correctly. * tr/fd-gotcha-fixes: run-command: dup_devnull(): guard against syscalls failing git_mkstemps: correctly test return value of open()
2013-07-12run-command: dup_devnull(): guard against syscalls failingLibravatar Thomas Rast1-1/+4
dup_devnull() did not check the return values of open() and dup2(). Fix this omission. Signed-off-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-05-08mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVELibravatar Jonathan Nieder1-5/+5
Throughout git, it is assumed that the WIN32 preprocessor symbol is defined on native Windows setups (mingw and msvc) and not on Cygwin. On Cygwin, most of the time git can pretend this is just another Unix machine, and Windows-specific magic is generally counterproductive. Unfortunately Cygwin *does* define the WIN32 symbol in some headers. Best to rely on a new git-specific symbol GIT_WINDOWS_NATIVE instead, defined as follows: #if defined(WIN32) && !defined(__CYGWIN__) # define GIT_WINDOWS_NATIVE #endif After this change, it should be possible to drop the CYGWIN_V15_WIN32API setting without any negative effect. [rj: %s/WINDOWS_NATIVE/GIT_WINDOWS_NATIVE/g ] Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-19Merge branch 'jk/a-thread-only-dies-once'Libravatar Junio C Hamano1-0/+11
A regression fix for the logic to detect die() handler triggering itself recursively. * jk/a-thread-only-dies-once: run-command: use thread-aware die_is_recursing routine usage: allow pluggable die-recursion checks
2013-04-16run-command: use thread-aware die_is_recursing routineLibravatar Jeff King1-0/+11
If we die from an async thread, we do not actually exit the program, but just kill the thread. This confuses the static counter in usage.c's default die_is_recursing function; it updates the counter once for the thread death, and then when the main program calls die() itself, it erroneously thinks we are recursing. The end result is that we print "recursion detected in die handler" instead of the real error in such a case (the easiest way to trigger this is having a remote connection hang up while running a sideband demultiplexer). This patch solves it by using a per-thread counter when the async_die function is installed; we detect recursion in each thread (including the main one), but they do not step on each other's toes. Other threaded code does not need to worry about this, as they do not install specialized die handlers; they just let a die() from a sub-thread take down the whole program. Since we are overriding the default recursion-check function, there is an interesting corner case that is not a problem, but bears some explanation. Imagine the main thread calls die(), and then in the die_routine starts an async call. We will switch to using thread-local storage, which starts at 0, for the main thread's counter, even though the original counter was actually at 1. That's OK, though, for two reasons: 1. It would miss only the first level of recursion, and would still find recursive failures inside the async helper. 2. We do not currently and are not likely to start doing anything as heavyweight as starting an async routine from within a die routine or helper function. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>