{"thread":{"id":"27552","subject":"[PATCH 0/3] sh-i18n--envsubst; case-sensitive getenv on Windows","startedAt":"2011-06-06T06:57:41Z","lastAt":"2011-10-05T16:21:14Z","messageCount":5,"participants":["Johannes Sixt","Karsten Blees"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"169336","messageId":"4DEC7A65.7020207@viscovery.net","threadId":"27552","inReplyTo":null,"subject":"[PATCH 0/3] sh-i18n--envsubst; case-sensitive getenv on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-06-06T06:57:41Z","receivedAt":"2011-06-06T06:57:41Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nHere are the fixes needed on Windows for the ab/i18n-scripts topic.\n\nJohannes Sixt (3):\n sh-i18n--envsubst: do not crash when no arguments are given\n mingw.c: move definition of mingw_getenv down\n Windows: teach getenv to do a case-sensitive search\n\n compat/mingw.c | 47 ++++++++++++++++++++++++++++++++---------------\n sh-i18n--envsubst.c | 1 +\n 2 files changed, 33 insertions(+), 15 deletions(-)\n\n-- \n1.7.6.rc0.1186.gfb4fd\n"},{"id":"169337","messageId":"4DEC7ABE.6070000@viscovery.net","threadId":"27552","inReplyTo":"4DEC7A65.7020207@viscovery.net","subject":"[PATCH 1/3] sh-i18n--envsubst: do not crash when no arguments are given","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-06-06T06:59:10Z","receivedAt":"2011-06-06T06:59:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nSigned-off-by: Johannes Sixt \n---\n sh-i18n--envsubst.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c\nindex 2eb0ee4..47a306f 100644\n--- a/sh-i18n--envsubst.c\n+++ b/sh-i18n--envsubst.c\n@@ -74,6 +74,7 @@ main (int argc, char *argv[])\n \t{\n \tcase 1:\n \t error (\"we won't substitute all variables on stdin for you\");\n+\t break;\n \t /*\n \t all_variables = 1;\n subst_from_stdin ();\n-- \n1.7.6.rc0.1186.gfb4fd\n"},{"id":"169338","messageId":"4DEC7C5A.8020201@viscovery.net","threadId":"27552","inReplyTo":"4DEC7A65.7020207@viscovery.net","subject":"[PATCH 2/3] mingw.c: move definition of mingw_getenv down","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-06-06T07:06:02Z","receivedAt":"2011-06-06T07:06:02Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nWe want to use static lookup_env() in a subsequent change.\n\nAt first sight, this change looks innocent. But it is not due to the\n#undef getenv. There is one caller of getenv between the old location and\nthe new location whose behavior could change. But as can be seen from the\ndefintion of mingw_getenv, the behavior for this caller does not change\nsubstantially.\n\nTo ensure consistent behavior in the future, change all getenv callers\nin mingw.c to use mingw_getenv.\n\nWith this patch, this is not a big deal, yet, but with the subsequent\nchange, where we teach getenv to do a case-sensitive lookup, the behavior\nof all call sites is changed.\n\nSigned-off-by: Johannes Sixt \n---\n Erik, Dscho,\n\n the 4msysgit tree has additional calls of getenv in mingw.c. You might\n want to update them to call mingw_getenv() instead.\n\n But then not doing so would not have a dramatic effect. It is similar\n to GIT_ASK_YESNO: People can still set Git_Ask_YesNo, and it will\n work in the same way as usual, *unless* they also have GIT_ASK_YESNO\n set as well (and with a different value).\n\n compat/mingw.c | 30 +++++++++++++++---------------\n 1 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex e085e8b..ee480f9 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -178,7 +178,7 @@ static int ask_yes_no_if_possible(const char *format, ...)\n \tvsnprintf(question, sizeof(question), format, args);\n \tva_end(args);\n \n-\tif ((retry_hook[0] = getenv(\"GIT_ASK_YESNO\"))) {\n+\tif ((retry_hook[0] = mingw_getenv(\"GIT_ASK_YESNO\"))) {\n \t\tretry_hook[1] = question;\n \t\treturn !run_command_v_opt(retry_hook, 0);\n \t}\n@@ -599,19 +599,6 @@ char *mingw_getcwd(char *pointer, int len)\n \treturn ret;\n }\n \n-#undef getenv\n-char *mingw_getenv(const char *name)\n-{\n-\tchar *result = getenv(name);\n-\tif (!result && !strcmp(name, \"TMPDIR\")) {\n-\t\t/* on Windows it is TMP and TEMP */\n-\t\tresult = getenv(\"TMP\");\n-\t\tif (!result)\n-\t\t\tresult = getenv(\"TEMP\");\n-\t}\n-\treturn result;\n-}\n-\n /*\n * See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx\n * (Parsing C++ Command-Line Arguments)\n@@ -711,7 +698,7 @@ static const char *parse_interpreter(const char *cmd)\n */\n static char **get_path_split(void)\n {\n-\tchar *p, **path, *envpath = getenv(\"PATH\");\n+\tchar *p, **path, *envpath = mingw_getenv(\"PATH\");\n \tint i, n = 0;\n \n \tif (!envpath || !*envpath)\n@@ -1128,6 +1115,19 @@ char **make_augmented_environ(const char *const *vars)\n \treturn env;\n }\n \n+#undef getenv\n+char *mingw_getenv(const char *name)\n+{\n+\tchar *result = getenv(name);\n+\tif (!result && !strcmp(name, \"TMPDIR\")) {\n+\t\t/* on Windows it is TMP and TEMP */\n+\t\tresult = getenv(\"TMP\");\n+\t\tif (!result)\n+\t\t\tresult = getenv(\"TEMP\");\n+\t}\n+\treturn result;\n+}\n+\n /*\n * Note, this isn't a complete replacement for getaddrinfo. It assumes\n * that service contains a numerical port, or that it is null. It\n-- \n1.7.6.rc0.1186.gfb4fd\n"},{"id":"169339","messageId":"4DEC7CDD.10403@viscovery.net","threadId":"27552","inReplyTo":"4DEC7A65.7020207@viscovery.net","subject":"[PATCH 3/3] Windows: teach getenv to do a case-sensitive search","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-06-06T07:08:13Z","receivedAt":"2011-06-06T07:08:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\ngetenv() on Windows looks up environment variables in a case-insensitive\nmanner. Even though all documentations claim that the environment is\ncase-insensitive, it is possible for applications to pass an environment\nto child processes that has variables that differ only in case. Bash on\nWindows does this, for example, and sh-i18n--envsubst depends on this\nbehavior.\n\nWith this patch environment variables are first looked up in a\ncase-sensitive manner; only if this finds nothing, the system's getenv() is\nused as a fallback.\n\nSigned-off-by: Johannes Sixt \n---\n compat/mingw.c | 23 ++++++++++++++++++++---\n 1 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex ee480f9..6e5af32 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1116,14 +1116,31 @@ char **make_augmented_environ(const char *const *vars)\n }\n \n #undef getenv\n+\n+/*\n+ * The system's getenv looks up the name in a case-insensitive manner.\n+ * This version tries a case-sensitive lookup and falls back to\n+ * case-insensitive if nothing was found. This is necessary because,\n+ * as a prominent example, CMD sets 'Path', but not 'PATH'.\n+ * Warning: not thread-safe.\n+ */\n+static char *getenv_cs(const char *name)\n+{\n+\tsize_t len = strlen(name);\n+\tint i = lookup_env(environ, name, len);\n+\tif (i >= 0)\n+\t\treturn environ[i] + len + 1;\t/* skip past name and '=' */\n+\treturn getenv(name);\n+}\n+\n char *mingw_getenv(const char *name)\n {\n-\tchar *result = getenv(name);\n+\tchar *result = getenv_cs(name);\n \tif (!result && !strcmp(name, \"TMPDIR\")) {\n \t\t/* on Windows it is TMP and TEMP */\n-\t\tresult = getenv(\"TMP\");\n+\t\tresult = getenv_cs(\"TMP\");\n \t\tif (!result)\n-\t\t\tresult = getenv(\"TEMP\");\n+\t\t\tresult = getenv_cs(\"TEMP\");\n \t}\n \treturn result;\n }\n-- \n1.7.6.rc0.1186.gfb4fd\n"},{"id":"176933","messageId":"loom.20111005T175934-633@post.gmane.org","threadId":"27552","inReplyTo":"4DEC7CDD.10403@viscovery.net","subject":"Re: [PATCH 3/3] Windows: teach getenv to do a case-sensitive search","fromName":"Karsten Blees","fromEmail":"blees@dcon.de","sentAt":"2011-10-05T16:21:14Z","receivedAt":"2011-10-05T16:21:14Z","isPatch":true,"sender":{"key":"karsten.blees@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1111200?v=4"},"body":"Hmm...this looks like a pretty fragile solution to me. Wouldn't it be simpler\nand safer to just fix the conflicting variables, instead of inventing entirely\nnew environment semantics for Windows?\n\nI looked at the eval_gettext occurences, and only found '$path' in\ngit-submodule.sh to obviously conflict with existing environment variables. So\nthe straightforward solution IMO would be to fix the variable in that script.\n\nSmall solution (only affects gettext): in git-submodule.sh, replace all\n'eval_gettext...$path' with 'modulepath=$path eval_gettext...$modulepath'\n\nBig solution (enables git-submodule-foreach scripts on Windows, but is a\nbreaking change for existing foreach scripts on Unix): in git-submodule.sh,\nt/t7407-submodule-foreach.sh, Documentation/git-submodule.txt, replace all\n'$path' with '$modulepath' (also a few 'path=...' and 'while read...path')\n\n\nJust a few failure scenarios that come to mind with the current solution:\n- Given environment variables \"Path\" and \"path\", the 'case-sensitive first'\napproach works fine for 'getenv(\"path\")', but 'getenv(\"PATH\")' still has a 50%\nchance of failure.\n- The other environment functions have not been changed to reflect the\n'case-sensitive first' logic: setenv(\"path\"...) and setenv(\"PATH\"...) both have\na chance of overwriting the wrong entry, same for putenv.\n- Windows applications generally don't support case-sensitive environment\nvariables, e.g. all MSYS and Cygwin programs convert environment variable names\nto upper case on startup, eliminating duplicates in the process. With git.exe\nbeeing the only case-sensitive tool, any change to git-sh-i18n.sh (e.g.\nreplacing git-envsubst with a real gettext-envsubst) is likely to break again.\n- As you already mentioned, Windows doesn't support case-sensitive environment\nvariable names, MSDN is pretty clear on that. Expressly violating this\ndocumentation may cease to work with any new Windows version or patch.\n\nCheers,\nKarsten\n"}]}