Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Commit acfe268

Browse files
shanashana
committed
Fix a bunch of stuff in the git installation process
- Fix a bunch of typos - Make sure the download failing doesn't abort the whole process - Install lfs if there is a git but lfs is missing - Fix the download tests to download the real files that we're shipping - Mock the unzipper for the git installer test so it's faster - a bunch of other fixes
1 parent 5562b63 commit acfe268

File tree

15 files changed

+402
-170
lines changed

15 files changed

+402
-170
lines changed

‎src/GitHub.Api/Application/ApplicationManagerBase.cs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ public void Run(bool firstRun)
7474
}
7575
};
7676

77-
var initEnvironmentTask = new ActionTask<NPath>(CancellationToken,
78-
(_, path) => InitializeEnvironment(path))
77+
var initEnvironmentTask = new ActionTask<GitInstaller.GitInstallationState>(CancellationToken,
78+
(_, state) => InitializeEnvironment(state))
7979
{ Affinity = TaskAffinity.UI };
8080

8181
isBusy = true;
@@ -227,20 +227,22 @@ protected void SetupMetrics(string unityVersion, bool firstRun)
227227
/// </summary>
228228
/// <param name="gitExecutablePath"></param>
229229
/// <param name="octorunScriptPath"></param>
230-
private void InitializeEnvironment(NPath gitExecutablePath)
230+
private void InitializeEnvironment(GitInstaller.GitInstallationState installationState)
231231
{
232232
isBusy = false;
233233
SetupMetrics();
234234

235-
if (!gitExecutablePath.IsInitialized)
235+
if (!installationState.GitIsValid)
236236
{
237237
return;
238238
}
239-
239+
240240
var gitInstallDetails = new GitInstaller.GitInstallDetails(Environment.UserCachePath, Environment.IsWindows);
241-
var isCustomGitExec = gitExecutablePath != gitInstallDetails.GitExecutablePath;
241+
var isCustomGitExec = installationState.GitExecutablePath != gitInstallDetails.GitExecutablePath;
242+
243+
Environment.GitExecutablePath = installationState.GitExecutablePath;
244+
Environment.GitLfsExecutablePath = installationState.GitLfsExecutablePath;
242245

243-
Environment.GitExecutablePath = gitExecutablePath;
244246
Environment.IsCustomGitExecutable = isCustomGitExec;
245247
Environment.User.Initialize(GitClient);
246248

‎src/GitHub.Api/Installer/GitInstaller.cs‎

Lines changed: 123 additions & 75 deletions
Large diffs are not rendered by default.

‎src/GitHub.Api/Installer/ZipHelper.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ public static IZipHelper Instance
1919
instance = new ZipHelper();
2020
return instance;
2121
}
22+
set { instance = value; }
2223
}
2324

2425
public bool Extract(string archive, string outFolder, CancellationToken cancellationToken,

‎src/GitHub.Api/Managers/Downloader.cs‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ public Task<DownloadData> Run()
8383
{
8484
foreach (var task in queuedTasks)
8585
task.Start();
86+
if (queuedTasks.Count == 0)
87+
DownloadComplete(null);
8688
return aggregateDownloads.Task;
8789
}
8890

@@ -121,32 +123,55 @@ public Task<DownloadData> QueueDownload(UriString url, UriString md5Url, NPath t
121123
var md5Exists = destinationMd5.FileExists();
122124
var fileExists = destinationFile.FileExists();
123125

126+
if (fileExists && md5Exists)
127+
{
128+
var verification = new FuncTask<NPath>(cancellationToken, () => destinationFile);
129+
verification.OnStart += _ => DownloadStart?.Invoke(result);
130+
verification.OnEnd += (t, res, success, ex) =>
131+
{
132+
if (!Utils.VerifyFileIntegrity(destinationFile, destinationMd5))
133+
{
134+
destinationMd5.Delete();
135+
destinationFile.Delete();
136+
var fileDownload = DownloadFile(url, targetDirectory, result, verifyDownload);
137+
queuedTasks.Add(fileDownload);
138+
var md5Download = DownloadFile(md5Url, targetDirectory, result, verifyDownload);
139+
queuedTasks.Add(md5Download);
140+
fileDownload.Start();
141+
md5Download.Start();
142+
}
143+
else
144+
{
145+
DownloadComplete(result);
146+
}
147+
};
148+
queuedTasks.Add(verification);
149+
}
150+
124151
if (!md5Exists)
125152
{
126-
destinationMd5.DeleteIfExists();
127-
var md5Download = new DownloadTask(cancellationToken, fs, md5Url, targetDirectory)
128-
.Catch(e => DownloadFailed(result, e));
129-
md5Download.OnEnd += verifyDownload;
153+
var md5Download = DownloadFile(md5Url, targetDirectory, result, verifyDownload);
154+
md5Download.OnStart += _ => DownloadStart?.Invoke(result);
130155
queuedTasks.Add(md5Download);
131156
}
132157

133158
if (!fileExists)
134159
{
135-
var fileDownload = new DownloadTask(cancellationToken, fs, url, targetDirectory)
136-
.Catch(e => DownloadFailed(result, e));
137-
fileDownload.OnStart += _ => DownloadStart?.Invoke(result);
138-
fileDownload.OnEnd += verifyDownload;
160+
var fileDownload = DownloadFile(url, targetDirectory, result, verifyDownload);
161+
if (md5Exists) // only invoke DownloadStart if it hasn't been invoked before in the md5 download
162+
fileDownload.OnStart += _ => DownloadStart?.Invoke(result);
139163
queuedTasks.Add(fileDownload);
140164
}
141-
142-
if (fileExists && md5Exists)
143-
{
144-
var verification = new FuncTask<NPath>(cancellationToken, () => destinationFile);
145-
verification.OnEnd += verifyDownload;
146-
queuedTasks.Add(verification);
147-
}
148165
return aggregateDownloads.Task;
149166
}
167+
168+
private ITask<NPath> DownloadFile(UriString url, NPath targetDirectory, DownloadData result, Action<ITask<NPath>, NPath, bool, Exception> verifyDownload)
169+
{
170+
var download = new DownloadTask(cancellationToken, fs, url, targetDirectory)
171+
.Catch(e => { DownloadFailed(result, e); return true; });
172+
download.OnEnd += verifyDownload;
173+
return download;
174+
}
150175
}
151176

152177
public static bool Download(ILogging logger, UriString url,

‎src/GitHub.Api/OutputProcessors/ProcessManager.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public void RunCommandLineWindow(NPath workingDirectory)
8484
if (environment.IsWindows)
8585
{
8686
startInfo.FileName = "cmd";
87-
gitEnvironment.Configure(startInfo, workingDirectory, environment.IsCustomGitExecutable);
87+
gitEnvironment.Configure(startInfo, workingDirectory);
8888
}
8989
else if (environment.IsMac)
9090
{
@@ -94,20 +94,20 @@ public void RunCommandLineWindow(NPath workingDirectory)
9494
var envVarFile = NPath.GetTempFilename();
9595
startInfo.FileName = "open";
9696
startInfo.Arguments = $"-a Terminal {envVarFile}";
97-
gitEnvironment.Configure(startInfo, workingDirectory, environment.IsCustomGitExecutable);
97+
gitEnvironment.Configure(startInfo, workingDirectory);
9898

9999
var envVars = startInfo.EnvironmentVariables;
100100
var scriptContents = new[] {
101101
$"cd \"{envVars["GHU_WORKINGDIR"]}\"",
102-
environment.IsCustomGitExecutable? "/bin/bash" : $"PATH=\"{envVars["GHU_FULLPATH"]}\":$PATH /bin/bash"
102+
$"PATH=\"{envVars["GHU_FULLPATH"]}\" /bin/bash"
103103
};
104104
environment.FileSystem.WriteAllLines(envVarFile, scriptContents);
105105
Mono.Unix.Native.Syscall.chmod(envVarFile, (Mono.Unix.Native.FilePermissions)493); // -rwxr-xr-x mode (0755)
106106
}
107107
else
108108
{
109109
startInfo.FileName = "sh";
110-
gitEnvironment.Configure(startInfo, workingDirectory, environment.IsCustomGitExecutable);
110+
gitEnvironment.Configure(startInfo, workingDirectory);
111111
}
112112

113113
Process.Start(startInfo);

‎src/GitHub.Api/Platform/DefaultEnvironment.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ public class DefaultEnvironment : IEnvironment
1313
private static bool? onMac;
1414

1515
private NPath gitExecutablePath;
16+
private NPath gitLfsExecutablePath;
1617
private NPath nodeJsExecutablePath;
1718
private NPath octorunScriptPath;
1819

@@ -163,6 +164,17 @@ public NPath GitExecutablePath
163164
GitInstallPath = GitExecutablePath.Resolve().Parent.Parent;
164165
}
165166
}
167+
168+
public NPath GitLfsExecutablePath
169+
{
170+
get { return gitLfsExecutablePath; }
171+
set
172+
{
173+
gitLfsExecutablePath = value;
174+
GitLfsInstallPath = gitLfsExecutablePath.IsInitialized ? gitLfsExecutablePath.Parent : NPath.Default;
175+
}
176+
}
177+
166178
public NPath NodeJsExecutablePath
167179
{
168180
get
@@ -177,6 +189,7 @@ public NPath NodeJsExecutablePath
177189
}
178190
}
179191
public NPath GitInstallPath { get; private set; }
192+
public NPath GitLfsInstallPath { get; private set; }
180193
public NPath RepositoryPath { get; private set; }
181194
public ICacheContainer CacheContainer { get; private set; }
182195
public IRepository Repository { get; set; }

‎src/GitHub.Api/Platform/IEnvironment.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,7 @@ public interface IEnvironment
3535
IRepository Repository { get; set; }
3636
string ExecutableExtension { get; }
3737
ICacheContainer CacheContainer { get; }
38+
NPath GitLfsInstallPath { get; }
39+
NPath GitLfsExecutablePath { get; set; }
3840
}
3941
}

‎src/GitHub.Api/Platform/ProcessEnvironment.cs‎

Lines changed: 64 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using GitHub.Logging;
22
using System;
3+
using System.Collections.Generic;
34
using System.Diagnostics;
45

56
namespace GitHub.Unity
@@ -17,58 +18,86 @@ public ProcessEnvironment(IEnvironment environment)
1718

1819
public void Configure(ProcessStartInfo psi, NPath workingDirectory, bool dontSetupGit = false)
1920
{
20-
Guard.ArgumentNotNull(psi, "psi");
21-
2221
psi.WorkingDirectory = workingDirectory;
2322
psi.EnvironmentVariables["HOME"] = NPath.HomeDirectory;
2423
psi.EnvironmentVariables["TMP"] = psi.EnvironmentVariables["TEMP"] = NPath.SystemTemp;
24+
25+
var path = Environment.Path;
2526
psi.EnvironmentVariables["GHU_WORKINGDIR"] = workingDirectory;
26-
psi.EnvironmentVariables["PATH"] = Environment.Path;
27-
psi.EnvironmentVariables["GHU_FULLPATH"] = Environment.Path;
2827

29-
// if we don't know where git is, then there's nothing else to configure
30-
if (!Environment.GitInstallPath.IsInitialized || dontSetupGit)
28+
if (dontSetupGit)
29+
{
30+
psi.EnvironmentVariables["GHU_FULLPATH"] = path;
31+
psi.EnvironmentVariables["PATH"] = path;
3132
return;
32-
33-
var httpProxy = Environment.GetEnvironmentVariable("HTTP_PROXY");
34-
if (!String.IsNullOrEmpty(httpProxy))
35-
psi.EnvironmentVariables["HTTP_PROXY"] = httpProxy;
36-
37-
var httpsProxy = Environment.GetEnvironmentVariable("HTTPS_PROXY");
38-
if (!String.IsNullOrEmpty(httpsProxy))
39-
psi.EnvironmentVariables["HTTPS_PROXY"] = httpsProxy;
40-
41-
//TODO: Remove with Git LFS Locking becomes standard
42-
psi.EnvironmentVariables["GITLFSLOCKSENABLED"] = "1";
33+
}
34+
35+
Guard.ArgumentNotNull(psi, "psi");
36+
37+
var pathEntries = new List<string>();
38+
string separator = Environment.IsWindows ? ";" : ":";
4339

44-
if (Environment.IsWindows)
40+
if (Environment.GitInstallPath.IsInitialized)
4541
{
46-
// We need to essentially fake up what git-cmd.bat does
4742
var gitPathRoot = Environment.GitInstallPath;
48-
43+
var gitExecutableDir = Environment.GitExecutablePath.Parent; // original path to git (might be different from install path if it's a symlink)
44+
4945
var baseExecPath = gitPathRoot;
50-
if (baseExecPath.DirectoryExists("mingw32"))
51-
baseExecPath = baseExecPath.Combine("mingw32");
52-
else
53-
baseExecPath = baseExecPath.Combine("mingw64");
54-
var binPath = baseExecPath.Combine("bin");
55-
46+
var binPath = baseExecPath;
47+
if (Environment.IsWindows)
48+
{
49+
if (baseExecPath.DirectoryExists("mingw32"))
50+
baseExecPath = baseExecPath.Combine("mingw32");
51+
else
52+
baseExecPath = baseExecPath.Combine("mingw64");
53+
binPath = baseExecPath.Combine("bin");
54+
}
55+
5656
var execPath = baseExecPath.Combine("libexec", "git-core");
5757
if (!execPath.DirectoryExists())
5858
execPath = NPath.Default;
5959

60-
var userPath = @"C:\windows\system32;C:\windows";
61-
var path = $"{gitPathRoot}\\cmd;{gitPathRoot}\\usr\\bin;{execPath};{binPath}";
62-
60+
if (Environment.IsWindows)
61+
{
62+
pathEntries.AddRange(new[] { gitPathRoot.Combine("cmd").ToString(), gitPathRoot.Combine("usr", "bin") });
63+
}
64+
else
65+
{
66+
pathEntries.Add(gitExecutableDir.ToString());
67+
}
68+
if (execPath.IsInitialized)
69+
pathEntries.Add(execPath);
70+
pathEntries.Add(binPath);
71+
6372
if (execPath.IsInitialized)
6473
psi.EnvironmentVariables["GIT_EXEC_PATH"] = execPath.ToString();
65-
66-
psi.EnvironmentVariables["PATH"] = path;
67-
psi.EnvironmentVariables["GHU_FULLPATH"] = path;
68-
69-
psi.EnvironmentVariables["PLINK_PROTOCOL"] = "ssh";
70-
psi.EnvironmentVariables["TERM"] = "msys";
7174
}
75+
76+
if (Environment.GitLfsInstallPath.IsInitialized && Environment.GitInstallPath != Environment.GitLfsInstallPath)
77+
{
78+
pathEntries.Add(Environment.GitLfsInstallPath);
79+
}
80+
81+
pathEntries.Add("END");
82+
83+
path = String.Join(separator, pathEntries.ToArray()) + separator + path;
84+
85+
psi.EnvironmentVariables["GHU_FULLPATH"] = path;
86+
psi.EnvironmentVariables["PATH"] = path;
87+
88+
//TODO: Remove with Git LFS Locking becomes standard
89+
psi.EnvironmentVariables["GITLFSLOCKSENABLED"] = "1";
90+
91+
psi.EnvironmentVariables["PLINK_PROTOCOL"] = "ssh";
92+
psi.EnvironmentVariables["TERM"] = "msys";
93+
94+
var httpProxy = Environment.GetEnvironmentVariable("HTTP_PROXY");
95+
if (!String.IsNullOrEmpty(httpProxy))
96+
psi.EnvironmentVariables["HTTP_PROXY"] = httpProxy;
97+
98+
var httpsProxy = Environment.GetEnvironmentVariable("HTTPS_PROXY");
99+
if (!String.IsNullOrEmpty(httpsProxy))
100+
psi.EnvironmentVariables["HTTPS_PROXY"] = httpsProxy;
72101
}
73102
}
74103
}

‎src/GitHub.Api/Tasks/ProcessTask.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ public override string ToString()
350350

351351
public Process Process { get; set; }
352352
public int ProcessId { get { return Process.Id; } }
353-
public override bool Successful { get { return Task.Status == TaskStatus.RanToCompletion && Process.ExitCode == 0; } }
353+
public override bool Successful { get { return !taskFailed && Task.Status == TaskStatus.RanToCompletion && Process.ExitCode == 0; } }
354354
public StreamWriter StandardInput { get { return wrapper?.Input; } }
355355
public virtual string ProcessName { get; protected set; }
356356
public virtual string ProcessArguments { get; }

‎src/UnityExtension/Assets/Editor/GitHub.Unity/UI/GitPathView.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ public override void OnGUI()
142142

143143
new FindExecTask("git", Manager.CancellationToken)
144144
.Configure(Manager.ProcessManager, false, true)
145+
.Catch(ex => true)
145146
.FinallyInUI((success, ex, path) => {
146147
if (success)
147148
{
@@ -242,7 +243,7 @@ private void ValidateAndSetGitInstallPath(string value)
242243
EntryPoint.ApplicationManager.TaskManager);
243244

244245
gitInstaller.SetupGitIfNeeded()
245-
.FinallyInUI((success, exception, result) =>
246+
.FinallyInUI((success, exception, installationState) =>
246247
{
247248
Logger.Trace("Setup Git Using the installer:{0}", success);
248249

@@ -254,7 +255,8 @@ private void ValidateAndSetGitInstallPath(string value)
254255
else
255256
{
256257
Manager.SystemSettings.Unset(Constants.GitInstallPathKey);
257-
Environment.GitExecutablePath = result;
258+
Environment.GitExecutablePath = installationState.GitExecutablePath;
259+
Environment.GitLfsExecutablePath = installationState.GitLfsExecutablePath;
258260
Environment.IsCustomGitExecutable = false;
259261

260262
gitExecHasChanged = true;

0 commit comments

Comments
 (0)