From fd531f4f4cb2a53ef8812e9bd1371a1290dbe02c Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Wed, 28 Mar 2018 11:29:53 -0400 Subject: [PATCH 1/5] ValidateGitInstall may fail in error --- src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs index b2031f83e..2705c008f 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs @@ -216,7 +216,7 @@ private void ValidateAndSetGitInstallPath(string value) gitVersionErrorMessage = null; GitClient.ValidateGitInstall(value.ToNPath()) - .ThenInUI((sucess, result) => + .FinallyInUI((sucess, exception, result) => { if (!sucess) { From 53529ed43f69c2504b091b0c3bb2076ac8844422 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Wed, 28 Mar 2018 13:12:13 -0400 Subject: [PATCH 2/5] Capturing the situation where versions are not returned at all --- .../Editor/GitHub.Unity/UI/GitPathView.cs | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs index 2705c008f..2366f7249 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs @@ -15,7 +15,9 @@ class GitPathView : Subview private const string BrowseButton = "..."; private const string GitInstallBrowseTitle = "Select git binary"; private const string ErrorInvalidPathMessage = "Invalid Path."; - private const string ErrorGettingSoftwareVersionMessage = "Error getting software versions."; + private const string ErrorValidatingGitPath = "Error validating Git Path."; + private const string ErrorGitNotFoundMessage = "Git not found."; + private const string ErrorGitLfsNotFoundMessage = "Git LFS not found."; private const string ErrorMinimumGitVersionMessageFormat = "Git version {0} found. Git version {1} is required."; private const string ErrorMinimumGitLfsVersionMessageFormat = "Git LFS version {0} found. Git LFS version {1} is required."; @@ -220,8 +222,8 @@ private void ValidateAndSetGitInstallPath(string value) { if (!sucess) { - Logger.Trace(ErrorGettingSoftwareVersionMessage); - gitVersionErrorMessage = ErrorGettingSoftwareVersionMessage; + Logger.Trace(ErrorValidatingGitPath); + gitVersionErrorMessage = ErrorValidatingGitPath; } else if (!result.IsValid) { @@ -232,21 +234,32 @@ private void ValidateAndSetGitInstallPath(string value) var errorMessageStringBuilder = new StringBuilder(); - if (result.GitVersion < Constants.MinimumGitVersion) + if (result.GitVersion == null) { - errorMessageStringBuilder.AppendFormat(ErrorMinimumGitVersionMessageFormat, - result.GitVersion, Constants.MinimumGitVersion); + errorMessageStringBuilder.Append(ErrorGitNotFoundMessage); } - - if (result.GitLfsVersion < Constants.MinimumGitLfsVersion) + else if (result.GitLfsVersion == null) + { + errorMessageStringBuilder.Append(ErrorGitLfsNotFoundMessage); + } + else { - if (errorMessageStringBuilder.Length > 0) + if (result.GitVersion < Constants.MinimumGitVersion) { - errorMessageStringBuilder.Append(Environment.NewLine); + errorMessageStringBuilder.AppendFormat(ErrorMinimumGitVersionMessageFormat, + result.GitVersion, Constants.MinimumGitVersion); } - errorMessageStringBuilder.AppendFormat(ErrorMinimumGitLfsVersionMessageFormat, - result.GitLfsVersion, Constants.MinimumGitLfsVersion); + if (result.GitLfsVersion < Constants.MinimumGitLfsVersion) + { + if (errorMessageStringBuilder.Length > 0) + { + errorMessageStringBuilder.Append(Environment.NewLine); + } + + errorMessageStringBuilder.AppendFormat(ErrorMinimumGitLfsVersionMessageFormat, + result.GitLfsVersion, Constants.MinimumGitLfsVersion); + } } gitVersionErrorMessage = errorMessageStringBuilder.ToString(); From 25a83b695eda029775a388f6bb79630ad2eda70d Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Wed, 28 Mar 2018 14:19:38 -0400 Subject: [PATCH 3/5] Structuring tasks to bail early --- src/GitHub.Api/Git/GitClient.cs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/GitHub.Api/Git/GitClient.cs b/src/GitHub.Api/Git/GitClient.cs index e3f8f5b7b..c7439b504 100644 --- a/src/GitHub.Api/Git/GitClient.cs +++ b/src/GitHub.Api/Git/GitClient.cs @@ -120,18 +120,29 @@ public ITask ValidateGitInstall(NPath path) Version gitVersion = null; Version gitLfsVersion = null; - var gitVersionTask = new GitVersionTask(cancellationToken).Configure(processManager, path); - var gitLfsVersionTask = new GitLfsVersionTask(cancellationToken).Configure(processManager, path); - - return gitVersionTask - .Then((result, version) => gitVersion = version) - .Then(gitLfsVersionTask) - .Then((result, version) => gitLfsVersion = version) - .Then(success => new ValidateGitInstallResult(success && + var endTask = new FuncTask(cancellationToken, + () => new ValidateGitInstallResult( gitVersion?.CompareTo(Constants.MinimumGitVersion) >= 0 && gitLfsVersion?.CompareTo(Constants.MinimumGitLfsVersion) >= 0, - gitVersion, gitLfsVersion) - ); + gitVersion, gitLfsVersion)); + + var gitLfsVersionTask = new GitLfsVersionTask(cancellationToken).Configure(processManager, path); + + gitLfsVersionTask + .Then((result, version) => {return gitLfsVersion = version;}) + .Then(endTask, taskIsTopOfChain: true); + + gitLfsVersionTask.Then(endTask, TaskRunOptions.OnFailure, taskIsTopOfChain:true); + + var gitVersionTask = new GitVersionTask(cancellationToken).Configure(processManager, path); + + gitVersionTask + .Then((result, version) => { return gitVersion = version; }) + .Then(gitLfsVersionTask, taskIsTopOfChain: true); + + gitVersionTask.Then(endTask, TaskRunOptions.OnFailure, taskIsTopOfChain:true); + + return endTask; } public ITask Init(IOutputProcessor processor = null) From 0aee974f6cbf023f51c2c12cdac3760d61b2b8a0 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 29 Mar 2018 09:57:49 -0400 Subject: [PATCH 4/5] Changing log message type --- src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs index 2366f7249..e9f57d7b7 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs @@ -266,7 +266,7 @@ private void ValidateAndSetGitInstallPath(string value) } else { - Logger.Warning("Software versions meet minimums Git:{0} GitLfs:{1}", + Logger.Trace("Software versions meet minimums Git:{0} GitLfs:{1}", result.GitVersion, result.GitLfsVersion); From ce8f75fba0f7da2fdb815740ec11313e7d92f88a Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 29 Mar 2018 12:06:26 -0400 Subject: [PATCH 5/5] Fix mispelling --- .../Assets/Editor/GitHub.Unity/UI/GitPathView.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs index e9f57d7b7..17128c4ac 100644 --- a/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs +++ b/src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs @@ -218,9 +218,9 @@ private void ValidateAndSetGitInstallPath(string value) gitVersionErrorMessage = null; GitClient.ValidateGitInstall(value.ToNPath()) - .FinallyInUI((sucess, exception, result) => + .FinallyInUI((success, exception, result) => { - if (!sucess) + if (!success) { Logger.Trace(ErrorValidatingGitPath); gitVersionErrorMessage = ErrorValidatingGitPath;