From 80d5098cbb6c8c4243d15f7ff2e20132b6992f4b Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 11 Nov 2023 14:36:24 -0500 Subject: [PATCH] Rename `ShaIsParent` to `CommittishIsParent` Update functionality to reflect this. --- .../Remote/GitHubRemoteDeploymentManager.cs | 2 +- .../Remote/GitLabRemoteDeploymentManager.cs | 2 +- .../Components/Engine/OpenDreamInstaller.cs | 2 +- .../Components/Repository/IRepository.cs | 8 +++--- .../Components/Repository/Repository.cs | 26 ++++++++++++++++--- .../Engine/TestOpenDreamInstaller.cs | 2 +- .../Tgstation.Server.Tests/TestRepository.cs | 4 +-- 7 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Deployment/Remote/GitHubRemoteDeploymentManager.cs b/src/Tgstation.Server.Host/Components/Deployment/Remote/GitHubRemoteDeploymentManager.cs index a31780bf21..751bb33646 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/Remote/GitHubRemoteDeploymentManager.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/Remote/GitHubRemoteDeploymentManager.cs @@ -198,7 +198,7 @@ namespace Tgstation.Server.Host.Components.Deployment.Remote return; // We don't just assume, actually check the repo contains the merge commit. - if (await repository.ShaIsParent(pr.MergeCommitSha, cancellationToken)) + if (await repository.CommittishIsParent(pr.MergeCommitSha, cancellationToken)) { if (lastMerged == null || lastMerged.MergedAt < pr.MergedAt) lastMerged = pr; diff --git a/src/Tgstation.Server.Host/Components/Deployment/Remote/GitLabRemoteDeploymentManager.cs b/src/Tgstation.Server.Host/Components/Deployment/Remote/GitLabRemoteDeploymentManager.cs index 3154b2ee03..2686764cca 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/Remote/GitLabRemoteDeploymentManager.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/Remote/GitLabRemoteDeploymentManager.cs @@ -83,7 +83,7 @@ namespace Tgstation.Server.Host.Components.Deployment.Remote return; // We don't just assume, actually check the repo contains the merge commit. - if (await repository.ShaIsParent(mergeRequest.MergeCommitSha, cancellationToken)) + if (await repository.CommittishIsParent(mergeRequest.MergeCommitSha, cancellationToken)) { if (lastMerged == null || lastMerged.ClosedAt < mergeRequest.ClosedAt) lastMerged = mergeRequest; diff --git a/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs b/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs index e279f0d0cc..ceb95a810f 100644 --- a/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs +++ b/src/Tgstation.Server.Host/Components/Engine/OpenDreamInstaller.cs @@ -152,7 +152,7 @@ namespace Tgstation.Server.Host.Components.Engine progressSection2, cancellationToken); - if (!await repo.ShaIsParent("fab769776dada6b9bcad546094d78c604049e0e9", cancellationToken)) + if (!await repo.CommittishIsParent("fab769776dada6b9bcad546094d78c604049e0e9", cancellationToken)) throw new JobException(ErrorCode.OpenDreamTooOld); return new RepositoryEngineInstallationData(IOManager, repo, InstallationSourceSubDirectory); diff --git a/src/Tgstation.Server.Host/Components/Repository/IRepository.cs b/src/Tgstation.Server.Host/Components/Repository/IRepository.cs index 33c4b9a97c..0482eaaa90 100644 --- a/src/Tgstation.Server.Host/Components/Repository/IRepository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/IRepository.cs @@ -170,13 +170,13 @@ namespace Tgstation.Server.Host.Components.Repository ValueTask CopyTo(string path, CancellationToken cancellationToken); /// - /// Check if a given is a parent of the current . + /// Check if a given is a parent of the current . /// - /// The SHA to check. + /// The committish of the SHA to check. /// The for the operation. - /// A resulting in if is a parent of , otherwise. + /// A resulting in if is a parent of , otherwise. /// This function is NOT reentrant. - Task ShaIsParent(string sha, CancellationToken cancellationToken); + Task CommittishIsParent(string committish, CancellationToken cancellationToken); /// /// Get the tracked reference's current SHA. diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 5f3602675e..cae1677f60 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -791,16 +791,34 @@ namespace Tgstation.Server.Host.Components.Repository TaskScheduler.Current); /// - public Task ShaIsParent(string sha, CancellationToken cancellationToken) => Task.Factory.StartNew( + public Task CommittishIsParent(string committish, CancellationToken cancellationToken) => Task.Factory.StartNew( () => { - var targetCommit = libGitRepo.Lookup(sha); - if (targetCommit == null) + var targetObject = libGitRepo.Lookup(committish); + if (targetObject == null) { - logger.LogTrace("Commit {sha} not found in repository", sha); + logger.LogTrace("Committish {committish} not found in repository", committish); return false; } + if (targetObject is not Commit targetCommit) + { + if (targetObject is not TagAnnotation) + { + logger.LogTrace("Committish {committish} is a {type} and does not point to a commit!", committish, targetObject.GetType().Name); + return false; + } + + targetCommit = targetObject.Peel(); + if (targetCommit == null) + { + logger.LogError( + "TagAnnotation {committish} was found but the commit associated with it could not be found in repository!", + committish); + return false; + } + } + cancellationToken.ThrowIfCancellationRequested(); var startSha = Head; var mergeResult = libGitRepo.Merge( diff --git a/tests/Tgstation.Server.Host.Tests/Components/Engine/TestOpenDreamInstaller.cs b/tests/Tgstation.Server.Host.Tests/Components/Engine/TestOpenDreamInstaller.cs index 6330ef1b10..6b8024ddac 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Engine/TestOpenDreamInstaller.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Engine/TestOpenDreamInstaller.cs @@ -40,7 +40,7 @@ namespace Tgstation.Server.Host.Components.Engine.Tests var cloneAttempts = 0; var mockRepository = new Mock(); - mockRepository.Setup(x => x.ShaIsParent(It.IsNotNull(), It.IsAny())).ReturnsAsync(true); + mockRepository.Setup(x => x.CommittishIsParent(It.IsNotNull(), It.IsAny())).ReturnsAsync(true); var mockRepositoryManager = new Mock(); mockRepositoryManager.Setup(x => x.CloneRepository( generalConfig.OpenDreamGitUrl, diff --git a/tests/Tgstation.Server.Tests/TestRepository.cs b/tests/Tgstation.Server.Tests/TestRepository.cs index e03fbe984a..8a16f0ddd9 100644 --- a/tests/Tgstation.Server.Tests/TestRepository.cs +++ b/tests/Tgstation.Server.Tests/TestRepository.cs @@ -42,10 +42,10 @@ namespace Tgstation.Server.Tests const string StartSha = "af4da8beb9f9b374b04a3cc4d65acca662e8cc1a"; await repo.CheckoutObject(StartSha, null, null, true, new JobProgressReporter(Mock.Of>(), null, (stage, progress) => { }), CancellationToken.None); - var result = await repo.ShaIsParent("2f8588a3ca0f6b027704a2a04381215619de3412", CancellationToken.None); + var result = await repo.CommittishIsParent("2f8588a3ca0f6b027704a2a04381215619de3412", CancellationToken.None); Assert.IsTrue(result); Assert.AreEqual(StartSha, repo.Head); - result = await repo.ShaIsParent("f636418bf47d238d33b0e4a34f0072b23a8aad0e", CancellationToken.None); + result = await repo.CommittishIsParent("f636418bf47d238d33b0e4a34f0072b23a8aad0e", CancellationToken.None); Assert.IsFalse(result); Assert.AreEqual(StartSha, repo.Head); }