From 6c47fe06c186c3f3216e514e278e852b360a6e09 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Tue, 16 Apr 2019 10:33:48 +0200 Subject: [PATCH 1/2] Possible fix for the wrong git being picked up in some cases Even though environment variable keys are case-insensitive, they're exposed via a hashset that is case-sensitive. This means that in some cases, when the environment variable is named `Path` for eg, setting PATH=something causes a duplication of the keys Path/PATH, and the actual original environment variable that the system reads is not updated. WTF. This change looks up environment variables by a case insensitive search. It might fix #1044, since that problem is being caused by the wrong git being used in some operations, so I'm going to take a wild guess that maybe in those cases the wrong environment variable is getting read. --- src/GitHub.Api/Platform/DefaultEnvironment.cs | 22 +++++++++++++++---- src/GitHub.Api/Platform/IEnvironment.cs | 1 + src/GitHub.Api/Platform/ProcessEnvironment.cs | 12 +++++----- .../IntegrationTestEnvironment.cs | 17 ++++++++++++-- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/GitHub.Api/Platform/DefaultEnvironment.cs b/src/GitHub.Api/Platform/DefaultEnvironment.cs index 2caede5a5..c99bf3b68 100644 --- a/src/GitHub.Api/Platform/DefaultEnvironment.cs +++ b/src/GitHub.Api/Platform/DefaultEnvironment.cs @@ -1,5 +1,6 @@ using GitHub.Logging; using System; +using System.Globalization; using System.IO; using System.Linq; @@ -116,12 +117,25 @@ public string GetSpecialFolder(Environment.SpecialFolder folder) public string ExpandEnvironmentVariables(string name) { - return Environment.ExpandEnvironmentVariables(name); + var key = GetEnvironmentVariableKey(name); + return Environment.ExpandEnvironmentVariables(key); } - public string GetEnvironmentVariable(string variable) + public string GetEnvironmentVariable(string name) { - return Environment.GetEnvironmentVariable(variable); + var key = GetEnvironmentVariableKey(name); + return Environment.GetEnvironmentVariable(key); + } + + public string GetEnvironmentVariableKey(string name) + { + return GetEnvironmentVariableKeyInternal(name); + } + + private static string GetEnvironmentVariableKeyInternal(string name) + { + return Environment.GetEnvironmentVariables().Keys.Cast() + .FirstOrDefault(k => string.Compare(name, k, true, CultureInfo.InvariantCulture) == 0); } public NPath LogPath { get; } @@ -134,7 +148,7 @@ public string GetEnvironmentVariable(string variable) public NPath ExtensionInstallPath { get; set; } public NPath UserCachePath { get; set; } public NPath SystemCachePath { get; set; } - public string Path { get; set; } = Environment.GetEnvironmentVariable("PATH"); + public string Path { get; set; } = Environment.GetEnvironmentVariable(GetEnvironmentVariableKeyInternal("PATH")); public string NewLine => Environment.NewLine; public NPath OctorunScriptPath diff --git a/src/GitHub.Api/Platform/IEnvironment.cs b/src/GitHub.Api/Platform/IEnvironment.cs index df543ab62..f21c22eab 100644 --- a/src/GitHub.Api/Platform/IEnvironment.cs +++ b/src/GitHub.Api/Platform/IEnvironment.cs @@ -41,5 +41,6 @@ public interface IEnvironment ISettings LocalSettings { get; } ISettings SystemSettings { get; } ISettings UserSettings { get; } + string GetEnvironmentVariableKey(string name); } } diff --git a/src/GitHub.Api/Platform/ProcessEnvironment.cs b/src/GitHub.Api/Platform/ProcessEnvironment.cs index 685a07b60..bf97546cb 100644 --- a/src/GitHub.Api/Platform/ProcessEnvironment.cs +++ b/src/GitHub.Api/Platform/ProcessEnvironment.cs @@ -1,5 +1,4 @@ using GitHub.Logging; -using System; using System.Collections.Generic; using System.Diagnostics; @@ -24,11 +23,12 @@ public void Configure(ProcessStartInfo psi, NPath workingDirectory, bool dontSet var path = Environment.Path; psi.EnvironmentVariables["GHU_WORKINGDIR"] = workingDirectory; + var pathEnvVarKey = Environment.GetEnvironmentVariableKey("PATH"); if (dontSetupGit) { psi.EnvironmentVariables["GHU_FULLPATH"] = path; - psi.EnvironmentVariables["PATH"] = path; + psi.EnvironmentVariables[pathEnvVarKey] = path; return; } @@ -87,10 +87,10 @@ public void Configure(ProcessStartInfo psi, NPath workingDirectory, bool dontSet pathEntries.Add("END"); - path = String.Join(separator, pathEntries.ToArray()) + separator + path; + path = string.Join(separator, pathEntries.ToArray()) + separator + path; psi.EnvironmentVariables["GHU_FULLPATH"] = path; - psi.EnvironmentVariables["PATH"] = path; + psi.EnvironmentVariables[pathEnvVarKey] = path; //TODO: Remove with Git LFS Locking becomes standard psi.EnvironmentVariables["GITLFSLOCKSENABLED"] = "1"; @@ -102,11 +102,11 @@ public void Configure(ProcessStartInfo psi, NPath workingDirectory, bool dontSet } var httpProxy = Environment.GetEnvironmentVariable("HTTP_PROXY"); - if (!String.IsNullOrEmpty(httpProxy)) + if (!string.IsNullOrEmpty(httpProxy)) psi.EnvironmentVariables["HTTP_PROXY"] = httpProxy; var httpsProxy = Environment.GetEnvironmentVariable("HTTPS_PROXY"); - if (!String.IsNullOrEmpty(httpsProxy)) + if (!string.IsNullOrEmpty(httpsProxy)) psi.EnvironmentVariables["HTTPS_PROXY"] = httpsProxy; psi.EnvironmentVariables["DISPLAY"] = "0"; } diff --git a/src/tests/IntegrationTests/IntegrationTestEnvironment.cs b/src/tests/IntegrationTests/IntegrationTestEnvironment.cs index eb65b8187..d184bd672 100644 --- a/src/tests/IntegrationTests/IntegrationTestEnvironment.cs +++ b/src/tests/IntegrationTests/IntegrationTestEnvironment.cs @@ -1,5 +1,6 @@ using System; using System.Globalization; +using System.Linq; using GitHub.Unity; using GitHub.Logging; @@ -60,7 +61,7 @@ public void InitializeRepository(NPath? expectedPath = null) public string ExpandEnvironmentVariables(string name) { - return name; + return defaultEnvironment.ExpandEnvironmentVariables(name); } public string GetEnvironmentVariable(string v) @@ -73,6 +74,18 @@ public string GetEnvironmentVariable(string v) return environmentVariable; } + + public string GetEnvironmentVariableKey(string name) + { + return GetEnvironmentVariableKeyInternal(name); + } + + private static string GetEnvironmentVariableKeyInternal(string name) + { + return Environment.GetEnvironmentVariables().Keys.Cast() + .FirstOrDefault(k => string.Compare(name, k, true, CultureInfo.InvariantCulture) == 0); + } + public string GetSpecialFolder(Environment.SpecialFolder folder) { var ensureDirectoryExists = UserCachePath.Parent.EnsureDirectoryExists(folder.ToString()); @@ -88,7 +101,7 @@ public string GetSpecialFolder(Environment.SpecialFolder folder) public string UserProfilePath => UserCachePath.Parent.CreateDirectory("user profile path"); - public string Path { get; set; } = Environment.GetEnvironmentVariable("PATH").ToNPath(); + public string Path { get; set; } = Environment.GetEnvironmentVariable(GetEnvironmentVariableKeyInternal("PATH")); public string NewLine => Environment.NewLine; public string UnityVersion => "5.6"; From 4d0e15a40d112f9349e8cbce65ae515334175df3 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Tue, 16 Apr 2019 10:45:57 +0200 Subject: [PATCH 2/2] Oh yeah, should probably not blow up here --- src/GitHub.Api/Platform/DefaultEnvironment.cs | 2 +- src/tests/IntegrationTests/IntegrationTestEnvironment.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/GitHub.Api/Platform/DefaultEnvironment.cs b/src/GitHub.Api/Platform/DefaultEnvironment.cs index c99bf3b68..eece16e48 100644 --- a/src/GitHub.Api/Platform/DefaultEnvironment.cs +++ b/src/GitHub.Api/Platform/DefaultEnvironment.cs @@ -135,7 +135,7 @@ public string GetEnvironmentVariableKey(string name) private static string GetEnvironmentVariableKeyInternal(string name) { return Environment.GetEnvironmentVariables().Keys.Cast() - .FirstOrDefault(k => string.Compare(name, k, true, CultureInfo.InvariantCulture) == 0); + .FirstOrDefault(k => string.Compare(name, k, true, CultureInfo.InvariantCulture) == 0) ?? name; } public NPath LogPath { get; } diff --git a/src/tests/IntegrationTests/IntegrationTestEnvironment.cs b/src/tests/IntegrationTests/IntegrationTestEnvironment.cs index d184bd672..4690ac6df 100644 --- a/src/tests/IntegrationTests/IntegrationTestEnvironment.cs +++ b/src/tests/IntegrationTests/IntegrationTestEnvironment.cs @@ -77,13 +77,13 @@ public string GetEnvironmentVariable(string v) public string GetEnvironmentVariableKey(string name) { - return GetEnvironmentVariableKeyInternal(name); + return defaultEnvironment.GetEnvironmentVariableKey(name); } private static string GetEnvironmentVariableKeyInternal(string name) { return Environment.GetEnvironmentVariables().Keys.Cast() - .FirstOrDefault(k => string.Compare(name, k, true, CultureInfo.InvariantCulture) == 0); + .FirstOrDefault(k => string.Compare(name, k, true, CultureInfo.InvariantCulture) == 0) ?? name; } public string GetSpecialFolder(Environment.SpecialFolder folder)