Merge pull request #118 from github-for-unity/fixes/settings-should-save-git-exec-path

Fix to load git path from settings

Author: shana

src/GitHub.Api/Application/ApplicationManagerBase.cs

@@ -52,20 +52,10 @@ public virtual async Task Run(bool firstRun)
             }
             else
             {
-                var progress = new ProgressReport();
-
-                var gitClient = new GitClient(Environment, ProcessManager, Platform.CredentialManager, TaskManager);
-                var gitSetup = new GitInstaller(Environment, CancellationToken);
-                var expectedPath = gitSetup.GitInstallationPath;
-                var setupDone = await gitSetup.SetupIfNeeded(progress.Percentage, progress.Remaining);
-                if (setupDone)
-                    Environment.GitExecutablePath = gitSetup.GitExecutablePath;
-                else
-                    Environment.GitExecutablePath = await LookForGitInstallationPath(gitClient, SystemSettings).SafeAwait();
-
-                GitClient = gitClient;
+                GitClient = new GitClient(Environment, ProcessManager, Platform.CredentialManager, TaskManager);
+                Environment.GitExecutablePath = await DetermineGitExecutablePath();
 
-                Logger.Trace("Environment.GitExecutablePath \"{0}\" Exists:{1}", gitSetup.GitExecutablePath, gitSetup.GitExecutablePath.FileExists());
+                Logger.Trace("Environment.GitExecutablePath \"{0}\" Exists:{1}", Environment.GitExecutablePath, Environment.GitExecutablePath.FileExists());
 
                 if (Environment.IsWindows)
                 {
@@ -164,19 +154,25 @@ private async Task LoadKeychain()
             }
         }
 
-        private static async Task<NPath> LookForGitInstallationPath(IGitClient gitClient, ISettings systemSettings)
+        private async Task<NPath> DetermineGitExecutablePath(ProgressReport progress = null)
         {
-            NPath cachedGitInstallPath = null;
-            var path = systemSettings.Get(Constants.GitInstallPathKey);
-            if (!String.IsNullOrEmpty(path))
-                cachedGitInstallPath = path.ToNPath();
+            var gitExecutablePath = SystemSettings.Get(Constants.GitInstallPathKey)?.ToNPath();
+            if (gitExecutablePath != null && gitExecutablePath.FileExists())
+            {
+                Logger.Trace("Using git install path from settings");
+                return gitExecutablePath;
+            }
 
-            // Root paths
-            if (cachedGitInstallPath != null && cachedGitInstallPath.DirectoryExists())
+            var gitInstaller = new GitInstaller(Environment, CancellationToken);
+            var setupDone = await gitInstaller.SetupIfNeeded(progress?.Percentage, progress?.Remaining);
+            if (setupDone)
             {
-                return cachedGitInstallPath;
+                Logger.Trace("Setup performed using new path");
+                return gitInstaller.GitExecutablePath;
             }
-            return await gitClient.FindGitInstallation();
+
+            Logger.Trace("Finding git install path");
+            return await GitClient.FindGitInstallation().SafeAwait();
         }
 
         protected void SetupMetrics(string unityVersion, bool firstRun)

src/GitHub.Api/GitHub.Api.csproj

@@ -145,7 +145,7 @@
     <Compile Include="Installer\GitInstaller.cs" />
     <Compile Include="Installer\ZipHelper.cs" />
     <Compile Include="Extensions\AsyncExtensions.cs" />
-    <Compile Include="Platform\LocalSettings.cs" />
+    <Compile Include="Platform\Settings.cs" />
     <Compile Include="OutputProcessors\BranchListOutputProcessor.cs" />
     <Compile Include="OutputProcessors\LockOutputProcessor.cs" />
     <Compile Include="OutputProcessors\LogEntryOutputProcessor.cs" />

src/GitHub.Api/Platform/LocalSettings.cs src/GitHub.Api/Platform/Settings.cssrc/GitHub.Api/Platform/LocalSettings.cs renamed to src/GitHub.Api/Platform/Settings.cs

@@ -44,9 +44,10 @@ public JsonBackedSettings()
 
         public override void Initialize()
         {
-            logger.Debug($"Initializing settings file {SettingsPath} {GetType()}");
+            logger.Debug($"Initializing settings {GetType()}");
             cachePath = SettingsPath.Combine(SettingsFileName);
-            logger.Trace("Initializing settings file at {0}", cachePath);
+
+            logger.Debug("Initializing settings file at {0}", cachePath);
             LoadFromCache(cachePath);
         }
 
@@ -65,19 +66,31 @@ public override string Get(string key, string fallback = "")
             object value = null;
             if (cacheData.GitHubUnity.TryGetValue(key, out value))
             {
+                logger.Debug("Get: {0}", key);
                 return (T)value;
             }
 
+            logger.Debug("Miss: {0}", key);
             return fallback;
         }
 
         public override void Set<T>(string key, T value)
         {
-            if (!cacheData.GitHubUnity.ContainsKey(key))
-                cacheData.GitHubUnity.Add(key, value);
-            else
-                cacheData.GitHubUnity[key] = value;
-            SaveToCache(cachePath);
+            try
+            {
+                logger.Trace("Set: {0}", key);
+
+                if (!cacheData.GitHubUnity.ContainsKey(key))
+                    cacheData.GitHubUnity.Add(key, value);
+                else
+                    cacheData.GitHubUnity[key] = value;
+                SaveToCache(cachePath);
+            }
+            catch (Exception e)
+            {
+                logger.Error(e, "Error storing to cache");
+                throw;
+            }
         }
 
         public override void Unset(string key)

src/UnityExtension/Assets/Editor/GitHub.Unity/UI/SettingsView.cs

@@ -607,6 +607,9 @@ private void OnInstallPathGUI()
             }
             if (EditorGUI.EndChangeCheck())
             {
+                Logger.Trace("Setting GitExecPath: " + gitExecPath);
+
+                Manager.SystemSettings.Set(Constants.GitInstallPathKey, gitExecPath);
                 Environment.GitExecutablePath = gitExecPath.ToNPath();
             }