{"thread":{"id":"26221","subject":"[PATCH v2] alias: use run_command api to execute aliases","startedAt":"2011-01-06T19:54:09Z","lastAt":"2011-01-06T20:27:56Z","messageCount":4,"participants":["Erik Faye-Lund","Jonathan Nieder","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"159059","messageId":"1294343649-6272-1-git-send-email-kusmabite@gmail.com","threadId":"26221","inReplyTo":null,"subject":"[PATCH v2] alias: use run_command api to execute aliases","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-01-06T19:54:09Z","receivedAt":"2011-01-06T19:54:09Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Windows, system() executes with cmd.exe instead of /bin/sh. This\nmeans that aliases currently has to be batch-scripts instead of\nbourne-scripts. On top of that, cmd.exe does not handle single quotes,\nwhich is what the code-path currently uses to handle arguments with\nspaces.\n\nTo solve both problems in one go, use run_command_v_opt() to execute\nthe alias. It already does the right thing prepend \"sh -c \" to the\nalias.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n\nFixed the issues pointed out by Jonathan Nieder.\n\n git.c |   30 +++++++++++++++---------------\n 1 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex 68334f6..f3666a8 100644\n--- a/git.c\n+++ b/git.c\n@@ -177,22 +177,22 @@ static int handle_alias(int *argcp, const char ***argv)\n \talias_string = alias_lookup(alias_command);\n \tif (alias_string) {\n \t\tif (alias_string[0] == '!') {\n+\t\t\tconst char **alias_argv;\n+\t\t\tint argc = *argcp, i;\n+\n \t\t\tcommit_pager_choice();\n-\t\t\tif (*argcp > 1) {\n-\t\t\t\tstruct strbuf buf;\n-\n-\t\t\t\tstrbuf_init(&buf, PATH_MAX);\n-\t\t\t\tstrbuf_addstr(&buf, alias_string);\n-\t\t\t\tsq_quote_argv(&buf, (*argv) + 1, PATH_MAX);\n-\t\t\t\tfree(alias_string);\n-\t\t\t\talias_string = buf.buf;\n-\t\t\t}\n-\t\t\ttrace_printf(\"trace: alias to shell cmd: %s => %s\\n\",\n-\t\t\t\t     alias_command, alias_string + 1);\n-\t\t\tret = system(alias_string + 1);\n-\t\t\tif (ret >= 0 && WIFEXITED(ret) &&\n-\t\t\t    WEXITSTATUS(ret) != 127)\n-\t\t\t\texit(WEXITSTATUS(ret));\n+\n+\t\t\t/* build alias_argv */\n+\t\t\talias_argv = xmalloc(sizeof(char *) * argc + 1);\n+\t\t\talias_argv[0] = alias_string + 1;\n+\t\t\tfor (i = 1; i < argc; ++i)\n+\t\t\t\talias_argv[i] = (*argv)[i];\n+\t\t\talias_argv[argc] = NULL;\n+\n+\t\t\tret = run_command_v_opt(alias_argv, RUN_USING_SHELL);\n+\t\t\tif (ret >= 0)   /* normal exit */\n+\t\t\t\texit(ret);\n+\n \t\t\tdie(\"Failed to run '%s' when expanding alias '%s'\",\n \t\t\t    alias_string + 1, alias_command);\n \t\t}\n-- \n1.7.3.3.585.g74f6e\n"},{"id":"159060","messageId":"20110106200518.GA14945@burratino","threadId":"26221","inReplyTo":"1294343649-6272-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v2] alias: use run_command api to execute aliases","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-06T20:05:18Z","receivedAt":"2011-01-06T20:05:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Erik Faye-Lund wrote:\n\n> --- a/git.c\n> +++ b/git.c\n> @@ -177,22 +177,22 @@ static int handle_alias(int *argcp, const char ***argv)\n[...]\n> +\t\t\talias_argv = xmalloc(sizeof(char *) * argc + 1);\n\nMissed a spot?  (missing parens)\n\nThanks for this.  It also should provide a minor speedup in the case\nof commands with no shell metacharacters.\n"},{"id":"159061","messageId":"AANLkTi=aPjtfFVKX6XqHyZYVaT07ExnWVF140JJ==U+m@mail.gmail.com","threadId":"26221","inReplyTo":"20110106200518.GA14945@burratino","subject":"Re: [PATCH v2] alias: use run_command api to execute aliases","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-01-06T20:14:07Z","receivedAt":"2011-01-06T20:14:07Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Jan 6, 2011 9:05 PM, \"Jonathan Nieder\" <jrnieder@gmail.com> wrote:\n>\n> Erik Faye-Lund wrote:\n>\n> > --- a/git.c\n> > +++ b/git.c\n> > @@ -177,22 +177,22 @@ static int handle_alias(int *argcp, const char\n***argv)\n> [...]\n> > +                     alias_argv = xmalloc(sizeof(char *) * argc + 1);\n>\n> Missed a spot?  (missing parens)\n>\n\nBlergh... I guess that teaches me a lesson about sending out patches in a\nhurry. New version a bit later, then ;)\n\n> Thanks for this.  It also should provide a minor speedup in the case\n> of commands with no shell metacharacters.\n\nNice. Not intentional, but hey :)\n"},{"id":"159063","messageId":"201101062127.56597.j6t@kdbg.org","threadId":"26221","inReplyTo":"1294343649-6272-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v2] alias: use run_command api to execute aliases","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2011-01-06T20:27:56Z","receivedAt":"2011-01-06T20:27:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Donnerstag, 6. Januar 2011, Erik Faye-Lund wrote:\n> @@ -177,22 +177,22 @@ static int handle_alias(int *argcp, const char\n> ***argv) alias_string = alias_lookup(alias_command);\n>  \tif (alias_string) {\n>  \t\tif (alias_string[0] == '!') {\n> +\t\t\tconst char **alias_argv;\n> +\t\t\tint argc = *argcp, i;\n> +\n>  \t\t\tcommit_pager_choice();\n> -\t\t\tif (*argcp > 1) {\n> -\t\t\t\tstruct strbuf buf;\n> -\n> -\t\t\t\tstrbuf_init(&buf, PATH_MAX);\n> -\t\t\t\tstrbuf_addstr(&buf, alias_string);\n> -\t\t\t\tsq_quote_argv(&buf, (*argv) + 1, PATH_MAX);\n> -\t\t\t\tfree(alias_string);\n> -\t\t\t\talias_string = buf.buf;\n> -\t\t\t}\n> -\t\t\ttrace_printf(\"trace: alias to shell cmd: %s => %s\\n\",\n> -\t\t\t\t     alias_command, alias_string + 1);\n> -\t\t\tret = system(alias_string + 1);\n> -\t\t\tif (ret >= 0 && WIFEXITED(ret) &&\n> -\t\t\t    WEXITSTATUS(ret) != 127)\n> -\t\t\t\texit(WEXITSTATUS(ret));\n> +\n> +\t\t\t/* build alias_argv */\n> +\t\t\talias_argv = xmalloc(sizeof(char *) * argc + 1);\n> +\t\t\talias_argv[0] = alias_string + 1;\n> +\t\t\tfor (i = 1; i < argc; ++i)\n> +\t\t\t\talias_argv[i] = (*argv)[i];\n> +\t\t\talias_argv[argc] = NULL;\n> +\n> +\t\t\tret = run_command_v_opt(alias_argv, RUN_USING_SHELL);\n> +\t\t\tif (ret >= 0)   /* normal exit */\n> +\t\t\t\texit(ret);\n> +\n>  \t\t\tdie(\"Failed to run '%s' when expanding alias '%s'\",\n>  \t\t\t    alias_string + 1, alias_command);\n\nAt this point, errno has a meaning. I'd perhaps change this to:\n\n\t\t\tdie_errno(\"While expanding alias '%s': '%s'\",\n\t\t\t\talias_command, alias_string + 1);\n\n-- Hannes\n"}]}