From 45817be86e68b11ec338d2f78ba6123677d852a2 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 20 Feb 2020 11:29:48 -0500 Subject: [PATCH] Make InvalidRepoException exit normally Signed-off-by: Derrick Stolee --- Scalar.Common/Enlistment.cs | 6 +++--- Scalar.Common/Git/GitAuthentication.cs | 4 ++-- Scalar.Common/Git/GitSsl.cs | 9 +++++---- Scalar.Common/Http/CacheServerResolver.cs | 6 +++--- Scalar.Common/InvalidRepoException.cs | 5 ++++- Scalar.Common/ScalarEnlistment.cs | 4 ++-- Scalar.UnitTests/Git/GitAuthenticationTests.cs | 16 ++++++++-------- Scalar/CommandLine/ScalarVerb.cs | 4 ++++ 8 files changed, 31 insertions(+), 23 deletions(-) diff --git a/Scalar.Common/Enlistment.cs b/Scalar.Common/Enlistment.cs index 7161810cc51..00fb8558ac6 100644 --- a/Scalar.Common/Enlistment.cs +++ b/Scalar.Common/Enlistment.cs @@ -36,18 +36,18 @@ protected Enlistment( GitProcess.ConfigResult originResult = gitProcess.GetOriginUrl(); if (!originResult.TryParseAsString(out string originUrl, out string error)) { - throw new InvalidRepoException("Could not get origin url. git error: " + error); + throw new InvalidRepoException(this.WorkingDirectoryRoot, "Could not get origin url. git error: " + error); } if (originUrl == null) { - throw new InvalidRepoException("Could not get origin url. remote 'origin' is not configured for this repo.'"); + throw new InvalidRepoException(this.WorkingDirectoryRoot, "Could not get origin url. remote 'origin' is not configured for this repo.'"); } this.RepoUrl = originUrl.Trim(); } - this.Authentication = authentication ?? new GitAuthentication(gitProcess, this.RepoUrl); + this.Authentication = authentication ?? new GitAuthentication(gitProcess, this.RepoUrl, this.WorkingDirectoryRoot); } public string EnlistmentRoot { get; } diff --git a/Scalar.Common/Git/GitAuthentication.cs b/Scalar.Common/Git/GitAuthentication.cs index e6b2c43da67..15045e4a259 100644 --- a/Scalar.Common/Git/GitAuthentication.cs +++ b/Scalar.Common/Git/GitAuthentication.cs @@ -25,14 +25,14 @@ public class GitAuthentication private bool isInitialized; - public GitAuthentication(GitProcess git, string repoUrl) + public GitAuthentication(GitProcess git, string repoUrl, string repoPath) { this.credentialStore = git; this.repoUrl = repoUrl; if (git.TryGetConfigUrlMatch("http", this.repoUrl, out Dictionary configSettings)) { - this.GitSsl = new GitSsl(configSettings); + this.GitSsl = new GitSsl(repoPath, configSettings); } } diff --git a/Scalar.Common/Git/GitSsl.cs b/Scalar.Common/Git/GitSsl.cs index 1c448643e1b..e2feb8385a9 100644 --- a/Scalar.Common/Git/GitSsl.cs +++ b/Scalar.Common/Git/GitSsl.cs @@ -20,6 +20,7 @@ public class GitSsl private readonly PhysicalFileSystem fileSystem; public GitSsl( + string repoPath, IDictionary configSettings, Func createCertificateStore = null, CertificateVerifier certificateVerifier = null, @@ -32,9 +33,9 @@ public GitSsl( this.certificatePathOrSubjectCommonName = sslCerts.Values.Last(); } - this.isCertificatePasswordProtected = SetBoolSettingOrThrow(configSettings, GitConfigSetting.HttpSslCertPasswordProtected, this.isCertificatePasswordProtected); + this.isCertificatePasswordProtected = SetBoolSettingOrThrow(repoPath, configSettings, GitConfigSetting.HttpSslCertPasswordProtected, this.isCertificatePasswordProtected); - this.ShouldVerify = SetBoolSettingOrThrow(configSettings, GitConfigSetting.HttpSslVerify, this.ShouldVerify); + this.ShouldVerify = SetBoolSettingOrThrow(repoPath, configSettings, GitConfigSetting.HttpSslVerify, this.ShouldVerify); } } @@ -87,7 +88,7 @@ public X509Certificate2 GetCertificate(ITracer tracer, GitProcess gitProcess) return result; } - private static bool SetBoolSettingOrThrow(IDictionary configSettings, string settingName, bool currentValue) + private static bool SetBoolSettingOrThrow(string repoPath, IDictionary configSettings, string settingName, bool currentValue) { if (configSettings.TryGetValue(settingName, out GitConfigSetting settingValues)) { @@ -97,7 +98,7 @@ private static bool SetBoolSettingOrThrow(IDictionary } catch (FormatException) { - throw new InvalidRepoException($"{settingName} git setting did not have a bool-parsable value. Found: {string.Join(" ", settingValues.Values)}"); + throw new InvalidRepoException(repoPath, $"{settingName} git setting did not have a bool-parsable value. Found: {string.Join(" ", settingValues.Values)}"); } } diff --git a/Scalar.Common/Http/CacheServerResolver.cs b/Scalar.Common/Http/CacheServerResolver.cs index e25a90ac413..fc0f4d7b43a 100644 --- a/Scalar.Common/Http/CacheServerResolver.cs +++ b/Scalar.Common/Http/CacheServerResolver.cs @@ -32,7 +32,7 @@ public static string GetUrlFromConfig(Enlistment enlistment) // TODO 1057500: Remove support for encoded-repo-url cache config setting return - GetValueFromConfig(git, ScalarConstants.GitConfig.CacheServer, localOnly: true) + GetValueFromConfig(enlistment.WorkingDirectoryRoot, git, ScalarConstants.GitConfig.CacheServer, localOnly: true) ?? enlistment.RepoUrl; } @@ -128,7 +128,7 @@ public bool TrySaveUrlToLocalConfig(CacheServerInfo cache, out string error) return result.ExitCodeIsSuccess; } - private static string GetValueFromConfig(GitProcess git, string configName, bool localOnly) + private static string GetValueFromConfig(string repoPath, GitProcess git, string configName, bool localOnly) { GitProcess.ConfigResult result = localOnly @@ -137,7 +137,7 @@ private static string GetValueFromConfig(GitProcess git, string configName, bool if (!result.TryParseAsString(out string value, out string error)) { - throw new InvalidRepoException(error); + throw new InvalidRepoException(repoPath, error); } return value; diff --git a/Scalar.Common/InvalidRepoException.cs b/Scalar.Common/InvalidRepoException.cs index b78a64b54ea..abd959b9ab5 100644 --- a/Scalar.Common/InvalidRepoException.cs +++ b/Scalar.Common/InvalidRepoException.cs @@ -4,9 +4,12 @@ namespace Scalar.Common { public class InvalidRepoException : Exception { - public InvalidRepoException(string message) + public string RepoPath { get; } + + public InvalidRepoException(string repoPath, string message) : base(message) { + this.RepoPath = repoPath; } } } diff --git a/Scalar.Common/ScalarEnlistment.cs b/Scalar.Common/ScalarEnlistment.cs index 4cda07d14ed..a51fa0a439e 100644 --- a/Scalar.Common/ScalarEnlistment.cs +++ b/Scalar.Common/ScalarEnlistment.cs @@ -69,7 +69,7 @@ public static ScalarEnlistment CreateFromDirectory( if (!TryGetScalarEnlistmentRoot(directory, out enlistmentRoot, out workingDirectory)) { - throw new InvalidRepoException($"Could not get enlistment root."); + throw new InvalidRepoException(directory, $"Could not get enlistment root."); } if (createWithoutRepoURL) @@ -80,7 +80,7 @@ public static ScalarEnlistment CreateFromDirectory( return new ScalarEnlistment(enlistmentRoot, workingDirectory, null, gitBinRoot, authentication); } - throw new InvalidRepoException($"Directory '{directory}' does not exist"); + throw new InvalidRepoException(directory, $"Directory '{directory}' does not exist"); } public static string GetNewScalarLogFileName( diff --git a/Scalar.UnitTests/Git/GitAuthenticationTests.cs b/Scalar.UnitTests/Git/GitAuthenticationTests.cs index a0662a6a9fe..92af83298ef 100644 --- a/Scalar.UnitTests/Git/GitAuthenticationTests.cs +++ b/Scalar.UnitTests/Git/GitAuthenticationTests.cs @@ -27,7 +27,7 @@ public void AuthShouldBackoffAfterFirstRetryFailure() MockTracer tracer = new MockTracer(); MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl", "fake path"); dut.TryInitializeAndRequireAuth(tracer, out _); string authString; @@ -54,7 +54,7 @@ public void BackoffIsNotInEffectAfterSuccess() MockTracer tracer = new MockTracer(); MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl", "fake path"); dut.TryInitializeAndRequireAuth(tracer, out _); string authString; @@ -78,7 +78,7 @@ public void ContinuesToBackoffIfTryGetCredentialsFails() MockTracer tracer = new MockTracer(); MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl", "fake path"); dut.TryInitializeAndRequireAuth(tracer, out _); string authString; @@ -105,7 +105,7 @@ public void TwoThreadsFailAtOnceStillRetriesOnce() MockTracer tracer = new MockTracer(); MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl", "fake path"); dut.TryInitializeAndRequireAuth(tracer, out _); string authString; @@ -132,7 +132,7 @@ public void TwoThreadsInterleavingFailuresStillRetriesOnce() MockTracer tracer = new MockTracer(); MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl", "fake path"); dut.TryInitializeAndRequireAuth(tracer, out _); string thread1Auth; @@ -168,7 +168,7 @@ public void TwoThreadsInterleavingFailuresShouldntStompASuccess() MockTracer tracer = new MockTracer(); MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl", "fake path"); dut.TryInitializeAndRequireAuth(tracer, out _); string thread1Auth; @@ -205,7 +205,7 @@ public void DontDoubleStoreExistingCredential() MockTracer tracer = new MockTracer(); MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl", "fake path"); dut.TryInitializeAndRequireAuth(tracer, out _); string authString; @@ -228,7 +228,7 @@ public void DontStoreDifferentCredentialFromCachedValue() MockTracer tracer = new MockTracer(); MockGitProcess gitProcess = this.GetGitProcess(); - GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl"); + GitAuthentication dut = new GitAuthentication(gitProcess, "mock://repoUrl", "fake path"); dut.TryInitializeAndRequireAuth(tracer, out _); // Get and store an initial value that will be cached diff --git a/Scalar/CommandLine/ScalarVerb.cs b/Scalar/CommandLine/ScalarVerb.cs index 294aee9b64a..81ea1a3fe71 100644 --- a/Scalar/CommandLine/ScalarVerb.cs +++ b/Scalar/CommandLine/ScalarVerb.cs @@ -107,6 +107,10 @@ protected ReturnCode Execute( { verb.Execute(); } + catch (InvalidRepoException ire) + { + this.ReportErrorAndExit($"Invalid repository: {ire.Message}"); + } catch (VerbAbortedException) { }