{"thread":{"id":"20904","subject":"[PATCH 1/2] start_command: do not clobber cmd->env on Windows code path","startedAt":"2009-09-11T17:40:08Z","lastAt":"2009-09-12T04:32:22Z","messageCount":3,"participants":["Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"122903","messageId":"200909111940.08652.j6t@kdbg.org","threadId":"20904","inReplyTo":"1252560077-1725-1-git-send-email-snaury@gmail.com","subject":"[PATCH 1/2] start_command: do not clobber cmd->env on Windows code path","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-09-11T17:40:08Z","receivedAt":"2009-09-11T17:40:08Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Previously, it would not be possible to call start_command twice for the\nsame struct child_process that has env set.\n\nThe fix is achieved by moving the loop that modifies the environment block\ninto a helper function. This also allows us to make two other helper\nfunctions static.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n We don't call start_command twice in a row anywhere in git.git, but\n msysgit has a patch that does, and with the next patch the buglet would\n be triggered.\n\n Even after this patch, other members of *cmd are clobbered, so it could\n be argued that this patch is unnecessary, but at least the Windows code\n path now keeps the same members that the Unix code path keeps, and it\n makes start_command easier to read by moving a loop into a helper.\n\n -- Hannes\n\n compat/mingw.c |   16 ++++++++++++++--\n compat/mingw.h |    3 +--\n run-command.c  |    7 ++-----\n 3 files changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex bed4178..36ef8d3 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -824,7 +824,7 @@ void mingw_execvp(const char *cmd, char *const *argv)\n \tfree_path_split(path);\n }\n \n-char **copy_environ()\n+static char **copy_environ(void)\n {\n \tchar **env;\n \tint i = 0;\n@@ -861,7 +861,7 @@ static int lookup_env(char **env, const char *name, size_t nmln)\n /*\n  * If name contains '=', then sets the variable, otherwise it unsets it\n  */\n-char **env_setenv(char **env, const char *name)\n+static char **env_setenv(char **env, const char *name)\n {\n \tchar *eq = strchrnul(name, '=');\n \tint i = lookup_env(env, name, eq-name);\n@@ -886,6 +886,18 @@ char **env_setenv(char **env, const char *name)\n \treturn env;\n }\n \n+/*\n+ * Copies global environ and adjusts variables as specified by vars.\n+ */\n+char **make_augmented_environ(const char *const *vars)\n+{\n+\tchar **env = copy_environ();\n+\n+\twhile (*vars)\n+\t\tenv = env_setenv(env, *vars++);\n+\treturn env;\n+}\n+\n /* this is the first function to call into WS_32; initialize it */\n #undef gethostbyname\n struct hostent *mingw_gethostbyname(const char *host)\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 948de66..c43917c 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -222,9 +222,8 @@ void mingw_open_html(const char *path);\n  * helpers\n  */\n \n-char **copy_environ(void);\n+char **make_augmented_environ(const char *const *vars);\n void free_environ(char **env);\n-char **env_setenv(char **env, const char *name);\n \n /*\n  * A replacement of main() that ensures that argv[0] has a path\ndiff --git a/run-command.c b/run-command.c\nindex f3e7abb..ac314a5 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -173,11 +173,8 @@ fail_pipe:\n \n \tif (cmd->dir)\n \t\tdie(\"chdir in start_command() not implemented\");\n-\tif (cmd->env) {\n-\t\tenv = copy_environ();\n-\t\tfor (; *cmd->env; cmd->env++)\n-\t\t\tenv = env_setenv(env, *cmd->env);\n-\t}\n+\tif (cmd->env)\n+\t\tenv = make_augmented_environ(cmd->env);\n \n \tif (cmd->git_cmd) {\n \t\tcmd->argv = prepare_git_cmd(cmd->argv);\n-- \n1.6.5.rc0.28.gfb9b\n"},{"id":"122904","messageId":"200909111945.07842.j6t@kdbg.org","threadId":"20904","inReplyTo":"200909111940.08652.j6t@kdbg.org","subject":"[PATCH 2/2] pager: set LESS=FRSX also on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-09-11T17:45:07Z","receivedAt":"2009-09-11T17:45:07Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Previously, this environment variable was set in the pager_preexec\ncallback, which is conditionally-compiled only on Unix, because it is not,\nand cannot be, called on Windows.\n\nWith this patch the env member of struct child_process is used to set\nthe environment variable, which also works on Windows.\n\nNoticed by Alexey Borzenkov.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n Alexey,\n\n this will result in a conflict when you apply it to your msysgit because\n there is an extra patch that shuffled the setup of pager_process around.\n\n -- Hannes\n\n pager.c |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 4921843..f416d38 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -21,8 +21,6 @@ static void pager_preexec(void)\n \tFD_ZERO(&in);\n \tFD_SET(0, &in);\n \tselect(1, &in, NULL, &in, NULL);\n-\n-\tsetenv(\"LESS\", \"FRSX\", 0);\n }\n #endif\n \n@@ -70,6 +68,10 @@ void setup_pager(void)\n \tpager_argv[2] = pager;\n \tpager_process.argv = pager_argv;\n \tpager_process.in = -1;\n+\tif (!getenv(\"LESS\")) {\n+\t\tstatic const char *env[] = { \"LESS=FRSX\", NULL };\n+\t\tpager_process.env = env;\n+\t}\n #ifndef __MINGW32__\n \tpager_process.preexec_cb = pager_preexec;\n #endif\n-- \n1.6.5.rc0.28.gfb9b\n"},{"id":"122931","messageId":"7vws44lr55.fsf@alter.siamese.dyndns.org","threadId":"20904","inReplyTo":"200909111940.08652.j6t@kdbg.org","subject":"Re: [PATCH 1/2] start_command: do not clobber cmd->env on Windows code path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-12T04:32:22Z","receivedAt":"2009-09-12T04:32:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\nThanks; both patches applied.\n"}]}