diff --git a/docs/DevGuide.md b/docs/DevGuide.md index fcb62541f9..daa5f9572c 100644 --- a/docs/DevGuide.md +++ b/docs/DevGuide.md @@ -17,6 +17,7 @@ ('https://github.com/maestro-auth-test/maestro-test', 289474), ('https://github.com/maestro-auth-test/maestro-test2', 289474), ('https://github.com/maestro-auth-test/maestro-test3', 289474), + ('https://github.com/maestro-auth-test/maestro-test-vmr', 289474), ('https://github.com/maestro-auth-test/arcade', 289474), ('https://github.com/maestro-auth-test/dnceng-vmr', 289474); ``` diff --git a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs index eecff7e628..36246defd8 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs @@ -140,6 +140,14 @@ Task CommitAsync( /// List of currently modified staged files Task GetStagedFiles(string repoPath); + /// + /// Determines if a given path is a git repository. + /// + /// Path to a git repository + /// Git reference to check for + /// True if the path is a git repository, false otherwise + Task GitRefExists(string repoPath, string gitRef, CancellationToken cancellationToken = default); + /// /// Fetches from all remotes. /// diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs index 2ad28c7179..ea48c1d76b 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs @@ -210,7 +210,6 @@ public async Task GetObjectTypeAsync(string repoPath, string obje }; var result = await _processManager.ExecuteGit(repoPath, args); - result.ThrowIfFailed($"Failed to find object {objectSha} in {repoPath}"); return result.StandardOutput.Trim() switch { @@ -443,6 +442,22 @@ public async Task BlameLineAsync(string repoPath, string relativeFilePat return result.StandardOutput.Trim().Split(' ').First(); } + public async Task GitRefExists(string repoPath, string gitRef, CancellationToken cancellationToken = default) + { + // If the ref is a SHA or local branch/tag, we can check it directly via git cat-file -t + var objectType = await GetObjectTypeAsync(repoPath, gitRef); + if (objectType != GitObjectType.Unknown) + { + return true; + } + + // If it's a remote branch that has been fetched git cat-file -t won't work, + // because we would have to query for [remote name]/gitRef + var result = await RunGitCommandAsync(repoPath, ["branch", "-a", "--list", "*/" + gitRef], cancellationToken); + result.ThrowIfFailed($"Failed to verify if git ref '{gitRef}' exists in {repoPath}"); + return result.StandardOutput.Contains(gitRef); + } + public async Task HasWorkingTreeChangesAsync(string repoPath) { var result = await _processManager.ExecuteGit(repoPath, ["diff", "--exit-code"]); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs index 5e38b7ada0..4e5e387733 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs @@ -76,28 +76,17 @@ protected async Task PrepareCloneInternalAsync( // Path should be returned the same for all invocations // We checkout a default ref path = await PrepareCloneInternal(remoteUri, dirName, cancellationToken); - var missingCommit = false; // Verify that all requested commits are available - foreach (string commit in refsToVerify.ToArray()) + foreach (string gitRef in refsToVerify.ToArray()) { - try + if (await _localGitRepo.GitRefExists(path, gitRef, cancellationToken)) { - var objectType = await _localGitRepo.GetObjectTypeAsync(path, commit); - if (objectType == GitObjectType.Commit) - { - refsToVerify.Remove(commit); - } - } - catch - { - // Ref not found yet, let's try another remote - missingCommit = true; - break; + refsToVerify.Remove(gitRef); } } - if (!missingCommit) + if (!refsToVerify.Any()) { _logger.LogDebug("All requested refs ({refs}) found in {repo}", string.Join(", ", requestedRefs), path); break; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs index eb8e806672..30027b7724 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs @@ -45,7 +45,6 @@ public class VmrDependencyTracker : IVmrDependencyTracker { private readonly AllVersionsPropsFile _repoVersions; private readonly ISourceManifest _sourceManifest; - private readonly LocalPath _allVersionsFilePath; private readonly IVmrInfo _vmrInfo; private readonly IFileSystem _fileSystem; private readonly ISourceMappingParser _sourceMappingParser; @@ -63,7 +62,6 @@ public VmrDependencyTracker( ISourceManifest sourceManifest) { _vmrInfo = vmrInfo; - _allVersionsFilePath = vmrInfo.VmrPath / VmrInfo.GitInfoSourcesDir / AllVersionsPropsFile.FileName; _sourceManifest = sourceManifest; _repoVersions = new AllVersionsPropsFile(sourceManifest.Repositories); _fileSystem = fileSystem; @@ -94,7 +92,7 @@ public async Task InitializeSourceMappings(string? sourceMappingsPath = null) public void UpdateDependencyVersion(VmrDependencyUpdate update) { _repoVersions.UpdateVersion(update.Mapping.Name, update.TargetRevision, update.TargetVersion); - _repoVersions.SerializeToXml(_allVersionsFilePath); + _repoVersions.SerializeToXml(_vmrInfo.AllVersionsFilePath); _sourceManifest.UpdateVersion(update.Mapping.Name, update.RemoteUri, update.TargetRevision, update.TargetVersion); _fileSystem.WriteToFile(_vmrInfo.SourceManifestPath, _sourceManifest.ToJson()); @@ -126,7 +124,7 @@ public bool RemoveRepositoryVersion(string repo) if (_repoVersions.DeleteVersion(repo)) { - _repoVersions.SerializeToXml(_allVersionsFilePath); + _repoVersions.SerializeToXml(_vmrInfo.AllVersionsFilePath); hasChanges = true; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs index f2d7d9b616..d868df936a 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs @@ -59,6 +59,11 @@ public interface IVmrInfo /// Gets a full path leading to sources belonging to a given repo /// NativePath GetRepoSourcesPath(string mappingName); + + /// + /// Path to the AllRepoVersions.props + /// + NativePath AllVersionsFilePath { get; } } public class VmrInfo : IVmrInfo @@ -137,4 +142,6 @@ public VmrInfo(NativePath vmrPath, NativePath tmpPath) public static UnixPath GetRelativeRepoSourcesPath(string mappingName) => RelativeSourcesDir / mappingName; public NativePath SourceManifestPath { get; private set; } + + public NativePath AllVersionsFilePath => VmrPath / GitInfoSourcesDir / AllVersionsPropsFile.FileName; } diff --git a/src/ProductConstructionService/ProductConstructionService.WorkItems/ReminderManager.cs b/src/ProductConstructionService/ProductConstructionService.WorkItems/ReminderManager.cs index 5449e62fea..a4a4123b39 100644 --- a/src/ProductConstructionService/ProductConstructionService.WorkItems/ReminderManager.cs +++ b/src/ProductConstructionService/ProductConstructionService.WorkItems/ReminderManager.cs @@ -50,7 +50,7 @@ public async Task UnsetReminderAsync(bool isCodeFlow) { await client.DeleteWorkItemAsync(receipt.MessageId, receipt.PopReceipt); } - catch (RequestFailedException e) when (e.Message.Contains("The specified message does not exist") || e.Message.Contains("did not match the pop receipt")) + catch (RequestFailedException e) when (e.ErrorCode == "MessageNotFound" || e.ErrorCode == "PopReceiptMismatch") { // The message was already deleted, so we can ignore this exception. } diff --git a/src/ProductConstructionService/Readme.md b/src/ProductConstructionService/Readme.md index cff2d31bfb..4e225dfbe1 100644 --- a/src/ProductConstructionService/Readme.md +++ b/src/ProductConstructionService/Readme.md @@ -17,6 +17,7 @@ ('https://github.com/maestro-auth-test/maestro-test', 289474), ('https://github.com/maestro-auth-test/maestro-test2', 289474), ('https://github.com/maestro-auth-test/maestro-test3', 289474), + ('https://github.com/maestro-auth-test/maestro-test-vmr', 289474), ('https://github.com/maestro-auth-test/arcade', 289474), ('https://github.com/maestro-auth-test/dnceng-vmr', 289474); ``` diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs index 16ab961a38..bfec245b76 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs @@ -124,8 +124,8 @@ repo VMR │ x─┘ 5. 7. x │ │ │ │ │ 6.O◄┘ └──►O 8. - │ 9. │ - │ ◄────────────────┤ + │ │ + |────────────────────►O 9. │ │ */ [Test] @@ -178,7 +178,9 @@ await GitOperations.VerifyMergeConflict(VmrPath, forwardBranchName, // While the VMR accepted the content from the repo but it will get overriden by the VMR content again var hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branch: forwardBranchName); hadUpdates.ShouldHaveUpdates(); - await GitOperations.MergePrBranch(VmrPath, forwardBranchName); + await GitOperations.VerifyMergeConflict(VmrPath, forwardBranchName, + mergeTheirs: true, + expectedConflictingFile: VmrInfo.SourcesDir / Constants.ProductRepoName / _productRepoFileName); // Both VMR and repo need to have the version from the VMR as it flowed to the repo and back CheckFileContents(_productRepoFilePath, "New content from the VMR #2"); diff --git a/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs b/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs index b58291ce5a..ca8bab0542 100644 --- a/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs +++ b/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs @@ -276,7 +276,8 @@ public async Task CommitIsNotFound() var remotes = configuration.Values.Select(x => x.RemoteUri).ToArray(); - var action = async() => await _manager.PrepareCloneAsync(mapping, remotes, new[] { "sha1", "sha2", "sha4" }, "main", default); + var searchedRefs = new[] { "sha1", "sha2", "sha4" }; + var action = async() => await _manager.PrepareCloneAsync(mapping, remotes, searchedRefs, "main", default); await action.Should().ThrowAsync("because sha4 is not present anywhere"); foreach (var pair in configuration) @@ -284,6 +285,12 @@ public async Task CommitIsNotFound() _localGitRepo .Verify(x => x.AddRemoteIfMissingAsync(clonePath, pair.Value.RemoteUri, It.IsAny()), Times.Once); } + + foreach (var sha in searchedRefs) + { + _localGitRepo + .Verify(x => x.GitRefExists(clonePath, sha, It.IsAny()), Times.AtLeastOnce); + } } /// @@ -314,15 +321,11 @@ private void SetupLazyFetching(string clonePath, Dictionary .Returns(Task.CompletedTask); _localGitRepo - .Setup(x => x.GetObjectTypeAsync(clonePath, It.IsAny())) - .Callback((string _, string sha) => + .Setup(x => x.GitRefExists(clonePath, It.IsAny(), It.IsAny())) + .ReturnsAsync((string _, string sha, CancellationToken __) => { - if (!configuration.Any(p => p.Value.CommitsContained.Contains(sha) && p.Value.IsCloned)) - { - throw new Exception($"Could not find {sha}"); - } - }) - .ReturnsAsync(GitObjectType.Commit); + return configuration.Any(p => p.Value.CommitsContained.Contains(sha) && p.Value.IsCloned); + }); } }