code cleanup for GitVersionComputer & GitPrepare

GitTools · arturcic · Oct 25, 2019 · Oct 17, 2019 · Oct 17, 2019 · Oct 20, 2019
commit dbac722b18542b2750e38429d45f9142fca301b6
diff --git a/src/GitVersionCore.Tests/GitToolsTestingExtensions.cs b/src/GitVersionCore.Tests/GitToolsTestingExtensions.cs
@@ -80,7 +80,14 @@ private static SemanticVersion ExecuteGitVersion(GitVersionContext context)
         /// </summary>
         public static void InitialiseRepo(this RemoteRepositoryFixture fixture)
         {
-            new GitPreparer(new NullLog(), null, null, new Authentication(), false, fixture.LocalRepositoryFixture.RepositoryPath).Initialise(true, null);
+            var log = new NullLog();
+
+            var arguments = new Arguments
+            {
+                Authentication = new Authentication(),
+                TargetPath = fixture.LocalRepositoryFixture.RepositoryPath
+            };
+            new GitPreparer(log, arguments).Initialize(true, null);
         }
     }
 }
diff --git a/src/GitVersionCore.Tests/GitVersionExecutorTests.cs b/src/GitVersionCore.Tests/GitVersionExecutorTests.cs
@@ -46,11 +46,17 @@ public void CacheKeySameAfterReNormalizing()
             {
                 var targetUrl = "https://github.com/GitTools/GitVersion.git";
                 var targetBranch = "refs/head/master";
-                var gitPreparer = new GitPreparer(log, targetUrl, null, new Authentication(), false, fixture.RepositoryPath);
-                var configFileLocator = new DefaultConfigFileLocator(fileSystem, log);
-                gitPreparer.Initialise(true, targetBranch);
+
+                var arguments = new Arguments
+                {
+                    TargetUrl = targetUrl,
+                    TargetPath = fixture.RepositoryPath
+                };
+                var gitPreparer = new GitPreparer(log, arguments);
+                configFileLocator = new DefaultConfigFileLocator(fileSystem, log);
+                gitPreparer.Initialize(true, targetBranch);
                 var cacheKey1 = GitVersionCacheKeyFactory.Create(fileSystem, log, gitPreparer, null, configFileLocator);
-                gitPreparer.Initialise(true, targetBranch);
+                gitPreparer.Initialize(true, targetBranch);
                 var cacheKey2 = GitVersionCacheKeyFactory.Create(fileSystem, log, gitPreparer, null, configFileLocator);
 
                 cacheKey2.Value.ShouldBe(cacheKey1.Value);
@@ -74,8 +80,15 @@ public void CacheKeyForWorktree()
                     repo.Worktrees.Add("worktree", worktreePath, false);
 
                     var targetUrl = "https://github.com/GitTools/GitVersion.git";
-                    var gitPreparer = new GitPreparer(log, targetUrl, null, new Authentication(), false, worktreePath);
-                    var configFileLocator = new DefaultConfigFileLocator(fileSystem, log);
+
+                    var arguments = new Arguments
+                    {
+                        TargetUrl = targetUrl,
+                        TargetPath = worktreePath
+                    };
+
+                    var gitPreparer = new GitPreparer(log, arguments);
+                    configFileLocator = new DefaultConfigFileLocator(fileSystem, log);
                     var cacheKey = GitVersionCacheKeyFactory.Create(fileSystem, log, gitPreparer, null, configFileLocator);
                     cacheKey.Value.ShouldNotBeEmpty();
                 }
@@ -183,7 +196,12 @@ public void CacheFileExistsOnDiskWhenOverrideConfigIsSpecifiedVersionShouldBeDyn
             {
                 fileSystem.WriteAllText(vv.FileName, versionCacheFileContent);
 
-                var gitPreparer = new GitPreparer(log, null, null, null, false, fixture.RepositoryPath);
+                var arguments = new Arguments
+                {
+                    TargetPath = fixture.RepositoryPath
+                };
+
+                var gitPreparer = new GitPreparer(log, arguments);
                 var cacheDirectory = GitVersionCache.GetCacheDirectory(gitPreparer);
 
                 var cacheDirectoryTimestamp = fileSystem.GetLastDirectoryWrite(cacheDirectory);
@@ -353,7 +371,14 @@ public void GetProjectRootDirectory_WorkingDirectoryWithWorktree()
                     repo.Worktrees.Add("worktree", worktreePath, false);
 
                     var targetUrl = "https://github.com/GitTools/GitVersion.git";
-                    var gitPreparer = new GitPreparer(log, targetUrl, null, new Authentication(), false, worktreePath);
+
+                    var arguments = new Arguments
+                    {
+                        TargetUrl = targetUrl,
+                        TargetPath = worktreePath
+                    };
+
+                    var gitPreparer = new GitPreparer(log, arguments);
                     gitPreparer.GetProjectRootDirectory().TrimEnd('/', '\\').ShouldBe(worktreePath);
                 }
                 finally
@@ -371,7 +396,14 @@ public void GetProjectRootDirectory_NoWorktree()
             RepositoryScope(versionAndBranchFinder, (fixture, vv) =>
             {
                 var targetUrl = "https://github.com/GitTools/GitVersion.git";
-                var gitPreparer = new GitPreparer(log, targetUrl, null, new Authentication(), false, fixture.RepositoryPath);
+
+                var arguments = new Arguments
+                {
+                    TargetUrl = targetUrl,
+                    TargetPath = fixture.RepositoryPath
+                };
+
+                var gitPreparer = new GitPreparer(log, arguments);
                 var expectedPath = fixture.RepositoryPath.TrimEnd('/', '\\');
                 gitPreparer.GetProjectRootDirectory().TrimEnd('/', '\\').ShouldBe(expectedPath);
             });
@@ -403,7 +435,14 @@ public void GetDotGitDirectory_NoWorktree()
             RepositoryScope(versionAndBranchFinder, (fixture, vv) =>
             {
                 var targetUrl = "https://github.com/GitTools/GitVersion.git";
-                var gitPreparer = new GitPreparer(log, targetUrl, null, new Authentication(), false, fixture.RepositoryPath);
+
+                var arguments = new Arguments
+                {
+                    TargetUrl = targetUrl,
+                    TargetPath = fixture.RepositoryPath
+                };
+
+                var gitPreparer = new GitPreparer(log, arguments);
                 var expectedPath = Path.Combine(fixture.RepositoryPath, ".git");
                 gitPreparer.GetDotGitDirectory().ShouldBe(expectedPath);
             });
@@ -427,7 +466,14 @@ public void GetDotGitDirectory_Worktree()
                     repo.Worktrees.Add("worktree", worktreePath, false);
 
                     var targetUrl = "https://github.com/GitTools/GitVersion.git";
-                    var gitPreparer = new GitPreparer(log, targetUrl, null, new Authentication(), false, worktreePath);
+
+                    var arguments = new Arguments
+                    {
+                        TargetUrl = targetUrl,
+                        TargetPath = worktreePath
+                    };
+
+                    var gitPreparer = new GitPreparer(log, arguments);
                     var expectedPath = Path.Combine(fixture.RepositoryPath, ".git");
                     gitPreparer.GetDotGitDirectory().ShouldBe(expectedPath);
                 }
@@ -438,7 +484,7 @@ public void GetDotGitDirectory_Worktree()
             });
         }
 
-        private void RepositoryScope(GitVersionComputer gitVersionComputer, Action<EmptyRepositoryFixture, VersionVariables> fixtureAction = null)
+        private void RepositoryScope(IGitVersionComputer gitVersionComputer, Action<EmptyRepositoryFixture, VersionVariables> fixtureAction = null)
         {
             // Make sure GitVersion doesn't trigger build server mode when we are running the tests
             environment.SetEnvironmentVariable(AppVeyor.EnvironmentVariableName, null);

diff --git a/src/GitVersionCore/GitPreparer.cs b/src/GitVersionCore/GitPreparer.cs
@@ -15,58 +15,58 @@ public class GitPreparer
         private readonly bool noFetch;
 
         private const string defaultRemoteName = "origin";
+        private string dynamicGitRepositoryPath;
 
         public GitPreparer(ILog log, Arguments arguments)
-            : this(log, arguments.TargetUrl, arguments.DynamicRepositoryLocation, arguments.Authentication, arguments.NoFetch, arguments.TargetPath)
-        {
-        }
-
-        public GitPreparer(ILog log, string targetUrl, string dynamicRepositoryLocation, Authentication authentication, bool noFetch, string targetPath)
         {
             this.log = log;
-            TargetUrl = targetUrl;
-            this.dynamicRepositoryLocation = dynamicRepositoryLocation;
-            this.authentication =
-                new AuthenticationInfo
-                {
-                    Username = authentication?.Username,
-                    Password = authentication?.Password
-                };
-            this.noFetch = noFetch;
-            WorkingDirectory = targetPath.TrimEnd('/', '\\');
-        }
-
-        public string TargetUrl { get; }
 
-        public string WorkingDirectory { get; }
+            TargetUrl = arguments.TargetUrl;
+            WorkingDirectory = arguments.TargetPath.TrimEnd('/', '\\');
 
-        private bool IsDynamicGitRepository => !string.IsNullOrWhiteSpace(DynamicGitRepositoryPath);
+            dynamicRepositoryLocation = arguments.DynamicRepositoryLocation;
+            authentication = new AuthenticationInfo
+            {
+                Username = arguments.Authentication?.Username,
+                Password = arguments.Authentication?.Password
+            };
 
-        private string DynamicGitRepositoryPath { get; set; }
+            noFetch = arguments.NoFetch;
+        }
 
-        public void Initialise(bool normalizeGitDirectory, string currentBranch, bool shouldCleanUpRemotes = false)
+        public void Initialize(bool normalizeGitDirectory, string currentBranch, bool shouldCleanUpRemotes = false)
         {
             if (string.IsNullOrWhiteSpace(TargetUrl))
             {
-                if (normalizeGitDirectory)
+                if (!normalizeGitDirectory) return;
+                using (log.IndentLog($"Normalizing git directory for branch '{currentBranch}'"))
                 {
-                    using (log.IndentLog($"Normalizing git directory for branch '{currentBranch}'"))
+                    if (shouldCleanUpRemotes)
                     {
-                        if (shouldCleanUpRemotes)
-                        {
-                            CleanupDuplicateOrigin();
-                        }
-                        GitRepositoryHelper.NormalizeGitDirectory(log, GetDotGitDirectory(), authentication, noFetch, currentBranch, IsDynamicGitRepository);
+                        CleanupDuplicateOrigin();
                     }
+                    GitRepositoryHelper.NormalizeGitDirectory(log, GetDotGitDirectory(), authentication, noFetch, currentBranch, IsDynamicGitRepository());
                 }
                 return;
             }
 
             var tempRepositoryPath = CalculateTemporaryRepositoryPath(TargetUrl, dynamicRepositoryLocation);
 
-            DynamicGitRepositoryPath = CreateDynamicRepository(tempRepositoryPath, authentication, TargetUrl, currentBranch, noFetch);
+            dynamicGitRepositoryPath = CreateDynamicRepository(tempRepositoryPath, authentication, TargetUrl, currentBranch, noFetch);
         }
 
+        public TResult WithRepository<TResult>(Func<IRepository, TResult> action)
+        {
+            using IRepository repo = new Repository(GetDotGitDirectory());
+            return action(repo);
+        }
+
+        public string TargetUrl { get; }
+
+        public string WorkingDirectory { get; }
+
+        private bool IsDynamicGitRepository() => !string.IsNullOrWhiteSpace(dynamicGitRepositoryPath);
+
         private void CleanupDuplicateOrigin()
         {
             var remoteToKeep = defaultRemoteName;
@@ -91,12 +91,6 @@ private void CleanupDuplicateOrigin()
             }
         }
 
-        public TResult WithRepository<TResult>(Func<IRepository, TResult> action)
-        {
-            using IRepository repo = new Repository(GetDotGitDirectory());
-            return action(repo);
-        }
-
         private static string CalculateTemporaryRepositoryPath(string targetUrl, string dynamicRepositoryLocation)
         {
             var userTemp = dynamicRepositoryLocation ?? Path.GetTempPath();
@@ -137,7 +131,7 @@ private static bool GitRepoHasMatchingRemote(string possiblePath, string targetU
 
         public string GetDotGitDirectory()
         {
-            var dotGitDirectory = IsDynamicGitRepository ? DynamicGitRepositoryPath : Repository.Discover(WorkingDirectory);
+            var dotGitDirectory = IsDynamicGitRepository() ? dynamicGitRepositoryPath : Repository.Discover(WorkingDirectory);
 
             dotGitDirectory = dotGitDirectory?.TrimEnd('/', '\\');
             if (string.IsNullOrEmpty(dotGitDirectory))
@@ -151,8 +145,8 @@ public string GetDotGitDirectory()
 
         public string GetProjectRootDirectory()
         {
-            log.Info($"IsDynamicGitRepository: {IsDynamicGitRepository}");
-            if (IsDynamicGitRepository)
+            log.Info($"IsDynamicGitRepository: {IsDynamicGitRepository()}");
+            if (IsDynamicGitRepository())
             {
                 log.Info($"Returning Project Root as {WorkingDirectory}");
                 return WorkingDirectory;

diff --git a/src/GitVersionCore/GitVersionComputer.cs b/src/GitVersionCore/GitVersionComputer.cs
@@ -27,17 +27,43 @@ public GitVersionComputer(IFileSystem fileSystem, ILog log, IConfigFileLocator c
 
         public VersionVariables ComputeVersionVariables(Arguments arguments)
         {
-            return ComputeVersionVariables(
-                arguments.TargetUrl, arguments.DynamicRepositoryLocation, arguments.Authentication,
-                arguments.TargetBranch, arguments.NoFetch, arguments.TargetPath,
-                arguments.CommitId, arguments.OverrideConfig, arguments.NoCache, arguments.NoNormalize);
+            var buildServer = buildServerResolver.GetCurrentBuildServer();
+
+            // Normalize if we are running on build server
+            var normalizeGitDirectory = !arguments.NoNormalize && buildServer != null;
+            arguments.NoFetch = arguments.NoFetch || buildServer != null && buildServer.PreventFetch();
+            var shouldCleanUpRemotes = buildServer != null && buildServer.ShouldCleanUpRemotes();
+
+            var gitPreparer = new GitPreparer(log, arguments);
+
+            var currentBranch = ResolveCurrentBranch(buildServer, arguments.TargetBranch, !string.IsNullOrWhiteSpace(arguments.DynamicRepositoryLocation));
+
+            gitPreparer.Initialize(normalizeGitDirectory, currentBranch, shouldCleanUpRemotes);
+
+            var dotGitDirectory = gitPreparer.GetDotGitDirectory();
+            var projectRoot = gitPreparer.GetProjectRootDirectory();
+
+            log.Info($"Project root is: {projectRoot}");
+            log.Info($"DotGit directory is: {dotGitDirectory}");
+            if (string.IsNullOrEmpty(dotGitDirectory) || string.IsNullOrEmpty(projectRoot))
+            {
+                // TODO Link to wiki article
+                throw new Exception($"Failed to prepare or find the .git directory in path '{arguments.TargetPath}'.");
+            }
+
+            return GetCachedGitVersionInfo(arguments.TargetBranch, arguments.CommitId, arguments.OverrideConfig, arguments.NoCache, gitPreparer);
         }
 
-        public bool TryGetVersion(string directory, out VersionVariables versionVariables, bool noFetch)
+        public bool TryGetVersion(string directory, bool noFetch, out VersionVariables versionVariables)
         {
             try
             {
-                versionVariables = ComputeVersionVariables(null, null, null, null, noFetch, directory, null);
+                var arguments = new Arguments
+                {
+                    NoFetch = noFetch,
+                    TargetPath = directory
+                };
+                versionVariables = ComputeVersionVariables(arguments);
                 return true;
             }
             catch (Exception ex)
@@ -48,39 +74,21 @@ public bool TryGetVersion(string directory, out VersionVariables versionVariable
             }
         }
 
-        private VersionVariables ComputeVersionVariables(string targetUrl, string dynamicRepositoryLocation, Authentication authentication, string targetBranch, bool noFetch, string workingDirectory, string commitId, Config overrideConfig = null, bool noCache = false, bool noNormalize = false)
+        private string ResolveCurrentBranch(IBuildServer buildServer, string targetBranch, bool isDynamicRepository)
         {
-            // Normalize if we are running on build server
-            var buildServer = buildServerResolver.GetCurrentBuildServer();
-            var normalizeGitDirectory = !noNormalize && buildServer != null;
-            var fetch = noFetch || buildServer != null && buildServer.PreventFetch();
-            var shouldCleanUpRemotes = buildServer != null && buildServer.ShouldCleanUpRemotes();
-            var gitPreparer = new GitPreparer(log, targetUrl, dynamicRepositoryLocation, authentication, fetch, workingDirectory);
-
-            gitPreparer.Initialise(normalizeGitDirectory, ResolveCurrentBranch(buildServer, targetBranch, !string.IsNullOrWhiteSpace(dynamicRepositoryLocation)), shouldCleanUpRemotes);
-            var dotGitDirectory = gitPreparer.GetDotGitDirectory();
-            var projectRoot = gitPreparer.GetProjectRootDirectory();
-
-            // TODO Can't use this, it still needs work
-            //var gitRepository = GitRepositoryFactory.CreateRepository(new RepositoryInfo
-            //{
-            //    Url = targetUrl,
-            //    Branch = targetBranch,
-            //    Authentication = new AuthenticationInfo
-            //    {
-            //        Username = authentication.Username,
-            //        Password = authentication.Password
-            //    },
-            //    Directory = workingDirectory
-            //});
-            log.Info($"Project root is: {projectRoot}");
-            log.Info($"DotGit directory is: {dotGitDirectory}");
-            if (string.IsNullOrEmpty(dotGitDirectory) || string.IsNullOrEmpty(projectRoot))
+            if (buildServer == null)
             {
-                // TODO Link to wiki article
-                throw new Exception($"Failed to prepare or find the .git directory in path '{workingDirectory}'.");
+                return targetBranch;
             }
 
+            var currentBranch = buildServer.GetCurrentBranch(isDynamicRepository) ?? targetBranch;
+            log.Info("Branch from build environment: " + currentBranch);
+
+            return currentBranch;
+        }
+
+        private VersionVariables GetCachedGitVersionInfo(string targetBranch, string commitId, Config overrideConfig, bool noCache, GitPreparer gitPreparer)
+        {
             var cacheKey = GitVersionCacheKeyFactory.Create(fileSystem, log, gitPreparer, overrideConfig, configFileLocator);
             var versionVariables = noCache ? default : gitVersionCache.LoadVersionVariablesFromDiskCache(gitPreparer, cacheKey);
             if (versionVariables == null)
@@ -91,7 +99,7 @@ private VersionVariables ComputeVersionVariables(string targetUrl, string dynami
                 {
                     try
                     {
-                      gitVersionCache.WriteVariablesToDiskCache(gitPreparer, cacheKey, versionVariables);
+                        gitVersionCache.WriteVariablesToDiskCache(gitPreparer, cacheKey, versionVariables);
                     }
                     catch (AggregateException e)
                     {
@@ -103,20 +111,7 @@ private VersionVariables ComputeVersionVariables(string targetUrl, string dynami
             return versionVariables;
         }
 
-        private string ResolveCurrentBranch(IBuildServer buildServer, string targetBranch, bool isDynamicRepository)
-        {
-            if (buildServer == null)
-            {
-                return targetBranch;
-            }
-
-            var currentBranch = buildServer.GetCurrentBranch(isDynamicRepository) ?? targetBranch;
-            log.Info("Branch from build environment: " + currentBranch);
-
-            return currentBranch;
-        }
-
-        private VersionVariables ExecuteInternal(string targetBranch, string commitId, GitPreparer gitPreparer, Config overrideConfig = null)
+        private VersionVariables ExecuteInternal(string targetBranch, string commitId, GitPreparer gitPreparer, Config overrideConfig)
         {
             var versionFinder = new GitVersionFinder();
             var configuration = ConfigurationProvider.Provide(gitPreparer, overrideConfig: overrideConfig, configFileLocator: configFileLocator);