Fix a problem with the git installation process

The installation state was getting passed into ExtractPortableGit
too early - that method needs to have the result of the download
tasks

Author: shana

src/GitHub.Api/Installer/GitInstaller.cs

@@ -30,14 +30,13 @@ public GitInstaller(IEnvironment environment, IProcessManager processManager,
         public ITask<GitInstallationState> SetupGitIfNeeded()
         {
             //Logger.Trace("SetupGitIfNeeded");
-
+            GitInstallationState installationState = new GitInstallationState();
             installationTask = new FuncTask<GitInstallationState, GitInstallationState>(cancellationToken, (success, path) => path)
                 { Name = "Git Installation - Complete" };
             installationTask.OnStart += thisTask => thisTask.UpdateProgress(0, 100);
             installationTask.OnEnd += (thisTask, result, success, exception) => thisTask.UpdateProgress(100, 100);
 
             ITask<GitInstallationState> startTask = null;
-            GitInstallationState installationState = new GitInstallationState();
             if (!environment.IsWindows)
             {
                 var findTask = new FindExecTask("git", cancellationToken)
@@ -72,37 +71,47 @@ public ITask<GitInstallationState> SetupGitIfNeeded()
             {
                 startTask = new FuncTask<GitInstallationState>(cancellationToken, () =>
                     {
-                        installationState = VerifyGitInstallation();
-                        if (!installationState.GitIsValid && !installationState.GitLfsIsValid)
-                            installationState = GrabZipFromResources(installationState);
-                        else
-                            Logger.Trace("SetupGitIfNeeded: Skipped");
-                        return installationState;
+                        return VerifyPortableGitInstallation();
                     })
                 { Name = "Git Installation - Extract" };
             }
 
+            startTask = startTask.Then(new FuncTask<GitInstallationState, GitInstallationState>(cancellationToken, (success, installState) =>
+                {
+                    if (installState.GitIsValid && installState.GitLfsIsValid)
+                    {
+                        return installState;
+                    }
+
+                    installState = VerifyZipFiles(installState);
+                    installState = GrabZipFromResourcesIfNeeded(installState);
+                    return installState;
+                })
+                { Name = "Git Installation - Validate" }
+            );
+
             startTask.OnEnd += (thisTask, installState, success, exception) =>
             {
-                if (!installState.GitIsValid && !installState.GitLfsIsValid)
+                if (installState.GitIsValid && installState.GitLfsIsValid)
                 {
-                    if (!installState.GitZipExists || !installState.GitLfsZipExists)
-                        thisTask = thisTask.Then(CreateDownloadTask(installState));
-                    thisTask = thisTask.Then(ExtractPortableGit(installState));
+                    Logger.Trace("Skipping git installation");
+                    thisTask.Then(installationTask);
+                    return;
                 }
-                thisTask = thisTask.Then(installationTask);
+
+                var downloadZipTask = DownloadZipsIfNeeded(installState);
+                downloadZipTask.OnEnd += ExtractPortableGit;
+                thisTask.Then(downloadZipTask);
             };
 
             return startTask;
         }
 
-        private GitInstallationState VerifyGitInstallation()
+        private GitInstallationState VerifyPortableGitInstallation()
         {
             var state = new GitInstallationState();
             var gitExists = installDetails.GitExecutablePath.IsInitialized && installDetails.GitExecutablePath.FileExists();
             var gitLfsExists = installDetails.GitLfsExecutablePath.IsInitialized && installDetails.GitLfsExecutablePath.FileExists();
-            state.GitZipExists = installDetails.GitZipPath.FileExists();
-            state.GitLfsZipExists = installDetails.GitLfsZipPath.FileExists();
 
             if (gitExists)
             {
@@ -144,26 +153,44 @@ private GitInstallationState VerifyGitInstallation()
             return state;
         }
 
-        private GitInstallationState GrabZipFromResources(GitInstallationState state)
+        private GitInstallationState VerifyZipFiles(GitInstallationState state)
+        {
+            var md5 = AssemblyResources.ToFile(ResourceType.Platform, "git.zip.md5", installDetails.ZipPath, environment);
+            if (!md5.FileExists() || (installDetails.GitZipPath.FileExists() && !Utils.VerifyFileIntegrity(installDetails.GitZipPath, md5)))
+            {
+                installDetails.GitZipPath.DeleteIfExists();
+            }
+            state.GitZipExists = installDetails.GitZipPath.FileExists();
+
+            md5 = AssemblyResources.ToFile(ResourceType.Platform, "git-lfs.zip.md5", installDetails.ZipPath, environment);
+            // check whether the git-lfs zip file exists and is valid
+            if (!md5.FileExists() || (installDetails.GitLfsZipPath.FileExists() && !Utils.VerifyFileIntegrity(installDetails.GitLfsZipPath, md5)))
+            {
+                installDetails.GitLfsZipPath.DeleteIfExists();
+            }
+            state.GitLfsZipExists = installDetails.GitLfsZipPath.FileExists();
+            installationTask.UpdateProgress(20, 100);
+            return state;
+        }
+
+        private GitInstallationState GrabZipFromResourcesIfNeeded(GitInstallationState state)
         {
             if (!state.GitZipExists)
             {
                 AssemblyResources.ToFile(ResourceType.Platform, "git.zip", installDetails.ZipPath, environment);
-                AssemblyResources.ToFile(ResourceType.Platform, "git.zip.md5", installDetails.ZipPath, environment);
             }
             state.GitZipExists = installDetails.GitZipPath.FileExists();
 
             if (!state.GitLfsZipExists)
             {
                 AssemblyResources.ToFile(ResourceType.Platform, "git-lfs.zip", installDetails.ZipPath, environment);
-                AssemblyResources.ToFile(ResourceType.Platform, "git-lfs.zip.md5", installDetails.ZipPath, environment);
             }
             state.GitLfsZipExists = installDetails.GitLfsZipPath.FileExists();
-            installationTask.UpdateProgress(20, 100);
+            installationTask.UpdateProgress(30, 100);
             return state;
         }
 
-        private ITask<GitInstallationState> CreateDownloadTask(GitInstallationState state)
+        private ITask<GitInstallationState> DownloadZipsIfNeeded(GitInstallationState state)
         {
             var downloader = new Downloader();
             downloader.Catch(e => true);
@@ -180,7 +207,8 @@ private ITask<GitInstallationState> CreateDownloadTask(GitInstallationState stat
             });
         }
 
-        private FuncTask<GitInstallationState> ExtractPortableGit(GitInstallationState state)
+        private void ExtractPortableGit(ITask<GitInstallationState> thisTask,
+            GitInstallationState state, bool s, Exception exception)
         {
             ITask<NPath> task = null;
             var tempZipExtractPath = NPath.CreateTempDirectory("git_zip_extract_zip_paths");
@@ -252,7 +280,9 @@ private FuncTask<GitInstallationState> ExtractPortableGit(GitInstallationState s
                 endTask = task.Then(endTask);
             }
 
-            return endTask;
+            thisTask
+                .Then(endTask)
+                .Then(installationTask);
         }
 
         public class GitInstallationState

src/GitHub.Api/Managers/Downloader.cs

@@ -29,7 +29,9 @@ class Downloader : FuncListTask<DownloadData>
         private readonly List<PairDownloader> downloaders = new List<PairDownloader>();
 
         public Downloader() : base(TaskManager.Instance.Token, RunDownloaders)
-        {}
+        {
+            Name = "Downloader";
+        }
 
         public void QueueDownload(UriString url, UriString md5Url, NPath targetDirectory)
         {
@@ -124,31 +126,6 @@ public Task<DownloadData> QueueDownload(UriString url, UriString md5Url, NPath t
                 var md5Exists = destinationMd5.FileExists();
                 var fileExists = destinationFile.FileExists();
 
-                if (fileExists && md5Exists)
-                {
-                    var verification = new FuncTask<NPath>(cancellationToken, () => destinationFile);
-                    verification.OnStart += _ => DownloadStart?.Invoke(result);
-                    verification.OnEnd += (t, res, success, ex) =>
-                    {
-                        if (!Utils.VerifyFileIntegrity(destinationFile, destinationMd5))
-                        {
-                            destinationMd5.Delete();
-                            destinationFile.Delete();
-                            var fileDownload = DownloadFile(url, targetDirectory, result, verifyDownload);
-                            queuedTasks.Add(fileDownload);
-                            var md5Download = DownloadFile(md5Url, targetDirectory, result, verifyDownload);
-                            queuedTasks.Add(md5Download);
-                            fileDownload.Start();
-                            md5Download.Start();
-                        }
-                        else
-                        {
-                            DownloadComplete(result);
-                        }
-                    };
-                    queuedTasks.Add(verification);
-                }
-
                 if (!md5Exists)
                 {
                     var md5Download = DownloadFile(md5Url, targetDirectory, result, verifyDownload);

src/tests/IntegrationTests/Installer/GitInstallerTests.cs

@@ -62,12 +62,12 @@ public void GitInstallWindows()
             }), Arg.Any<CancellationToken>(), Arg.Any<Func<long, long, bool>>()).Returns(true);
             ZipHelper.Instance = zipHelper;
             var gitInstaller = new GitInstaller(Environment, ProcessManager, TaskManager, installDetails);
-            var startTask = gitInstaller.SetupGitIfNeeded();
-            var endTask = new FuncTask<GitInstaller.GitInstallationState, GitInstaller.GitInstallationState>(TaskManager.Token, (s, state) => state);
-            startTask.OnEnd += (thisTask, path, success, exception) => thisTask.GetEndOfChain().Then(endTask);
+
+            TaskCompletionSource<GitInstaller.GitInstallationState> end = new TaskCompletionSource<GitInstaller.GitInstallationState>();
+            var startTask = gitInstaller.SetupGitIfNeeded().Finally((_, state) => end.TrySetResult(state));
             startTask.Start();
             GitInstaller.GitInstallationState result = null;
-            Assert.DoesNotThrow(async () => result = await endTask.Task);
+            Assert.DoesNotThrow(async () => result = await end.Task);
             result.Should().NotBeNull();
 
             Assert.AreEqual(gitInstallationPath.Combine(installDetails.PackageNameWithVersion), result.GitInstallationPath);