Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

really don't cache environment variables

As of c67941 "don't cache environment variables, fix non-ASCII environment
variables", git-cheetah fails to show context menu items unless the git
installation directory is in the PATH.

This is due to mingw_spawnvpe_cwd using getenv("PATH") internally, which is
the state of the PATH variable as cached by MSVCRT on startup.

To allow for environment changes in the hosting exporer process, we shall
not use any cached variant of the environment.

Reimplement mingw_getenv to access the Win32 environment directly (instead
of MSVCRT's cached version).

Update fork_process to use the improved non-caching getenv instead of the
more complex Win32 function.

Also add a minimal setenv implementation and use it in fork_process, so
that we don't mix POSIX and Win32 APIs.

Reported-by: Peter Oberndorfer <kumbayo84@gmail.com>
Signed-off-by: Karsten Blees <blees@dcon.de>
  • Loading branch information...
commit c8dc84566a06069d9b04bbf40e7cffe8889e6048 1 parent 703df3f
Karsten Blees kblees authored
Showing with 49 additions and 11 deletions.
  1. +41 −4 compat/mingw.c
  2. +3 −0  compat/mingw.h
  3. +5 −7 explorer/systeminfo.c
45 compat/mingw.c
View
@@ -1,5 +1,6 @@
#include "../common/git-compat-util.h"
#include "../common/strbuf.h"
+#include "../common/cache.h"
unsigned int _CRT_fmode = _O_BINARY;
@@ -369,19 +370,55 @@ char *mingw_getcwd(char *pointer, int len)
return ret;
}
-#undef getenv
+/*
+ * Use GetEnvironmentVariable() instead of getenv() to see updates to the
+ * hosting explorer process's environment (e.g. PATH). MSVCRT's getenv() is
+ * just a copy which may not be up to date.
+ *
+ * This implementation is *not* thread safe. This is not a problem as the
+ * git-cheetah DLL is appartment threaded, so all calls should be made from
+ * the same COM thread.
+ *
+ * Additionally, each call overwrites the previous return value (i.e. use
+ * strdup(getenv(...)) if necessary).
+ *
+ * Note: both of these limitations are POSIX compliant.
+ */
+static char *do_getenv(const char *name)
+{
+ static char *value;
+ static unsigned value_len;
+ unsigned len = GetEnvironmentVariableA(name, NULL, 0);
+ if (!len)
+ return NULL;
+ ALLOC_GROW(value, len, value_len);
+ if (!GetEnvironmentVariableA(name, value, value_len))
+ return NULL;
+ return value;
+}
+
char *mingw_getenv(const char *name)
{
- char *result = getenv(name);
+ char *result = do_getenv(name);
if (!result && !strcmp(name, "TMPDIR")) {
/* on Windows it is TMP and TEMP */
- result = getenv("TMP");
+ result = do_getenv("TMP");
if (!result)
- result = getenv("TEMP");
+ result = do_getenv("TEMP");
}
return result;
}
+int mingw_setenv(const char *name, const char *value, int overwrite)
+{
+ if (!overwrite && GetEnvironmentVariableA(name, NULL, 0))
+ return 0;
+ if (SetEnvironmentVariableA(name, value))
+ return 0;
+ errno = EINVAL;
+ return -1;
+}
+
/*
* See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx
* (Parsing C++ Command-Line Arguments)
3  compat/mingw.h
View
@@ -154,6 +154,9 @@ char *mingw_getcwd(char *pointer, int len);
char *mingw_getenv(const char *name);
#define getenv mingw_getenv
+int mingw_setenv(const char *name, const char *value, int overwrite);
+#define setenv mingw_setenv
+
struct hostent *mingw_gethostbyname(const char *host);
#define gethostbyname mingw_gethostbyname
12 explorer/systeminfo.c
View
@@ -109,17 +109,15 @@ pid_t fork_process(const char *cmd, const char **args, const char *wd)
/* if gitpath is set, temporarily set PATH=<gitpath>;%PATH% */
if (gitpath) {
- int len;
struct strbuf path = STRBUF_INIT;
strbuf_addstr(&path, gitpath);
- len = GetEnvironmentVariable("PATH", NULL, 0);
- if (len) {
- oldpath = malloc(len);
- GetEnvironmentVariable("PATH", oldpath, len);
+ oldpath = getenv("PATH");
+ if (oldpath) {
+ oldpath = xstrdup(oldpath);
strbuf_addch(&path, ';');
strbuf_addstr(&path, oldpath);
}
- SetEnvironmentVariable("PATH", path.buf);
+ setenv("PATH", path.buf, 1);
strbuf_release(&path);
}
@@ -128,7 +126,7 @@ pid_t fork_process(const char *cmd, const char **args, const char *wd)
/* reset PATH to previous value */
if (gitpath) {
- SetEnvironmentVariable("PATH", oldpath);
+ setenv("PATH", oldpath, 1);
free(oldpath);
}
return result;
Please sign in to comment.
Something went wrong with that request. Please try again.