From 4612f1ebe9f34eb88462773d7f93070ad812983a Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 5 Apr 2018 22:30:54 +0200 Subject: [PATCH 1/5] These should not be tracked in the test and package projects --- unity/PackageProject/.gitignore | 1 + .../Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 | 1 - unity/TestProject/.gitignore | 1 + .../Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 | 1 - 4 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 unity/PackageProject/Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 delete mode 100644 unity/TestProject/Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 diff --git a/unity/PackageProject/.gitignore b/unity/PackageProject/.gitignore index 9fac08e77..42cb35a28 100644 --- a/unity/PackageProject/.gitignore +++ b/unity/PackageProject/.gitignore @@ -5,6 +5,7 @@ *.xml *.local.json *.zip +*.md5 *.dylib *.so *.bundle diff --git a/unity/PackageProject/Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 b/unity/PackageProject/Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 deleted file mode 100644 index b10619984..000000000 --- a/unity/PackageProject/Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 +++ /dev/null @@ -1 +0,0 @@ -7cdaa49008b8c996343e07670100bce2 \ No newline at end of file diff --git a/unity/TestProject/.gitignore b/unity/TestProject/.gitignore index 05006f52d..9d11e7781 100644 --- a/unity/TestProject/.gitignore +++ b/unity/TestProject/.gitignore @@ -8,6 +8,7 @@ *.dylib *.so *.bundle +*.md5 # ignoring this for now *.meta \ No newline at end of file diff --git a/unity/TestProject/Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 b/unity/TestProject/Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 deleted file mode 100644 index b10619984..000000000 --- a/unity/TestProject/Assets/Plugins/GitHub/Editor/Resources/octorun.zip.md5 +++ /dev/null @@ -1 +0,0 @@ -7cdaa49008b8c996343e07670100bce2 \ No newline at end of file From 7215176118e676d6f2e1ba0fc12d03e7f4df0577 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 5 Apr 2018 22:31:26 +0200 Subject: [PATCH 2/5] Name the task appropriately --- src/GitHub.Api/Installer/GitInstaller.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHub.Api/Installer/GitInstaller.cs b/src/GitHub.Api/Installer/GitInstaller.cs index 469b13437..772878462 100644 --- a/src/GitHub.Api/Installer/GitInstaller.cs +++ b/src/GitHub.Api/Installer/GitInstaller.cs @@ -73,7 +73,7 @@ public ITask SetupGitIfNeeded() { return VerifyPortableGitInstallation(); }) - { Name = "Git Installation - Extract" }; + { Name = "Git Installation - Verify" }; } startTask = startTask.Then(new FuncTask(cancellationToken, (success, installState) => From bd19997d0e38c806b79f803c03b8aa62825019dd Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 5 Apr 2018 22:32:36 +0200 Subject: [PATCH 3/5] Fix running the installation tasks on start --- .../Application/ApplicationManagerBase.cs | 92 +++++++++---------- 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/src/GitHub.Api/Application/ApplicationManagerBase.cs b/src/GitHub.Api/Application/ApplicationManagerBase.cs index 31bf7f5fd..2228e51e8 100644 --- a/src/GitHub.Api/Application/ApplicationManagerBase.cs +++ b/src/GitHub.Api/Application/ApplicationManagerBase.cs @@ -52,6 +52,11 @@ protected void Initialize() public void Run(bool firstRun) { Logger.Trace("Run - CurrentDirectory {0}", NPath.CurrentDirectory); + isBusy = true; + + var endTask = new ActionTask(CancellationToken, + (_, state) => InitializeEnvironment(state)) + { Affinity = TaskAffinity.UI }; ITask setExistingEnvironmentPath; if (Environment.IsMac) @@ -59,14 +64,15 @@ public void Run(bool firstRun) setExistingEnvironmentPath = new SimpleProcessTask(CancellationToken, "bash".ToNPath(), "-c \"/usr/libexec/path_helper\"") .Configure(ProcessManager, dontSetupGit: true) .Catch(e => true) // make sure this doesn't throw if the task fails - .Then((success, path) => success ? path.Split(new[] { "\"" }, StringSplitOptions.None)[1] : null); + .Then((success, path) => success ? path?.Split(new[] { "\"" }, StringSplitOptions.None)[1] : null); } else { setExistingEnvironmentPath = new FuncTask(CancellationToken, () => null); } - setExistingEnvironmentPath.OnEnd += (t, path, success, ex) => { + setExistingEnvironmentPath.OnEnd += (t, path, success, ex) => + { if (path != null) { Logger.Trace("Existing Environment Path Original:{0} Updated:{1}", Environment.Path, path); @@ -74,60 +80,50 @@ public void Run(bool firstRun) } }; - var initEnvironmentTask = new ActionTask(CancellationToken, - (_, state) => InitializeEnvironment(state)) - { Affinity = TaskAffinity.UI }; - - isBusy = true; + var setupOctorun = new OctorunInstaller(Environment, TaskManager).SetupOctorunIfNeeded(); + var setOctorunEnvironment = new ActionTask(CancellationToken, + (s, octorunPath) => Environment.OctorunScriptPath = octorunPath); - var octorunInstaller = new OctorunInstaller(Environment, TaskManager); - var setupTask = setExistingEnvironmentPath.Then(octorunInstaller.SetupOctorunIfNeeded()); - - var initializeGitTask = new FuncTask(CancellationToken, () => - { - var gitExecutablePath = SystemSettings.Get(Constants.GitInstallPathKey)?.ToNPath(); - if (gitExecutablePath.HasValue && gitExecutablePath.Value.FileExists()) // we have a git path - { - Logger.Trace("Using git install path from settings: {0}", gitExecutablePath); - return gitExecutablePath.Value; - } - return NPath.Default; - }); - var setOctorunEnvironmentTask = new ActionTask(CancellationToken, (s, octorunPath) => + var getGitFromSettings = new FuncTask(CancellationToken, () => + { + var gitExecutablePath = SystemSettings.Get(Constants.GitInstallPathKey)?.ToNPath(); + if (gitExecutablePath.HasValue && gitExecutablePath.Value.FileExists()) // we have a git path { - Environment.OctorunScriptPath = octorunPath; - }); + Logger.Trace("Using git install path from settings: {0}", gitExecutablePath); + return gitExecutablePath.Value; + } + return NPath.Default; + }); - setupTask.OnEnd += (t, path, _, __) => + getGitFromSettings.OnEnd += (t, path, _, __) => + { + if (path.IsInitialized) { - t.GetEndOfChain().Then(setOctorunEnvironmentTask).Then(initializeGitTask); - }; + var state = new GitInstaller.GitInstallationState { GitExecutablePath = path }; + endTask.PreviousResult = state; + endTask.Start(); + return; + } + Logger.Trace("Using portable git"); - initializeGitTask.OnEnd += (t, path, _, __) => + var setupGit = new GitInstaller(Environment, ProcessManager, TaskManager).SetupGitIfNeeded(); + setupGit.Finally((s, state) => { - if (path.IsInitialized) - { - t.GetEndOfChain() - .Then(initEnvironmentTask, taskIsTopOfChain: true); - return; - } - Logger.Trace("Using portable git"); - - var gitInstaller = new GitInstaller(Environment, ProcessManager, TaskManager); - - var task = gitInstaller.SetupGitIfNeeded(); - task.Progress(progressReporter.UpdateProgress); - task.OnEnd += (thisTask, result, success, exception) => - { - thisTask.GetEndOfChain() - .Then(initEnvironmentTask, taskIsTopOfChain: true); - }; + endTask.PreviousResult = state; + endTask.Start(); + }); + setupGit.Progress(progressReporter.UpdateProgress); + // append installer task to top chain + t.Then(setupGit); + }; - // append installer task to top chain - t.Then(task, taskIsTopOfChain: true); - }; + var setupChain = setExistingEnvironmentPath.Then(setupOctorun); + setupChain.OnEnd += (t, path, _, __) => + { + t.GetEndOfChain().Then(setOctorunEnvironment).Then(getGitFromSettings); + }; - setupTask.Start(); + setupChain.Start(); } public ITask InitializeRepository() From 05fafb506b0588f05f5a8d943cc6ceea70eeda42 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 5 Apr 2018 22:51:57 +0200 Subject: [PATCH 4/5] We really don't want to call the finally handler twice --- src/GitHub.Api/Tasks/TaskBase.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/GitHub.Api/Tasks/TaskBase.cs b/src/GitHub.Api/Tasks/TaskBase.cs index b959c530e..c9b5d4d52 100644 --- a/src/GitHub.Api/Tasks/TaskBase.cs +++ b/src/GitHub.Api/Tasks/TaskBase.cs @@ -671,7 +671,8 @@ protected virtual void RaiseOnEnd(TResult data) protected override void CallFinallyHandler() { finallyHandler?.Invoke(!taskFailed, result); - base.CallFinallyHandler(); + if (finallyHandler == null) + base.CallFinallyHandler(); } public new Task Task From a82ee33cc6a90fc8ccba257a5d8420d17d1780f1 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 5 Apr 2018 23:13:15 +0200 Subject: [PATCH 5/5] Get a better fix that doesn't break the current assumptions of how finally handlers are called --- src/GitHub.Api/Application/ApplicationManagerBase.cs | 2 +- src/GitHub.Api/Tasks/TaskBase.cs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/GitHub.Api/Application/ApplicationManagerBase.cs b/src/GitHub.Api/Application/ApplicationManagerBase.cs index 2228e51e8..848f6d302 100644 --- a/src/GitHub.Api/Application/ApplicationManagerBase.cs +++ b/src/GitHub.Api/Application/ApplicationManagerBase.cs @@ -107,6 +107,7 @@ public void Run(bool firstRun) Logger.Trace("Using portable git"); var setupGit = new GitInstaller(Environment, ProcessManager, TaskManager).SetupGitIfNeeded(); + t.Then(setupGit); setupGit.Finally((s, state) => { endTask.PreviousResult = state; @@ -114,7 +115,6 @@ public void Run(bool firstRun) }); setupGit.Progress(progressReporter.UpdateProgress); // append installer task to top chain - t.Then(setupGit); }; var setupChain = setExistingEnvironmentPath.Then(setupOctorun); diff --git a/src/GitHub.Api/Tasks/TaskBase.cs b/src/GitHub.Api/Tasks/TaskBase.cs index c9b5d4d52..b959c530e 100644 --- a/src/GitHub.Api/Tasks/TaskBase.cs +++ b/src/GitHub.Api/Tasks/TaskBase.cs @@ -671,8 +671,7 @@ protected virtual void RaiseOnEnd(TResult data) protected override void CallFinallyHandler() { finallyHandler?.Invoke(!taskFailed, result); - if (finallyHandler == null) - base.CallFinallyHandler(); + base.CallFinallyHandler(); } public new Task Task