Merge branch 'jk/run-command-use-shell'

* jk/run-command-use-shell:
t4030, t4031: work around bogus MSYS bash path conversion
diff: run external diff helper with shell
textconv: use shell to run helper
editor: use run_command's shell feature
run-command: optimize out useless shell calls
run-command: convert simple callsites to use_shell
t0021: use $SHELL_PATH for the filter script
run-command: add "use shell" option

diff --git a/convert.c b/convert.c
index 491e7141b4ea29b3cf754cbaf2656a0c3ca8c46c..950b1f9840663e7536256babb05f68de01a4689b 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -249,10 +249,11 @@ static int filter_buffer(int fd, void *data)
        struct child_process child_process;
        struct filter_params *params = (struct filter_params *)data;
        int write_err, status;
-       const char *argv[] = { "sh", "-c", params->cmd, NULL };
+       const char *argv[] = { params->cmd, NULL };
 
        memset(&child_process, 0, sizeof(child_process));
        child_process.argv = argv;
+       child_process.use_shell = 1;
        child_process.in = -1;
        child_process.out = fd;
 

diff --git a/diff.c b/diff.c
index 04beb26a6a8aa46b5b0721b71236553cf549061b..5d713145879d88339a3ea6bef207129f3cac3fef 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -2294,7 +2294,7 @@ static void run_external_diff(const char *pgm,
        }
        *arg = NULL;
        fflush(NULL);
-       retval = run_command_v_opt(spawn_arg, 0);
+       retval = run_command_v_opt(spawn_arg, RUN_USING_SHELL);
        remove_tempfile();
        if (retval) {
                fprintf(stderr, "external diff died, stopping at %s.\n", name);
@@ -3818,6 +3818,7 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,
        *arg = NULL;
 
        memset(&child, 0, sizeof(child));
+       child.use_shell = 1;
        child.argv = argv;
        child.out = -1;
        if (start_command(&child) != 0 ||

diff --git a/editor.c b/editor.c
index 615f5754d66ba7ea611a4837b1ff802f9f9de598..d8340031d24828483697c10257806efedfe041f5 100644 (file)
--- a/editor.c
+++ b/editor.c
@@ -36,26 +36,9 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
                return error("Terminal is dumb, but EDITOR unset");
 
        if (strcmp(editor, ":")) {
-               size_t len = strlen(editor);
-               int i = 0;
-               int failed;
-               const char *args[6];
-               struct strbuf arg0 = STRBUF_INIT;
+               const char *args[] = { editor, path, NULL };
 
-               if (strcspn(editor, "|&;<>()$`\\\"' \t\n*?[#~=%") != len) {
-                       /* there are specials */
-                       strbuf_addf(&arg0, "%s \"$@\"", editor);
-                       args[i++] = "sh";
-                       args[i++] = "-c";
-                       args[i++] = arg0.buf;
-               }
-               args[i++] = editor;
-               args[i++] = path;
-               args[i] = NULL;
-
-               failed = run_command_v_opt_cd_env(args, 0, NULL, env);
-               strbuf_release(&arg0);
-               if (failed)
+               if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env))
                        return error("There was a problem with the editor '%s'.",
                                        editor);
        }

diff --git a/imap-send.c b/imap-send.c
index de8114bac010ef095cf9de17671566e248366dcb..51f371ba9f08637657ec9198cf3f16d0c0407232 100644 (file)
--- a/imap-send.c
+++ b/imap-send.c
@@ -965,17 +965,13 @@ static struct store *imap_open_store(struct imap_server_conf *srvc)
        /* open connection to IMAP server */
 
        if (srvc->tunnel) {
-               const char *argv[4];
+               const char *argv[] = { srvc->tunnel, NULL };
                struct child_process tunnel = {0};
 
                imap_info("Starting tunnel '%s'... ", srvc->tunnel);
 
-               argv[0] = "sh";
-               argv[1] = "-c";
-               argv[2] = srvc->tunnel;
-               argv[3] = NULL;
-
                tunnel.argv = argv;
+               tunnel.use_shell = 1;
                tunnel.in = -1;
                tunnel.out = -1;
                if (start_command(&tunnel))

diff --git a/ll-merge.c b/ll-merge.c
index 2d6b6d6cb1d2bc2d334bf058feb3444e94b5a781..18511e281fe7435c2ae93ba3faf502cc606e62b9 100644 (file)
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -175,7 +175,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
                { "B", temp[2] },
                { NULL }
        };
-       const char *args[] = { "sh", "-c", NULL, NULL };
+       const char *args[] = { NULL, NULL };
        int status, fd, i;
        struct stat st;
 
@@ -190,8 +190,8 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
 
        strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict);
 
-       args[2] = cmd.buf;
-       status = run_command_v_opt(args, 0);
+       args[0] = cmd.buf;
+       status = run_command_v_opt(args, RUN_USING_SHELL);
        fd = open(temp[1], O_RDONLY);
        if (fd < 0)
                goto bad;

diff --git a/pager.c b/pager.c
index 92c03f654abd0333bd0dd48b4aebf9ae42ac4de5..2c7e8ecb3c0860b00113e348008415ca28a7ff72 100644 (file)
--- a/pager.c
+++ b/pager.c
@@ -28,7 +28,7 @@ static void pager_preexec(void)
 }
 #endif
 
-static const char *pager_argv[] = { "sh", "-c", NULL, NULL };
+static const char *pager_argv[] = { NULL, NULL };
 static struct child_process pager_process;
 
 static void wait_for_pager(void)
@@ -81,7 +81,8 @@ void setup_pager(void)
        spawned_pager = 1; /* means we are emitting to terminal */
 
        /* spawn the pager */
-       pager_argv[2] = pager;
+       pager_argv[0] = pager;
+       pager_process.use_shell = 1;
        pager_process.argv = pager_argv;
        pager_process.in = -1;
        if (!getenv("LESS")) {

diff --git a/run-command.c b/run-command.c
index cf2d8f7fae1356e50736cb9d599625df79738a2a..47ced570bd58d7146bf226fcbb0fdfdf06b99be5 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -15,6 +15,50 @@ static inline void dup_devnull(int to)
        close(fd);
 }
 
+static const char **prepare_shell_cmd(const char **argv)
+{
+       int argc, nargc = 0;
+       const char **nargv;
+
+       for (argc = 0; argv[argc]; argc++)
+               ; /* just counting */
+       /* +1 for NULL, +3 for "sh -c" plus extra $0 */
+       nargv = xmalloc(sizeof(*nargv) * (argc + 1 + 3));
+
+       if (argc < 1)
+               die("BUG: shell command is empty");
+
+       if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
+               nargv[nargc++] = "sh";
+               nargv[nargc++] = "-c";
+
+               if (argc < 2)
+                       nargv[nargc++] = argv[0];
+               else {
+                       struct strbuf arg0 = STRBUF_INIT;
+                       strbuf_addf(&arg0, "%s \"$@\"", argv[0]);
+                       nargv[nargc++] = strbuf_detach(&arg0, NULL);
+               }
+       }
+
+       for (argc = 0; argv[argc]; argc++)
+               nargv[nargc++] = argv[argc];
+       nargv[nargc] = NULL;
+
+       return nargv;
+}
+
+#ifndef WIN32
+static int execv_shell_cmd(const char **argv)
+{
+       const char **nargv = prepare_shell_cmd(argv);
+       trace_argv_printf(nargv, "trace: exec:");
+       execvp(nargv[0], (char **)nargv);
+       free(nargv);
+       return -1;
+}
+#endif
+
 int start_command(struct child_process *cmd)
 {
        int need_in, need_out, need_err;
@@ -123,6 +167,8 @@ int start_command(struct child_process *cmd)
                        cmd->preexec_cb();
                if (cmd->git_cmd) {
                        execv_git_cmd(cmd->argv);
+               } else if (cmd->use_shell) {
+                       execv_shell_cmd(cmd->argv);
                } else {
                        execvp(cmd->argv[0], (char *const*) cmd->argv);
                }
@@ -179,6 +225,8 @@ int start_command(struct child_process *cmd)
 
        if (cmd->git_cmd) {
                cmd->argv = prepare_git_cmd(cmd->argv);
+       } else if (cmd->use_shell) {
+               cmd->argv = prepare_shell_cmd(cmd->argv);
        }
 
        cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env);
@@ -297,6 +345,7 @@ static void prepare_run_command_v_opt(struct child_process *cmd,
        cmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
        cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
        cmd->silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0;
+       cmd->use_shell = opt & RUN_USING_SHELL ? 1 : 0;
 }
 
 int run_command_v_opt(const char **argv, int opt)

diff --git a/run-command.h b/run-command.h
index fb342090e3cac49f41689f9610bfe2d6c87c87c1..967ba8cc09786934724132b629587419f195b245 100644 (file)
--- a/run-command.h
+++ b/run-command.h
@@ -33,6 +33,7 @@ struct child_process {
        unsigned git_cmd:1; /* if this is to be git sub-command */
        unsigned silent_exec_failure:1;
        unsigned stdout_to_stderr:1;
+       unsigned use_shell:1;
        void (*preexec_cb)(void);
 };
 
@@ -46,6 +47,7 @@ extern int run_hook(const char *index_file, const char *name, ...);
 #define RUN_GIT_CMD         2  /*If this is to be git sub-command */
 #define RUN_COMMAND_STDOUT_TO_STDERR 4
 #define RUN_SILENT_EXEC_FAILURE 8
+#define RUN_USING_SHELL 16
 int run_command_v_opt(const char **argv, int opt);
 
 /*

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 8fc39d77cec6168dae930beef785597dace24aa3..6cb8d60ea2649495c0e3c8bbb8b7cc75c36799b7 100755 (executable)
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -4,7 +4,8 @@ test_description='blob conversion via gitattributes'
 
 . ./test-lib.sh
 
-cat <<\EOF >rot13.sh
+cat <<EOF >rot13.sh
+#!$SHELL_PATH
 tr \
   'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' \
   'nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM'

diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index a3f0897a52ce2147388baeac6fc64d3b8501b516..88c5619ae7471ab0d3286259d88c437ae3953b4a 100755 (executable)
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -48,7 +48,7 @@ test_expect_success 'file is considered binary by plumbing' '
 
 test_expect_success 'setup textconv filters' '
        echo file diff=foo >.gitattributes &&
-       git config diff.foo.textconv "$PWD"/hexdump &&
+       git config diff.foo.textconv "\"$(pwd)\""/hexdump &&
        git config diff.fail.textconv false
 '
 

diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh
index a894c6062271c53b688830a3230d1c78fb561d70..7e7b307a24606131b4880817a0056af11973f3d2 100755 (executable)
--- a/t/t4031-diff-rewrite-binary.sh
+++ b/t/t4031-diff-rewrite-binary.sh
@@ -54,7 +54,7 @@ chmod +x dump
 
 test_expect_success 'setup textconv' '
        echo file diff=foo >.gitattributes &&
-       git config diff.foo.textconv "$PWD"/dump
+       git config diff.foo.textconv "\"$(pwd)\""/dump
 '
 
 test_expect_success 'rewrite diff respects textconv' '