From b136f7ef59b9bc08b3560968d66ed5b7271ed0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Tue, 26 Nov 2024 14:22:47 +0100 Subject: [PATCH 01/11] Support multiple VMRs in PCS (#4181) Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com> --- .../VirtualMonoRepo/BackflowOperation.cs | 10 +- .../VirtualMonoRepo/CodeFlowOperation.cs | 7 - .../VirtualMonoRepo/ForwardFlowOperation.cs | 12 +- .../CodeFlowCommandLineOptions.cs | 4 - .../VmrCommandLineOptionsBase.cs | 2 +- .../DarcLib/ILocalGitClient.cs | 8 ++ .../DarcLib/ILocalGitRepo.cs | 5 + .../DarcLib/LocalGitClient.cs | 6 + .../DarcLib/LocalGitRepo.cs | 3 + .../VirtualMonoRepo/PcsVmrBackFlower.cs | 13 +- .../VirtualMonoRepo/PcsVmrForwardFlower.cs | 6 +- .../DarcLib/VirtualMonoRepo/VmrBackflower.cs | 122 ++++++++++++------ .../VirtualMonoRepo/VmrCloneManager.cs | 40 +++--- .../DarcLib/VirtualMonoRepo/VmrCodeflower.cs | 16 +-- .../VirtualMonoRepo/VmrForwardFlower.cs | 113 ++++++++++------ .../DarcLib/VirtualMonoRepo/VmrInitializer.cs | 2 +- .../DarcLib/VirtualMonoRepo/VmrManagerBase.cs | 6 +- .../VirtualMonoRepo/VmrRegistrations.cs | 91 ++++++++----- .../DarcLib/VirtualMonoRepo/VmrUpdater.cs | 13 +- .../InitializationBackgroundService.cs | 5 +- .../VirtualMonoRepo/VmrConfiguration.cs | 8 +- .../PullRequestUpdater.cs | 10 +- .../VmrBackflowTest.cs | 10 +- .../VmrCodeFlowTests.cs | 44 ------- .../VmrForwardFlowTest.cs | 6 +- .../VmrTestsBase.cs | 65 ++++++++-- .../PullRequestUpdaterTests.cs | 2 +- 27 files changed, 364 insertions(+), 265 deletions(-) diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/BackflowOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/BackflowOperation.cs index 287f62b6f9..5ad9798bb2 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/BackflowOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/BackflowOperation.cs @@ -28,18 +28,14 @@ internal class BackflowOperation( protected override async Task FlowAsync( string mappingName, NativePath targetDirectory, - string? targetRepoPath, CancellationToken cancellationToken) { - targetRepoPath ??= Environment.CurrentDirectory!; - var targetRepo = new NativePath(targetRepoPath); return await vmrBackFlower.FlowBackAsync( mappingName, - targetRepo, - shaToFlow: null, - _options.Build, + targetDirectory, + _options.Build ?? throw new Exception("Please specify a build to flow"), excludedAssets: null, - await GetBaseBranch(targetRepo), + await GetBaseBranch(targetDirectory), await GetTargetBranch(_vmrInfo.VmrPath), _options.DiscardPatches, cancellationToken); diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs index 65b59592a6..9c91b0de44 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs @@ -56,24 +56,17 @@ protected override async Task ExecuteInternalAsync( _vmrInfo.TmpPath = new NativePath(_options.RepositoryDirectory); } - if (_options.Build.HasValue && _options.Commit != null) - { - throw new ArgumentException("Cannot specify both --build and --commit"); - } - await _dependencyTracker.InitializeSourceMappings(); await FlowAsync( repoName, new NativePath(targetDirectory), - _options.Commit, cancellationToken); } protected abstract Task FlowAsync( string mappingName, NativePath targetDirectory, - string? shaToFlow, CancellationToken cancellationToken); protected async Task GetBaseBranch(NativePath repoPath) diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/ForwardFlowOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/ForwardFlowOperation.cs index 71eac3fd43..15086ba32c 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/ForwardFlowOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/ForwardFlowOperation.cs @@ -26,20 +26,16 @@ internal class ForwardFlowOperation( protected override async Task FlowAsync( string mappingName, - NativePath targetDirectory, - string? sourceRepoPath, + NativePath repoPath, CancellationToken cancellationToken) { - var sourceRepo = new NativePath(sourceRepoPath ?? Environment.CurrentDirectory!); - return await vmrForwardFlower.FlowForwardAsync( mappingName, - sourceRepo, - shaToFlow: null, - _options.Build, + repoPath, + _options.Build ?? throw new Exception("Please specify a build to flow"), excludedAssets: null, await GetBaseBranch(new NativePath(_options.VmrPath)), - await GetTargetBranch(sourceRepo), + await GetTargetBranch(repoPath), _options.DiscardPatches, cancellationToken); } diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/CodeFlowCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/CodeFlowCommandLineOptions.cs index 4d5387e61c..b9948ac71f 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/CodeFlowCommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/CodeFlowCommandLineOptions.cs @@ -10,7 +10,6 @@ namespace Microsoft.DotNet.Darc.Options.VirtualMonoRepo; internal interface ICodeFlowCommandLineOptions : IBaseVmrCommandLineOptions { int? Build { get; set; } - string Commit { get; set; } bool DiscardPatches { get; set; } string RepositoryDirectory { get; set; } public string BaseBranch { get; set; } @@ -37,9 +36,6 @@ internal abstract class CodeFlowCommandLineOptions : VmrCommandLineOptions [Option("build", Required = false, HelpText = "If specified, flows the given build. Cannot be used with --ref.")] public int? Build { get; set; } - [Option("commit", Required = false, HelpText = "If specified, flows the given commit. Cannot be used with --build.")] - public string Commit { get; set; } - [Option("base-branch", Required = false, HelpText = "Name of the branch of the target repository to apply changes on top of. Defaults to the checked out branch")] public string BaseBranch { get; set; } diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs index cf31484c2b..4ff1833701 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs @@ -46,7 +46,7 @@ protected void RegisterVmrServices(IServiceCollection services, string? tmpPath) tmpPath = Path.GetFullPath(tmpPath ?? Path.GetTempPath()); - services.AddVmrManagers(GitLocation, VmrPath, tmpPath, gitHubToken, azureDevOpsToken); + services.AddSingleVmrSupport(GitLocation, VmrPath, tmpPath, gitHubToken, azureDevOpsToken); services.TryAddTransient(); } } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs index 2a193c6680..eecff7e628 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs @@ -150,6 +150,14 @@ Task FetchAllAsync( IReadOnlyCollection remoteUris, CancellationToken cancellationToken = default); + /// + /// Performs `git pull` + /// + /// Path to a git repository + Task PullAsync( + string repoPath, + CancellationToken cancellationToken = default); + /// /// Stages files from the given path. /// diff --git a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitRepo.cs b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitRepo.cs index 25d9e6f9ef..b7bc5db83f 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitRepo.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitRepo.cs @@ -155,6 +155,11 @@ Task CommitAsync( /// List of remotes to fetch from Task FetchAllAsync(IReadOnlyCollection remoteUris, CancellationToken cancellationToken = default); + /// + /// Performs `git pull` + /// + Task PullAsync(CancellationToken cancellationToken = default); + /// /// Returns a list of modified staged files. /// diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs index 25530465d8..2ad28c7179 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs @@ -240,6 +240,12 @@ public async Task FetchAllAsync( } } + public async Task PullAsync(string repoPath, CancellationToken cancellationToken = default) + { + var result = await _processManager.ExecuteGit(repoPath, ["pull"], cancellationToken: cancellationToken); + result.ThrowIfFailed($"Failed to pull updates in {repoPath}"); + } + /// /// Add a remote to a local repo if does not already exist. /// diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitRepo.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitRepo.cs index 3089be4f51..ade9387522 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitRepo.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitRepo.cs @@ -71,6 +71,9 @@ public async Task GetCheckedOutBranchAsync() public async Task FetchAllAsync(IReadOnlyCollection remoteUris, CancellationToken cancellationToken = default) => await _localGitClient.FetchAllAsync(Path, remoteUris, cancellationToken); + public async Task PullAsync(CancellationToken cancellationToken = default) + => await _localGitClient.PullAsync(Path, cancellationToken); + public async Task GetStagedFiles() => await _localGitClient.GetStagedFiles(Path); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs index 4318cc7e8b..74fe6719d7 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs @@ -77,9 +77,8 @@ public PcsVmrBackFlower( CancellationToken cancellationToken = default) { (bool targetBranchExisted, SourceMapping mapping, ILocalGitRepo targetRepo) = await PrepareVmrAndRepo( - subscription.SourceDirectory, + subscription, build, - subscription.TargetBranch, targetBranch, cancellationToken); @@ -89,7 +88,6 @@ public PcsVmrBackFlower( mapping, targetRepo, lastFlow, - build.Commit, build, subscription.ExcludedAssets, subscription.TargetBranch, @@ -102,9 +100,8 @@ public PcsVmrBackFlower( } private async Task<(bool, SourceMapping, ILocalGitRepo)> PrepareVmrAndRepo( - string mappingName, + Subscription subscription, Build build, - string baseBranch, string targetBranch, CancellationToken cancellationToken) { @@ -116,7 +113,7 @@ await _vmrCloneManager.PrepareVmrAsync( cancellationToken); // Prepare repo - SourceMapping mapping = _dependencyTracker.GetMapping(mappingName); + SourceMapping mapping = _dependencyTracker.GetMapping(subscription.SourceDirectory); var remotes = new[] { mapping.DefaultRemote, _sourceManifest.GetRepoVersion(mapping.Name).RemoteUri } .Distinct() .OrderRemotesByLocalPublicOther() @@ -131,7 +128,7 @@ await _vmrCloneManager.PrepareVmrAsync( targetRepo = await _repositoryCloneManager.PrepareCloneAsync( mapping, remotes, - [baseBranch, targetBranch], + [subscription.TargetBranch, targetBranch], targetBranch, cancellationToken); targetBranchExisted = true; @@ -142,7 +139,7 @@ await _vmrCloneManager.PrepareVmrAsync( targetRepo = await _repositoryCloneManager.PrepareCloneAsync( mapping, remotes, - baseBranch, + subscription.TargetBranch, cancellationToken); await targetRepo.CreateBranchAsync(targetBranch); targetBranchExisted = false; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs index 1c90c3ee4f..7f2fbf8a33 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs @@ -72,7 +72,11 @@ public async Task FlowForwardAsync( CancellationToken cancellationToken = default) { var baseBranch = subscription.TargetBranch; - bool targetBranchExisted = await PrepareVmr(baseBranch, targetBranch, cancellationToken); + bool targetBranchExisted = await PrepareVmr( + subscription.TargetRepository, + baseBranch, + targetBranch, + cancellationToken); // Prepare repo SourceMapping mapping = _dependencyTracker.GetMapping(subscription.TargetDirectory); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs index e226fb6aab..3207901674 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs @@ -23,7 +23,6 @@ public interface IVmrBackFlower /// /// Mapping to flow /// Local checkout of the repository - /// SHA to flow /// Build to flow /// Assets to exclude from the dependency flow /// If target branch does not exist, it is created off of this branch @@ -32,8 +31,28 @@ public interface IVmrBackFlower Task FlowBackAsync( string mapping, NativePath targetRepo, - string? shaToFlow, - int? buildToFlow, + int buildToFlow, + IReadOnlyCollection? excludedAssets, + string baseBranch, + string targetBranch, + bool discardPatches = false, + CancellationToken cancellationToken = default); + + /// + /// Flows backward the code from the VMR to the target branch of a product repo. + /// This overload is used in the context of the darc CLI. + /// + /// Mapping to flow + /// Local checkout of the repository + /// Build to flow + /// Assets to exclude from the dependency flow + /// If target branch does not exist, it is created off of this branch + /// Target branch to make the changes on + /// Keep patch files? + Task FlowBackAsync( + string mapping, + NativePath targetRepo, + Build buildToFlow, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -46,7 +65,6 @@ Task FlowBackAsync( /// /// Mapping to flow /// Local checkout of the repository - /// SHA to flow /// Build to flow /// Assets to exclude from the dependency flow /// If target branch does not exist, it is created off of this branch @@ -55,8 +73,7 @@ Task FlowBackAsync( Task FlowBackAsync( string mapping, ILocalGitRepo targetRepo, - string? shaToFlow, - int? buildToFlow, + int buildToFlow, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -116,8 +133,7 @@ public VmrBackFlower( public Task FlowBackAsync( string mapping, NativePath targetRepoPath, - string? shaToFlow, - int? buildToFlow, + int buildToFlow, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -126,7 +142,6 @@ public Task FlowBackAsync( => FlowBackAsync( mapping, _localGitRepoFactory.Create(targetRepoPath), - shaToFlow, buildToFlow, excludedAssets, baseBranch, @@ -134,42 +149,74 @@ public Task FlowBackAsync( discardPatches, cancellationToken); + public Task FlowBackAsync( + string mapping, + NativePath targetRepoPath, + Build buildToFlow, + IReadOnlyCollection? excludedAssets, + string baseBranch, + string targetBranch, + bool discardPatches = false, + CancellationToken cancellationToken = default) + { + return FlowBackAsync( + mapping, + _localGitRepoFactory.Create(targetRepoPath), + buildToFlow, + excludedAssets, + baseBranch, + targetBranch, + discardPatches, + cancellationToken); + } + public async Task FlowBackAsync( string mappingName, ILocalGitRepo targetRepo, - string? shaToFlow, - int? buildToFlow, + int buildToFlow, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, bool discardPatches = false, CancellationToken cancellationToken = default) { - Build? build = null; - if (buildToFlow.HasValue) - { - build = await _barClient.GetBuildAsync(buildToFlow.Value) - ?? throw new Exception($"Failed to find build with BAR ID {buildToFlow}"); - } + Build build = await _barClient.GetBuildAsync(buildToFlow) + ?? throw new Exception($"Failed to find build with BAR ID {buildToFlow}"); - // SHA comes either directly or from the build or if none supplied, from tip of the VMR - shaToFlow ??= build?.Commit; - (bool targetBranchExisted, SourceMapping mapping, shaToFlow) = await PrepareVmrAndRepo( + return await FlowBackAsync( mappingName, targetRepo, - shaToFlow, + build, + excludedAssets, baseBranch, targetBranch, + discardPatches, cancellationToken); + } - shaToFlow ??= await _localGitClient.GetShaForRefAsync(_vmrInfo.VmrPath); + protected async Task FlowBackAsync( + string mappingName, + ILocalGitRepo targetRepo, + Build build, + IReadOnlyCollection? excludedAssets, + string baseBranch, + string targetBranch, + bool discardPatches = false, + CancellationToken cancellationToken = default) + { + (bool targetBranchExisted, SourceMapping mapping) = await PrepareVmrAndRepo( + mappingName, + targetRepo, + build, + baseBranch, + targetBranch, + cancellationToken); Codeflow lastFlow = await GetLastFlowAsync(mapping, targetRepo, currentIsBackflow: true); return await FlowBackAsync( mapping, targetRepo, lastFlow, - shaToFlow, build, excludedAssets, baseBranch, @@ -183,8 +230,7 @@ protected async Task FlowBackAsync( SourceMapping mapping, ILocalGitRepo targetRepo, Codeflow lastFlow, - string shaToFlow, - Build? build, + Build build, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -194,7 +240,7 @@ protected async Task FlowBackAsync( { var hasChanges = await FlowCodeAsync( lastFlow, - new Backflow(lastFlow.TargetSha, shaToFlow), + new Backflow(lastFlow.TargetSha, build.Commit), targetRepo, mapping, build, @@ -210,7 +256,7 @@ protected async Task FlowBackAsync( targetRepo, build, excludedAssets, - sourceElementSha: shaToFlow, + sourceElementSha: build.Commit, cancellationToken); return hasChanges; @@ -221,7 +267,7 @@ protected override async Task SameDirectionFlowAsync( Codeflow lastFlow, Codeflow currentFlow, ILocalGitRepo targetRepo, - Build? build, + Build build, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -313,7 +359,8 @@ await FlowCodeAsync( new Backflow(lastLastFlow.SourceSha, lastFlow.SourceSha), targetRepo, mapping, - /* TODO (https://github.com/dotnet/arcade-services/issues/4166): Find a previous build? */ null, + // TODO (https://github.com/dotnet/arcade-services/issues/4166): Find a previous build? + new Build(-1, DateTimeOffset.Now, 0, false, false, lastLastFlow.TargetSha, [], [], [], []), excludedAssets, targetBranch, targetBranch, @@ -353,7 +400,7 @@ protected override async Task OppositeDirectionFlowAsync( Codeflow lastFlow, Codeflow currentFlow, ILocalGitRepo targetRepo, - Build? build, + Build build, string baseBranch, string targetBranch, bool discardPatches, @@ -441,22 +488,15 @@ .. mapping.Exclude.Select(VmrPatchHandler.GetExclusionRule), return true; } - private async Task<(bool, SourceMapping, string)> PrepareVmrAndRepo( + private async Task<(bool, SourceMapping)> PrepareVmrAndRepo( string mappingName, ILocalGitRepo targetRepo, - string? shaToFlow, + Build build, string baseBranch, string targetBranch, CancellationToken cancellationToken) { - if (shaToFlow is null) - { - shaToFlow = await _localGitClient.GetShaForRefAsync(_vmrInfo.VmrPath); - } - else - { - await _vmrCloneManager.PrepareVmrAsync(shaToFlow, CancellationToken.None); - } + await _vmrCloneManager.PrepareVmrAsync([build.GetRepository()], [build.Commit], build.Commit, cancellationToken); SourceMapping mapping = _dependencyTracker.GetMapping(mappingName); ISourceComponent repoInfo = _sourceManifest.GetRepoVersion(mappingName); @@ -490,6 +530,6 @@ await _repositoryCloneManager.PrepareCloneAsync( targetBranchExisted = false; }; - return (targetBranchExisted, mapping, shaToFlow); + return (targetBranchExisted, mapping); } } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs index 12a8cf672b..2cb6e26493 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.IO; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.DarcLib.Helpers; @@ -25,10 +27,6 @@ Task PrepareVmrAsync( IReadOnlyCollection requestedRefs, string checkoutRef, CancellationToken cancellationToken); - - Task PrepareVmrAsync( - string checkoutRef, - CancellationToken cancellationToken); } public class VmrCloneManager : CloneManager, IVmrCloneManager @@ -60,30 +58,36 @@ public async Task PrepareVmrAsync( string checkoutRef, CancellationToken cancellationToken) { + // TODO https://github.com/dotnet/arcade-services/issues/3318: We should check if working tree is clean + + // This makes sure we keep different VMRs separate + // We expect to have up to 3: + // 1. The GitHub VMR (dotnet/dotnet) + // 2. The AzDO mirror (dotnet-dotnet) + // 3. The E2E test VMR (maestro-auth-tests/maestro-test-vmr) + var folderName = StringUtils.GetXxHash64( + string.Join(';', remoteUris.Distinct().OrderBy(u => u))); + ILocalGitRepo vmr = await PrepareCloneInternalAsync( - Constants.VmrFolderName, + Path.Combine("vmrs", folderName), remoteUris, requestedRefs, checkoutRef, cancellationToken); + _vmrInfo.VmrPath = vmr.Path; await _dependencyTracker.InitializeSourceMappings(); _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); return vmr; } - public async Task PrepareVmrAsync(string checkoutRef, CancellationToken cancellationToken) - { - ILocalGitRepo vmr = await PrepareVmrAsync( - [_vmrInfo.VmrUri], - [checkoutRef], - checkoutRef, - cancellationToken); - - _vmrInfo.VmrPath = vmr.Path; - return vmr; - } - - protected override NativePath GetClonePath(string dirName) => _vmrInfo.VmrPath; + // When we initialize with a single static VMR, + // we will have the path in the newly initialized VmrPath (from VmrRegistrations). + // When we initialize with a different new VMR for each background job, + // the vmrPath will be empty and we will set it to the suggested dirName. + protected override NativePath GetClonePath(string dirName) + => !string.IsNullOrEmpty(_vmrInfo.VmrPath) + ? _vmrInfo.VmrPath + : _vmrInfo.TmpPath / dirName; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs index c0c219f97d..b6babd4fcf 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs @@ -29,6 +29,7 @@ internal abstract class VmrCodeFlower private readonly IVmrDependencyTracker _dependencyTracker; private readonly ILocalGitClient _localGitClient; private readonly ILocalLibGit2Client _libGit2Client; + private readonly ILocalGitRepoFactory _localGitRepoFactory; private readonly IVersionDetailsParser _versionDetailsParser; private readonly IDependencyFileManager _dependencyFileManager; private readonly ICoherencyUpdateResolver _coherencyUpdateResolver; @@ -36,8 +37,6 @@ internal abstract class VmrCodeFlower private readonly IFileSystem _fileSystem; private readonly ILogger _logger; - protected ILocalGitRepo LocalVmr { get; } - protected VmrCodeFlower( IVmrInfo vmrInfo, ISourceManifest sourceManifest, @@ -57,14 +56,13 @@ protected VmrCodeFlower( _dependencyTracker = dependencyTracker; _localGitClient = localGitClient; _libGit2Client = libGit2Client; + _localGitRepoFactory = localGitRepoFactory; _versionDetailsParser = versionDetailsParser; _dependencyFileManager = dependencyFileManager; _coherencyUpdateResolver = coherencyUpdateResolver; _assetLocationResolver = assetLocationResolver; _fileSystem = fileSystem; _logger = logger; - - LocalVmr = localGitRepoFactory.Create(_vmrInfo.VmrPath); } /// @@ -78,7 +76,7 @@ protected async Task FlowCodeAsync( Codeflow currentFlow, ILocalGitRepo repo, SourceMapping mapping, - Build? build, + Build build, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -158,7 +156,7 @@ protected abstract Task SameDirectionFlowAsync( Codeflow lastFlow, Codeflow currentFlow, ILocalGitRepo repo, - Build? build, + Build build, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -184,7 +182,7 @@ protected abstract Task OppositeDirectionFlowAsync( Codeflow lastFlow, Codeflow currentFlow, ILocalGitRepo repo, - Build? build, + Build build, string baseBranch, string targetBranch, bool discardPatches, @@ -238,7 +236,7 @@ protected async Task GetLastFlowAsync(SourceMapping mapping, ILocalGit if (currentIsBackflow) { (backwardSha, forwardSha) = (lastBackflow.VmrSha, lastForwardFlow.VmrSha); - sourceRepo = LocalVmr; + sourceRepo = _localGitRepoFactory.Create(_vmrInfo.VmrPath); } else { @@ -322,7 +320,7 @@ private async Task GetLastForwardFlow(string mappingName) protected async Task UpdateDependenciesAndToolset( NativePath sourceRepo, ILocalGitRepo targetRepo, - Build? build, + Build build, IReadOnlyCollection? excludedAssets, string? sourceElementSha, CancellationToken cancellationToken) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs index 9c5f57f955..be1595ebc8 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs @@ -27,7 +27,6 @@ public interface IVmrForwardFlower /// /// Mapping to flow /// Local checkout of the repository - /// SHA to flow /// Build to flow /// Assets to exclude from the dependency flow /// If target branch does not exist, it is created off of this branch @@ -37,8 +36,29 @@ public interface IVmrForwardFlower Task FlowForwardAsync( string mapping, NativePath sourceRepo, - string? shaToFlow, - int? buildToFlow, + int buildToFlow, + IReadOnlyCollection? excludedAssets, + string baseBranch, + string targetBranch, + bool discardPatches = false, + CancellationToken cancellationToken = default); + + /// + /// Flows forward the code from the source repo to the target branch of the VMR. + /// This overload is used in the context of the darc CLI. + /// + /// Mapping to flow + /// Local checkout of the repository + /// Build to flow + /// Assets to exclude from the dependency flow + /// If target branch does not exist, it is created off of this branch + /// Target branch to make the changes on + /// Keep patch files? + /// True when there were changes to be flown + Task FlowForwardAsync( + string mapping, + NativePath sourceRepo, + Build buildToFlow, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -91,22 +111,34 @@ public VmrForwardFlower( public async Task FlowForwardAsync( string mappingName, NativePath repoPath, - string? shaToFlow, - int? buildToFlow, + int buildToFlow, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, bool discardPatches = false, CancellationToken cancellationToken = default) - { - bool targetBranchExisted = await PrepareVmr(baseBranch, targetBranch, cancellationToken); + => await FlowForwardAsync( + mappingName, + repoPath, + await _barClient.GetBuildAsync(buildToFlow) + ?? throw new Exception($"Failed to find build with BAR ID {buildToFlow}"), + excludedAssets, + baseBranch, + targetBranch, + discardPatches, + cancellationToken); - Build? build = null; - if (buildToFlow.HasValue) - { - build = await _barClient.GetBuildAsync(buildToFlow.Value) - ?? throw new Exception($"Failed to find build with BAR ID {buildToFlow}"); - } + public async Task FlowForwardAsync( + string mappingName, + NativePath repoPath, + Build build, + IReadOnlyCollection? excludedAssets, + string baseBranch, + string targetBranch, + bool discardPatches = false, + CancellationToken cancellationToken = default) + { + bool targetBranchExisted = await PrepareVmr(_vmrInfo.VmrUri, baseBranch, targetBranch, cancellationToken); ILocalGitRepo sourceRepo = _localGitRepoFactory.Create(repoPath); SourceMapping mapping = _dependencyTracker.GetMapping(mappingName); @@ -115,22 +147,13 @@ public async Task FlowForwardAsync( // Refresh the repo await sourceRepo.FetchAllAsync([mapping.DefaultRemote, repoInfo.RemoteUri], cancellationToken); - // SHA comes either directly or from the build or if none supplied, from tip of the repo - shaToFlow ??= build?.Commit; - if (shaToFlow is null) - { - shaToFlow = await sourceRepo.GetShaForRefAsync(); - } - else - { - await sourceRepo.CheckoutAsync(shaToFlow); - } + await sourceRepo.CheckoutAsync(build.Commit); Codeflow lastFlow = await GetLastFlowAsync(mapping, sourceRepo, currentIsBackflow: false); bool hasChanges = await FlowCodeAsync( lastFlow, - new ForwardFlow(lastFlow.TargetSha, shaToFlow), + new ForwardFlow(lastFlow.TargetSha, build.Commit), sourceRepo, mapping, build, @@ -143,7 +166,7 @@ public async Task FlowForwardAsync( hasChanges |= await UpdateDependenciesAndToolset( sourceRepo.Path, - LocalVmr, + _localGitRepoFactory.Create(_vmrInfo.VmrPath), build, excludedAssets, sourceElementSha: null, @@ -152,14 +175,18 @@ public async Task FlowForwardAsync( return hasChanges; } - protected async Task PrepareVmr(string baseBranch, string targetBranch, CancellationToken cancellationToken) + protected async Task PrepareVmr( + string vmrUri, + string baseBranch, + string targetBranch, + CancellationToken cancellationToken) { bool branchExisted; try { await _vmrCloneManager.PrepareVmrAsync( - [_vmrInfo.VmrUri], + [vmrUri], [baseBranch, targetBranch], targetBranch, cancellationToken); @@ -169,13 +196,17 @@ await _vmrCloneManager.PrepareVmrAsync( { // This means the target branch does not exist yet // We will create it off of the base branch - await LocalVmr.CheckoutAsync(baseBranch); - await LocalVmr.CreateBranchAsync(targetBranch); + var vmr = await _vmrCloneManager.PrepareVmrAsync( + [vmrUri], + [baseBranch], + baseBranch, + cancellationToken); + + await vmr.CheckoutAsync(baseBranch); + await vmr.CreateBranchAsync(targetBranch); branchExisted = false; } - await _dependencyTracker.InitializeSourceMappings(); - _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); return branchExisted; } @@ -184,7 +215,7 @@ protected override async Task SameDirectionFlowAsync( Codeflow lastFlow, Codeflow currentFlow, ILocalGitRepo sourceRepo, - Build? build, + Build build, IReadOnlyCollection? excludedAssets, string baseBranch, string targetBranch, @@ -196,14 +227,10 @@ protected override async Task SameDirectionFlowAsync( List additionalRemotes = [ - new AdditionalRemote(mapping.Name, sourceRepo.Path) + new AdditionalRemote(mapping.Name, sourceRepo.Path), + new AdditionalRemote(mapping.Name, build.GetRepository()), ]; - if (build is not null) - { - additionalRemotes.Add(new AdditionalRemote(mapping.Name, build.GetRepository())); - } - bool hadUpdates; try @@ -247,8 +274,8 @@ protected override async Task SameDirectionFlowAsync( _vmrInfo.SourceManifestPath, line => line.Contains(lastFlow.SourceSha), lastFlow.TargetSha); - await _vmrCloneManager.PrepareVmrAsync(previousFlowTargetSha, cancellationToken); - await LocalVmr.CreateBranchAsync(targetBranch, overwriteExistingBranch: true); + var vmr = await _vmrCloneManager.PrepareVmrAsync([_vmrInfo.VmrUri], [previousFlowTargetSha], previousFlowTargetSha, cancellationToken); + await vmr.CreateBranchAsync(targetBranch, overwriteExistingBranch: true); // Reconstruct the previous flow's branch var lastLastFlow = await GetLastFlowAsync(mapping, sourceRepo, currentIsBackflow: true); @@ -257,7 +284,8 @@ await FlowCodeAsync( new ForwardFlow(lastLastFlow.SourceSha, lastFlow.SourceSha), sourceRepo, mapping, - build, // TODO (https://github.com/dotnet/arcade-services/issues/4166): This is an interesting one - should we try to find a build for that previous SHA? + // TODO (https://github.com/dotnet/arcade-services/issues/4166): Find a previous build? + new Build(-1, DateTimeOffset.Now, 0, false, false, lastLastFlow.SourceSha, [], [], [], []), excludedAssets, baseBranch, targetBranch, @@ -270,8 +298,7 @@ await FlowCodeAsync( hadUpdates = await _vmrUpdater.UpdateRepository( mapping.Name, currentFlow.TargetSha, - // TODO - all parameters below should come from BAR build / options - "1.2.3", + build.Assets?.FirstOrDefault()?.Version ?? "0.0.0", updateDependencies: false, additionalRemotes, componentTemplatePath: null, diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs index 4fd61a09bf..1e3a8a5b81 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs @@ -103,7 +103,7 @@ public async Task InitializeRepository( workBranchName += $"/{targetRevision}"; } - IWorkBranch workBranch = await _workBranchFactory.CreateWorkBranchAsync(LocalVmr, workBranchName); + IWorkBranch workBranch = await _workBranchFactory.CreateWorkBranchAsync(GetLocalVmr(), workBranchName); var rootUpdate = new VmrDependencyUpdate( mapping, diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs index d504529778..525d59fb70 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs @@ -33,11 +33,12 @@ public abstract class VmrManagerBase private readonly ICodeownersGenerator _codeownersGenerator; private readonly ICredScanSuppressionsGenerator _credScanSuppressionsGenerator; private readonly ILocalGitClient _localGitClient; + private readonly ILocalGitRepoFactory _localGitRepoFactory; private readonly IDependencyFileManager _dependencyFileManager; private readonly IFileSystem _fileSystem; private readonly ILogger _logger; - protected ILocalGitRepo LocalVmr { get; } + protected ILocalGitRepo GetLocalVmr() => _localGitRepoFactory.Create(_vmrInfo.VmrPath); protected VmrManagerBase( IVmrInfo vmrInfo, @@ -66,10 +67,9 @@ protected VmrManagerBase( _codeownersGenerator = codeownersGenerator; _credScanSuppressionsGenerator = credScanSuppressionsGenerator; _localGitClient = localGitClient; + _localGitRepoFactory = localGitRepoFactory; _dependencyFileManager = dependencyFileManager; _fileSystem = fileSystem; - - LocalVmr = localGitRepoFactory.Create(_vmrInfo.VmrPath); } public async Task> UpdateRepoToRevisionAsync( diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs index 9ead506826..f35387bebc 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs @@ -19,7 +19,26 @@ namespace Microsoft.DotNet.DarcLib.VirtualMonoRepo; public static class VmrRegistrations { - public static IServiceCollection AddVmrManagers( + // This one is used in the context of the PCS service + public static IServiceCollection AddMultiVmrSupport( + this IServiceCollection services, + string tmpPath, + string gitLocation = "git") + { + tmpPath = Path.GetFullPath(tmpPath); + + // When running in the context of the PCS service, we don't have one VMR path + // We will always set the VmrPath whenever we call VmrCloneManager to prepare a specific VMR for us + // Then we will initialize VmrInfo and SourceManifest data from that path + // We assume only one VMR will be within a DI scope (one per background job basically) + services.TryAddScoped(sp => new VmrInfo(string.Empty, tmpPath)); + services.TryAddScoped(sp => new SourceManifest([], [])); + + return AddVmrManagers(services, gitLocation, null, null); + } + + // This one is used in the context of darc and E2E tests + public static IServiceCollection AddSingleVmrSupport( this IServiceCollection services, string gitLocation, string vmrPath, @@ -27,8 +46,26 @@ public static IServiceCollection AddVmrManagers( string? gitHubToken, string? azureDevOpsToken) { - // Configuration based registrations + // When running in the context of darc or E2E tests, we have one VMR path for the whole lifetime of the process + // We can statically initialize the information right away services.TryAddSingleton(new VmrInfo(Path.GetFullPath(vmrPath), Path.GetFullPath(tmpPath))); + services.TryAddScoped(sp => + { + var vmrInfo = sp.GetRequiredService(); + return SourceManifest.FromJson(vmrInfo.SourceManifestPath); + }); + + return AddVmrManagers(services, gitLocation, gitHubToken, azureDevOpsToken); + } + + private static IServiceCollection AddVmrManagers( + IServiceCollection services, + string gitLocation, + string? gitHubToken, + string? azureDevOpsToken) + { + // Configuration based registrations + services.TryAddSingleton(); services.TryAddSingleton(sp => { if (!string.IsNullOrEmpty(azureDevOpsToken)) @@ -58,7 +95,6 @@ public static IServiceCollection AddVmrManagers( services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); - services.TryAddSingleton(); services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); @@ -67,42 +103,37 @@ public static IServiceCollection AddVmrManagers( services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); + services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); + services.TryAddScoped(); services.TryAddScoped(); - services.TryAddSingleton(); + services.TryAddScoped(); - services.AddHttpClient("GraphQL", httpClient => - { - httpClient.DefaultRequestHeaders.Add(HeaderNames.Accept, "application/json"); - httpClient.DefaultRequestHeaders.Add(HeaderNames.UserAgent, "Darc"); - }).ConfigurePrimaryHttpMessageHandler((handler, service) => - { - if (handler is HttpClientHandler httpClientHandler) + services + .AddHttpClient("GraphQL", httpClient => { - httpClientHandler.CheckCertificateRevocationList = true; - } - else if (handler is SocketsHttpHandler socketsHttpHandler) + httpClient.DefaultRequestHeaders.Add(HeaderNames.Accept, "application/json"); + httpClient.DefaultRequestHeaders.Add(HeaderNames.UserAgent, "Darc"); + }) + .ConfigurePrimaryHttpMessageHandler((handler, service) => { - socketsHttpHandler.SslOptions.CertificateRevocationCheckMode = X509RevocationMode.Online; - } - else - { - throw new InvalidOperationException($"Could not create client with CRL check, HttpMessageHandler type {handler.GetType().FullName ?? handler.GetType().Name} is unknown."); - } - }); - - services.TryAddTransient(); - - // These initialize the configuration by reading the JSON files in VMR's src/ - services.TryAddSingleton(sp => - { - var configuration = sp.GetRequiredService(); - return SourceManifest.FromJson(configuration.SourceManifestPath); - }); + if (handler is HttpClientHandler httpClientHandler) + { + httpClientHandler.CheckCertificateRevocationList = true; + } + else if (handler is SocketsHttpHandler socketsHttpHandler) + { + socketsHttpHandler.SslOptions.CertificateRevocationCheckMode = X509RevocationMode.Online; + } + else + { + throw new InvalidOperationException($"Could not create client with CRL check, HttpMessageHandler type {handler.GetType().FullName ?? handler.GetType().Name} is unknown."); + } + }); return services; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs index aa3c886c72..64e886c45c 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs @@ -300,7 +300,7 @@ private async Task UpdateRepositoryRecursively( var workBranchName = "sync" + $"/{rootUpdate.Mapping.Name}" + $"/{Commit.GetShortSha(GetCurrentVersion(rootUpdate.Mapping))}-{rootUpdate.TargetRevision}"; - IWorkBranch workBranch = await _workBranchFactory.CreateWorkBranchAsync(LocalVmr, workBranchName); + IWorkBranch workBranch = await _workBranchFactory.CreateWorkBranchAsync(GetLocalVmr(), workBranchName); // Collection of all affected VMR patches we will need to restore after the sync var vmrPatchesToReapply = new List(); @@ -465,13 +465,14 @@ private async Task ReapplyVmrPatches(IWorkBranch workBranch, List r.Path)); await CommitAsync(commitMessage); } @@ -690,7 +691,7 @@ private async Task UpdateTargetVersionOnly( }; cancellationToken.ThrowIfCancellationRequested(); - await LocalVmr.StageAsync(filesToAdd, cancellationToken); + await GetLocalVmr().StageAsync(filesToAdd, cancellationToken); cancellationToken.ThrowIfCancellationRequested(); await CommitAsync($"Updated package version of {mapping.Name} to {targetVersion}", author: null); } diff --git a/src/ProductConstructionService/ProductConstructionService.Api/InitializationBackgroundService.cs b/src/ProductConstructionService/ProductConstructionService.Api/InitializationBackgroundService.cs index c59ce3924d..f9dda92141 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/InitializationBackgroundService.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/InitializationBackgroundService.cs @@ -27,11 +27,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) // If Vmr cloning is taking more than 20 min, something is wrong var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(stoppingToken, new CancellationTokenSource(TimeSpan.FromMinutes(20)).Token); - // TODO: VmrUri should be loaded from configuration and passed to the singleton via Configure() - IVmrInfo vmrInfo = scope.ServiceProvider.GetRequiredService(); - vmrInfo.VmrUri = options.VmrUri; IVmrCloneManager vmrCloneManager = scope.ServiceProvider.GetRequiredService(); - await vmrCloneManager.PrepareVmrAsync("main", linkedTokenSource.Token); + await vmrCloneManager.PrepareVmrAsync([options.VmrUri], ["main"], "main", linkedTokenSource.Token); linkedTokenSource.Token.ThrowIfCancellationRequested(); telemetryScope.SetSuccess(); diff --git a/src/ProductConstructionService/ProductConstructionService.Api/VirtualMonoRepo/VmrConfiguration.cs b/src/ProductConstructionService/ProductConstructionService.Api/VirtualMonoRepo/VmrConfiguration.cs index bf4dc6beb5..a3a3145ff0 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/VirtualMonoRepo/VmrConfiguration.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/VirtualMonoRepo/VmrConfiguration.cs @@ -16,10 +16,8 @@ internal static class VmrConfiguration public static void AddVmrRegistrations(this WebApplicationBuilder builder) { - string vmrPath = builder.Configuration.GetRequiredValue(VmrPathKey); string tmpPath = builder.Configuration.GetRequiredValue(TmpPathKey); - - builder.Services.AddVmrManagers("git", vmrPath, tmpPath, gitHubToken: null, azureDevOpsToken: null); + builder.Services.AddMultiVmrSupport(tmpPath); } public static void InitializeVmrFromRemote(this WebApplicationBuilder builder) @@ -39,9 +37,5 @@ public static void InitializeVmrFromDisk(this WebApplicationBuilder builder) throw new InvalidOperationException($"VMR not found at {vmrPath}. " + $"Either run the service in initialization mode or clone a VMR into {vmrPath}."); } - - // TODO: Change IVmrInfo to be loaded from configurations and call Configure() here - var vmrInfo = builder.Services.BuildServiceProvider().GetRequiredService(); - vmrInfo.VmrUri = builder.Configuration.GetRequiredValue(VmrUriKey); } } diff --git a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs index 00eab5209a..5571134bdb 100644 --- a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs +++ b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs @@ -992,12 +992,12 @@ private async Task UpdateAssetsAndSources(SubscriptionUpdateWorkItem updat { if (isForwardFlow) { - targetRepo = _vmrInfo.VmrPath; hadUpdates = await _vmrForwardFlower.FlowForwardAsync( subscription, build, pullRequest.HeadBranch, cancellationToken: default); + targetRepo = _vmrInfo.VmrPath; } else { @@ -1020,7 +1020,7 @@ private async Task UpdateAssetsAndSources(SubscriptionUpdateWorkItem updat { _logger.LogInformation("Code changes for {subscriptionId} ready in local branch {branch}", subscription.Id, - subscription.TargetBranch); + pullRequest.HeadBranch); // TODO https://github.com/dotnet/arcade-services/issues/3318: Handle failures (conflict, non-ff etc) using (var scope = _telemetryRecorder.RecordGitOperation(TrackedGitOperation.Push, subscription.TargetRepository)) @@ -1075,7 +1075,7 @@ private async Task UpdateAssetsAndSources(SubscriptionUpdateWorkItem updat string newBranchName = GetNewBranchName(targetBranch); _logger.LogInformation( - "New code flow request for subscription {subscriptionId}. Requesting branch {branch} from PCS", + "New code flow request for subscription {subscriptionId} / branch {branchName}", update.SubscriptionId, newBranchName); @@ -1107,12 +1107,12 @@ private async Task UpdateAssetsAndSources(SubscriptionUpdateWorkItem updat { if (isForwardFlow) { - targetRepo = _vmrInfo.VmrPath; hadUpdates = await _vmrForwardFlower.FlowForwardAsync( subscription, build, newBranchName, cancellationToken: default); + targetRepo = _vmrInfo.VmrPath; } else { @@ -1140,7 +1140,7 @@ private async Task UpdateAssetsAndSources(SubscriptionUpdateWorkItem updat _logger.LogInformation("Code changes for {subscriptionId} ready in local branch {branch}", subscription.Id, - subscription.TargetBranch); + newBranchName); // TODO https://github.com/dotnet/arcade-services/issues/3318: Handle failures (conflict, non-ff etc) using (var scope = _telemetryRecorder.RecordGitOperation(TrackedGitOperation.Push, subscription.TargetRepository)) diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs index 41b97a9c8e..1c25a1a646 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs @@ -177,7 +177,7 @@ await File.WriteAllTextAsync(ProductRepoPath / VersionFiles.VersionProps, Constants.ProductRepoName, ProductRepoPath, branchName + "-backflow", - buildToFlow: build1.Id, + buildToFlow: build1, excludedAssets: ["Package.C2"]); hadUpdates.ShouldHaveUpdates(); await GitOperations.MergePrBranch(ProductRepoPath, branchName + "-backflow"); @@ -229,7 +229,7 @@ .. GetExpectedVersionFiles(ProductRepoPath), ("Package.D3", "1.0.2"), ]); - hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName + "-pr", buildToFlow: build2.Id); + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName + "-pr", buildToFlow: build2); hadUpdates.ShouldHaveUpdates(); dependencies = await productRepo.GetDependenciesAsync(); dependencies.Should().BeEquivalentTo(GetDependencies(build2)); @@ -278,7 +278,7 @@ .. GetExpectedVersionFiles(ProductRepoPath), ]; // We flow this latest build back into the PR that is waiting in the product repo - hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName + "-pr", buildToFlow: build3.Id); + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName + "-pr", buildToFlow: build3); hadUpdates.ShouldHaveUpdates(); dependencies = await productRepo.GetDependenciesAsync(); dependencies.Should().BeEquivalentTo(expectedDependencies); @@ -333,7 +333,7 @@ .. GetExpectedVersionFiles(ProductRepoPath), ]); // Flow the first build - hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName + "-pr2", buildToFlow: build4.Id); + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName + "-pr2", buildToFlow: build4); hadUpdates.ShouldHaveUpdates(); expectedDependencies = @@ -358,7 +358,7 @@ .. GetExpectedVersionFiles(ProductRepoPath), await GitOperations.CommitAll(ProductRepoPath, "Changing a repo file in the PR"); // Flow the second build - this should throw as there's a conflict in the PR branch - await this.Awaiting(_ => CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName + "-pr2", buildToFlow: build5.Id)) + await this.Awaiting(_ => CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName + "-pr2", buildToFlow: build5)) .Should().ThrowAsync(); // The state of the branch should be the same as before diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrCodeFlowTests.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrCodeFlowTests.cs index aac7c38d65..da666deec5 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrCodeFlowTests.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrCodeFlowTests.cs @@ -1,9 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.IO; using System.Linq; using System.Threading.Tasks; @@ -25,8 +23,6 @@ internal abstract class VmrCodeFlowTests : VmrTestsBase protected const string FakePackageName = "Fake.Package"; protected const string FakePackageVersion = "1.0.0"; - private int _buildId = 100; - protected readonly string _productRepoFileName = Constants.GetRepoFileName(Constants.ProductRepoName); private readonly Mock _basicBarClient = new(); @@ -123,46 +119,6 @@ await GetLocal(VmrPath).AddDependencyAsync(new DependencyDetail await GitOperations.Checkout(ProductRepoPath, "main"); } - protected async Task CreateNewVmrBuild((string name, string version)[] assets) - => await CreateNewBuild(VmrPath, assets); - - protected async Task CreateNewRepoBuild((string name, string version)[] assets) - => await CreateNewBuild(ProductRepoPath, assets); - - protected async Task CreateNewBuild(NativePath repoPath, (string name, string version)[] assets) - { - var assetId = 1; - _buildId++; - - var build = new Build( - id: _buildId, - dateProduced: DateTimeOffset.Now, - staleness: 0, - released: false, - stable: true, - commit: await GitOperations.GetRepoLastCommit(repoPath), - channels: ImmutableList.Empty, - assets: - [ - ..assets.Select(a => new Asset(++assetId, _buildId, true, a.name, a.version, - [ - new AssetLocation(assetId, LocationType.NugetFeed, "https://source.feed/index.json") - ])) - ], - dependencies: ImmutableList.Empty, - incoherencies: ImmutableList.Empty) - { - GitHubBranch = "main", - GitHubRepository = repoPath, - }; - - _basicBarClient - .Setup(x => x.GetBuildAsync(build.Id)) - .ReturnsAsync(build); - - return build; - } - protected static List GetDependencies(Build build) => build.Assets .Select(a => new DependencyDetail diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrForwardFlowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrForwardFlowTest.cs index 802834886c..9b42bc482d 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrForwardFlowTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrForwardFlowTest.cs @@ -88,7 +88,7 @@ await GetLocal(VmrPath).AddDependencyAsync(new DependencyDetail ("Package.A1", "1.0.1"), ]); - var hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branchName, buildToFlow: build1.Id); + var hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branchName, buildToFlow: build1); hadUpdates.ShouldHaveUpdates(); await GitOperations.MergePrBranch(VmrPath, branchName); @@ -121,7 +121,7 @@ await GetLocal(VmrPath).AddDependencyAsync(new DependencyDetail ]); // Flow the first build - hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branchName, buildToFlow: build2.Id); + hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branchName, buildToFlow: build2); hadUpdates.ShouldHaveUpdates(); // We make a conflicting change in the PR branch @@ -130,7 +130,7 @@ await GetLocal(VmrPath).AddDependencyAsync(new DependencyDetail await GitOperations.CommitAll(VmrPath, "Changing a file in the PR"); // Flow the second build - this should throw as there's a conflict in the PR branch - await this.Awaiting(_ => CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branchName, buildToFlow: build3.Id)) + await this.Awaiting(_ => CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branchName, buildToFlow: build3)) .Should().ThrowAsync(); // The state of the branch should be the same as before diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs index f506086136..4e64b450bc 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Text; @@ -15,8 +16,10 @@ using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; using Microsoft.DotNet.DarcLib.VirtualMonoRepo; using Microsoft.DotNet.Maestro.Client; +using Microsoft.DotNet.Maestro.Client.Models; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Moq; using NUnit.Framework; namespace Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests; @@ -35,6 +38,9 @@ internal abstract class VmrTestsBase protected IServiceProvider ServiceProvider { get; private set; } = null!; private readonly CancellationTokenSource _cancellationToken = new(); + private readonly Mock _basicBarClient = new(); + + private int _buildId = 100; [SetUp] public async Task Setup() @@ -58,6 +64,8 @@ public async Task Setup() ServiceProvider = CreateServiceProvider().BuildServiceProvider(); ServiceProvider.GetRequiredService().VmrUri = VmrPath; + + _basicBarClient.Reset(); } [TearDown] @@ -82,7 +90,7 @@ public void DeleteCurrentTestDirectory() protected virtual IServiceCollection CreateServiceProvider() => new ServiceCollection() .AddLogging(b => b.AddConsole().AddFilter(l => l >= LogLevel.Debug)) - .AddVmrManagers("git", VmrPath, TmpPath, null, null) + .AddSingleVmrSupport("git", VmrPath, TmpPath, null, null) .AddSingleton(new BarApiClient( buildAssetRegistryPat: null, managedIdentityId: null, @@ -189,8 +197,7 @@ protected async Task CallDarcBackflow( string mappingName, NativePath repoPath, string branch, - string? shaToFlow = null, - int? buildToFlow = null, + Build? buildToFlow = null, IReadOnlyCollection? excludedAssets = null) { using var scope = ServiceProvider.CreateScope(); @@ -198,8 +205,7 @@ protected async Task CallDarcBackflow( return await codeflower.FlowBackAsync( mappingName, repoPath, - shaToFlow, - buildToFlow, + buildToFlow ?? await CreateNewVmrBuild([]), excludedAssets, "main", branch, @@ -210,8 +216,7 @@ protected async Task CallDarcForwardflow( string mappingName, NativePath repoPath, string branch, - string? shaToFlow = null, - int? buildToFlow = null, + Build? buildToFlow = null, IReadOnlyCollection? excludedAssets = null) { using var scope = ServiceProvider.CreateScope(); @@ -219,8 +224,7 @@ protected async Task CallDarcForwardflow( return await codeflower.FlowForwardAsync( mappingName, repoPath, - shaToFlow, - buildToFlow, + buildToFlow ?? await CreateNewRepoBuild(repoPath, []), excludedAssets, "main", branch, @@ -338,4 +342,47 @@ protected async Task WriteSourceMappingsInVmr(SourceMappingFile sourceMappings) // Needed for some local git operations protected Local GetLocal(NativePath repoPath) => ActivatorUtilities.CreateInstance(ServiceProvider, repoPath.ToString()); protected DependencyFileManager GetDependencyFileManager() => ActivatorUtilities.CreateInstance(ServiceProvider); + + protected async Task CreateNewVmrBuild((string name, string version)[] assets) + => await CreateNewBuild(VmrPath, assets); + + protected async Task CreateNewRepoBuild((string name, string version)[] assets) + => await CreateNewBuild(ProductRepoPath, assets); + + protected async Task CreateNewRepoBuild(NativePath repoPath, (string name, string version)[] assets) + => await CreateNewBuild(repoPath, assets); + + protected async Task CreateNewBuild(NativePath repoPath, (string name, string version)[] assets) + { + var assetId = 1; + _buildId++; + + var build = new Build( + id: _buildId, + dateProduced: DateTimeOffset.Now, + staleness: 0, + released: false, + stable: true, + commit: await GitOperations.GetRepoLastCommit(repoPath), + channels: ImmutableList.Empty, + assets: + [ + ..assets.Select(a => new Asset(++assetId, _buildId, true, a.name, a.version, + [ + new AssetLocation(assetId, LocationType.NugetFeed, "https://source.feed/index.json") + ])) + ], + dependencies: ImmutableList.Empty, + incoherencies: ImmutableList.Empty) + { + GitHubBranch = "main", + GitHubRepository = repoPath, + }; + + _basicBarClient + .Setup(x => x.GetBuildAsync(build.Id)) + .ReturnsAsync(build); + + return build; + } } diff --git a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs index 707e9a2fbc..35cbf16015 100644 --- a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs +++ b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs @@ -57,7 +57,7 @@ protected override void RegisterServices(IServiceCollection services) services.AddSingleton(); services.AddScoped(); services.AddTransient(); - services.AddVmrManagers("git", VmrPath, TmpPath, null, null); + services.AddSingleVmrSupport("git", VmrPath, TmpPath, null, null); } protected override Task BeforeExecute(IServiceProvider context) From 127f7b095d9e345f7fb1477c79923b91c4cc9a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Wed, 27 Nov 2024 11:53:03 +0100 Subject: [PATCH 02/11] Order VMR patches before stripping/restoration (#4188) --- .../DarcLib/VirtualMonoRepo/VmrInitializer.cs | 2 +- .../DarcLib/VirtualMonoRepo/VmrManagerBase.cs | 6 +++--- .../DarcLib/VirtualMonoRepo/VmrUpdater.cs | 20 +++++++++---------- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs index 1e3a8a5b81..7f0942e067 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs @@ -223,6 +223,6 @@ await UpdateRepoToRevisionAsync( // VMR initialization does not need to restore patches, // the repository is new and does not have those applied - protected override Task> RestoreVmrPatchedFilesAsync(IReadOnlyCollection patches, IReadOnlyCollection additionalRemotes, CancellationToken cancellationToken) + protected override Task> StripVmrPatchesAsync(IReadOnlyCollection patches, IReadOnlyCollection additionalRemotes, CancellationToken cancellationToken) => Task.FromResult>([]); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs index 525d59fb70..3a9d250785 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs @@ -101,7 +101,7 @@ public async Task> UpdateRepoToRevisionAs // This includes all patches that are also modified by the current change // (happens when we update repo from which the VMR patches come) IReadOnlyCollection vmrPatchesToRestore = restoreVmrPatches - ? await RestoreVmrPatchedFilesAsync(patches, additionalRemotes, cancellationToken) + ? await StripVmrPatchesAsync(patches, additionalRemotes, cancellationToken) : []; foreach (var patch in patches) @@ -175,7 +175,7 @@ protected async Task ReapplyVmrPatchesAsync( patches.Count, patches.Count > 1 ? "es" : string.Empty); - foreach (var patch in patches) + foreach (var patch in patches.DistinctBy(p => p.Path).OrderBy(p => p.Path)) { if (!_fileSystem.FileExists(patch.Path)) { @@ -329,7 +329,7 @@ protected async Task UpdateThirdPartyNoticesAsync(string templatePath, Cancellat } } - protected abstract Task> RestoreVmrPatchedFilesAsync( + protected abstract Task> StripVmrPatchesAsync( IReadOnlyCollection patches, IReadOnlyCollection additionalRemotes, CancellationToken cancellationToken); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs index 64e886c45c..4ef4979f14 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs @@ -415,7 +415,7 @@ private async Task UpdateRepositoryRecursively( .AppendLine($" {update.RemoteUri}/compare/{update.TargetVersion}..{update.TargetRevision}"); } - await ReapplyVmrPatches(workBranch, vmrPatchesToReapply, cancellationToken); + await ApplyVmrPatches(workBranch, vmrPatchesToReapply, cancellationToken); await CleanUpRemovedRepos(componentTemplatePath, tpnTemplatePath); @@ -437,7 +437,7 @@ private async Task UpdateRepositoryRecursively( return updatedDependencies.Any(); } - private async Task ReapplyVmrPatches(IWorkBranch workBranch, List vmrPatchesToReapply, CancellationToken cancellationToken) + private async Task ApplyVmrPatches(IWorkBranch workBranch, List vmrPatchesToReapply, CancellationToken cancellationToken) { if (!vmrPatchesToReapply.Any()) { @@ -446,7 +446,7 @@ private async Task ReapplyVmrPatches(IWorkBranch workBranch, List p.Path).ToArray(), cancellationToken); + await ReapplyVmrPatchesAsync(vmrPatchesToReapply, cancellationToken); } catch (Exception) { @@ -461,7 +461,7 @@ private async Task ReapplyVmrPatches(IWorkBranch workBranch, List - /// Detects VMR patches affected by a given set of patches and restores files patched by these - /// VMR patches into their original state. - /// Detects whether patched files are coming from a mapped repository or a submodule too. + /// Removes changes applied by VMR patches and restores the original state of the files. /// /// Mapping that is currently being updated (so we get its patches) /// Patches with incoming changes to be checked whether they affect some VMR patch - protected override async Task> RestoreVmrPatchedFilesAsync( + protected override async Task> StripVmrPatchesAsync( IReadOnlyCollection patches, IReadOnlyCollection additionalRemotes, CancellationToken cancellationToken) { - IReadOnlyCollection vmrPatchesToRestore = await GetVmrPatchesToRestore( + IReadOnlyCollection vmrPatchesToRestore = await GetVmrPatches( patches, cancellationToken); @@ -510,7 +508,7 @@ protected override async Task> RestoreVmr return vmrPatchesToRestore; } - foreach (var patch in vmrPatchesToRestore) + foreach (var patch in vmrPatchesToRestore.OrderByDescending(p => p.Path)) { if (!_fileSystem.FileExists(patch.Path)) { @@ -545,7 +543,7 @@ await _patchHandler.ApplyPatch( /// - A new version of patch is synchronized from installer - we must remove the old version and apply the new. /// /// Patches of currently synchronized changes - private async Task> GetVmrPatchesToRestore( + private async Task> GetVmrPatches( IReadOnlyCollection patches, CancellationToken cancellationToken) { From 5ba7d22efbee21a04321a7ef3ad410b6077bb5ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Wed, 27 Nov 2024 14:28:09 +0100 Subject: [PATCH 03/11] Configure forward flow correctly (#4189) --- .../VirtualMonoRepo/UpdateOperation.cs | 1 + .../DarcLib/VirtualMonoRepo/IVmrUpdater.cs | 2 ++ .../VirtualMonoRepo/PcsVmrForwardFlower.cs | 1 - .../VirtualMonoRepo/VmrForwardFlower.cs | 19 +++++++++++-------- .../DarcLib/VirtualMonoRepo/VmrInfo.cs | 4 ++++ .../DarcLib/VirtualMonoRepo/VmrUpdater.cs | 9 ++++++++- .../VmrTestsBase.cs | 2 +- 7 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs index 709b5cc0a1..eef10f00fe 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs @@ -42,5 +42,6 @@ protected override async Task ExecuteInternalAsync( _options.GenerateCodeowners, _options.GenerateCredScanSuppressions, _options.DiscardPatches, + reapplyVmrPatches: false, cancellationToken); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs index 27bcd99faf..cbf4010f2d 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs @@ -24,6 +24,7 @@ public interface IVmrUpdater /// Whether to generate a CODEOWNERS file /// Whether to generate a .config/CredScanSuppressions.json file /// Whether to clean up genreated .patch files after their used + /// Whether to reapply patches stored in the VMR /// True if the repository was updated, false if it was already up to date Task UpdateRepository( string mappingName, @@ -36,5 +37,6 @@ Task UpdateRepository( bool generateCodeowners, bool generateCredScanSuppressions, bool discardPatches, + bool reapplyVmrPatches, CancellationToken cancellationToken); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs index 7f2fbf8a33..9e6a95ad02 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs @@ -24,7 +24,6 @@ public interface IPcsVmrForwardFlower /// /// Subscription to flow /// Build to flow - /// If target branch does not exist, it is created off of this branch /// Target branch to make the changes on /// True when there were changes to be flown Task FlowForwardAsync( diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs index be1595ebc8..0baa4c3613 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs @@ -249,11 +249,12 @@ protected override async Task SameDirectionFlowAsync( targetVersion, updateDependencies: false, additionalRemotes: additionalRemotes, - componentTemplatePath: null, - tpnTemplatePath: null, - generateCodeowners: true, + componentTemplatePath: _vmrInfo.VmrPath / VmrInfo.ComponentTemplatePath, + tpnTemplatePath: _vmrInfo.VmrPath / VmrInfo.ThirdPartyNoticesTemplatePath, + generateCodeowners: false, generateCredScanSuppressions: true, discardPatches, + reapplyVmrPatches: true, cancellationToken); } catch (PatchApplicationFailedException e) @@ -301,11 +302,12 @@ await FlowCodeAsync( build.Assets?.FirstOrDefault()?.Version ?? "0.0.0", updateDependencies: false, additionalRemotes, - componentTemplatePath: null, - tpnTemplatePath: null, + componentTemplatePath: _vmrInfo.VmrPath / VmrInfo.ComponentTemplatePath, + tpnTemplatePath: _vmrInfo.VmrPath / VmrInfo.ThirdPartyNoticesTemplatePath, generateCodeowners: false, generateCredScanSuppressions: false, discardPatches, + reapplyVmrPatches: true, cancellationToken); } @@ -377,11 +379,12 @@ .. submodules.Select(s => s.Path).Distinct().Select(VmrPatchHandler.GetExclusion targetVersion, updateDependencies: false, additionalRemote, - componentTemplatePath: null, - tpnTemplatePath: null, + componentTemplatePath: _vmrInfo.VmrPath / VmrInfo.ComponentTemplatePath, + tpnTemplatePath: _vmrInfo.VmrPath / VmrInfo.ThirdPartyNoticesTemplatePath, generateCodeowners: false, - generateCredScanSuppressions: false, + generateCredScanSuppressions: true, discardPatches, + reapplyVmrPatches: true, cancellationToken); } } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs index 2f7c540de6..f2d7d9b616 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs @@ -75,6 +75,10 @@ public class VmrInfo : IVmrInfo public const string KeepAttribute = "vmr-preserve"; public const string IgnoreAttribute = "vmr-ignore"; + // TODO (https://github.com/dotnet/arcade-services/issues/4186): Read these from source-mappings.json + public const string ComponentTemplatePath = "src/sdk/src/VirtualMonoRepo/Component.template.md"; + public const string ThirdPartyNoticesTemplatePath = "src/sdk/src/VirtualMonoRepo/THIRD-PARTY-NOTICES.template.txt"; + public const string ComponentListPath = "Components.md"; public const string ThirdPartyNoticesFileName = "THIRD-PARTY-NOTICES.txt"; public const string CodeownersFileName = "CODEOWNERS"; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs index 4ef4979f14..4fb000891b 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs @@ -104,6 +104,7 @@ public async Task UpdateRepository( bool generateCodeowners, bool generateCredScanSuppressions, bool discardPatches, + bool reapplyVmrPatches, CancellationToken cancellationToken) { await _dependencyTracker.InitializeSourceMappings(); @@ -142,7 +143,7 @@ public async Task UpdateRepository( { try { - var patchesToReapply = await UpdateRepositoryInternal( + IReadOnlyCollection patchesToReapply = await UpdateRepositoryInternal( dependencyUpdate, restoreVmrPatches: true, additionalRemotes, @@ -152,6 +153,12 @@ public async Task UpdateRepository( generateCredScanSuppressions, discardPatches, cancellationToken); + + if (reapplyVmrPatches) + { + await ReapplyVmrPatchesAsync(patchesToReapply, cancellationToken); + } + return true; } catch (EmptySyncException e) diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs index 4e64b450bc..69f7f22c54 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs @@ -190,7 +190,7 @@ protected async Task CallDarcUpdate(string repository, string commit, Additional { using var scope = ServiceProvider.CreateScope(); var vmrUpdater = scope.ServiceProvider.GetRequiredService(); - await vmrUpdater.UpdateRepository(repository, commit, null, true, additionalRemotes, null, null, generateCodeowners, generateCredScanSuppressions, true, _cancellationToken.Token); + await vmrUpdater.UpdateRepository(repository, commit, null, true, additionalRemotes, null, null, generateCodeowners, generateCredScanSuppressions, discardPatches: true, reapplyVmrPatches: false, _cancellationToken.Token); } protected async Task CallDarcBackflow( From 4dd2d585113631a23f636da9767477c499c6d903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Thu, 28 Nov 2024 10:59:19 +0100 Subject: [PATCH 04/11] Fix E2E code flow tests (fetching refs, updating version files) (#4190) --- docs/DevGuide.md | 1 + .../DarcLib/ILocalGitClient.cs | 8 +++++++ .../DarcLib/LocalGitClient.cs | 17 ++++++++++++++- .../DarcLib/VirtualMonoRepo/CloneManager.cs | 19 ++++------------- .../VirtualMonoRepo/VmrDependencyTracker.cs | 6 ++---- .../DarcLib/VirtualMonoRepo/VmrInfo.cs | 7 +++++++ .../ReminderManager.cs | 2 +- src/ProductConstructionService/Readme.md | 1 + .../VmrTwoWayCodeflowTest.cs | 8 ++++--- .../RepositoryCloneManagerTests.cs | 21 +++++++++++-------- 10 files changed, 57 insertions(+), 33 deletions(-) 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); + }); } } From 2bd1d95588ae0eaaf7480340a19f25094608583a Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa <91743470+dkurepa@users.noreply.github.com> Date: Fri, 29 Nov 2024 12:26:05 +0100 Subject: [PATCH 05/11] Propagate build info from BAR to VMR outputs (#4192) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Přemek Vysoký --- .../VirtualMonoRepo/CodeFlowOperation.cs | 2 +- .../VirtualMonoRepo/GenerateTpnOperation.cs | 2 +- .../GetRepoVersionOperation.cs | 2 +- .../VirtualMonoRepo/InitializeOperation.cs | 14 ++++++- .../VirtualMonoRepo/UpdateOperation.cs | 39 ++++++++++++------- .../DarcLib/Helpers/DependencyFileManager.cs | 3 ++ .../DarcLib/Helpers/VersionDetailsParser.cs | 4 +- .../DarcLib/Models/VersionDetails.cs | 2 +- .../Models/VirtualMonoRepo/GitInfoFile.cs | 8 +--- .../Models/VirtualMonoRepo/ManifestRecord.cs | 9 ++++- .../Models/VirtualMonoRepo/SourceManifest.cs | 10 +++-- .../VirtualMonoRepo/IVmrInitializer.cs | 4 ++ .../DarcLib/VirtualMonoRepo/IVmrUpdater.cs | 5 ++- .../VirtualMonoRepo/VmrCloneManager.cs | 3 +- .../DarcLib/VirtualMonoRepo/VmrCodeflower.cs | 7 ++-- .../VirtualMonoRepo/VmrDependencyTracker.cs | 36 ++++++++++++----- .../VirtualMonoRepo/VmrDependencyUpdate.cs | 8 +++- .../VirtualMonoRepo/VmrForwardFlower.cs | 34 ++++++++-------- .../DarcLib/VirtualMonoRepo/VmrInitializer.cs | 13 +++++-- .../DarcLib/VirtualMonoRepo/VmrManagerBase.cs | 24 +++++++++++- .../VirtualMonoRepo/VmrRepoVersionResolver.cs | 2 +- .../DarcLib/VirtualMonoRepo/VmrScanner.cs | 2 +- .../DarcLib/VirtualMonoRepo/VmrUpdater.cs | 23 ++++++++--- .../LoadSourceMappingsFromInstallerTest.cs | 1 + .../VmrBackflowTest.cs | 4 +- .../VmrTestsBase.cs | 27 ++++++++----- ...rsionFileTests.cs => VersionFilesTests.cs} | 2 +- .../VirtualMonoRepo/GitInfoFileTests.cs | 6 --- .../VirtualMonoRepo/ManifestRecordTests.cs | 9 +++-- .../VirtualMonoRepo/VmrPusherTests.cs | 7 +++- .../CodeFlowScenarioTestBase.cs | 5 ++- .../ScenarioTestBase.cs | 4 +- .../ScenarioTests/ScenarioTests_CodeFlow.cs | 2 +- 33 files changed, 214 insertions(+), 109 deletions(-) rename test/Microsoft.DotNet.DarcLib.Tests/Helpers/{VersionFileTests.cs => VersionFilesTests.cs} (94%) diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs index 9c91b0de44..6b58c00cd9 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs @@ -56,7 +56,7 @@ protected override async Task ExecuteInternalAsync( _vmrInfo.TmpPath = new NativePath(_options.RepositoryDirectory); } - await _dependencyTracker.InitializeSourceMappings(); + await _dependencyTracker.RefreshMetadata(); await FlowAsync( repoName, diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/GenerateTpnOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/GenerateTpnOperation.cs index 5e15911834..1b556b6522 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/GenerateTpnOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/GenerateTpnOperation.cs @@ -26,7 +26,7 @@ public GenerateTpnOperation( public override async Task ExecuteAsync() { - await _dependencyTracker.InitializeSourceMappings(); + await _dependencyTracker.RefreshMetadata(); await _generator.UpdateThirdPartyNotices(_options.TpnTemplate); return 0; } diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/GetRepoVersionOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/GetRepoVersionOperation.cs index 78b854ac4f..cdc4bd1104 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/GetRepoVersionOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/GetRepoVersionOperation.cs @@ -37,7 +37,7 @@ public override async Task ExecuteAsync() // If there are no repositories, list all. if (!repositories.Any()) { - await _dependencyTracker.InitializeSourceMappings(); + await _dependencyTracker.RefreshMetadata(); repositories = _dependencyTracker.Mappings.Select(m => m.Name).ToList(); } diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/InitializeOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/InitializeOperation.cs index d281d7ee9f..93bcdff4aa 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/InitializeOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/InitializeOperation.cs @@ -2,9 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.Darc.Options.VirtualMonoRepo; +using Microsoft.DotNet.DarcLib; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.VirtualMonoRepo; using Microsoft.Extensions.Logging; @@ -16,15 +18,18 @@ internal class InitializeOperation : VmrOperationBase { private readonly InitializeCommandLineOptions _options; private readonly IVmrInitializer _vmrInitializer; + private readonly IBarApiClient _barClient; public InitializeOperation( InitializeCommandLineOptions options, IVmrInitializer vmrInitializer, - ILogger logger) + ILogger logger, + IBarApiClient barClient) : base(options, logger) { _options = options; _vmrInitializer = vmrInitializer; + _barClient = barClient; } protected override async Task ExecuteInternalAsync( @@ -32,10 +37,14 @@ protected override async Task ExecuteInternalAsync( string? targetRevision, IReadOnlyCollection additionalRemotes, CancellationToken cancellationToken) - => await _vmrInitializer.InitializeRepository( + { + var build = (await _barClient.GetBuildsAsync(repoName, targetRevision)).SingleOrDefault(); + await _vmrInitializer.InitializeRepository( repoName, targetRevision, null, + build?.AzureDevOpsBuildNumber, + build?.Id, _options.Recursive, new NativePath(_options.SourceMappings), additionalRemotes, @@ -45,4 +54,5 @@ protected override async Task ExecuteInternalAsync( _options.GenerateCredScanSuppressions, _options.DiscardPatches, cancellationToken); + } } diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs index eef10f00fe..a8e569ce16 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs @@ -2,9 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.Darc.Options.VirtualMonoRepo; +using Microsoft.DotNet.DarcLib; using Microsoft.DotNet.DarcLib.VirtualMonoRepo; using Microsoft.Extensions.Logging; @@ -15,15 +17,18 @@ internal class UpdateOperation : VmrOperationBase { private readonly UpdateCommandLineOptions _options; private readonly IVmrUpdater _vmrUpdater; + private readonly IBarApiClient _barClient; public UpdateOperation( UpdateCommandLineOptions options, IVmrUpdater vmrUpdater, - ILogger logger) + ILogger logger, + IBarApiClient barClient) : base(options, logger) { _options = options; _vmrUpdater = vmrUpdater; + _barClient = barClient; } protected override async Task ExecuteInternalAsync( @@ -31,17 +36,23 @@ protected override async Task ExecuteInternalAsync( string? targetRevision, IReadOnlyCollection additionalRemotes, CancellationToken cancellationToken) - => await _vmrUpdater.UpdateRepository( - repoName, - targetRevision, - targetVersion: null, - _options.Recursive, - additionalRemotes, - _options.ComponentTemplate, - _options.TpnTemplate, - _options.GenerateCodeowners, - _options.GenerateCredScanSuppressions, - _options.DiscardPatches, - reapplyVmrPatches: false, - cancellationToken); + { + var build = (await _barClient.GetBuildsAsync(repoName, targetRevision)).SingleOrDefault(); + + await _vmrUpdater.UpdateRepository( + repoName, + targetRevision, + targetVersion: null, + build?.AzureDevOpsBuildNumber, + build?.Id, + _options.Recursive, + additionalRemotes, + _options.ComponentTemplate, + _options.TpnTemplate, + _options.GenerateCodeowners, + _options.GenerateCredScanSuppressions, + _options.DiscardPatches, + reapplyVmrPatches: false, + cancellationToken); + } } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Helpers/DependencyFileManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/Helpers/DependencyFileManager.cs index 8707493e84..20f3524c11 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Helpers/DependencyFileManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Helpers/DependencyFileManager.cs @@ -240,6 +240,9 @@ public void UpdateVersionDetails( SetAttribute(versionDetails, sourceNode, VersionDetailsParser.UriElementName, sourceDependency.Uri); SetAttribute(versionDetails, sourceNode, VersionDetailsParser.ShaElementName, sourceDependency.Sha); + if (sourceDependency.BarId != null) { + SetAttribute(versionDetails, sourceNode, VersionDetailsParser.BarIdElementName, sourceDependency.BarId.ToString()); + } } foreach (DependencyDetail itemToUpdate in itemsToUpdate) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Helpers/VersionDetailsParser.cs b/src/Microsoft.DotNet.Darc/DarcLib/Helpers/VersionDetailsParser.cs index a3bbfbbdc4..0e040cf332 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Helpers/VersionDetailsParser.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Helpers/VersionDetailsParser.cs @@ -28,6 +28,7 @@ public class VersionDetailsParser : IVersionDetailsParser public const string VersionPropsAlternateVersionElementSuffix = "Version"; public const string ShaElementName = "Sha"; public const string UriElementName = "Uri"; + public const string BarIdElementName = "BarId"; public const string DependencyElementName = "Dependency"; public const string DependenciesElementName = "Dependencies"; public const string NameAttributeName = "Name"; @@ -158,7 +159,8 @@ private static List ParseDependencyDetails(XmlNodeList depende var sha = sourceNode.Attributes[ShaElementName]?.Value?.Trim() ?? throw new DarcException($"Malformed {SourceElementName} section - expected {ShaElementName} attribute"); - return new SourceDependency(uri, sha); + int.TryParse(sourceNode.Attributes[BarIdElementName]?.Value?.Trim(), out int barId); + return new SourceDependency(uri, sha, barId); } private static bool ParseBooleanAttribute(XmlAttributeCollection attributes, string attributeName) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Models/VersionDetails.cs b/src/Microsoft.DotNet.Darc/DarcLib/Models/VersionDetails.cs index 094bdf6644..2b4623aabc 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Models/VersionDetails.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Models/VersionDetails.cs @@ -11,5 +11,5 @@ public record VersionDetails( IReadOnlyCollection Dependencies, SourceDependency? Source); -public record SourceDependency(string Uri, string Sha); +public record SourceDependency(string Uri, string Sha, int? BarId); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/GitInfoFile.cs b/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/GitInfoFile.cs index 0ae9575fe0..6b254bf100 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/GitInfoFile.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/GitInfoFile.cs @@ -28,8 +28,6 @@ public class GitInfoFile : MsBuildPropsFile public string GitCommitHash { get; set; } public string OfficialBuildId { get; set; } public string OutputPackageVersion { get; set; } - public string PreReleaseVersionLabel { get; set; } - public bool IsStable { get; set; } public int? GitCommitCount { get; set; } public GitInfoFile() @@ -43,9 +41,7 @@ protected override void SerializeProperties(XmlElement propertyGroup, Func repositories, IEnumerable r.Path == repository); if (repo != null) @@ -59,10 +59,14 @@ public void UpdateVersion(string repository, string uri, string sha, string? pac { repo.PackageVersion = packageVersion; } + if (barId != null) + { + repo.BarId = barId; + } } else { - _repositories.Add(new RepositoryRecord(repository, uri, sha, packageVersion)); + _repositories.Add(new RepositoryRecord(repository, uri, sha, packageVersion, barId)); } } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrInitializer.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrInitializer.cs index d79a3710fc..91054ad2fb 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrInitializer.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrInitializer.cs @@ -17,6 +17,8 @@ public interface IVmrInitializer /// Name of a repository mapping /// Revision (commit SHA, branch, tag..) onto which to synchronize, leave empty for HEAD /// Version of packages, that the SHA we're updating to, produced + /// Azdo build id of the build that's being flown, if applicable + /// Bar id of the build that's being flown, if applicable /// When true, initializes dependencies (from Version.Details.xml) recursively /// Path to the source-mappings.json file /// Additional git remotes to use when fetching @@ -29,6 +31,8 @@ Task InitializeRepository( string mappingName, string? targetRevision, string? targetVersion, + string? officialBuildId, + int? barId, bool initializeDependencies, LocalPath sourceMappingsPath, IReadOnlyCollection additionalRemotes, diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs index cbf4010f2d..fcc73b63da 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs @@ -16,7 +16,8 @@ public interface IVmrUpdater /// Name of a repository mapping /// Revision (commit SHA, branch, tag..) onto which to synchronize, leave empty for HEAD /// Version of packages, that the SHA we're updating to, produced - /// Whether to pull changes commit by commit instead of squashing all updates into one + /// Azdo build id of the build that's being flown, if applicable + /// Bar id of the build that's being flown, if applicable /// When true, updates dependencies (from Version.Details.xml) recursively /// Additional git remotes to use when fetching /// Path to VMR's Component.md template @@ -30,6 +31,8 @@ Task UpdateRepository( string mappingName, string? targetRevision, string? targetVersion, + string? officialBuildId, + int? barId, bool updateDependencies, IReadOnlyCollection additionalRemotes, string? componentTemplatePath, diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs index 2cb6e26493..2823c6b604 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs @@ -76,8 +76,7 @@ public async Task PrepareVmrAsync( cancellationToken); _vmrInfo.VmrPath = vmr.Path; - await _dependencyTracker.InitializeSourceMappings(); - _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); + await _dependencyTracker.RefreshMetadata(); return vmr; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs index b6babd4fcf..a7a3ff4845 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs @@ -220,7 +220,7 @@ protected async Task BlameLineAsync(string filePath, Func /// protected async Task GetLastFlowAsync(SourceMapping mapping, ILocalGitRepo repoClone, bool currentIsBackflow) { - await _dependencyTracker.InitializeSourceMappings(); + await _dependencyTracker.RefreshMetadata(); _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); ForwardFlow lastForwardFlow = await GetLastForwardFlow(mapping.Name); @@ -337,8 +337,9 @@ protected async Task UpdateDependenciesAndToolset( if (sourceElementSha != null) { sourceOrigin = new SourceDependency( - build?.GetRepository() ?? Constants.DefaultVmrUri, - sourceElementSha); + build.GetRepository(), + sourceElementSha, + build.Id); if (versionDetails.Source?.Sha != sourceElementSha) { diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs index 30027b7724..9768bbb07d 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs @@ -23,10 +23,10 @@ public interface IVmrDependencyTracker IReadOnlyCollection Mappings { get; } /// - /// Loads repository mappings from source-mappings.json + /// Refreshes all metadata: source mappings, source manifest and AllRepoVersions /// /// Leave empty for default (src/source-mappings.json) - Task InitializeSourceMappings(string? sourceMappingsPath = null); + Task RefreshMetadata(string? sourceMappingsPath = null); void UpdateDependencyVersion(VmrDependencyUpdate update); @@ -43,7 +43,7 @@ public interface IVmrDependencyTracker /// public class VmrDependencyTracker : IVmrDependencyTracker { - private readonly AllVersionsPropsFile _repoVersions; + private AllVersionsPropsFile _repoVersions; private readonly ISourceManifest _sourceManifest; private readonly IVmrInfo _vmrInfo; private readonly IFileSystem _fileSystem; @@ -83,18 +83,30 @@ public SourceMapping GetMapping(string name) public VmrDependencyVersion? GetDependencyVersion(SourceMapping mapping) => _sourceManifest.GetVersion(mapping.Name); - public async Task InitializeSourceMappings(string? sourceMappingsPath = null) + private async Task InitializeSourceMappings(string? sourceMappingsPath = null) { sourceMappingsPath ??= _vmrInfo.VmrPath / VmrInfo.DefaultRelativeSourceMappingsPath; _mappings = await _sourceMappingParser.ParseMappings(sourceMappingsPath); } + public async Task RefreshMetadata(string? sourceMappingsPath = null) + { + await InitializeSourceMappings(sourceMappingsPath); + _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); + _repoVersions = new AllVersionsPropsFile(_sourceManifest.Repositories); + } + public void UpdateDependencyVersion(VmrDependencyUpdate update) { _repoVersions.UpdateVersion(update.Mapping.Name, update.TargetRevision, update.TargetVersion); _repoVersions.SerializeToXml(_vmrInfo.AllVersionsFilePath); - _sourceManifest.UpdateVersion(update.Mapping.Name, update.RemoteUri, update.TargetRevision, update.TargetVersion); + _sourceManifest.UpdateVersion( + update.Mapping.Name, + update.RemoteUri, + update.TargetRevision, + update.TargetVersion, + update.BarId); _fileSystem.WriteToFile(_vmrInfo.SourceManifestPath, _sourceManifest.ToJson()); // Root repository of an update does not have a package version associated with it @@ -104,14 +116,18 @@ public void UpdateDependencyVersion(VmrDependencyUpdate update) ?? _sourceManifest.GetVersion(update.Mapping.Name)?.PackageVersion ?? "0.0.0"; - var (buildId, releaseLabel) = VersionFiles.DeriveBuildInfo(update.Mapping.Name, packageVersion); - + // If we didn't find a Bar build for the update, calculate it the old way + string? officialBuildId = update.OfficialBuildId; + if (string.IsNullOrEmpty(officialBuildId)) + { + var (calculatedOfficialBuildId, _) = VersionFiles.DeriveBuildInfo(update.Mapping.Name, packageVersion); + officialBuildId = calculatedOfficialBuildId; + } + var gitInfo = new GitInfoFile { GitCommitHash = update.TargetRevision, - OfficialBuildId = buildId, - PreReleaseVersionLabel = releaseLabel, - IsStable = string.IsNullOrWhiteSpace(releaseLabel), + OfficialBuildId = officialBuildId, OutputPackageVersion = packageVersion, }; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyUpdate.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyUpdate.cs index d376672c9a..5ec0c5bd5e 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyUpdate.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyUpdate.cs @@ -13,10 +13,14 @@ namespace Microsoft.DotNet.DarcLib.VirtualMonoRepo; /// Remote URI from which we will pull the new updates /// Target revision (usually commit SHA but also a branch or a tag) to update to /// Version of packages built for that given SHA (e.g. 8.0.0-alpha.1.22614.1) -/// Parent dependency in the dependency tree that caused this update,null for root (installer) +/// Parent dependency in the dependency tree that caused this update,null for root (installer) +/// Id of the build that triggered the codeflow. Empty when flowing non bar builds/code +/// Bar Id of the build that triggered the codeflow. Empty when flowing non bar builds/code public record VmrDependencyUpdate( SourceMapping Mapping, string RemoteUri, string TargetRevision, string? TargetVersion, - SourceMapping? Parent); + SourceMapping? Parent, + string? OfficialBuildId, + int? BarId); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs index 0baa4c3613..e5ee4ec1df 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs @@ -237,16 +237,14 @@ protected override async Task SameDirectionFlowAsync( { // If the build produced any assets, we use the number to update VMR's git info files // The git info files won't be important by then and probably removed but let's keep it for now - string? targetVersion = null; - if (build?.Assets.Count > 0) - { - targetVersion = build?.Assets[0].Version; - } + string? targetVersion = build.Assets.FirstOrDefault()?.Version; hadUpdates = await _vmrUpdater.UpdateRepository( mapping.Name, currentFlow.TargetSha, targetVersion, + build.AzureDevOpsBuildNumber, + build.Id, updateDependencies: false, additionalRemotes: additionalRemotes, componentTemplatePath: _vmrInfo.VmrPath / VmrInfo.ComponentTemplatePath, @@ -299,7 +297,9 @@ await FlowCodeAsync( hadUpdates = await _vmrUpdater.UpdateRepository( mapping.Name, currentFlow.TargetSha, - build.Assets?.FirstOrDefault()?.Version ?? "0.0.0", + build.Assets.FirstOrDefault()?.Version ?? "0.0.0", + build.AzureDevOpsBuildNumber, + build.Id, updateDependencies: false, additionalRemotes, componentTemplatePath: _vmrInfo.VmrPath / VmrInfo.ComponentTemplatePath, @@ -319,7 +319,7 @@ protected override async Task OppositeDirectionFlowAsync( Codeflow lastFlow, Codeflow currentFlow, ILocalGitRepo sourceRepo, - Build? build, + Build build, string baseBranch, string targetBranch, bool discardPatches, @@ -337,7 +337,7 @@ .. await sourceRepo.GetGitSubmodulesAsync(currentFlow.TargetSha), ]; // We will remove everything not-cloaked and replace it with current contents of the source repo - // When flowing to the VMR, we remove all files but sobmodules and cloaked files + // When flowing to the VMR, we remove all files but submodules and cloaked files List removalFilters = [ .. mapping.Include.Select(VmrPatchHandler.GetInclusionRule), @@ -356,20 +356,16 @@ .. submodules.Select(s => s.Path).Distinct().Select(VmrPatchHandler.GetExclusion // We make the VMR believe it has the zero commit of the repo as it matches the dir/git state at the moment _dependencyTracker.UpdateDependencyVersion(new VmrDependencyUpdate( mapping, - sourceRepo.Path, // TODO = URL from BAR build + build.GetRepository(), Constants.EmptyGitObject, _dependencyTracker.GetDependencyVersion(mapping)!.PackageVersion, - Parent: null)); + Parent: null, + build.AzureDevOpsBuildNumber, + build.Id)); - IReadOnlyCollection? additionalRemote = build is not null - ? [new AdditionalRemote(mapping.Name, build.GetRepository())] - : []; + IReadOnlyCollection? additionalRemote = [new AdditionalRemote(mapping.Name, build.GetRepository())]; - string? targetVersion = null; - if (build?.Assets.Count > 0) - { - targetVersion = build.Assets[0].Version; - } + var targetVersion = build.Assets.FirstOrDefault()?.Version; // TODO: Detect if no changes // TODO: Technically, if we only changed metadata files, there are no updates still @@ -377,6 +373,8 @@ .. submodules.Select(s => s.Path).Distinct().Select(VmrPatchHandler.GetExclusion mapping.Name, currentFlow.TargetSha, targetVersion, + build.AzureDevOpsBuildNumber, + build.Id, updateDependencies: false, additionalRemote, componentTemplatePath: _vmrInfo.VmrPath / VmrInfo.ComponentTemplatePath, diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs index 7f0942e067..c74000c32f 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs @@ -62,8 +62,9 @@ public VmrInitializer( IFileSystem fileSystem, ILogger logger, ISourceManifest sourceManifest, - IVmrInfo vmrInfo) - : base(vmrInfo, sourceManifest, dependencyTracker, patchHandler, versionDetailsParser, thirdPartyNoticesGenerator, readmeComponentListGenerator, codeownersGenerator, credScanSuppressionsGenerator, localGitClient, localGitRepoFactory, dependencyFileManager, fileSystem, logger) + IVmrInfo vmrInfo, + IServiceProvider serviceProvider) + : base(vmrInfo, sourceManifest, dependencyTracker, patchHandler, versionDetailsParser, thirdPartyNoticesGenerator, readmeComponentListGenerator, codeownersGenerator, credScanSuppressionsGenerator, localGitClient, localGitRepoFactory, dependencyFileManager, fileSystem, logger, serviceProvider) { _vmrInfo = vmrInfo; _dependencyTracker = dependencyTracker; @@ -78,6 +79,8 @@ public async Task InitializeRepository( string mappingName, string? targetRevision, string? targetVersion, + string? officialBuildId, + int? barId, bool initializeDependencies, LocalPath sourceMappingsPath, IReadOnlyCollection additionalRemotes, @@ -88,7 +91,7 @@ public async Task InitializeRepository( bool discardPatches, CancellationToken cancellationToken) { - await _dependencyTracker.InitializeSourceMappings(sourceMappingsPath); + await _dependencyTracker.RefreshMetadata(sourceMappingsPath); var mapping = _dependencyTracker.GetMapping(mappingName); @@ -110,7 +113,9 @@ public async Task InitializeRepository( mapping.DefaultRemote, targetRevision ?? mapping.DefaultRef, targetVersion, - null); + null, + officialBuildId, + barId); try { diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs index 3a9d250785..f3fcd2749e 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs @@ -12,6 +12,7 @@ using Microsoft.DotNet.DarcLib.Models; using Microsoft.DotNet.DarcLib.Models.Darc; using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; #nullable enable @@ -37,6 +38,7 @@ public abstract class VmrManagerBase private readonly IDependencyFileManager _dependencyFileManager; private readonly IFileSystem _fileSystem; private readonly ILogger _logger; + private readonly IServiceProvider _serviceProvider; protected ILocalGitRepo GetLocalVmr() => _localGitRepoFactory.Create(_vmrInfo.VmrPath); @@ -54,7 +56,8 @@ protected VmrManagerBase( ILocalGitRepoFactory localGitRepoFactory, IDependencyFileManager dependencyFileManager, IFileSystem fileSystem, - ILogger logger) + ILogger logger, + IServiceProvider serviceProvider) { _logger = logger; _vmrInfo = vmrInfo; @@ -70,6 +73,7 @@ protected VmrManagerBase( _localGitRepoFactory = localGitRepoFactory; _dependencyFileManager = dependencyFileManager; _fileSystem = fileSystem; + _serviceProvider = serviceProvider; } public async Task> UpdateRepoToRevisionAsync( @@ -222,6 +226,8 @@ protected async Task> GetAllDependenciesAsync( _logger.LogInformation("Finding transitive dependencies for {mapping}:{revision}..", root.Mapping.Name, root.TargetRevision); + var barClient = _serviceProvider.GetRequiredService(); + while (reposToScan.TryDequeue(out var repo)) { cancellationToken.ThrowIfCancellationRequested(); @@ -271,12 +277,26 @@ protected async Task> GetAllDependenciesAsync( $"for a {VersionFiles.VersionDetailsXml} dependency of {dependency.Name}"); } + var builds = await barClient.GetBuildsAsync(dependency.RepoUri, dependency.Commit); + + if (builds.Count() != 1) + { + _logger.LogInformation("Expected to find one build for repo {repo} and commit {commit}, but found {number} builds" + + "Will proceed with the code flow normally, but won't have any BAR data for this repo", + dependency.RepoUri, + dependency.Commit, + builds.Count()); + } + var build = builds.SingleOrDefault(); + var update = new VmrDependencyUpdate( mapping, dependency.RepoUri, dependency.Commit, dependency.Version, - repo.Mapping); + repo.Mapping, + build?.AzureDevOpsBuildNumber, + build?.Id); if (transitiveDependencies.TryAdd(mapping, update)) { diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRepoVersionResolver.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRepoVersionResolver.cs index 23ccd03efc..0ddc64ea5f 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRepoVersionResolver.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRepoVersionResolver.cs @@ -28,7 +28,7 @@ public VmrRepoVersionResolver(IVmrDependencyTracker dependencyTracker) public async Task GetVersion(string mappingName) { - await _dependencyTracker.InitializeSourceMappings(); + await _dependencyTracker.RefreshMetadata(); SourceMapping mapping = _dependencyTracker.GetMapping(mappingName); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrScanner.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrScanner.cs index 0e86114494..fc2b7ab3f1 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrScanner.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrScanner.cs @@ -39,7 +39,7 @@ public VmrScanner( public async Task> ScanVmr(string? baselineFilePath, CancellationToken cancellationToken) { - await _dependencyTracker.InitializeSourceMappings(); + await _dependencyTracker.RefreshMetadata(); var taskList = new List>>(); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs index 4fb000891b..855bfb9487 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs @@ -76,8 +76,9 @@ public VmrUpdater( IFileSystem fileSystem, ILogger logger, ISourceManifest sourceManifest, - IVmrInfo vmrInfo) - : base(vmrInfo, sourceManifest, dependencyTracker, patchHandler, versionDetailsParser, thirdPartyNoticesGenerator, readmeComponentListGenerator, codeownersGenerator, credScanSuppressionsGenerator, localGitClient, localGitRepoFactory, dependencyFileManager, fileSystem, logger) + IVmrInfo vmrInfo, + IServiceProvider serviceProvider) + : base(vmrInfo, sourceManifest, dependencyTracker, patchHandler, versionDetailsParser, thirdPartyNoticesGenerator, readmeComponentListGenerator, codeownersGenerator, credScanSuppressionsGenerator, localGitClient, localGitRepoFactory, dependencyFileManager, fileSystem, logger, serviceProvider) { _logger = logger; _sourceManifest = sourceManifest; @@ -97,6 +98,8 @@ public async Task UpdateRepository( string mappingName, string? targetRevision, string? targetVersion, + string? officialBuildId, + int? barId, bool updateDependencies, IReadOnlyCollection additionalRemotes, string? componentTemplatePath, @@ -107,7 +110,7 @@ public async Task UpdateRepository( bool reapplyVmrPatches, CancellationToken cancellationToken) { - await _dependencyTracker.InitializeSourceMappings(); + await _dependencyTracker.RefreshMetadata(); var mapping = _dependencyTracker.GetMapping(mappingName); @@ -125,7 +128,9 @@ public async Task UpdateRepository( mapping.DefaultRemote, targetRevision ?? mapping.DefaultRef, targetVersion, - Parent: null); + Parent: null, + officialBuildId, + barId); if (updateDependencies) { @@ -197,6 +202,8 @@ private async Task> UpdateRepositoryInter await UpdateTargetVersionOnly( update.TargetRevision, update.TargetVersion, + update.OfficialBuildId, + update.BarId, update.Mapping, currentVersion, cancellationToken); @@ -671,6 +678,8 @@ private void DeleteRepository(string repo) private async Task UpdateTargetVersionOnly( string targetRevision, string targetVersion, + string? officialBuildId, + int? barId, SourceMapping mapping, VmrDependencyVersion currentVersion, CancellationToken cancellationToken) @@ -687,7 +696,9 @@ private async Task UpdateTargetVersionOnly( TargetRevision: targetRevision, TargetVersion: targetVersion, Parent: null, - RemoteUri: _sourceManifest.GetRepoVersion(mapping.Name).RemoteUri)); + RemoteUri: _sourceManifest.GetRepoVersion(mapping.Name).RemoteUri, + OfficialBuildId: officialBuildId, + BarId: barId)); var filesToAdd = new List { @@ -747,7 +758,7 @@ private async Task LoadNewSourceMappings( try { _fileSystem.WriteToFile(tempFile, sourceMappingContent); - await _dependencyTracker.InitializeSourceMappings(tempFile); + await _dependencyTracker.RefreshMetadata(tempFile); _logger.LogInformation("Initialized a new version of {file}", relativePath); } catch (Exception e) diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/LoadSourceMappingsFromInstallerTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/LoadSourceMappingsFromInstallerTest.cs index 2352d37642..fba11b92a3 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/LoadSourceMappingsFromInstallerTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/LoadSourceMappingsFromInstallerTest.cs @@ -115,6 +115,7 @@ public async Task NewRepoAddedDuringSyncTest() JsonSerializer.Serialize(_sourceMappings, _jsonSettings)); await GitOperations.CommitAll(InstallerRepoPath, "Added new dependency"); + await CreateNewBuild(ProductRepoPath, []); // We will sync new installer, which should bring in the new product repo await UpdateRepoToLastCommit(Constants.InstallerRepoName, InstallerRepoPath); diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs index 1c25a1a646..7faa49f29e 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs @@ -28,9 +28,9 @@ public async Task OnlyBackflowsTest() hadUpdates.ShouldHaveUpdates(); await GitOperations.MergePrBranch(ProductRepoPath, branchName); CheckFileContents(_productRepoFilePath, "New content from the VMR"); - // Backflow again - should be a no-op - hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName); + // We want to flow the same build again, so the BarId doesn't change + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branchName, useLatestBuild: true); hadUpdates.ShouldNotHaveUpdates(); await GitOperations.Checkout(ProductRepoPath, "main"); await GitOperations.DeleteBranch(ProductRepoPath, branchName); diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs index 69f7f22c54..b8396fdfcf 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs @@ -91,11 +91,7 @@ public void DeleteCurrentTestDirectory() protected virtual IServiceCollection CreateServiceProvider() => new ServiceCollection() .AddLogging(b => b.AddConsole().AddFilter(l => l >= LogLevel.Debug)) .AddSingleVmrSupport("git", VmrPath, TmpPath, null, null) - .AddSingleton(new BarApiClient( - buildAssetRegistryPat: null, - managedIdentityId: null, - disableInteractiveAuth: true, - buildAssetRegistryBaseUri: MaestroApiOptions.StagingMaestroUri)); + .AddSingleton(_basicBarClient.Object); protected static List GetExpectedFilesInVmr( NativePath vmrPath, @@ -163,6 +159,7 @@ protected static void CompareFileContents(NativePath filePath, string resourceFi protected async Task InitializeRepoAtLastCommit(string repoName, NativePath repoPath, LocalPath? sourceMappingsPath = null) { + await CreateNewBuild(repoPath, []); var commit = await GitOperations.GetRepoLastCommit(repoPath); var sourceMappings = sourceMappingsPath ?? VmrPath / VmrInfo.DefaultRelativeSourceMappingsPath; await CallDarcInitialize(repoName, commit, sourceMappings); @@ -170,6 +167,7 @@ protected async Task InitializeRepoAtLastCommit(string repoName, NativePath repo protected async Task UpdateRepoToLastCommit(string repoName, NativePath repoPath, bool generateCodeowners = false, bool generateCredScanSuppressions = false) { + await CreateNewBuild(repoPath, []); var commit = await GitOperations.GetRepoLastCommit(repoPath); await CallDarcUpdate(repoName, commit, generateCodeowners, generateCredScanSuppressions); } @@ -178,7 +176,7 @@ private async Task CallDarcInitialize(string repository, string commit, LocalPat { using var scope = ServiceProvider.CreateScope(); var vmrInitializer = scope.ServiceProvider.GetRequiredService(); - await vmrInitializer.InitializeRepository(repository, commit, null, true, sourceMappingsPath, Array.Empty(), null, null, false, false, true, _cancellationToken.Token); + await vmrInitializer.InitializeRepository(repository, commit, null, null, null, true, sourceMappingsPath, Array.Empty(), null, null, false, false, true, _cancellationToken.Token); } protected async Task CallDarcUpdate(string repository, string commit, bool generateCodeowners = false, bool generateCredScanSuppressions = false) @@ -190,7 +188,7 @@ protected async Task CallDarcUpdate(string repository, string commit, Additional { using var scope = ServiceProvider.CreateScope(); var vmrUpdater = scope.ServiceProvider.GetRequiredService(); - await vmrUpdater.UpdateRepository(repository, commit, null, true, additionalRemotes, null, null, generateCodeowners, generateCredScanSuppressions, discardPatches: true, reapplyVmrPatches: false, _cancellationToken.Token); + await vmrUpdater.UpdateRepository(repository, commit, null, null, null, true, additionalRemotes, null, null, generateCodeowners, generateCredScanSuppressions, discardPatches: true, reapplyVmrPatches: false, _cancellationToken.Token); } protected async Task CallDarcBackflow( @@ -198,10 +196,17 @@ protected async Task CallDarcBackflow( NativePath repoPath, string branch, Build? buildToFlow = null, - IReadOnlyCollection? excludedAssets = null) + IReadOnlyCollection? excludedAssets = null, + bool useLatestBuild = false) { using var scope = ServiceProvider.CreateScope(); var codeflower = scope.ServiceProvider.GetRequiredService(); + + if (useLatestBuild) + { + buildToFlow = await _basicBarClient.Object.GetBuildAsync(_buildId); + } + return await codeflower.FlowBackAsync( mappingName, repoPath, @@ -356,6 +361,7 @@ protected async Task CreateNewBuild(NativePath repoPath, (string name, st { var assetId = 1; _buildId++; + var commit = await GitOperations.GetRepoLastCommit(repoPath); var build = new Build( id: _buildId, @@ -363,7 +369,7 @@ protected async Task CreateNewBuild(NativePath repoPath, (string name, st staleness: 0, released: false, stable: true, - commit: await GitOperations.GetRepoLastCommit(repoPath), + commit: commit, channels: ImmutableList.Empty, assets: [ @@ -382,6 +388,9 @@ protected async Task CreateNewBuild(NativePath repoPath, (string name, st _basicBarClient .Setup(x => x.GetBuildAsync(build.Id)) .ReturnsAsync(build); + _basicBarClient + .Setup(x => x.GetBuildsAsync(repoPath.Path, commit)) + .ReturnsAsync([build]); return build; } diff --git a/test/Microsoft.DotNet.DarcLib.Tests/Helpers/VersionFileTests.cs b/test/Microsoft.DotNet.DarcLib.Tests/Helpers/VersionFilesTests.cs similarity index 94% rename from test/Microsoft.DotNet.DarcLib.Tests/Helpers/VersionFileTests.cs rename to test/Microsoft.DotNet.DarcLib.Tests/Helpers/VersionFilesTests.cs index 3115d689ac..64f917e9ad 100644 --- a/test/Microsoft.DotNet.DarcLib.Tests/Helpers/VersionFileTests.cs +++ b/test/Microsoft.DotNet.DarcLib.Tests/Helpers/VersionFilesTests.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; diff --git a/test/Microsoft.DotNet.DarcLib.Tests/Models/VirtualMonoRepo/GitInfoFileTests.cs b/test/Microsoft.DotNet.DarcLib.Tests/Models/VirtualMonoRepo/GitInfoFileTests.cs index b26169b210..6feda03523 100644 --- a/test/Microsoft.DotNet.DarcLib.Tests/Models/VirtualMonoRepo/GitInfoFileTests.cs +++ b/test/Microsoft.DotNet.DarcLib.Tests/Models/VirtualMonoRepo/GitInfoFileTests.cs @@ -45,9 +45,7 @@ public void GitInfoXmlIsDeSerializedTest() GitCommitHash = "4ee620cc1b57da45d93135e064d43a83e65bbb6e", OfficialBuildId = "20220803.1", OutputPackageVersion = "7.0.0-beta.22403.1", - PreReleaseVersionLabel = "beta", GitCommitCount = 1432, - IsStable = true, }; // Act @@ -63,8 +61,6 @@ public void GitInfoXmlIsDeSerializedTest() 4ee620cc1b57da45d93135e064d43a83e65bbb6e 20220803.1 7.0.0-beta.22403.1 - beta - true 1432 @@ -75,8 +71,6 @@ public void GitInfoXmlIsDeSerializedTest() gitInfoFile.GitCommitHash.Should().Be("4ee620cc1b57da45d93135e064d43a83e65bbb6e"); gitInfoFile.OfficialBuildId.Should().Be("20220803.1"); gitInfoFile.OutputPackageVersion.Should().Be("7.0.0-beta.22403.1"); - gitInfoFile.PreReleaseVersionLabel.Should().Be("beta"); gitInfoFile.GitCommitCount.Should().Be(1432); - gitInfoFile.IsStable.Should().BeTrue(); } } diff --git a/test/Microsoft.DotNet.DarcLib.Tests/Models/VirtualMonoRepo/ManifestRecordTests.cs b/test/Microsoft.DotNet.DarcLib.Tests/Models/VirtualMonoRepo/ManifestRecordTests.cs index 9fe6b66366..a538af6012 100644 --- a/test/Microsoft.DotNet.DarcLib.Tests/Models/VirtualMonoRepo/ManifestRecordTests.cs +++ b/test/Microsoft.DotNet.DarcLib.Tests/Models/VirtualMonoRepo/ManifestRecordTests.cs @@ -18,7 +18,8 @@ public void GitHubCommitUrlIsConstructedTest() path: "arcade", remoteUri: "https://github.com/dotnet/arcade", commitSha: "4ee620cc1b57da45d93135e064d43a83e65bbb6e", - packageVersion: "5.0.0"); + packageVersion: "5.0.0", + barId: null); record.GetPublicUrl().Should().Be("https://github.com/dotnet/arcade/tree/4ee620cc1b57da45d93135e064d43a83e65bbb6e"); @@ -26,7 +27,8 @@ public void GitHubCommitUrlIsConstructedTest() path: "arcade", remoteUri: "https://github.com/dotnet/some.git.repo.git", commitSha: "4ee620cc1b57da45d93135e064d43a83e65bbb6e", - packageVersion: "5.0.0"); + packageVersion: "5.0.0", + barId: null); record.GetPublicUrl().Should().Be("https://github.com/dotnet/some.git.repo/tree/4ee620cc1b57da45d93135e064d43a83e65bbb6e"); } @@ -38,7 +40,8 @@ public void AzDOCommitUrlIsConstructedTest() path: "command-line-api", remoteUri: "https://dev.azure.com/dnceng/internal/_git/dotnet-command-line-api", commitSha: "4ee620cc1b57da45d93135e064d43a83e65bbb6e", - packageVersion: "5.0.0"); + packageVersion: "5.0.0", + barId: null); record.GetPublicUrl().Should().Be("https://dev.azure.com/dnceng/internal/_git/dotnet-command-line-api/?version=GC4ee620cc1b57da45d93135e064d43a83e65bbb6e"); } diff --git a/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/VmrPusherTests.cs b/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/VmrPusherTests.cs index fd8906a25a..7683f31209 100644 --- a/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/VmrPusherTests.cs +++ b/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/VmrPusherTests.cs @@ -32,7 +32,12 @@ public class VmrPusherTests [SetUp] public void SetUp() { - var repo = new RepositoryRecord("some-repo", "https://github.com/org/some-repo", Sha, "8.0"); + var repo = new RepositoryRecord( + "some-repo", + "https://github.com/org/some-repo", + Sha, + "8.0", + barId: null); _sourceManifest.Reset(); _sourceManifest.SetupGet(s => s.Repositories).Returns(new List() { repo }); diff --git a/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs b/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs index 7871c738e2..4c860c6592 100644 --- a/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs +++ b/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs @@ -47,7 +47,8 @@ protected async Task CheckBackwardFlowGitHubPullRequest( string targetBranch, string[] testFiles, Dictionary testFilePatches, - string commitSha) + string commitSha, + int buildId) { var expectedPRTitle = GetCodeFlowPRName(targetBranch, sourceRepoName); @@ -56,7 +57,7 @@ protected async Task CheckBackwardFlowGitHubPullRequest( var versionDetailsFile = files.FirstOrDefault(file => file.FileName == "eng/Version.Details.xml"); versionDetailsFile.Should().NotBeNull(); - versionDetailsFile!.Patch.Should().Contain(GetExpectedCodeFlowDependencyVersionEntry(sourceRepoName, commitSha)); + versionDetailsFile!.Patch.Should().Contain(GetExpectedCodeFlowDependencyVersionEntry(sourceRepoName, commitSha, buildId)); // Verify new files are in the PR foreach (var testFile in testFiles) diff --git a/test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs b/test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs index f74a9f5c6d..d2d18ec8e8 100644 --- a/test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs +++ b/test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs @@ -247,8 +247,8 @@ protected async Task CheckNonBatchedGitHubPullRequest(string sourceRepoName, str } protected static string GetCodeFlowPRName(string targetBranch, string sourceRepoName) => $"[{targetBranch}] Source code changes from {TestParameters.GitHubTestOrg}/{sourceRepoName}"; - protected static string GetExpectedCodeFlowDependencyVersionEntry(string repo, string sha) => - $"Source Uri=\"{GetGitHubRepoUrl(repo)}\" Sha=\"{sha}\" />"; + protected static string GetExpectedCodeFlowDependencyVersionEntry(string repo, string sha, int buildId) => + $"Source Uri=\"{GetGitHubRepoUrl(repo)}\" Sha=\"{sha}\" BarId=\"{buildId}\" />"; protected async Task CheckGitHubPullRequest(string expectedPRTitle, string targetRepoName, string targetBranch, List expectedDependencies, string repoDirectory, bool isCompleted, bool isUpdated) diff --git a/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs b/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs index 6976414935..699adbd419 100644 --- a/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs +++ b/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs @@ -151,7 +151,7 @@ await CreateTargetBranchAndExecuteTest(targetBranchName, testRepoFolder, async ( await TriggerSubscriptionAsync(subscriptionId.Value); TestContext.WriteLine("Verifying subscription PR"); - await CheckBackwardFlowGitHubPullRequest(TestRepository.VmrTestRepoName, TestRepository.TestRepo1Name, targetBranchName, [TestFileName], TestFilePatches, repoSha); + await CheckBackwardFlowGitHubPullRequest(TestRepository.VmrTestRepoName, TestRepository.TestRepo1Name, targetBranchName, [TestFileName], TestFilePatches, repoSha, build.Id); } } } From 40cc628cfee841d7a43d0a0cba53733dc931044d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Fri, 29 Nov 2024 12:54:14 +0100 Subject: [PATCH 06/11] Add a test for out-of-order merging without conflicts (#4193) --- .../VmrTwoWayCodeflowTest.cs | 115 ++++++++++++++++-- 1 file changed, 104 insertions(+), 11 deletions(-) diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs index bfec245b76..63505e65ed 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs @@ -10,6 +10,7 @@ using Microsoft.DotNet.DarcLib.Models.Darc; using Microsoft.DotNet.DarcLib.VirtualMonoRepo; using Microsoft.Extensions.DependencyInjection; +using Microsoft.TeamFoundation.SourceControl.WebApi.Legacy; using Moq; using NUnit.Framework; @@ -111,6 +112,93 @@ public async Task SubmoduleCodeFlowTest() await GitOperations.CheckAllIsCommitted(ProductRepoPath); } + // This one simulates what would happen if PR both ways are open and the one that was open later merges first. + // The diagram it follows is here (O are commits): + /* + repo VMR + O────────────────────►O + │ 2. │ + │ O◄────────────────O 1. + │ │ 4. │ + 3.O───┼────────────►O │ + │ │ │ │ + │ ┌─┘ │ │ + │ │ │ │ + 5.O◄┘ └──►O 6. + │ │ + |────────────────────►O 7. + │ │ + */ + [Test] + public async Task OutOfOrderMergesTest() + { + await EnsureTestRepoIsInitialized(); + + const string backBranchName = nameof(OutOfOrderMergesTest); + const string forwardBranchName = nameof(OutOfOrderMergesTest) + "-ff"; + + // 1. Change file in VMR + await File.WriteAllTextAsync(_productRepoVmrPath / "1a.txt", "one"); + await GitOperations.CommitAll(VmrPath, "1a.txt"); + + // 2. Open a backflow PR + var hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, backBranchName); + hadUpdates.ShouldHaveUpdates(); + // We make another commit in the repo and add it to the PR branch (this is not in the diagram above) + await GitOperations.Checkout(VmrPath, "main"); + await File.WriteAllTextAsync(_productRepoVmrPath / "1b.txt", "one again"); + await GitOperations.CommitAll(VmrPath, "1b.txt"); + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, backBranchName); + hadUpdates.ShouldHaveUpdates(); + + // 3. Change file in the repo + await GitOperations.Checkout(ProductRepoPath, "main"); + await File.WriteAllTextAsync(ProductRepoPath / "3a.txt", "three"); + await GitOperations.CommitAll(ProductRepoPath, "3a.txt"); + + // 4. Open a forward flow PR + hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, forwardBranchName); + hadUpdates.ShouldHaveUpdates(); + // We make another commit in the repo and add it to the PR branch (this is not in the diagram above) + await GitOperations.Checkout(ProductRepoPath, "main"); + await File.WriteAllTextAsync(ProductRepoPath / "3b.txt", "three again"); + await GitOperations.CommitAll(ProductRepoPath, "3b.txt"); + hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, forwardBranchName); + hadUpdates.ShouldHaveUpdates(); + + // 5. Merge the backflow PR + await GitOperations.MergePrBranch(ProductRepoPath, backBranchName); + + // 6. Merge the forward flow PR + await GitOperations.MergePrBranch(VmrPath, forwardBranchName); + + // 7. Forward flow again so the VMR version of the file will flow back to the VMR + // While the VMR accepted the content from the repo but it will get overriden by the VMR content again + await GitOperations.Checkout(ProductRepoPath, "main"); + await GitOperations.Checkout(VmrPath, "main"); + hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branch: forwardBranchName); + hadUpdates.ShouldHaveUpdates(); + await GitOperations.VerifyMergeConflict(VmrPath, forwardBranchName, mergeTheirs: true); + + // Both VMR and repo need to have the version from the VMR as it flowed to the repo and back + (string, string)[] expectedFiles = + [ + ("1a.txt", "one"), + ("1b.txt", "one again"), + ("3a.txt", "three"), + ("3b.txt", "three again"), + ]; + + foreach (var (file, content) in expectedFiles) + { + CheckFileContents(_productRepoVmrPath / file, content); + CheckFileContents(ProductRepoPath / file, content); + } + + await GitOperations.CheckAllIsCommitted(VmrPath); + await GitOperations.CheckAllIsCommitted(ProductRepoPath); + } + // This one simulates what would happen if PR both ways are open and the one that was open later merges first. // The diagram it follows is here (O are commits, x are conflicts): /* @@ -129,7 +217,7 @@ repo VMR │ │ */ [Test] - public async Task OutOfOrderMergesTest() + public async Task OutOfOrderMergesWithConflictsTest() { await EnsureTestRepoIsInitialized(); @@ -142,26 +230,28 @@ public async Task OutOfOrderMergesTest() // 2. Open a backflow PR await File.WriteAllTextAsync(_productRepoVmrPath / "b.txt", bFileContent); await GitOperations.CommitAll(VmrPath, bFileContent); - var backflowBranch = await ChangeVmrFileAndFlowIt("New content from the VMR #1", backBranchName); - backflowBranch.ShouldHaveUpdates(); + var hadUpdates = await ChangeVmrFileAndFlowIt("New content from the VMR #1", backBranchName); + hadUpdates.ShouldHaveUpdates(); // We make another commit in the repo and add it to the PR branch (this is not in the diagram above) - backflowBranch = await ChangeVmrFileAndFlowIt("New content from the VMR #2", backBranchName); - backflowBranch.ShouldHaveUpdates(); await GitOperations.Checkout(ProductRepoPath, "main"); + hadUpdates = await ChangeVmrFileAndFlowIt("New content from the VMR #2", backBranchName); + hadUpdates.ShouldHaveUpdates(); // 3. Change file in the repo // 4. Open a forward flow PR + await GitOperations.Checkout(ProductRepoPath, "main"); await File.WriteAllTextAsync(ProductRepoPath / "a.txt", aFileContent); await GitOperations.CommitAll(ProductRepoPath, aFileContent); - var forwardFlowBranch = await ChangeRepoFileAndFlowIt("New content from the individual repo #1", forwardBranchName); - forwardFlowBranch.ShouldHaveUpdates(); + hadUpdates = await ChangeRepoFileAndFlowIt("New content from the individual repo #1", forwardBranchName); + hadUpdates.ShouldHaveUpdates(); // We make another commit in the repo and add it to the PR branch (this is not in the diagram above) - forwardFlowBranch = await ChangeRepoFileAndFlowIt("New content from the individual repo #2", forwardBranchName); - forwardFlowBranch.ShouldHaveUpdates(); - await GitOperations.Checkout(VmrPath, "main"); + await GitOperations.Checkout(ProductRepoPath, "main"); + hadUpdates = await ChangeRepoFileAndFlowIt("New content from the individual repo #2", forwardBranchName); + hadUpdates.ShouldHaveUpdates(); // 5. The backflow PR is now in conflict - repo has the content from step 3 but VMR has the one from step 1 // 6. We resolve the conflict by using the content from the VMR + await GitOperations.Checkout(ProductRepoPath, "main"); await GitOperations.VerifyMergeConflict(ProductRepoPath, backBranchName, mergeTheirs: true, expectedConflictingFile: _productRepoFileName); @@ -169,6 +259,7 @@ await GitOperations.VerifyMergeConflict(ProductRepoPath, backBranchName, // 7. The forward flow PR will have a conflict the opposite way - repo has the content from step 3 but VMR has the one from step 1 // 8. We resolve the conflict by using the content from the VMR too + await GitOperations.Checkout(VmrPath, "main"); await GitOperations.VerifyMergeConflict(VmrPath, forwardBranchName, mergeTheirs: true, expectedConflictingFile: VmrInfo.SourcesDir / Constants.ProductRepoName / _productRepoFileName); @@ -176,7 +267,9 @@ await GitOperations.VerifyMergeConflict(VmrPath, forwardBranchName, // 9. We try to forward flow again so the VMR version of the file will flow back to the VMR // 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); + await GitOperations.Checkout(ProductRepoPath, "main"); + await GitOperations.Checkout(VmrPath, "main"); + hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branch: forwardBranchName); hadUpdates.ShouldHaveUpdates(); await GitOperations.VerifyMergeConflict(VmrPath, forwardBranchName, mergeTheirs: true, From e641e37030e7536ef037fd16be8201a891d850cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Fri, 29 Nov 2024 12:56:42 +0100 Subject: [PATCH 07/11] Create and assign issues to TODOs (#4200) --- .../DarcLib/VirtualMonoRepo/VmrCloneManager.cs | 2 +- .../PullRequestUpdater.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs index 2823c6b604..d68bba8692 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs @@ -58,7 +58,7 @@ public async Task PrepareVmrAsync( string checkoutRef, CancellationToken cancellationToken) { - // TODO https://github.com/dotnet/arcade-services/issues/3318: We should check if working tree is clean + // TODO https://github.com/dotnet/arcade-services/issues/4197: Make sure VMR is ready for use (working tree is clean..) // This makes sure we keep different VMRs separate // We expect to have up to 3: diff --git a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs index 5571134bdb..fb5ec38225 100644 --- a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs +++ b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs @@ -953,7 +953,7 @@ await CreateCodeFlowPullRequestAsync( } catch (Exception e) { - // TODO https://github.com/dotnet/arcade-services/issues/3318: Handle this - Maybe we need to set a reminder and try again? + // TODO https://github.com/dotnet/arcade-services/issues/4198: Notify us about these kind of failures _logger.LogError(e, "Failed to update sources and packages for PR {url} of subscription {subscriptionId}", pr.Url, update.SubscriptionId); @@ -1022,7 +1022,7 @@ private async Task UpdateAssetsAndSources(SubscriptionUpdateWorkItem updat subscription.Id, pullRequest.HeadBranch); - // TODO https://github.com/dotnet/arcade-services/issues/3318: Handle failures (conflict, non-ff etc) + // TODO https://github.com/dotnet/arcade-services/issues/4199: Handle failures (conflict, non-ff etc) using (var scope = _telemetryRecorder.RecordGitOperation(TrackedGitOperation.Push, subscription.TargetRepository)) { await _gitClient.Push(targetRepo, pullRequest.HeadBranch, subscription.TargetRepository); @@ -1142,7 +1142,7 @@ private async Task UpdateAssetsAndSources(SubscriptionUpdateWorkItem updat subscription.Id, newBranchName); - // TODO https://github.com/dotnet/arcade-services/issues/3318: Handle failures (conflict, non-ff etc) + // TODO https://github.com/dotnet/arcade-services/issues/4199: Handle failures (conflict, non-ff etc) using (var scope = _telemetryRecorder.RecordGitOperation(TrackedGitOperation.Push, subscription.TargetRepository)) { await _gitClient.Push(targetRepo, newBranchName, subscription.TargetRepository); From b5694239876d5bd18314fe515b0a0f892b2f428d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Fri, 29 Nov 2024 14:58:04 +0100 Subject: [PATCH 08/11] Stop generating the `AllRepoVersions.props` file (#4202) --- .../VirtualMonoRepo/AllVersionsPropsFile.cs | 79 ------------------- .../VirtualMonoRepo/VmrDependencyTracker.cs | 16 +--- .../DarcLib/VirtualMonoRepo/VmrInfo.cs | 7 -- .../VmrForwardFlowTest.cs | 4 - .../VmrRepoDeletionTest.cs | 3 - .../VmrSyncRepoChangesTest.cs | 1 - .../VmrTestsBase.cs | 3 +- .../CodeFlowScenarioTestBase.cs | 6 +- 8 files changed, 4 insertions(+), 115 deletions(-) delete mode 100644 src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/AllVersionsPropsFile.cs diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/AllVersionsPropsFile.cs b/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/AllVersionsPropsFile.cs deleted file mode 100644 index 00b20a6d6f..0000000000 --- a/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/AllVersionsPropsFile.cs +++ /dev/null @@ -1,79 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Generic; -using System.Xml; - -#nullable enable -namespace Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; - -public interface IAllVersionsPropsFile : IMsBuildPropsFile -{ - Dictionary Versions { get; } - - void UpdateVersion(string repository, string sha, string packageVersion); - - bool DeleteVersion(string repository); -} - -/// -/// A model for a file AllRepoVersions.props which is part of the VMR and contains list of all versions -/// of all synchronized individual repositories. -/// -public class AllVersionsPropsFile : MsBuildPropsFile, IAllVersionsPropsFile -{ - public const string FileName = "AllRepoVersions.props"; - private const string ShaPropertyName = "GitCommitHash"; - private const string PackageVersionPropertyName = "OutputPackageVersion"; - - public Dictionary Versions { get; } - - public AllVersionsPropsFile(Dictionary versions) - : base(orderPropertiesAscending: true) - { - Versions = versions; - } - - public AllVersionsPropsFile(IReadOnlyCollection repositoryRecords) - : base(orderPropertiesAscending: true) - { - Versions = []; - foreach (var repo in repositoryRecords) - { - UpdateVersion(repo.Path, repo.CommitSha, repo.PackageVersion); - } - } - - public void UpdateVersion(string repository, string sha, string? packageVersion) - { - var key = SanitizePropertyName(repository); - Versions[key + ShaPropertyName] = sha; - - if (packageVersion != null) - { - Versions[key + PackageVersionPropertyName] = packageVersion; - } - } - - public bool DeleteVersion(string repository) - { - var key = SanitizePropertyName(repository); - var deleted = Versions.Remove(key + ShaPropertyName); - deleted |= Versions.Remove(key + PackageVersionPropertyName); - return deleted; - } - - public static AllVersionsPropsFile DeserializeFromXml(string path) - { - var versions = DeserializeProperties(path); - return new AllVersionsPropsFile(versions); - } - - protected override void SerializeProperties(XmlElement propertyGroup, Func createElement) - => SerializeDictionary(Versions, propertyGroup, createElement); - - private static string SanitizePropertyName(string propertyName) => propertyName - .Replace("-", string.Empty) - .Replace(".", string.Empty); -} diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs index 9768bbb07d..be7d9c05ea 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs @@ -23,7 +23,7 @@ public interface IVmrDependencyTracker IReadOnlyCollection Mappings { get; } /// - /// Refreshes all metadata: source mappings, source manifest and AllRepoVersions + /// Refreshes all metadata: source mappings, source manifest, .. /// /// Leave empty for default (src/source-mappings.json) Task RefreshMetadata(string? sourceMappingsPath = null); @@ -39,11 +39,10 @@ public interface IVmrDependencyTracker /// /// Holds information about versions of individual repositories synchronized in the VMR. -/// Uses the AllRepoVersions.props file as source of truth and propagates changes into the git-info files. +/// Uses the source-manifest.json file as source of truth and propagates changes into the git-info files. /// public class VmrDependencyTracker : IVmrDependencyTracker { - private AllVersionsPropsFile _repoVersions; private readonly ISourceManifest _sourceManifest; private readonly IVmrInfo _vmrInfo; private readonly IFileSystem _fileSystem; @@ -63,7 +62,6 @@ public VmrDependencyTracker( { _vmrInfo = vmrInfo; _sourceManifest = sourceManifest; - _repoVersions = new AllVersionsPropsFile(sourceManifest.Repositories); _fileSystem = fileSystem; _sourceMappingParser = sourceMappingParser; _mappings = null; @@ -93,14 +91,10 @@ public async Task RefreshMetadata(string? sourceMappingsPath = null) { await InitializeSourceMappings(sourceMappingsPath); _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); - _repoVersions = new AllVersionsPropsFile(_sourceManifest.Repositories); } public void UpdateDependencyVersion(VmrDependencyUpdate update) { - _repoVersions.UpdateVersion(update.Mapping.Name, update.TargetRevision, update.TargetVersion); - _repoVersions.SerializeToXml(_vmrInfo.AllVersionsFilePath); - _sourceManifest.UpdateVersion( update.Mapping.Name, update.RemoteUri, @@ -137,12 +131,6 @@ public void UpdateDependencyVersion(VmrDependencyUpdate update) public bool RemoveRepositoryVersion(string repo) { var hasChanges = false; - - if (_repoVersions.DeleteVersion(repo)) - { - _repoVersions.SerializeToXml(_vmrInfo.AllVersionsFilePath); - hasChanges = true; - } var gitInfoFilePath = GetGitInfoFilePath(repo); if (_fileSystem.FileExists(gitInfoFilePath)) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs index d868df936a..f2d7d9b616 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs @@ -59,11 +59,6 @@ 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 @@ -142,6 +137,4 @@ 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/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrForwardFlowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrForwardFlowTest.cs index 9b42bc482d..8e7de383d1 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrForwardFlowTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrForwardFlowTest.cs @@ -98,10 +98,6 @@ await GetLocal(VmrPath).AddDependencyAsync(new DependencyDetail dependencies.Where(d => d.Name != DependencyFileManager.ArcadeSdkPackageName) .Should().BeEquivalentTo(GetDependencies(build1)); - var propName = VersionFiles.GetVersionPropsPackageVersionElementName("Package.A1"); - var vmrVersionProps = AllVersionsPropsFile.DeserializeFromXml(VmrPath / VersionFiles.VersionProps); - vmrVersionProps.Versions[propName].Should().Be("1.0.1"); - // Now we will change something in the repo and flow it to the VMR // Then we will change something in the repo again but before we flow it, we will make a conflicting change in the PR branch await File.WriteAllTextAsync(_productRepoFilePath, "New content again in the repo #1"); diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrRepoDeletionTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrRepoDeletionTest.cs index ef27c0a37b..07d90180eb 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrRepoDeletionTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrRepoDeletionTest.cs @@ -86,9 +86,6 @@ await File.WriteAllTextAsync(InstallerRepoPath / _sourceMappingsRelativePath, CheckDirectoryContents(VmrPath, expectedFiles); - var versions = AllVersionsPropsFile.DeserializeFromXml(VmrPath / VmrInfo.GitInfoSourcesDir / AllVersionsPropsFile.FileName); - versions.Versions.Keys.Should().BeEquivalentTo(["installerGitCommitHash"]); - var sourceManifest = SourceManifest.FromJson(VmrPath / VmrInfo.SourcesDir / VmrInfo.SourceManifestFileName); sourceManifest.Repositories.Should().HaveCount(1); sourceManifest.Repositories.First().Path.Should().Be("installer"); diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs index 84604b125b..4c2f315ccc 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrSyncRepoChangesTest.cs @@ -89,7 +89,6 @@ public async Task PackageVersionIsUpdatedOnlyTest() sourceManifest.GetVersion(Constants.DependencyRepoName)!.PackageVersion.Should().Be("8.0.1"); (await File.ReadAllTextAsync(VmrPath / VmrInfo.GitInfoSourcesDir / Constants.DependencyRepoName + ".props")).Should().Contain("8.0.1"); - (await File.ReadAllTextAsync(VmrPath / VmrInfo.GitInfoSourcesDir / AllVersionsPropsFile.FileName)).Should().Contain("8.0.1"); } [Test] diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs index b8396fdfcf..b1ae7e7710 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs @@ -91,7 +91,7 @@ public void DeleteCurrentTestDirectory() protected virtual IServiceCollection CreateServiceProvider() => new ServiceCollection() .AddLogging(b => b.AddConsole().AddFilter(l => l >= LogLevel.Debug)) .AddSingleVmrSupport("git", VmrPath, TmpPath, null, null) - .AddSingleton(_basicBarClient.Object); + .AddSingleton(_basicBarClient.Object); protected static List GetExpectedFilesInVmr( NativePath vmrPath, @@ -101,7 +101,6 @@ protected static List GetExpectedFilesInVmr( { List expectedFiles = [ - vmrPath / VmrInfo.GitInfoSourcesDir / AllVersionsPropsFile.FileName, vmrPath / VmrInfo.DefaultRelativeSourceManifestPath, vmrPath / VmrInfo.DefaultRelativeSourceMappingsPath, ]; diff --git a/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs b/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs index 4c860c6592..f784bbbdd5 100644 --- a/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs +++ b/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs @@ -19,16 +19,12 @@ protected async Task CheckForwardFlowGitHubPullRequest( Octokit.PullRequest pullRequest = await WaitForPullRequestAsync(targetRepoName, targetBranch); IReadOnlyList files = await GitHubApi.PullRequest.Files(TestParameters.GitHubTestOrg, targetRepoName, pullRequest.Number); - files.Count.Should().Be(testFiles.Length + 3); + files.Count.Should().Be(testFiles.Length + 2); // Verify source-manifest has changes var sourceManifestFile = files.FirstOrDefault(file => file.FileName == "src/source-manifest.json"); sourceManifestFile.Should().NotBeNull(); - // Verify git-info - var allRepoVersionsFile = files.FirstOrDefault(file => file.FileName == "prereqs/git-info/AllRepoVersions.props"); - allRepoVersionsFile.Should().NotBeNull(); - var repoPropsFile = files.FirstOrDefault(file => file.FileName == $"prereqs/git-info/{sourceRepoName}.props"); repoPropsFile.Should().NotBeNull(); From 71512901368a7bf61aa2eb2933d35b2b6fa6cda7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Mon, 2 Dec 2024 17:01:28 +0100 Subject: [PATCH 09/11] Allow skipping BAR build look-up (+fix how we do it) (#4205) --- .../VirtualMonoRepo/InitializeOperation.cs | 11 +---- .../VirtualMonoRepo/UpdateOperation.cs | 14 ++----- .../InitializeCommandLineOptions.cs | 3 ++ .../UpdateCommandLineOptions.cs | 3 ++ .../VirtualMonoRepo/IVmrInitializer.cs | 6 +-- .../DarcLib/VirtualMonoRepo/IVmrUpdater.cs | 2 + .../VirtualMonoRepo/VmrForwardFlower.cs | 3 ++ .../DarcLib/VirtualMonoRepo/VmrInitializer.cs | 26 ++++++++---- .../DarcLib/VirtualMonoRepo/VmrManagerBase.cs | 41 +++++++++++-------- .../DarcLib/VirtualMonoRepo/VmrUpdater.cs | 22 ++++++++-- .../VmrTestsBase.cs | 40 +++++++++++++++--- 11 files changed, 113 insertions(+), 58 deletions(-) diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/InitializeOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/InitializeOperation.cs index 93bcdff4aa..c7270eef92 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/InitializeOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/InitializeOperation.cs @@ -2,11 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; -using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.Darc.Options.VirtualMonoRepo; -using Microsoft.DotNet.DarcLib; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.VirtualMonoRepo; using Microsoft.Extensions.Logging; @@ -18,18 +16,15 @@ internal class InitializeOperation : VmrOperationBase { private readonly InitializeCommandLineOptions _options; private readonly IVmrInitializer _vmrInitializer; - private readonly IBarApiClient _barClient; public InitializeOperation( InitializeCommandLineOptions options, IVmrInitializer vmrInitializer, - ILogger logger, - IBarApiClient barClient) + ILogger logger) : base(options, logger) { _options = options; _vmrInitializer = vmrInitializer; - _barClient = barClient; } protected override async Task ExecuteInternalAsync( @@ -38,13 +33,10 @@ protected override async Task ExecuteInternalAsync( IReadOnlyCollection additionalRemotes, CancellationToken cancellationToken) { - var build = (await _barClient.GetBuildsAsync(repoName, targetRevision)).SingleOrDefault(); await _vmrInitializer.InitializeRepository( repoName, targetRevision, null, - build?.AzureDevOpsBuildNumber, - build?.Id, _options.Recursive, new NativePath(_options.SourceMappings), additionalRemotes, @@ -53,6 +45,7 @@ await _vmrInitializer.InitializeRepository( _options.GenerateCodeowners, _options.GenerateCredScanSuppressions, _options.DiscardPatches, + _options.EnableBuildLookUp, cancellationToken); } } diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs index a8e569ce16..51fc00c279 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/UpdateOperation.cs @@ -2,11 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; -using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.Darc.Options.VirtualMonoRepo; -using Microsoft.DotNet.DarcLib; using Microsoft.DotNet.DarcLib.VirtualMonoRepo; using Microsoft.Extensions.Logging; @@ -17,18 +15,15 @@ internal class UpdateOperation : VmrOperationBase { private readonly UpdateCommandLineOptions _options; private readonly IVmrUpdater _vmrUpdater; - private readonly IBarApiClient _barClient; public UpdateOperation( UpdateCommandLineOptions options, IVmrUpdater vmrUpdater, - ILogger logger, - IBarApiClient barClient) + ILogger logger) : base(options, logger) { _options = options; _vmrUpdater = vmrUpdater; - _barClient = barClient; } protected override async Task ExecuteInternalAsync( @@ -37,14 +32,12 @@ protected override async Task ExecuteInternalAsync( IReadOnlyCollection additionalRemotes, CancellationToken cancellationToken) { - var build = (await _barClient.GetBuildsAsync(repoName, targetRevision)).SingleOrDefault(); - await _vmrUpdater.UpdateRepository( repoName, targetRevision, targetVersion: null, - build?.AzureDevOpsBuildNumber, - build?.Id, + officialBuildId: null, + barId: null, _options.Recursive, additionalRemotes, _options.ComponentTemplate, @@ -53,6 +46,7 @@ await _vmrUpdater.UpdateRepository( _options.GenerateCredScanSuppressions, _options.DiscardPatches, reapplyVmrPatches: false, + _options.EnableBuildLookUp, cancellationToken); } } diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/InitializeCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/InitializeCommandLineOptions.cs index 6127dd2b38..a6f9641372 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/InitializeCommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/InitializeCommandLineOptions.cs @@ -16,4 +16,7 @@ internal class InitializeCommandLineOptions : VmrSyncCommandLineOptionsName of a repository mapping /// Revision (commit SHA, branch, tag..) onto which to synchronize, leave empty for HEAD /// Version of packages, that the SHA we're updating to, produced - /// Azdo build id of the build that's being flown, if applicable - /// Bar id of the build that's being flown, if applicable /// When true, initializes dependencies (from Version.Details.xml) recursively /// Path to the source-mappings.json file /// Additional git remotes to use when fetching @@ -27,12 +25,11 @@ public interface IVmrInitializer /// Whether to generate a CODEOWNERS file /// Whether to generate a .config/CredScanSuppressions.json file /// Whether to clean up genreated .patch files after their used + /// Whether to look up package versions and build number from BAR when populating version files Task InitializeRepository( string mappingName, string? targetRevision, string? targetVersion, - string? officialBuildId, - int? barId, bool initializeDependencies, LocalPath sourceMappingsPath, IReadOnlyCollection additionalRemotes, @@ -41,5 +38,6 @@ Task InitializeRepository( bool generateCodeowners, bool generateCredScanSuppressions, bool discardPatches, + bool lookUpBuilds, CancellationToken cancellationToken); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs index fcc73b63da..a6f1124602 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs @@ -26,6 +26,7 @@ public interface IVmrUpdater /// Whether to generate a .config/CredScanSuppressions.json file /// Whether to clean up genreated .patch files after their used /// Whether to reapply patches stored in the VMR + /// Whether to look up package versions and build number from BAR when populating version files /// True if the repository was updated, false if it was already up to date Task UpdateRepository( string mappingName, @@ -41,5 +42,6 @@ Task UpdateRepository( bool generateCredScanSuppressions, bool discardPatches, bool reapplyVmrPatches, + bool lookUpBuilds, CancellationToken cancellationToken); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs index e5ee4ec1df..303c5ee606 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs @@ -253,6 +253,7 @@ protected override async Task SameDirectionFlowAsync( generateCredScanSuppressions: true, discardPatches, reapplyVmrPatches: true, + lookUpBuilds: true, cancellationToken); } catch (PatchApplicationFailedException e) @@ -308,6 +309,7 @@ await FlowCodeAsync( generateCredScanSuppressions: false, discardPatches, reapplyVmrPatches: true, + lookUpBuilds: true, cancellationToken); } @@ -383,6 +385,7 @@ .. submodules.Select(s => s.Path).Distinct().Select(VmrPatchHandler.GetExclusion generateCredScanSuppressions: true, discardPatches, reapplyVmrPatches: true, + lookUpBuilds: true, cancellationToken); } } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs index c74000c32f..523544ea6a 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs @@ -39,6 +39,7 @@ public class VmrInitializer : VmrManagerBase, IVmrInitializer """; private readonly IVmrInfo _vmrInfo; + private readonly IBasicBarClient _barClient; private readonly IVmrDependencyTracker _dependencyTracker; private readonly IVmrPatchHandler _patchHandler; private readonly IRepositoryCloneManager _cloneManager; @@ -59,14 +60,15 @@ public VmrInitializer( ILocalGitRepoFactory localGitRepoFactory, IDependencyFileManager dependencyFileManager, IWorkBranchFactory workBranchFactory, + IBasicBarClient barClient, IFileSystem fileSystem, ILogger logger, ISourceManifest sourceManifest, - IVmrInfo vmrInfo, - IServiceProvider serviceProvider) - : base(vmrInfo, sourceManifest, dependencyTracker, patchHandler, versionDetailsParser, thirdPartyNoticesGenerator, readmeComponentListGenerator, codeownersGenerator, credScanSuppressionsGenerator, localGitClient, localGitRepoFactory, dependencyFileManager, fileSystem, logger, serviceProvider) + IVmrInfo vmrInfo) + : base(vmrInfo, sourceManifest, dependencyTracker, patchHandler, versionDetailsParser, thirdPartyNoticesGenerator, readmeComponentListGenerator, codeownersGenerator, credScanSuppressionsGenerator, localGitClient, localGitRepoFactory, dependencyFileManager, barClient, fileSystem, logger) { _vmrInfo = vmrInfo; + _barClient = barClient; _dependencyTracker = dependencyTracker; _patchHandler = patchHandler; _cloneManager = cloneManager; @@ -79,8 +81,6 @@ public async Task InitializeRepository( string mappingName, string? targetRevision, string? targetVersion, - string? officialBuildId, - int? barId, bool initializeDependencies, LocalPath sourceMappingsPath, IReadOnlyCollection additionalRemotes, @@ -89,12 +89,24 @@ public async Task InitializeRepository( bool generateCodeowners, bool generateCredScanSuppressions, bool discardPatches, + bool lookUpBuilds, CancellationToken cancellationToken) { await _dependencyTracker.RefreshMetadata(sourceMappingsPath); - var mapping = _dependencyTracker.GetMapping(mappingName); + string? officialBuildId = null; + int? barId = null; + + if (lookUpBuilds) + { + var build = (await _barClient.GetBuildsAsync(mapping.DefaultRemote, targetRevision)) + .FirstOrDefault(); + + officialBuildId = build?.AzureDevOpsBuildNumber; + barId = build?.Id; + } + if (_dependencyTracker.GetDependencyVersion(mapping) is not null) { throw new EmptySyncException($"Repository {mapping.Name} already exists"); @@ -120,7 +132,7 @@ public async Task InitializeRepository( try { IEnumerable updates = initializeDependencies - ? await GetAllDependenciesAsync(rootUpdate, additionalRemotes, cancellationToken) + ? await GetAllDependenciesAsync(rootUpdate, additionalRemotes, lookUpBuilds, cancellationToken) : [rootUpdate]; foreach (var update in updates) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs index f3fcd2749e..b5f076bad9 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrManagerBase.cs @@ -12,7 +12,6 @@ using Microsoft.DotNet.DarcLib.Models; using Microsoft.DotNet.DarcLib.Models.Darc; using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; #nullable enable @@ -20,7 +19,7 @@ namespace Microsoft.DotNet.DarcLib.VirtualMonoRepo; public abstract class VmrManagerBase { - protected const string InterruptedSyncExceptionMessage = + protected const string InterruptedSyncExceptionMessage = "A new branch was created for the sync and didn't get merged as the sync " + "was interrupted. A new sync should start from {original} branch."; @@ -36,9 +35,9 @@ public abstract class VmrManagerBase private readonly ILocalGitClient _localGitClient; private readonly ILocalGitRepoFactory _localGitRepoFactory; private readonly IDependencyFileManager _dependencyFileManager; + private readonly IBasicBarClient _barClient; private readonly IFileSystem _fileSystem; private readonly ILogger _logger; - private readonly IServiceProvider _serviceProvider; protected ILocalGitRepo GetLocalVmr() => _localGitRepoFactory.Create(_vmrInfo.VmrPath); @@ -55,9 +54,9 @@ protected VmrManagerBase( ILocalGitClient localGitClient, ILocalGitRepoFactory localGitRepoFactory, IDependencyFileManager dependencyFileManager, + IBasicBarClient barClient, IFileSystem fileSystem, - ILogger logger, - IServiceProvider serviceProvider) + ILogger logger) { _logger = logger; _vmrInfo = vmrInfo; @@ -72,8 +71,8 @@ protected VmrManagerBase( _localGitClient = localGitClient; _localGitRepoFactory = localGitRepoFactory; _dependencyFileManager = dependencyFileManager; + _barClient = barClient; _fileSystem = fileSystem; - _serviceProvider = serviceProvider; } public async Task> UpdateRepoToRevisionAsync( @@ -205,7 +204,7 @@ protected async Task CommitAsync(string commitMessage, (string Name, string Emai await _localGitClient.CommitAsync(_vmrInfo.VmrPath, commitMessage, allowEmpty: true, author); - _logger.LogInformation("Committed in {duration} seconds", (int) watch.Elapsed.TotalSeconds); + _logger.LogInformation("Committed in {duration} seconds", (int)watch.Elapsed.TotalSeconds); } /// @@ -214,6 +213,7 @@ protected async Task CommitAsync(string commitMessage, (string Name, string Emai protected async Task> GetAllDependenciesAsync( VmrDependencyUpdate root, IReadOnlyCollection additionalRemotes, + bool lookUpBuilds, CancellationToken cancellationToken) { var transitiveDependencies = new Dictionary @@ -226,8 +226,6 @@ protected async Task> GetAllDependenciesAsync( _logger.LogInformation("Finding transitive dependencies for {mapping}:{revision}..", root.Mapping.Name, root.TargetRevision); - var barClient = _serviceProvider.GetRequiredService(); - while (reposToScan.TryDequeue(out var repo)) { cancellationToken.ThrowIfCancellationRequested(); @@ -277,17 +275,24 @@ protected async Task> GetAllDependenciesAsync( $"for a {VersionFiles.VersionDetailsXml} dependency of {dependency.Name}"); } - var builds = await barClient.GetBuildsAsync(dependency.RepoUri, dependency.Commit); - - if (builds.Count() != 1) + Maestro.Client.Models.Build? build = null; + if (lookUpBuilds) { - _logger.LogInformation("Expected to find one build for repo {repo} and commit {commit}, but found {number} builds" + - "Will proceed with the code flow normally, but won't have any BAR data for this repo", - dependency.RepoUri, - dependency.Commit, - builds.Count()); + var builds = (await _barClient.GetBuildsAsync(dependency.RepoUri, dependency.Commit)) + .OrderByDescending(b => b.DateProduced) + .ToList(); + + if (builds.Count > 1) + { + _logger.LogInformation( + "Found {number} builds for repo {repo} and commit {commit}. Will use the latest one.", + builds.Count, + dependency.RepoUri, + dependency.Commit); + } + + build = builds.FirstOrDefault(); } - var build = builds.SingleOrDefault(); var update = new VmrDependencyUpdate( mapping, diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs index 855bfb9487..c1a26eef17 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs @@ -51,6 +51,7 @@ [Recursive sync] {name} / {oldShaShort}{{Constants.Arrow}}{newShaShort} private readonly IRepositoryCloneManager _cloneManager; private readonly IVmrPatchHandler _patchHandler; private readonly IFileSystem _fileSystem; + private readonly IBasicBarClient _barClient; private readonly ILogger _logger; private readonly ISourceManifest _sourceManifest; private readonly IThirdPartyNoticesGenerator _thirdPartyNoticesGenerator; @@ -74,11 +75,11 @@ public VmrUpdater( IGitRepoFactory gitRepoFactory, IWorkBranchFactory workBranchFactory, IFileSystem fileSystem, + IBasicBarClient barClient, ILogger logger, ISourceManifest sourceManifest, - IVmrInfo vmrInfo, - IServiceProvider serviceProvider) - : base(vmrInfo, sourceManifest, dependencyTracker, patchHandler, versionDetailsParser, thirdPartyNoticesGenerator, readmeComponentListGenerator, codeownersGenerator, credScanSuppressionsGenerator, localGitClient, localGitRepoFactory, dependencyFileManager, fileSystem, logger, serviceProvider) + IVmrInfo vmrInfo) + : base(vmrInfo, sourceManifest, dependencyTracker, patchHandler, versionDetailsParser, thirdPartyNoticesGenerator, readmeComponentListGenerator, codeownersGenerator, credScanSuppressionsGenerator, localGitClient, localGitRepoFactory, dependencyFileManager, barClient, fileSystem, logger) { _logger = logger; _sourceManifest = sourceManifest; @@ -87,6 +88,7 @@ public VmrUpdater( _cloneManager = cloneManager; _patchHandler = patchHandler; _fileSystem = fileSystem; + _barClient = barClient; _thirdPartyNoticesGenerator = thirdPartyNoticesGenerator; _readmeComponentListGenerator = readmeComponentListGenerator; _localGitClient = localGitClient; @@ -108,12 +110,22 @@ public async Task UpdateRepository( bool generateCredScanSuppressions, bool discardPatches, bool reapplyVmrPatches, + bool lookUpBuilds, CancellationToken cancellationToken) { await _dependencyTracker.RefreshMetadata(); var mapping = _dependencyTracker.GetMapping(mappingName); + if (lookUpBuilds && (!barId.HasValue || string.IsNullOrEmpty(officialBuildId))) + { + var build = (await _barClient.GetBuildsAsync(mapping.DefaultRemote, targetRevision)) + .FirstOrDefault(); + + officialBuildId = build?.AzureDevOpsBuildNumber; + barId = build?.Id; + } + // Reload source-mappings.json if it's getting updated if (_vmrInfo.SourceMappingsPath != null && targetRevision != null @@ -142,6 +154,7 @@ public async Task UpdateRepository( generateCodeowners, generateCredScanSuppressions, discardPatches, + lookUpBuilds, cancellationToken); } else @@ -286,6 +299,7 @@ private async Task UpdateRepositoryRecursively( bool generateCodeowners, bool generateCredScanSuppressions, bool discardPatches, + bool lookUpBuilds, CancellationToken cancellationToken) { string originalRootSha = GetCurrentVersion(rootUpdate.Mapping); @@ -296,7 +310,7 @@ private async Task UpdateRepositoryRecursively( Constants.Arrow, rootUpdate.TargetRevision); - var updates = (await GetAllDependenciesAsync(rootUpdate, additionalRemotes, cancellationToken)).ToList(); + var updates = (await GetAllDependenciesAsync(rootUpdate, additionalRemotes, lookUpBuilds, cancellationToken)).ToList(); var extraneousMappings = _dependencyTracker.Mappings .Where(mapping => !updates.Any(update => update.Mapping == mapping) && !mapping.DisableSynchronization) diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs index b1ae7e7710..d0196d67f9 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTestsBase.cs @@ -171,23 +171,51 @@ protected async Task UpdateRepoToLastCommit(string repoName, NativePath repoPath await CallDarcUpdate(repoName, commit, generateCodeowners, generateCredScanSuppressions); } - private async Task CallDarcInitialize(string repository, string commit, LocalPath sourceMappingsPath) + private async Task CallDarcInitialize(string mapping, string commit, LocalPath sourceMappingsPath) { using var scope = ServiceProvider.CreateScope(); var vmrInitializer = scope.ServiceProvider.GetRequiredService(); - await vmrInitializer.InitializeRepository(repository, commit, null, null, null, true, sourceMappingsPath, Array.Empty(), null, null, false, false, true, _cancellationToken.Token); + await vmrInitializer.InitializeRepository( + mappingName: mapping, + targetRevision: commit, + targetVersion: null, + initializeDependencies: true, + sourceMappingsPath: sourceMappingsPath, + additionalRemotes: Array.Empty(), + componentTemplatePath: null, + tpnTemplatePath: null, + generateCodeowners: false, + generateCredScanSuppressions: false, + discardPatches: true, + lookUpBuilds: false, + cancellationToken: _cancellationToken.Token); } - protected async Task CallDarcUpdate(string repository, string commit, bool generateCodeowners = false, bool generateCredScanSuppressions = false) + protected async Task CallDarcUpdate(string mapping, string commit, bool generateCodeowners = false, bool generateCredScanSuppressions = false) { - await CallDarcUpdate(repository, commit, [], generateCodeowners, generateCredScanSuppressions); + await CallDarcUpdate(mapping, commit, [], generateCodeowners, generateCredScanSuppressions); } - protected async Task CallDarcUpdate(string repository, string commit, AdditionalRemote[] additionalRemotes, bool generateCodeowners = false, bool generateCredScanSuppressions = false) + protected async Task CallDarcUpdate(string mapping, string commit, AdditionalRemote[] additionalRemotes, bool generateCodeowners = false, bool generateCredScanSuppressions = false) { using var scope = ServiceProvider.CreateScope(); var vmrUpdater = scope.ServiceProvider.GetRequiredService(); - await vmrUpdater.UpdateRepository(repository, commit, null, null, null, true, additionalRemotes, null, null, generateCodeowners, generateCredScanSuppressions, discardPatches: true, reapplyVmrPatches: false, _cancellationToken.Token); + await vmrUpdater.UpdateRepository( + mappingName: mapping, + targetRevision: commit, + targetVersion: null, + officialBuildId: null, + barId: null, + updateDependencies: true, + additionalRemotes: additionalRemotes, + componentTemplatePath: null, + tpnTemplatePath: null, + generateCodeowners: generateCodeowners, + generateCredScanSuppressions: generateCredScanSuppressions, + discardPatches: true, + reapplyVmrPatches: false, + lookUpBuilds: false, + cancellationToken: _cancellationToken.Token); } protected async Task CallDarcBackflow( From 3e47dc387afa6083c9c1efe8125dac48b54185ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Tue, 3 Dec 2024 11:53:22 +0100 Subject: [PATCH 10/11] Make darc verify codeflow subscriptions not sourcing/targeting VMRs correctly (#4203) --- src/Maestro/DependencyUpdater/Program.cs | 2 +- ...{DarcRemoteFactory.cs => RemoteFactory.cs} | 30 +++++++----- .../Controllers/BuildsController.cs | 2 +- src/Maestro/Maestro.Web/Startup.cs | 2 +- .../DarcRemoteFactory.cs | 23 ++++++---- .../PullRequestActor.cs | 8 ++-- .../PullRequestBuilder.cs | 2 +- .../PullRequestPolicyFailureNotifier.cs | 2 +- .../Darc/Helpers/RemoteFactory.cs | 37 ++++++--------- .../Darc/Helpers/UxManager.cs | 10 ++-- .../Models/PopUps/AddSubscriptionPopUp.cs | 10 ++-- .../Models/PopUps/AuthenticateEditorPopUp.cs | 5 +- .../Darc/Models/PopUps/EditorPopUp.cs | 3 +- .../PopUps/SetRepositoryMergePoliciesPopUp.cs | 13 +++--- .../Darc/Models/PopUps/SubscriptionPopUp.cs | 46 +++++++++++++++++-- .../Models/PopUps/UpdateSubscriptionPopUp.cs | 14 ++++-- .../Operations/AddBuildToChannelOperation.cs | 5 +- .../Operations/AddDefaultChannelOperation.cs | 7 ++- .../Operations/AddSubscriptionOperation.cs | 22 ++++++--- .../Darc/Operations/CloneOperation.cs | 4 +- .../Darc/Operations/GatherDropOperation.cs | 2 +- .../Operations/GetDependencyGraphOperation.cs | 2 +- .../SetRepositoryMergePoliciesOperation.cs | 8 ++-- .../Operations/UpdateSubscriptionOperation.cs | 8 ++++ .../Darc/Options/CommandLineOptions.cs | 9 +++- .../Options/SubscriptionCommandLineOptions.cs | 3 ++ .../DarcLib/CoherencyUpdateResolver.cs | 4 +- .../ProductDependencyCyclesHealthMetric.cs | 2 +- .../HealthMetrics/SubscriptionHealthMetric.cs | 2 +- .../DarcLib/IRemoteFactory.cs | 5 +- src/Microsoft.DotNet.Darc/DarcLib/Local.cs | 2 +- .../DarcLib/Models/Darc/DependencyGraph.cs | 10 ++-- src/Microsoft.DotNet.Darc/DarcLib/Remote.cs | 4 +- .../Controllers/BuildsController.cs | 2 +- .../PcsStartup.cs | 2 +- .../PullRequestBuilder.cs | 2 +- .../PullRequestPolicyFailureNotifier.cs | 2 +- .../PullRequestUpdater.cs | 12 ++--- .../BuildController20200914Tests.cs | 2 +- .../DependencyCoherencyTests.cs | 42 ++++++++--------- .../BuildController20200914Tests.cs | 2 +- .../PullRequestBuilderTests.cs | 7 ++- .../PullRequestPolicyFailureNotifierTests.cs | 2 +- .../UpdaterTests.cs | 6 +-- .../PullRequestActorTests.cs | 7 ++- .../PullRequestBuilderTests.cs | 8 ++-- .../PullRequestPolicyFailureNotifierTests.cs | 2 +- 47 files changed, 242 insertions(+), 164 deletions(-) rename src/Maestro/Maestro.DataProviders/{DarcRemoteFactory.cs => RemoteFactory.cs} (79%) diff --git a/src/Maestro/DependencyUpdater/Program.cs b/src/Maestro/DependencyUpdater/Program.cs index 70bec120a5..8e24d48cbf 100644 --- a/src/Maestro/DependencyUpdater/Program.cs +++ b/src/Maestro/DependencyUpdater/Program.cs @@ -60,7 +60,7 @@ public static void Configure(IServiceCollection services) services.AddTransient(sp => ActivatorUtilities.CreateInstance(sp, "git")); services.AddTransient(); - services.AddScoped(); + services.AddScoped(); services.AddTransient(); services.AddKustoClientProvider("Kusto"); // TODO (https://github.com/dotnet/arcade-services/issues/3880) - Remove subscriptionIdGenerator diff --git a/src/Maestro/Maestro.DataProviders/DarcRemoteFactory.cs b/src/Maestro/Maestro.DataProviders/RemoteFactory.cs similarity index 79% rename from src/Maestro/Maestro.DataProviders/DarcRemoteFactory.cs rename to src/Maestro/Maestro.DataProviders/RemoteFactory.cs index 7cffd9f56d..c48c10da4d 100644 --- a/src/Maestro/Maestro.DataProviders/DarcRemoteFactory.cs +++ b/src/Maestro/Maestro.DataProviders/RemoteFactory.cs @@ -13,27 +13,30 @@ namespace Maestro.DataProviders; -public class DarcRemoteFactory : IRemoteFactory +public class RemoteFactory : IRemoteFactory { private readonly IVersionDetailsParser _versionDetailsParser; private readonly OperationManager _operations; private readonly IProcessManager _processManager; + private readonly ILoggerFactory _loggerFactory; private readonly BuildAssetRegistryContext _context; private readonly DarcRemoteMemoryCache _cache; private readonly IGitHubTokenProvider _gitHubTokenProvider; private readonly IAzureDevOpsTokenProvider _azdoTokenProvider; - public DarcRemoteFactory( + public RemoteFactory( BuildAssetRegistryContext context, IGitHubTokenProvider gitHubTokenProvider, IAzureDevOpsTokenProvider azdoTokenProvider, IVersionDetailsParser versionDetailsParser, DarcRemoteMemoryCache memoryCache, OperationManager operations, - IProcessManager processManager) + IProcessManager processManager, + ILoggerFactory loggerFactory) { _operations = operations; _processManager = processManager; + _loggerFactory = loggerFactory; _versionDetailsParser = versionDetailsParser; _context = context; _gitHubTokenProvider = gitHubTokenProvider; @@ -41,25 +44,25 @@ public DarcRemoteFactory( _cache = memoryCache; } - public async Task GetRemoteAsync(string repoUrl, ILogger logger) + public async Task CreateRemoteAsync(string repoUrl) { using (_operations.BeginOperation($"Getting remote for repo {repoUrl}.")) { - IRemoteGitRepo remoteGitClient = await GetRemoteGitClient(repoUrl, logger); - return new Remote(remoteGitClient, _versionDetailsParser, logger); + IRemoteGitRepo remoteGitClient = await GetRemoteGitClient(repoUrl); + return new Remote(remoteGitClient, _versionDetailsParser, _loggerFactory.CreateLogger()); } } - public async Task GetDependencyFileManagerAsync(string repoUrl, ILogger logger) + public async Task CreateDependencyFileManagerAsync(string repoUrl) { using (_operations.BeginOperation($"Getting remote file manager for repo {repoUrl}.")) { - IRemoteGitRepo remoteGitClient = await GetRemoteGitClient(repoUrl, logger); - return new DependencyFileManager(remoteGitClient, _versionDetailsParser, logger); + IRemoteGitRepo remoteGitClient = await GetRemoteGitClient(repoUrl); + return new DependencyFileManager(remoteGitClient, _versionDetailsParser, _loggerFactory.CreateLogger()); } } - private async Task GetRemoteGitClient(string repoUrl, ILogger logger) + private async Task GetRemoteGitClient(string repoUrl) { // Normalize the url with the AzDO client prior to attempting to // get a token. When we do coherency updates we build a repo graph and @@ -81,10 +84,13 @@ private async Task GetRemoteGitClient(string repoUrl, ILogger lo : new GitHubClient( new Microsoft.DotNet.DarcLib.GitHubTokenProvider(_gitHubTokenProvider), _processManager, - logger, + _loggerFactory.CreateLogger(), _cache.Cache), - GitRepoType.AzureDevOps => new AzureDevOpsClient(_azdoTokenProvider, _processManager, logger), + GitRepoType.AzureDevOps => new AzureDevOpsClient( + _azdoTokenProvider, + _processManager, + _loggerFactory.CreateLogger()), _ => throw new NotImplementedException($"Unknown repo url type {normalizedUrl}"), }; diff --git a/src/Maestro/Maestro.Web/Api/v2020_02_20/Controllers/BuildsController.cs b/src/Maestro/Maestro.Web/Api/v2020_02_20/Controllers/BuildsController.cs index 1610872e91..d2f19240ba 100644 --- a/src/Maestro/Maestro.Web/Api/v2020_02_20/Controllers/BuildsController.cs +++ b/src/Maestro/Maestro.Web/Api/v2020_02_20/Controllers/BuildsController.cs @@ -182,7 +182,7 @@ public async Task GetCommit(int buildId) return NotFound(); } - IRemote remote = await Factory.GetRemoteAsync(build.AzureDevOpsRepository ?? build.GitHubRepository, null); + IRemote remote = await Factory.CreateRemoteAsync(build.AzureDevOpsRepository ?? build.GitHubRepository); Microsoft.DotNet.DarcLib.Commit commit = await remote.GetCommitAsync(build.AzureDevOpsRepository ?? build.GitHubRepository, build.Commit); return Ok(new Maestro.Api.Model.v2020_02_20.Commit(commit.Author, commit.Sha, commit.Message)); } diff --git a/src/Maestro/Maestro.Web/Startup.cs b/src/Maestro/Maestro.Web/Startup.cs index ecb9f4ab1d..b64f3a1983 100644 --- a/src/Maestro/Maestro.Web/Startup.cs +++ b/src/Maestro/Maestro.Web/Startup.cs @@ -277,7 +277,7 @@ public override void ConfigureServices(IServiceCollection services) sp.GetRequiredService>(), "git")); services.AddTransient(); - services.AddScoped(); + services.AddScoped(); services.AddTransient(); services.AddSingleton(typeof(IActorProxyFactory<>), typeof(ActorProxyFactory<>)); diff --git a/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs b/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs index 87138e96d3..254a0993ce 100644 --- a/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs +++ b/src/Maestro/SubscriptionActorService/DarcRemoteFactory.cs @@ -27,6 +27,7 @@ public class DarcRemoteFactory : IRemoteFactory private readonly IVersionDetailsParser _versionDetailsParser; private readonly IProcessManager _processManager; private readonly OperationManager _operations; + private readonly ILoggerFactory _loggerFactory; public DarcRemoteFactory( IConfiguration configuration, @@ -37,12 +38,14 @@ public DarcRemoteFactory( TemporaryFiles tempFiles, IVersionDetailsParser versionDetailsParser, IProcessManager processManager, - OperationManager operations) + OperationManager operations, + ILoggerFactory loggerFactory) { _tempFiles = tempFiles; _versionDetailsParser = versionDetailsParser; _processManager = processManager; _operations = operations; + _loggerFactory = loggerFactory; _configuration = configuration; _gitHubTokenProvider = gitHubTokenProvider; _azureDevOpsTokenProvider = azureDevOpsTokenProvider; @@ -50,25 +53,25 @@ public DarcRemoteFactory( _context = context; } - public async Task GetRemoteAsync(string repoUrl, ILogger logger) + public async Task CreateRemoteAsync(string repoUrl) { using (_operations.BeginOperation($"Getting remote for repo {repoUrl}.")) { - IRemoteGitRepo remoteGitClient = await GetRemoteGitClient(repoUrl, logger); - return new Remote(remoteGitClient, _versionDetailsParser, logger); + IRemoteGitRepo remoteGitClient = await CreateRemoteGitClient(repoUrl); + return new Remote(remoteGitClient, _versionDetailsParser, _loggerFactory.CreateLogger()); } } - public async Task GetDependencyFileManagerAsync(string repoUrl, ILogger logger) + public async Task CreateDependencyFileManagerAsync(string repoUrl) { using (_operations.BeginOperation($"Getting remote file manager for repo {repoUrl}.")) { - IRemoteGitRepo remoteGitClient = await GetRemoteGitClient(repoUrl, logger); - return new DependencyFileManager(remoteGitClient, _versionDetailsParser, logger); + IRemoteGitRepo remoteGitClient = await CreateRemoteGitClient(repoUrl); + return new DependencyFileManager(remoteGitClient, _versionDetailsParser, _loggerFactory.CreateLogger()); } } - private async Task GetRemoteGitClient(string repoUrl, ILogger logger) + private async Task CreateRemoteGitClient(string repoUrl) { // Normalize the url with the AzDO client prior to attempting to // get a token. When we do coherency updates we build a repo graph and @@ -98,14 +101,14 @@ private async Task GetRemoteGitClient(string repoUrl, ILogger lo : new GitHubClient( new ResolvedTokenProvider(await _gitHubTokenProvider.GetTokenForInstallationAsync(installationId)), _processManager, - logger, + _loggerFactory.CreateLogger(), temporaryRepositoryRoot, _cache.Cache), GitRepoType.AzureDevOps => new AzureDevOpsClient( _azureDevOpsTokenProvider, _processManager, - logger, + _loggerFactory.CreateLogger(), temporaryRepositoryRoot), _ => throw new NotImplementedException($"Unknown repo url type {normalizedUrl}"), diff --git a/src/Maestro/SubscriptionActorService/PullRequestActor.cs b/src/Maestro/SubscriptionActorService/PullRequestActor.cs index b250673817..66955ce551 100644 --- a/src/Maestro/SubscriptionActorService/PullRequestActor.cs +++ b/src/Maestro/SubscriptionActorService/PullRequestActor.cs @@ -425,7 +425,7 @@ private async Task> SynchronizePullRe } (string targetRepository, _) = await GetTargetAsync(); - IRemote remote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote remote = await _remoteFactory.CreateRemoteAsync(targetRepository); _logger.LogInformation("Getting status for Pull Request: {url}", prUrl); PrStatus status = await remote.GetPullRequestStatusAsync(prUrl); @@ -704,7 +704,7 @@ public async Task> UpdateAssetsAsync( { (string targetRepository, string targetBranch) = await GetTargetAsync(); - IRemote darcRemote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote darcRemote = await _remoteFactory.CreateRemoteAsync(targetRepository); TargetRepoDependencyUpdate repoDependencyUpdate = await GetRequiredUpdates(updates, _remoteFactory, targetRepository, prBranch: null, targetBranch); @@ -805,7 +805,7 @@ private async Task UpdatePullRequestAsync(InProgressPullRequest pr, List GetRequiredUpdates( { _logger.LogInformation("Getting Required Updates for {branch} of {targetRepository}", targetBranch, targetRepository); // Get a remote factory for the target repo - IRemote darc = await remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote darc = await remoteFactory.CreateRemoteAsync(targetRepository); TargetRepoDependencyUpdate repoDependencyUpdate = new(); diff --git a/src/Maestro/SubscriptionActorService/PullRequestBuilder.cs b/src/Maestro/SubscriptionActorService/PullRequestBuilder.cs index 8788a69741..642e1568d3 100644 --- a/src/Maestro/SubscriptionActorService/PullRequestBuilder.cs +++ b/src/Maestro/SubscriptionActorService/PullRequestBuilder.cs @@ -122,7 +122,7 @@ public async Task CalculatePRDescriptionAndCommitUpdatesAsync( (UpdateAssetsParameters update, List deps) coherencyUpdate = requiredUpdates.Where(u => u.update.IsCoherencyUpdate).SingleOrDefault(); - IRemote remote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote remote = await _remoteFactory.CreateRemoteAsync(targetRepository); var locationResolver = new AssetLocationResolver(_barClient); // To keep a PR to as few commits as possible, if the number of diff --git a/src/Maestro/SubscriptionActorService/PullRequestPolicyFailureNotifier.cs b/src/Maestro/SubscriptionActorService/PullRequestPolicyFailureNotifier.cs index a1a6ca32a4..f4eee87982 100644 --- a/src/Maestro/SubscriptionActorService/PullRequestPolicyFailureNotifier.cs +++ b/src/Maestro/SubscriptionActorService/PullRequestPolicyFailureNotifier.cs @@ -65,7 +65,7 @@ public async Task TagSourceRepositoryGitHubContactsAsync(InProgressPullRequest p return; } - var darcRemote = await _remoteFactory.GetRemoteAsync($"https://github.com/{owner}/{repo}", _logger); + var darcRemote = await _remoteFactory.CreateRemoteAsync($"https://github.com/{owner}/{repo}"); var darcSubscriptionObject = await _barClient.GetSubscriptionAsync(subscriptionFromPr.SubscriptionId); string sourceRepository = darcSubscriptionObject.SourceRepository; string targetRepository = darcSubscriptionObject.TargetRepository; diff --git a/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs b/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs index cb748a5e6b..b9d9ea9c32 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Helpers/RemoteFactory.cs @@ -12,36 +12,29 @@ namespace Microsoft.DotNet.Darc.Helpers; internal class RemoteFactory : IRemoteFactory { + private readonly ILoggerFactory _loggerFactory; private readonly ICommandLineOptions _options; - public RemoteFactory(ICommandLineOptions options) + public RemoteFactory(ILoggerFactory loggerFactory, ICommandLineOptions options) { + _loggerFactory = loggerFactory; _options = options; } - public static IRemote GetRemote(ICommandLineOptions options, string repoUrl, ILogger logger) + public Task CreateRemoteAsync(string repoUrl) { - IRemoteGitRepo gitClient = GetRemoteGitClient(options, repoUrl, logger); - return new Remote(gitClient, new VersionDetailsParser(), logger); + IRemoteGitRepo gitClient = CreateRemoteGitClient(_options, repoUrl); + return Task.FromResult(new Remote(gitClient, new VersionDetailsParser(), _loggerFactory.CreateLogger())); } - public static IBarApiClient GetBarClient(ICommandLineOptions options) - => new BarApiClient( - options.BuildAssetRegistryToken, - managedIdentityId: null, - options.IsCi, - options.BuildAssetRegistryBaseUri); - - public Task GetRemoteAsync(string repoUrl, ILogger logger) - => Task.FromResult(GetRemote(_options, repoUrl, logger)); - - public Task GetDependencyFileManagerAsync(string repoUrl, ILogger logger) + public Task CreateDependencyFileManagerAsync(string repoUrl) { - IRemoteGitRepo gitClient = GetRemoteGitClient(_options, repoUrl, logger); - return Task.FromResult(new DependencyFileManager(gitClient, new VersionDetailsParser(), logger)); + IRemoteGitRepo gitClient = CreateRemoteGitClient(_options, repoUrl); + var dfm = new DependencyFileManager(gitClient, new VersionDetailsParser(), _loggerFactory.CreateLogger()); + return Task.FromResult(dfm); } - private static IRemoteGitRepo GetRemoteGitClient(ICommandLineOptions options, string repoUrl, ILogger logger) + private IRemoteGitRepo CreateRemoteGitClient(ICommandLineOptions options, string repoUrl) { string temporaryRepositoryRoot = Path.GetTempPath(); @@ -52,8 +45,8 @@ private static IRemoteGitRepo GetRemoteGitClient(ICommandLineOptions options, st GitRepoType.GitHub => new GitHubClient( options.GetGitHubTokenProvider(), - new ProcessManager(logger, options.GitLocation), - logger, + new ProcessManager(_loggerFactory.CreateLogger(), options.GitLocation), + _loggerFactory.CreateLogger(), temporaryRepositoryRoot, // Caching not in use for Darc local client. null), @@ -61,8 +54,8 @@ private static IRemoteGitRepo GetRemoteGitClient(ICommandLineOptions options, st GitRepoType.AzureDevOps => new AzureDevOpsClient( options.GetAzdoTokenProvider(), - new ProcessManager(logger, options.GitLocation), - logger, + new ProcessManager(_loggerFactory.CreateLogger(), options.GitLocation), + _loggerFactory.CreateLogger(), temporaryRepositoryRoot), _ => throw new System.InvalidOperationException($"Cannot create a remote of type {repoType}"), diff --git a/src/Microsoft.DotNet.Darc/Darc/Helpers/UxManager.cs b/src/Microsoft.DotNet.Darc/Darc/Helpers/UxManager.cs index a2866250a6..d8ab05b730 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Helpers/UxManager.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Helpers/UxManager.cs @@ -36,7 +36,7 @@ public UxManager(string gitLocation, ILogger logger) /// /// Popup to run /// Success or error code - public int ReadFromStdIn(EditorPopUp popUp) + public async Task ReadFromStdIn(EditorPopUp popUp) { int result; try @@ -46,7 +46,7 @@ public int ReadFromStdIn(EditorPopUp popUp) string dirPath = Path.GetDirectoryName(path); Directory.CreateDirectory(dirPath); - using (StreamWriter streamWriter = new StreamWriter(path)) + using (var streamWriter = new StreamWriter(path)) { string line; while ((line = Console.ReadLine()) != null) @@ -57,7 +57,7 @@ public int ReadFromStdIn(EditorPopUp popUp) // Now run the closed event and process the contents IList contents = EditorPopUp.OnClose(path); - result = popUp.ProcessContents(contents); + result = await popUp.ProcessContents(contents); Directory.Delete(dirPath, true); if (result != Constants.SuccessCode) { @@ -107,11 +107,11 @@ public int PopUp(EditorPopUp popUp) { _popUpClosed = false; process.EnableRaisingEvents = true; - process.Exited += (sender, e) => + process.Exited += async (sender, e) => { IList contents = EditorPopUp.OnClose(path); - result = popUp.ProcessContents(contents); + result = await popUp.ProcessContents(contents); // If succeeded, delete the temp file, otherwise keep it around // for another popup iteration. diff --git a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AddSubscriptionPopUp.cs b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AddSubscriptionPopUp.cs index dee9a86632..a061afe3db 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AddSubscriptionPopUp.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AddSubscriptionPopUp.cs @@ -4,6 +4,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; +using Microsoft.DotNet.DarcLib; using Microsoft.DotNet.Maestro.Client.Models; using Microsoft.Extensions.Logging; using YamlDotNet.Serialization; @@ -17,6 +19,8 @@ public class AddSubscriptionPopUp : SubscriptionPopUp public AddSubscriptionPopUp( string path, + bool forceCreation, + IGitRepoFactory gitRepoFactory, ILogger logger, string channel, string sourceRepository, @@ -34,7 +38,7 @@ public AddSubscriptionPopUp( string? sourceDirectory, string? targetDirectory, List excludedAssets) - : base(path, suggestedChannels, suggestedRepositories, availableMergePolicyHelp, logger, + : base(path, forceCreation, suggestedChannels, suggestedRepositories, availableMergePolicyHelp, logger, gitRepoFactory, new SubscriptionData { Channel = GetCurrentSettingForDisplay(channel, "", false), @@ -76,7 +80,7 @@ public AddSubscriptionPopUp( _logger = logger; } - public override int ProcessContents(IList contents) + public override async Task ProcessContents(IList contents) { SubscriptionData outputYamlData; @@ -95,7 +99,7 @@ public override int ProcessContents(IList contents) return Constants.ErrorCode; } - var result = ParseAndValidateData(outputYamlData); + var result = await ParseAndValidateData(outputYamlData); _data.TargetRepository = ParseSetting(outputYamlData.TargetRepository, _data.TargetRepository, false); if (string.IsNullOrEmpty(_data.TargetRepository)) diff --git a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AuthenticateEditorPopUp.cs b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AuthenticateEditorPopUp.cs index 8414f3f6c1..e42573f121 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AuthenticateEditorPopUp.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/AuthenticateEditorPopUp.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Threading.Tasks; using Microsoft.DotNet.Darc.Helpers; using Microsoft.Extensions.Logging; @@ -63,7 +64,7 @@ public AuthenticateEditorPopUp(string path, ILogger logger) public LocalSettings settings { get; set; } - public override int ProcessContents(IList contents) + public override Task ProcessContents(IList contents) { foreach (Line line in contents) { @@ -95,6 +96,6 @@ public override int ProcessContents(IList contents) } } - return settings.SaveSettingsFile(_logger); + return Task.FromResult(settings.SaveSettingsFile(_logger)); } } diff --git a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/EditorPopUp.cs b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/EditorPopUp.cs index 6555faa3a3..40b79193cc 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/EditorPopUp.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/EditorPopUp.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.IO; +using System.Threading.Tasks; using Newtonsoft.Json; #nullable enable @@ -34,7 +35,7 @@ public static IList OnClose(string path) return GetContentValues(updatedFileContents); } - public abstract int ProcessContents(IList contents); + public abstract Task ProcessContents(IList contents); private static List GetContentValues(IEnumerable contents) { diff --git a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/SetRepositoryMergePoliciesPopUp.cs b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/SetRepositoryMergePoliciesPopUp.cs index 0f7eb908a6..c6cfe5cd46 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/SetRepositoryMergePoliciesPopUp.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/SetRepositoryMergePoliciesPopUp.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.DotNet.Maestro.Client.Models; using Microsoft.Extensions.Logging; using YamlDotNet.Serialization; @@ -62,7 +63,7 @@ public SetRepositoryMergePoliciesPopUp(string path, } } - public override int ProcessContents(IList contents) + public override Task ProcessContents(IList contents) { RepositoryPoliciesData outputYamlData; @@ -76,13 +77,13 @@ public override int ProcessContents(IList contents) catch (Exception e) { _logger.LogError(e, "Failed to parse input yaml. Please see help for correct format."); - return Constants.ErrorCode; + return Task.FromResult(Constants.ErrorCode); } // Validate the merge policies if (!MergePoliciesPopUpHelpers.ValidateMergePolicies(MergePoliciesPopUpHelpers.ConvertMergePolicies(outputYamlData.MergePolicies), _logger)) { - return Constants.ErrorCode; + return Task.FromResult(Constants.ErrorCode); } _yamlData.MergePolicies = outputYamlData.MergePolicies; @@ -91,17 +92,17 @@ public override int ProcessContents(IList contents) if (string.IsNullOrEmpty(_yamlData.Repository)) { _logger.LogError("Repository URL must be non-empty"); - return Constants.ErrorCode; + return Task.FromResult(Constants.ErrorCode); } _yamlData.Branch = ParseSetting(outputYamlData.Branch, _yamlData.Branch, false); if (string.IsNullOrEmpty(_yamlData.Branch)) { _logger.LogError("Branch must be non-empty"); - return Constants.ErrorCode; + return Task.FromResult(Constants.ErrorCode); } - return Constants.SuccessCode; + return Task.FromResult(Constants.SuccessCode); } private class RepositoryPoliciesData diff --git a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/SubscriptionPopUp.cs b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/SubscriptionPopUp.cs index 3f025af843..3e5d8ff58f 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/SubscriptionPopUp.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/SubscriptionPopUp.cs @@ -4,6 +4,9 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; +using Microsoft.DotNet.DarcLib; +using Microsoft.DotNet.DarcLib.VirtualMonoRepo; using Microsoft.DotNet.Maestro.Client.Models; using Microsoft.Extensions.Logging; using YamlDotNet.Serialization; @@ -30,10 +33,12 @@ public abstract class SubscriptionPopUp : EditorPopUp private const string ExcludedAssetsElement = "Excluded Assets"; protected readonly SubscriptionData _data; + private readonly bool _forceCreation; private readonly IEnumerable _suggestedChannels; private readonly IEnumerable _suggestedRepositories; private readonly IEnumerable _availableMergePolicyHelp; private readonly ILogger _logger; + private readonly IGitRepoFactory _gitRepoFactory; public string Channel => _data.Channel; public string SourceRepository => _data.SourceRepository; @@ -50,20 +55,23 @@ public abstract class SubscriptionPopUp : EditorPopUp protected SubscriptionPopUp( string path, + bool forceCreation, IEnumerable suggestedChannels, IEnumerable suggestedRepositories, IEnumerable availableMergePolicyHelp, ILogger logger, + IGitRepoFactory gitRepoFactory, SubscriptionData data, IEnumerable header) : base(path) { _data = data; + _forceCreation = forceCreation; _suggestedChannels = suggestedChannels; _suggestedRepositories = suggestedRepositories; _availableMergePolicyHelp = availableMergePolicyHelp; _logger = logger; - + _gitRepoFactory = gitRepoFactory; GeneratePopUpContent(header); } @@ -109,7 +117,7 @@ private void GeneratePopUpContent(IEnumerable header) } } - protected int ParseAndValidateData(SubscriptionData outputYamlData) + protected async Task ParseAndValidateData(SubscriptionData outputYamlData) { if (!MergePoliciesPopUpHelpers.ValidateMergePolicies(MergePoliciesPopUpHelpers.ConvertMergePolicies(outputYamlData.MergePolicies), _logger)) { @@ -176,6 +184,25 @@ protected int ParseAndValidateData(SubscriptionData outputYamlData) _logger.LogError("Only one of source or target directory can be provided for source-enabled subscriptions"); return Constants.ErrorCode; } + + // For subscriptions targeting the VMR, we need to ensure that the target is indeed a VMR + try + { + if (!string.IsNullOrEmpty(outputYamlData.TargetDirectory) && !_forceCreation) + { + await CheckIfRepoIsVmr(outputYamlData.TargetRepository, outputYamlData.TargetBranch); + } + + if (!string.IsNullOrEmpty(outputYamlData.SourceDirectory) && !_forceCreation) + { + await CheckIfRepoIsVmr(outputYamlData.SourceRepository, "main"); + } + } + catch (DarcException e) + { + _logger.LogError(e.Message); + return Constants.ErrorCode; + } } // When we disable the source flow, we zero out the source/target directory @@ -193,11 +220,24 @@ protected int ParseAndValidateData(SubscriptionData outputYamlData) return Constants.SuccessCode; } + private async Task CheckIfRepoIsVmr(string repoUri, string branch) + { + try + { + var gitRepo = _gitRepoFactory.CreateClient(repoUri); + await gitRepo.GetFileContentsAsync(VmrInfo.DefaultRelativeSourceManifestPath, repoUri, branch); + } + catch (DependencyFileNotFoundException e) + { + throw new DarcException($"Target repository is not a VMR ({e.Message}). Use -f to override this check."); + } + } + /// /// Helper class for YAML encoding/decoding purposes. /// This is used so that we can have friendly alias names for elements. /// - #nullable disable +#nullable disable protected class SubscriptionData { [YamlMember(Alias = ChannelElement, ApplyNamingConventions = false)] diff --git a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/UpdateSubscriptionPopUp.cs b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/UpdateSubscriptionPopUp.cs index 8380fe81de..e216e01b5b 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/UpdateSubscriptionPopUp.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Models/PopUps/UpdateSubscriptionPopUp.cs @@ -4,6 +4,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; +using Microsoft.DotNet.DarcLib; using Microsoft.DotNet.Maestro.Client.Models; using Microsoft.Extensions.Logging; using YamlDotNet.Serialization; @@ -22,13 +24,15 @@ public class UpdateSubscriptionPopUp : SubscriptionPopUp private UpdateSubscriptionPopUp( string path, + bool forceCreation, + IGitRepoFactory gitRepoFactory, ILogger logger, Subscription subscription, IEnumerable suggestedChannels, IEnumerable suggestedRepositories, IEnumerable availableMergePolicyHelp, SubscriptionUpdateData data) - : base(path, suggestedChannels, suggestedRepositories, availableMergePolicyHelp, logger, data, + : base(path, forceCreation, suggestedChannels, suggestedRepositories, availableMergePolicyHelp, logger, gitRepoFactory, data, header: [ new Line($"Use this form to update the values of subscription '{subscription.Id}'.", true), new Line($"Note that if you are setting 'Is batchable' to true you need to remove all Merge Policies.", true), @@ -48,6 +52,8 @@ private UpdateSubscriptionPopUp( public UpdateSubscriptionPopUp( string path, + bool forceCreation, + IGitRepoFactory gitRepoFactory, ILogger logger, Subscription subscription, IEnumerable suggestedChannels, @@ -59,7 +65,7 @@ public UpdateSubscriptionPopUp( string sourceDirectory, string targetDirectory, List excludedAssets) - : this(path, logger, subscription, suggestedChannels, suggestedRepositories, availableMergePolicyHelp, + : this(path, forceCreation, gitRepoFactory, logger, subscription, suggestedChannels, suggestedRepositories, availableMergePolicyHelp, new SubscriptionUpdateData { Id = GetCurrentSettingForDisplay(subscription.Id.ToString(), subscription.Id.ToString(), false), @@ -80,7 +86,7 @@ public UpdateSubscriptionPopUp( { } - public override int ProcessContents(IList contents) + public override async Task ProcessContents(IList contents) { SubscriptionUpdateData outputYamlData; @@ -96,7 +102,7 @@ public override int ProcessContents(IList contents) return Constants.ErrorCode; } - var result = ParseAndValidateData(outputYamlData); + var result = await ParseAndValidateData(outputYamlData); if (!bool.TryParse(outputYamlData.Enabled, out bool enabled)) { diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/AddBuildToChannelOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/AddBuildToChannelOperation.cs index 6171a1de04..d59c2b70cb 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/AddBuildToChannelOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/AddBuildToChannelOperation.cs @@ -54,18 +54,21 @@ internal class AddBuildToChannelOperation : Operation private readonly AddBuildToChannelCommandLineOptions _options; private readonly ILogger _logger; private readonly IAzureDevOpsClient _azdoClient; + private readonly IRemoteFactory _remoteFactory; private readonly IBarApiClient _barClient; public AddBuildToChannelOperation( AddBuildToChannelCommandLineOptions options, IBarApiClient barClient, IAzureDevOpsClient azdoClient, + IRemoteFactory remoteFactory, ILogger logger) { _options = options; _barClient = barClient; _logger = logger; _azdoClient = azdoClient; + _remoteFactory = remoteFactory; } /// @@ -409,7 +412,7 @@ private async Task ValidateAzDOBuildAsync(IAzureDevOpsClient azdoClient, s build.AzureDevOpsRepository : build.GitHubRepository; - IRemote repoRemote = RemoteFactory.GetRemote(_options, sourceBuildRepo, _logger); + IRemote repoRemote = await _remoteFactory.CreateRemoteAsync(sourceBuildRepo); IEnumerable sourceBuildDependencies = await repoRemote.GetDependenciesAsync(sourceBuildRepo, build.Commit) .ConfigureAwait(false); diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/AddDefaultChannelOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/AddDefaultChannelOperation.cs index 3a584fba61..0584cc2ddd 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/AddDefaultChannelOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/AddDefaultChannelOperation.cs @@ -17,22 +17,25 @@ internal class AddDefaultChannelOperation : Operation private readonly AddDefaultChannelCommandLineOptions _options; private readonly ILogger _logger; private readonly IBarApiClient _barClient; + private readonly IRemoteFactory _remoteFactory; public AddDefaultChannelOperation( AddDefaultChannelCommandLineOptions options, ILogger logger, - IBarApiClient barClient) + IBarApiClient barClient, + IRemoteFactory remoteFactory) { _options = options; _logger = logger; _barClient = barClient; + _remoteFactory = remoteFactory; } public override async Task ExecuteAsync() { try { - IRemote repoRemote = RemoteFactory.GetRemote(_options, _options.Repository, _logger); + IRemote repoRemote = await _remoteFactory.CreateRemoteAsync(_options.Repository); // Users can ignore the flag and pass in -regex: but to prevent typos we'll avoid that. _options.Branch = _options.UseBranchAsRegex ? $"-regex:{_options.Branch}" : GitHelpers.NormalizeBranchName(_options.Branch); diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/AddSubscriptionOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/AddSubscriptionOperation.cs index 06cc997280..04484a1841 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/AddSubscriptionOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/AddSubscriptionOperation.cs @@ -24,15 +24,21 @@ internal class AddSubscriptionOperation : Operation private readonly AddSubscriptionCommandLineOptions _options; private readonly ILogger _logger; private readonly IBarApiClient _barClient; + private readonly IRemoteFactory _remoteFactory; + private readonly IGitRepoFactory _gitRepoFactory; public AddSubscriptionOperation( AddSubscriptionCommandLineOptions options, ILogger logger, - IBarApiClient barClient) + IBarApiClient barClient, + IRemoteFactory remoteFactory, + IGitRepoFactory gitRepoFactory) { _options = options; _logger = logger; _barClient = barClient; + _remoteFactory = remoteFactory; + _gitRepoFactory = gitRepoFactory; } /// @@ -167,6 +173,8 @@ public override async Task ExecuteAsync() // Help the user along with a form. We'll use the API to gather suggested values // from existing subscriptions based on the input parameters. var addSubscriptionPopup = new AddSubscriptionPopUp("add-subscription/add-subscription-todo", + _options.ForceCreation, + _gitRepoFactory, _logger, channel, sourceRepository, @@ -176,9 +184,9 @@ public override async Task ExecuteAsync() batchable, mergePolicies, (await suggestedChannels).Select(suggestedChannel => suggestedChannel.Name), - (await suggestedRepos).SelectMany(subscription => new List {subscription.SourceRepository, subscription.TargetRepository }).ToHashSet(), + (await suggestedRepos).SelectMany(subscription => new List { subscription.SourceRepository, subscription.TargetRepository }).ToHashSet(), Constants.AvailableFrequencies, - Constants.AvailableMergePolicyYamlHelp, + Constants.AvailableMergePolicyYamlHelp, failureNotificationTags, sourceEnabled, sourceDirectory, @@ -187,7 +195,7 @@ public override async Task ExecuteAsync() var uxManager = new UxManager(_options.GitLocation, _logger); int exitCode = _options.ReadStandardIn - ? uxManager.ReadFromStdIn(addSubscriptionPopup) + ? await uxManager.ReadFromStdIn(addSubscriptionPopup) : uxManager.PopUp(addSubscriptionPopup); if (exitCode != Constants.SuccessCode) @@ -237,15 +245,15 @@ public override async Task ExecuteAsync() } // Verify the target - IRemote targetVerifyRemote = RemoteFactory.GetRemote(_options, targetRepository, _logger); - if (!(await UxHelpers.VerifyAndConfirmBranchExistsAsync(targetVerifyRemote, targetRepository, targetBranch, !_options.Quiet, onlyCheckBranch: sourceEnabled))) + IRemote targetVerifyRemote = await _remoteFactory.CreateRemoteAsync(targetRepository); + if (!await UxHelpers.VerifyAndConfirmBranchExistsAsync(targetVerifyRemote, targetRepository, targetBranch, !_options.Quiet, onlyCheckBranch: sourceEnabled)) { Console.WriteLine("Aborting subscription creation."); return Constants.ErrorCode; } // Verify the source. - IRemote sourceVerifyRemote = RemoteFactory.GetRemote(_options, sourceRepository, _logger); + IRemote sourceVerifyRemote = await _remoteFactory.CreateRemoteAsync(sourceRepository); if (!await UxHelpers.VerifyAndConfirmRepositoryExistsAsync(sourceVerifyRemote, sourceRepository, !_options.Quiet)) { Console.WriteLine("Aborting subscription creation."); diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/CloneOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/CloneOperation.cs index 47132dc37a..1655a9a235 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/CloneOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/CloneOperation.cs @@ -274,7 +274,7 @@ private async Task HandleMasterCopyWithDefaultGitDir(IRemoteFactory remoteFactor if (!Directory.Exists(masterGitRepoPath)) { _logger.LogInformation($"Cloning master copy of {repoUrl} into {masterGitRepoPath}"); - IRemote repoRemote = await remoteFactory.GetRemoteAsync(repoUrl, _logger); + IRemote repoRemote = await remoteFactory.CreateRemoteAsync(repoUrl); await repoRemote.CloneAsync(repoUrl, null, masterGitRepoPath, checkoutSubmodules: true, masterRepoGitDirPath); } // The master folder already exists. We are probably resuming with a different --git-dir-parent setting, or the .gitdir parent was cleaned. @@ -315,7 +315,7 @@ private async Task HandleMasterCopyAndCreateGitDir(IRemoteFactory remoteFactory, if (!Directory.Exists(masterGitRepoPath)) { _logger.LogInformation($"Cloning master copy of {repoUrl} into {masterGitRepoPath} with .gitdir path {masterRepoGitDirPath}"); - IRemote repoRemote = await remoteFactory.GetRemoteAsync(repoUrl, _logger); + IRemote repoRemote = await remoteFactory.CreateRemoteAsync(repoUrl); await repoRemote.CloneAsync(repoUrl, null, masterGitRepoPath, checkoutSubmodules: true, masterRepoGitDirPath); } // The master folder already exists. We are probably resuming with a different --git-dir-parent setting, or the .gitdir parent was cleaned. diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/GatherDropOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/GatherDropOperation.cs index 8ffeeb8c8b..84219abe73 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/GatherDropOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/GatherDropOperation.cs @@ -172,7 +172,7 @@ public override async Task ExecuteAsync() private async Task> GetBuildDependenciesAsync(Build build) { var repoUri = build.GetRepository(); - IRemote remote = RemoteFactory.GetRemote(_options, repoUri, _logger); + IRemote remote = await _remoteFactory.CreateRemoteAsync(repoUri); return await remote.GetDependenciesAsync(repoUri, build.Commit); } diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependencyGraphOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependencyGraphOperation.cs index 0add4dd641..9b222cc7a0 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependencyGraphOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/GetDependencyGraphOperation.cs @@ -88,7 +88,7 @@ public override async Task ExecuteAsync() // Grab root dependency set. The graph build can do this, but // if an original asset name is passed, then this will do the initial filtering. - IRemote rootRepoRemote = await _remoteFactory.GetRemoteAsync(_options.RepoUri, _logger); + IRemote rootRepoRemote = await _remoteFactory.CreateRemoteAsync(_options.RepoUri); rootDependencies = await rootRepoRemote.GetDependenciesAsync( _options.RepoUri, _options.Version, diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/SetRepositoryMergePoliciesOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/SetRepositoryMergePoliciesOperation.cs index 9c314d6f5e..6316dafebd 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/SetRepositoryMergePoliciesOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/SetRepositoryMergePoliciesOperation.cs @@ -23,15 +23,18 @@ internal class SetRepositoryMergePoliciesOperation : Operation { private readonly SetRepositoryMergePoliciesCommandLineOptions _options; private readonly IBarApiClient _barClient; + private readonly IRemoteFactory _remoteFactory; private readonly ILogger _logger; public SetRepositoryMergePoliciesOperation( SetRepositoryMergePoliciesCommandLineOptions options, IBarApiClient barClient, + IRemoteFactory remoteFactory, ILogger logger) { _options = options; _barClient = barClient; + _remoteFactory = remoteFactory; _logger = logger; } @@ -137,7 +140,7 @@ public override async Task ExecuteAsync() mergePolicies = initEditorPopUp.MergePolicies; } - IRemote verifyRemote = RemoteFactory.GetRemote(_options, repository, _logger); + IRemote verifyRemote = await _remoteFactory.CreateRemoteAsync(repository); if (!await UxHelpers.VerifyAndConfirmBranchExistsAsync(verifyRemote, repository, branch, !_options.Quiet)) { @@ -147,8 +150,7 @@ public override async Task ExecuteAsync() try { - await _barClient.SetRepositoryMergePoliciesAsync( - repository, branch, mergePolicies); + await _barClient.SetRepositoryMergePoliciesAsync(repository, branch, mergePolicies); Console.WriteLine($"Successfully updated merge policies for {repository}@{branch}."); return Constants.SuccessCode; } diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/UpdateSubscriptionOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/UpdateSubscriptionOperation.cs index 938e7117e1..aa525d7532 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/UpdateSubscriptionOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/UpdateSubscriptionOperation.cs @@ -20,15 +20,21 @@ internal class UpdateSubscriptionOperation : Operation { private readonly UpdateSubscriptionCommandLineOptions _options; private readonly IBarApiClient _barClient; + private readonly IRemoteFactory _remoteFactory; + private readonly IGitRepoFactory _gitRepoFactory; private readonly ILogger _logger; public UpdateSubscriptionOperation( UpdateSubscriptionCommandLineOptions options, IBarApiClient barClient, + IRemoteFactory remoteFactory, + IGitRepoFactory gitRepoFactory, ILogger logger) { _options = options; _barClient = barClient; + _remoteFactory = remoteFactory; + _gitRepoFactory = gitRepoFactory; _logger = logger; } @@ -113,6 +119,8 @@ public override async Task ExecuteAsync() { var updateSubscriptionPopUp = new UpdateSubscriptionPopUp( "update-subscription/update-subscription-todo", + _options.ForceCreation, + _gitRepoFactory, _logger, subscription, (await suggestedChannels).Select(suggestedChannel => suggestedChannel.Name), diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs index 090dc3a442..69fbf04943 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/CommandLineOptions.cs @@ -55,7 +55,8 @@ public abstract class CommandLineOptions : ICommandLineOptions [Option("output-format", Default = DarcOutputType.text, HelpText = "Desired output type of darc. Valid values are 'json' and 'text'. Case sensitive.")] - public DarcOutputType OutputFormat { + public DarcOutputType OutputFormat + { get { return _outputFormat; @@ -142,7 +143,11 @@ public virtual IServiceCollection RegisterServices(IServiceCollection services) services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddTransient(sp => new ProcessManager(sp.GetRequiredService>(), GitLocation)); - services.TryAddSingleton(sp => RemoteFactory.GetBarClient(this)); + services.TryAddSingleton(sp => new BarApiClient( + BuildAssetRegistryToken, + managedIdentityId: null, + disableInteractiveAuth: IsCi, + BuildAssetRegistryBaseUri)); services.TryAddSingleton(sp => sp.GetRequiredService()); services.TryAddTransient(sp => sp.GetRequiredService>()); services.TryAddTransient(); diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/SubscriptionCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/SubscriptionCommandLineOptions.cs index c201a2d7e7..b5e1ff870d 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Options/SubscriptionCommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Options/SubscriptionCommandLineOptions.cs @@ -22,4 +22,7 @@ internal abstract class SubscriptionCommandLineOptions : CommandLineOptions> GetRequiredStrictCoherencyUpdatesAsyn if (!dependenciesCache.TryGetValue(parentCoherentDependencyCacheKey, out IEnumerable coherentParentsDependencies)) { - IRemote remoteClient = await remoteFactory.GetRemoteAsync(parentCoherentDependency.RepoUri, _logger); + IRemote remoteClient = await remoteFactory.CreateRemoteAsync(parentCoherentDependency.RepoUri); coherentParentsDependencies = await remoteClient.GetDependenciesAsync( parentCoherentDependency.RepoUri, parentCoherentDependency.Commit); @@ -389,7 +389,7 @@ private async Task DisambiguateAssetsAsync(IRemoteFactory remoteFactory, // coherent asset itself. if (!nugetConfigCache.TryGetValue(parentCoherentDependencyCacheKey, out IEnumerable nugetFeeds)) { - IRemote remoteClient = await remoteFactory.GetRemoteAsync(parentCoherentDependency.RepoUri, _logger); + IRemote remoteClient = await remoteFactory.CreateRemoteAsync(parentCoherentDependency.RepoUri); nugetFeeds = await remoteClient.GetPackageSourcesAsync(parentCoherentDependency.RepoUri, parentCoherentDependency.Commit); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/HealthMetrics/ProductDependencyCyclesHealthMetric.cs b/src/Microsoft.DotNet.Darc/DarcLib/HealthMetrics/ProductDependencyCyclesHealthMetric.cs index 5b5130ea56..2663b8f4fe 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/HealthMetrics/ProductDependencyCyclesHealthMetric.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/HealthMetrics/ProductDependencyCyclesHealthMetric.cs @@ -56,7 +56,7 @@ public override async Task EvaluateAsync() }; // Evaluate and find out what the latest is on the branch - var remote = await _remoteFactory.GetRemoteAsync(_repository, _logger); + var remote = await _remoteFactory.CreateRemoteAsync(_repository); var commit = await remote.GetLatestCommitAsync(_repository, _branch); if (commit == null) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/HealthMetrics/SubscriptionHealthMetric.cs b/src/Microsoft.DotNet.Darc/DarcLib/HealthMetrics/SubscriptionHealthMetric.cs index 8d8ccab872..dc13472bb7 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/HealthMetrics/SubscriptionHealthMetric.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/HealthMetrics/SubscriptionHealthMetric.cs @@ -91,7 +91,7 @@ public SubscriptionHealthMetric( /// True if the metric passed, false otherwise public override async Task EvaluateAsync() { - IRemote remote = await _remoteFactory.GetRemoteAsync(Repository, _logger); + IRemote remote = await _remoteFactory.CreateRemoteAsync(Repository); _logger.LogInformation("Evaluating subscription health metrics for {repo}@{branch}", Repository, Branch); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/IRemoteFactory.cs b/src/Microsoft.DotNet.Darc/DarcLib/IRemoteFactory.cs index 91760abdba..6a717c8596 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/IRemoteFactory.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/IRemoteFactory.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.DotNet.DarcLib.Helpers; -using Microsoft.Extensions.Logging; using System.Threading.Tasks; namespace Microsoft.DotNet.DarcLib; @@ -13,7 +12,7 @@ namespace Microsoft.DotNet.DarcLib; /// public interface IRemoteFactory { - Task GetRemoteAsync(string repoUrl, ILogger logger); + Task CreateRemoteAsync(string repoUrl); - Task GetDependencyFileManagerAsync(string repoUrl, ILogger logger); + Task CreateDependencyFileManagerAsync(string repoUrl); } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Local.cs b/src/Microsoft.DotNet.Darc/DarcLib/Local.cs index 950151b201..69046f9cd8 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Local.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Local.cs @@ -84,7 +84,7 @@ public async Task UpdateDependenciesAsync(List dependencies, I { try { - IRemote arcadeRemote = await remoteFactory.GetRemoteAsync(arcadeItem.RepoUri, _logger); + IRemote arcadeRemote = await remoteFactory.CreateRemoteAsync(arcadeItem.RepoUri); List engCommonFiles = await arcadeRemote.GetCommonScriptFilesAsync(arcadeItem.RepoUri, arcadeItem.Commit); filesToUpdate.AddRange(engCommonFiles); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/DependencyGraph.cs b/src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/DependencyGraph.cs index d1ed1562b1..b636bb5200 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/DependencyGraph.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Models/Darc/DependencyGraph.cs @@ -311,7 +311,7 @@ private static async Task DoLatestInChannelGraphNodeDiffAsync( // Perform diff if there is a latest commit. if (!string.IsNullOrEmpty(latestCommit)) { - IRemote repoRemote = await remoteFactory.GetRemoteAsync(node.Repository, logger); + IRemote repoRemote = await remoteFactory.CreateRemoteAsync(node.Repository); // This will return a no-diff if latestCommit == node.Commit node.DiffFrom = await repoRemote.GitDiffAsync(node.Repository, latestCommit, node.Commit); } @@ -369,7 +369,7 @@ private static async Task DoLatestInGraphNodeDiffAsync( // Compare all other nodes to the latest foreach (DependencyGraphNode node in nodes) { - IRemote repoRemote = await remoteFactory.GetRemoteAsync(node.Repository, logger); + IRemote repoRemote = await remoteFactory.CreateRemoteAsync(node.Repository); // If node == newestNode, returns no diff. node.DiffFrom = await repoRemote.GitDiffAsync(node.Repository, newestNode.Commit, node.Commit); } @@ -836,10 +836,8 @@ private static async Task> GetDependenciesAsync( } else if (remote) { - IRemote remoteClient = await remoteFactory.GetRemoteAsync(repoUri, logger); - dependencies = await remoteClient.GetDependenciesAsync( - repoUri, - commit); + IRemote remoteClient = await remoteFactory.CreateRemoteAsync(repoUri); + dependencies = await remoteClient.GetDependenciesAsync(repoUri, commit); } else { diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Remote.cs b/src/Microsoft.DotNet.Darc/DarcLib/Remote.cs index c76ab12542..4bae9ad589 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Remote.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Remote.cs @@ -191,7 +191,7 @@ public async Task> CommitUpdatesAsync( if (mayNeedArcadeUpdate) { - IDependencyFileManager arcadeFileManager = await remoteFactory.GetDependencyFileManagerAsync(arcadeItem.RepoUri, _logger); + IDependencyFileManager arcadeFileManager = await remoteFactory.CreateDependencyFileManagerAsync(arcadeItem.RepoUri); targetDotNetVersion = await arcadeFileManager.ReadToolsDotnetVersionAsync(arcadeItem.RepoUri, arcadeItem.Commit); } @@ -209,7 +209,7 @@ public async Task> CommitUpdatesAsync( { // Files in the source arcade repo. We use the remote factory because the // arcade repo may be in github while this remote is targeted at AzDO. - IRemote arcadeRemote = await remoteFactory.GetRemoteAsync(arcadeItem.RepoUri, _logger); + IRemote arcadeRemote = await remoteFactory.CreateRemoteAsync(arcadeItem.RepoUri); List engCommonFiles = await arcadeRemote.GetCommonScriptFilesAsync(arcadeItem.RepoUri, arcadeItem.Commit); filesToCommit.AddRange(engCommonFiles); diff --git a/src/ProductConstructionService/ProductConstructionService.Api/Api/v2020_02_20/Controllers/BuildsController.cs b/src/ProductConstructionService/ProductConstructionService.Api/Api/v2020_02_20/Controllers/BuildsController.cs index dce4730215..5813e50a4c 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/Api/v2020_02_20/Controllers/BuildsController.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/Api/v2020_02_20/Controllers/BuildsController.cs @@ -177,7 +177,7 @@ public async Task GetCommit(int buildId) return NotFound(); } - IRemote remote = await _factory.GetRemoteAsync(build.AzureDevOpsRepository ?? build.GitHubRepository, null); + IRemote remote = await _factory.CreateRemoteAsync(build.AzureDevOpsRepository ?? build.GitHubRepository); Microsoft.DotNet.DarcLib.Commit commit = await remote.GetCommitAsync(build.AzureDevOpsRepository ?? build.GitHubRepository, build.Commit); return Ok(new Maestro.Api.Model.v2020_02_20.Commit(commit.Author, commit.Sha, commit.Message)); } diff --git a/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs b/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs index c26bbfc16b..e89c1c890c 100644 --- a/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs +++ b/src/ProductConstructionService/ProductConstructionService.Api/PcsStartup.cs @@ -196,7 +196,7 @@ internal static async Task ConfigurePcs( builder.Configuration[ConfigurationKeys.GitHubClientId], builder.Configuration[ConfigurationKeys.GitHubClientSecret]); builder.Services.AddGitHubTokenProvider(); - builder.Services.AddScoped(); + builder.Services.AddScoped(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.Configure(_ => { }); diff --git a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs index a6c18beb2c..61eb931b32 100644 --- a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs +++ b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs @@ -117,7 +117,7 @@ public async Task CalculatePRDescriptionAndCommitUpdatesAsync( (SubscriptionUpdateWorkItem update, List deps) coherencyUpdate = requiredUpdates.Where(u => u.update.IsCoherencyUpdate).SingleOrDefault(); - IRemote remote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote remote = await _remoteFactory.CreateRemoteAsync(targetRepository); var locationResolver = new AssetLocationResolver(_barClient); // To keep a PR to as few commits as possible, if the number of diff --git a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestPolicyFailureNotifier.cs b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestPolicyFailureNotifier.cs index b8055bf539..4bd9a71ead 100644 --- a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestPolicyFailureNotifier.cs +++ b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestPolicyFailureNotifier.cs @@ -67,7 +67,7 @@ public async Task TagSourceRepositoryGitHubContactsAsync(InProgressPullRequest p return; } - var darcRemote = await _remoteFactory.GetRemoteAsync($"https://github.com/{owner}/{repo}", _logger); + var darcRemote = await _remoteFactory.CreateRemoteAsync($"https://github.com/{owner}/{repo}"); var darcSubscriptionObject = await _barClient.GetSubscriptionAsync(subscriptionFromPr.SubscriptionId); var sourceRepository = darcSubscriptionObject.SourceRepository; var targetRepository = darcSubscriptionObject.TargetRepository; diff --git a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs index fb5ec38225..b518ec177c 100644 --- a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs +++ b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs @@ -191,7 +191,7 @@ protected virtual Task TagSourceRepositoryGitHubContactsIfPossibleAsync(InProgre _logger.LogInformation("Querying status for pull request {prUrl}", pr.Url); (var targetRepository, _) = await GetTargetAsync(); - IRemote remote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote remote = await _remoteFactory.CreateRemoteAsync(targetRepository); PrStatus status = await remote.GetPullRequestStatusAsync(pr.Url); _logger.LogInformation("Pull request {url} is {status}", pr.Url, status); @@ -492,7 +492,7 @@ public async Task UpdateAssetsAsync( (var targetRepository, var targetBranch) = await GetTargetAsync(); bool isCodeFlow = update.SubscriptionType == SubscriptionType.DependenciesAndSources; - IRemote darcRemote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote darcRemote = await _remoteFactory.CreateRemoteAsync(targetRepository); TargetRepoDependencyUpdate repoDependencyUpdate = await GetRequiredUpdates(update, _remoteFactory, targetRepository, prBranch: null, targetBranch); @@ -598,7 +598,7 @@ private async Task UpdatePullRequestAsync(InProgressPullRequest pr, Subscription _logger.LogInformation("Updating pull request {url} branch {targetBranch} in {targetRepository}", pr.Url, targetBranch, targetRepository); - IRemote darcRemote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote darcRemote = await _remoteFactory.CreateRemoteAsync(targetRepository); PullRequest pullRequest = await darcRemote.GetPullRequestAsync(pr.Url); TargetRepoDependencyUpdate targetRepositoryUpdates = @@ -747,7 +747,7 @@ private async Task GetRequiredUpdates( { _logger.LogInformation("Getting Required Updates for {branch} of {targetRepository}", targetBranch, targetRepository); // Get a remote factory for the target repo - IRemote darc = await remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote darc = await remoteFactory.CreateRemoteAsync(targetRepository); TargetRepoDependencyUpdate repoDependencyUpdate = new(); @@ -1054,7 +1054,7 @@ private async Task UpdateAssetsAndSources(SubscriptionUpdateWorkItem updat var title = await _pullRequestBuilder.GenerateCodeFlowPRTitleAsync(update, subscription.TargetBranch); var description = await _pullRequestBuilder.GenerateCodeFlowPRDescriptionAsync(update); - var remote = await _remoteFactory.GetRemoteAsync(subscription.TargetRepository, _logger); + var remote = await _remoteFactory.CreateRemoteAsync(subscription.TargetRepository); await remote.UpdatePullRequestAsync(pullRequest.Url, new PullRequest { Title = title, @@ -1160,7 +1160,7 @@ private async Task CreateCodeFlowPullRequestAsync( string targetBranch) { bool isCodeFlow = update.SubscriptionType == SubscriptionType.DependenciesAndSources; - IRemote darcRemote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); + IRemote darcRemote = await _remoteFactory.CreateRemoteAsync(targetRepository); try { diff --git a/test/Maestro.Web.Tests/BuildController20200914Tests.cs b/test/Maestro.Web.Tests/BuildController20200914Tests.cs index 9321d4b998..816df1aa52 100644 --- a/test/Maestro.Web.Tests/BuildController20200914Tests.cs +++ b/test/Maestro.Web.Tests/BuildController20200914Tests.cs @@ -91,7 +91,7 @@ public static async Task Dependencies(IServiceCollection collection) var mockIRemoteFactory = new Mock(); var mockIRemote = new Mock(); - mockIRemoteFactory.Setup(f => f.GetRemoteAsync(Repository, It.IsAny())).ReturnsAsync(mockIRemote.Object); + mockIRemoteFactory.Setup(f => f.CreateRemoteAsync(Repository)).ReturnsAsync(mockIRemote.Object); mockIRemote.Setup(f => f.GetCommitAsync(Repository, CommitHash)).ReturnsAsync(new Microsoft.DotNet.DarcLib.Commit(Account, CommitHash, CommitMessage)); collection.AddSingleton(mockIRemote.Object); diff --git a/test/Microsoft.DotNet.Darc.Tests/DependencyCoherencyTests.cs b/test/Microsoft.DotNet.Darc.Tests/DependencyCoherencyTests.cs index 5e865d4271..0750bff318 100644 --- a/test/Microsoft.DotNet.Darc.Tests/DependencyCoherencyTests.cs +++ b/test/Microsoft.DotNet.Darc.Tests/DependencyCoherencyTests.cs @@ -187,7 +187,7 @@ public async Task CoherencyUpdateTests6() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -272,7 +272,7 @@ public async Task CoherencyUpdateTests7() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -334,7 +334,7 @@ public async Task CoherencyUpdateTests8() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -393,7 +393,7 @@ public async Task CoherencyUpdateTests9(bool pinHead) // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: pinHead); @@ -464,7 +464,7 @@ public async Task CoherencyUpdateTests10(bool pinHead) // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(dependencyGraphRemoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: pinHead); @@ -555,7 +555,7 @@ public async Task StrictCoherencyUpdateTests1() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -602,7 +602,7 @@ public async Task StrictCoherencyUpdateTests2() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -637,7 +637,7 @@ public async Task StrictCoherencyUpdateTests3() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -673,7 +673,7 @@ public async Task StrictCoherencyUpdateTests4() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -713,7 +713,7 @@ public async Task StrictCoherencyUpdateTests5() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -768,7 +768,7 @@ public async Task StrictCoherencyUpdateTests6() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -831,7 +831,7 @@ public async Task StrictCoherencyUpdateTests7() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -877,7 +877,7 @@ public async Task StrictCoherencyUpdateTests8() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -927,7 +927,7 @@ public async Task StrictCoherencyUpdateTests9() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -985,7 +985,7 @@ public async Task StrictCoherencyUpdateTests10() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -1052,7 +1052,7 @@ public async Task StrictCoherencyUpdateTests11() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -1118,7 +1118,7 @@ public async Task StrictCoherencyUpdateTests12() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -1190,7 +1190,7 @@ public async Task StrictCoherencyUpdateTests13() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -1263,7 +1263,7 @@ public async Task StrictCoherencyUpdateTests14() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -1334,7 +1334,7 @@ public async Task StrictCoherencyUpdateTests15() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); @@ -1377,7 +1377,7 @@ public async Task StrictCoherencyUpdateTests16() // Always return the main remote. var remoteFactoryMock = new Mock(); - remoteFactoryMock.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(remoteMock.Object); + remoteFactoryMock.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(remoteMock.Object); List existingDetails = []; DependencyDetail depA = AddDependency(existingDetails, "depA", "v1", "repoA", "commit1", pinned: false); diff --git a/test/ProductConstructionService.Api.Tests/BuildController20200914Tests.cs b/test/ProductConstructionService.Api.Tests/BuildController20200914Tests.cs index 5cc14651c2..cf463959ad 100644 --- a/test/ProductConstructionService.Api.Tests/BuildController20200914Tests.cs +++ b/test/ProductConstructionService.Api.Tests/BuildController20200914Tests.cs @@ -94,7 +94,7 @@ public static async Task Dependencies(IServiceCollection collection) var mockWorkItemProducerFactory = new Mock(); var mockWorkItemProducer = new Mock>(); mockWorkItemProducerFactory.Setup(f => f.CreateProducer(false)).Returns(mockWorkItemProducer.Object); - mockIRemoteFactory.Setup(f => f.GetRemoteAsync(Repository, It.IsAny())).ReturnsAsync(mockIRemote.Object); + mockIRemoteFactory.Setup(f => f.CreateRemoteAsync(Repository)).ReturnsAsync(mockIRemote.Object); mockIRemote.Setup(f => f.GetCommitAsync(Repository, CommitHash)).ReturnsAsync(new Microsoft.DotNet.DarcLib.Commit(Account, CommitHash, CommitMessage)); collection.AddSingleton(mockIRemote.Object); diff --git a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestBuilderTests.cs b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestBuilderTests.cs index 57c915eaec..a106d2845c 100644 --- a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestBuilderTests.cs +++ b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestBuilderTests.cs @@ -36,10 +36,9 @@ public void PullRequestBuilderTests_SetUp() protected override void RegisterServices(IServiceCollection services) { - _remoteFactory.Setup(f => f.GetRemoteAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync( - (string repo, ILogger logger) => - _darcRemotes.GetOrAddValue(repo, () => CreateMock()).Object); + _remoteFactory + .Setup(f => f.CreateRemoteAsync(It.IsAny())) + .ReturnsAsync((string repo) => _darcRemotes.GetOrAddValue(repo, () => CreateMock()).Object); services.AddSingleton(_remoteFactory.Object); services.AddSingleton(_barClient.Object); diff --git a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs index 57aea2649a..5a96a57694 100644 --- a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs +++ b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestPolicyFailureNotifierTests.cs @@ -94,7 +94,7 @@ where subscription.Id.Equals(subscriptionToFind) MockRemote = new Remote(GitRepo.Object, new VersionDetailsParser(), NullLogger.Instance); RemoteFactory = new Mock(MockBehavior.Strict); - RemoteFactory.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(MockRemote); + RemoteFactory.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(MockRemote); Provider = services.BuildServiceProvider(); Scope = Provider.CreateScope(); diff --git a/test/ProductConstructionService.DependencyFlow.Tests/UpdaterTests.cs b/test/ProductConstructionService.DependencyFlow.Tests/UpdaterTests.cs index 06b1816217..26731f3a0d 100644 --- a/test/ProductConstructionService.DependencyFlow.Tests/UpdaterTests.cs +++ b/test/ProductConstructionService.DependencyFlow.Tests/UpdaterTests.cs @@ -8,7 +8,6 @@ using Microsoft.DotNet.Kusto; using Microsoft.DotNet.Services.Utility; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.Services.Common; using Moq; using NUnit.Framework; @@ -64,9 +63,8 @@ protected override void RegisterServices(IServiceCollection services) services.AddSingleton(workItemProducerFactoryMock.Object); RemoteFactory - .Setup(f => f.GetRemoteAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync((string repo, ILogger logger) => - DarcRemotes.GetOrAddValue(repo, () => CreateMock()).Object); + .Setup(f => f.CreateRemoteAsync(It.IsAny())) + .ReturnsAsync((string repo) => DarcRemotes.GetOrAddValue(repo, () => CreateMock()).Object); } [SetUp] diff --git a/test/SubscriptionActorService.Tests/PullRequestActorTests.cs b/test/SubscriptionActorService.Tests/PullRequestActorTests.cs index 824c33086c..e07069d4f2 100644 --- a/test/SubscriptionActorService.Tests/PullRequestActorTests.cs +++ b/test/SubscriptionActorService.Tests/PullRequestActorTests.cs @@ -84,10 +84,9 @@ protected override void RegisterServices(IServiceCollection services) services.AddSingleton(_updateResolver.Object); services.AddSingleton(Mock.Of()); - _remoteFactory.Setup(f => f.GetRemoteAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync( - (string repo, ILogger logger) => - _darcRemotes.GetOrAddValue(repo, () => CreateMock()).Object); + _remoteFactory + .Setup(f => f.CreateRemoteAsync(It.IsAny())) + .ReturnsAsync((string repo) => _darcRemotes.GetOrAddValue(repo, () => CreateMock()).Object); services.AddSingleton(_remoteFactory.Object); base.RegisterServices(services); diff --git a/test/SubscriptionActorService.Tests/PullRequestBuilderTests.cs b/test/SubscriptionActorService.Tests/PullRequestBuilderTests.cs index d4c02142de..2100f6e984 100644 --- a/test/SubscriptionActorService.Tests/PullRequestBuilderTests.cs +++ b/test/SubscriptionActorService.Tests/PullRequestBuilderTests.cs @@ -12,7 +12,6 @@ using Microsoft.DotNet.DarcLib.Models.Darc; using Microsoft.DotNet.Maestro.Client.Models; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.Services.Common; using Moq; using NUnit.Framework; @@ -40,10 +39,9 @@ public void PullRequestBuilderTests_SetUp() protected override void RegisterServices(IServiceCollection services) { - _remoteFactory.Setup(f => f.GetRemoteAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync( - (string repo, ILogger logger) => - _darcRemotes.GetOrAddValue(repo, () => CreateMock()).Object); + _remoteFactory + .Setup(f => f.CreateRemoteAsync(It.IsAny())) + .ReturnsAsync((string repo) => _darcRemotes.GetOrAddValue(repo, () => CreateMock()).Object); services.AddSingleton(_remoteFactory.Object); services.AddSingleton(_barClient.Object); diff --git a/test/SubscriptionActorService.Tests/PullRequestPolicyFailureNotifierTests.cs b/test/SubscriptionActorService.Tests/PullRequestPolicyFailureNotifierTests.cs index 59077eef40..de1716aa4f 100644 --- a/test/SubscriptionActorService.Tests/PullRequestPolicyFailureNotifierTests.cs +++ b/test/SubscriptionActorService.Tests/PullRequestPolicyFailureNotifierTests.cs @@ -99,7 +99,7 @@ where subscription.Id.Equals(subscriptionToFind) MockRemote = new Remote(GitRepo.Object, new VersionDetailsParser(), NullLogger.Instance); RemoteFactory = new Mock(MockBehavior.Strict); - RemoteFactory.Setup(m => m.GetRemoteAsync(It.IsAny(), It.IsAny())).ReturnsAsync(MockRemote); + RemoteFactory.Setup(m => m.CreateRemoteAsync(It.IsAny())).ReturnsAsync(MockRemote); Provider = services.BuildServiceProvider(); Scope = Provider.CreateScope(); From eb59b4096f3bf49c43960656de994e586d91f320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Tue, 3 Dec 2024 12:14:47 +0100 Subject: [PATCH 11/11] Clean up clones before usage (#4204) --- .../DarcLib/VirtualMonoRepo/CloneManager.cs | 22 +++++++++++++++---- .../VirtualMonoRepo/RepositoryCloneManager.cs | 6 +++-- .../VirtualMonoRepo/VmrCloneManager.cs | 2 -- .../RepositoryCloneManagerTests.cs | 17 ++++++++++++++ 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs index 4e5e387733..7a7a5ff82b 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs @@ -71,11 +71,13 @@ protected async Task PrepareCloneInternalAsync( string.Join(", ", remoteUris)); NativePath path = null!; + bool cleanup = true; foreach (string remoteUri in remoteUris) { // Path should be returned the same for all invocations // We checkout a default ref - path = await PrepareCloneInternal(remoteUri, dirName, cancellationToken); + path = await PrepareCloneInternal(remoteUri, dirName, cleanup, cancellationToken); + cleanup = false; // Verify that all requested commits are available foreach (string gitRef in refsToVerify.ToArray()) @@ -110,7 +112,7 @@ protected async Task PrepareCloneInternalAsync( /// When clone is already present, it is re-used and we only fetch. /// When given remotes have already been fetched during this run, they are not fetched again. /// - protected async Task PrepareCloneInternal(string remoteUri, string dirName, CancellationToken cancellationToken) + protected async Task PrepareCloneInternal(string remoteUri, string dirName, bool performCleanup, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); @@ -139,7 +141,19 @@ protected async Task PrepareCloneInternal(string remoteUri, string d } else { - _logger.LogDebug("Clone of {repo} found in {clonePath}", remoteUri, clonePath); + _logger.LogDebug("Clone of {repo} found in {clonePath}. Preparing for use...", remoteUri, clonePath); + + // We make sure the clone is clean and we re-clone if it's unusable + if (performCleanup) + { + var result = await _localGitRepo.RunGitCommandAsync(clonePath, ["reset", "--hard"], cancellationToken); + if (!result.Succeeded) + { + _logger.LogWarning("Failed to clean up {clonePath}, re-cloning", clonePath); + _fileSystem.DeleteDirectory(clonePath, recursive: true); + return await PrepareCloneInternal(remoteUri, dirName, performCleanup: true, cancellationToken); + } + } string remote; @@ -151,7 +165,7 @@ protected async Task PrepareCloneInternal(string remoteUri, string d { _logger.LogWarning("Clone at {clonePath} is not a git repository, re-cloning", clonePath); _fileSystem.DeleteDirectory(clonePath, recursive: true); - return await PrepareCloneInternal(remoteUri, dirName, cancellationToken); + return await PrepareCloneInternal(remoteUri, dirName, performCleanup: true, cancellationToken); } // We cannot do `fetch --all` as tokens might be needed but fetch +refs/heads/*:+refs/remotes/origin/* doesn't fetch new refs diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/RepositoryCloneManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/RepositoryCloneManager.cs index fe6069a800..438012ec59 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/RepositoryCloneManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/RepositoryCloneManager.cs @@ -96,11 +96,13 @@ public async Task PrepareCloneAsync( } NativePath path = null!; + bool cleanup = true; foreach (string remoteUri in remoteUris) { // Path should be returned the same for all invocations // We checkout a default ref - path = await PrepareCloneInternal(remoteUri, mapping.Name, cancellationToken); + path = await PrepareCloneInternal(remoteUri, mapping.Name, cleanup, cancellationToken); + cleanup = false; } var repo = _localGitRepoFactory.Create(path); @@ -115,7 +117,7 @@ public async Task PrepareCloneAsync( { // We store clones in directories named as a hash of the repo URI var cloneDir = StringUtils.GetXxHash64(repoUri); - var path = await PrepareCloneInternal(repoUri, cloneDir, cancellationToken); + var path = await PrepareCloneInternal(repoUri, cloneDir, performCleanup: true, cancellationToken); var repo = _localGitRepoFactory.Create(path); await repo.CheckoutAsync(checkoutRef); return repo; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs index d68bba8692..4db8eede7b 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs @@ -58,8 +58,6 @@ public async Task PrepareVmrAsync( string checkoutRef, CancellationToken cancellationToken) { - // TODO https://github.com/dotnet/arcade-services/issues/4197: Make sure VMR is ready for use (working tree is clean..) - // This makes sure we keep different VMRs separate // We expect to have up to 3: // 1. The GitHub VMR (dotnet/dotnet) diff --git a/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs b/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs index ca8bab0542..5534b6ee97 100644 --- a/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs +++ b/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs @@ -49,6 +49,10 @@ public void SetUp() _localGitRepo.Reset(); _localGitRepoFactory.Reset(); + _localGitRepo.SetReturnsDefault(Task.FromResult(new ProcessExecutionResult() + { + ExitCode = 0, + })); _localGitRepoFactory .Setup(x => x.Create(It.IsAny())) .Returns((NativePath path) => new LocalGitRepo(path, _localGitRepo.Object, Mock.Of())); @@ -91,6 +95,7 @@ public async Task RepoIsClonedOnceTest() _repoCloner.Verify(x => x.CloneNoCheckoutAsync(RepoUri, _clonePath, null), Times.Once); _localGitRepo.Verify(x => x.CheckoutAsync(_clonePath, Ref), Times.Once); _localGitRepo.Verify(x => x.CheckoutAsync(_clonePath, "main"), Times.Exactly(2)); + _localGitRepo.Verify(x => x.RunGitCommandAsync(_clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Never); } [Test] @@ -108,6 +113,7 @@ public async Task CloneIsReusedTest() _repoCloner.Verify(x => x.CloneNoCheckoutAsync(RepoUri, _clonePath, null), Times.Never); _localGitRepo.Verify(x => x.CheckoutAsync(_clonePath, Ref), Times.Once); _localGitRepo.Verify(x => x.CheckoutAsync(_clonePath, "main"), Times.Once); + _localGitRepo.Verify(x => x.RunGitCommandAsync(_clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Once); } [Test] @@ -156,6 +162,7 @@ void ResetCalls() _repoCloner.Verify(x => x.CloneNoCheckoutAsync(mapping.DefaultRemote, clonePath, null), Times.Once); _localGitRepo.Verify(x => x.CheckoutAsync(clonePath, "main"), Times.Once); + _localGitRepo.Verify(x => x.RunGitCommandAsync(clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Never); // A second clone of the same ResetCalls(); @@ -165,6 +172,7 @@ void ResetCalls() _repoCloner.Verify(x => x.CloneNoCheckoutAsync(mapping.DefaultRemote, clonePath, null), Times.Never); _localGitRepo.Verify(x => x.UpdateRemoteAsync(clonePath, "default", default), Times.Never); _localGitRepo.Verify(x => x.CheckoutAsync(clonePath, Ref), Times.Once); + _localGitRepo.Verify(x => x.RunGitCommandAsync(clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Never); // A third clone with a new remote ResetCalls(); @@ -175,6 +183,7 @@ void ResetCalls() _localGitRepo.Verify(x => x.AddRemoteIfMissingAsync(clonePath, newRemote, It.IsAny()), Times.Once); _localGitRepo.Verify(x => x.CheckoutAsync(clonePath, Ref), Times.Once); _localGitRepo.Verify(x => x.UpdateRemoteAsync(clonePath, "new", default), Times.Once); + _localGitRepo.Verify(x => x.RunGitCommandAsync(clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Never); // Same again, should be cached ResetCalls(); @@ -185,6 +194,7 @@ void ResetCalls() _localGitRepo.Verify(x => x.AddRemoteIfMissingAsync(clonePath, newRemote, It.IsAny()), Times.Never); _localGitRepo.Verify(x => x.CheckoutAsync(clonePath, Ref + "3"), Times.Once); _localGitRepo.Verify(x => x.UpdateRemoteAsync(clonePath, "new", default), Times.Never); + _localGitRepo.Verify(x => x.RunGitCommandAsync(clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Never); // Call with URI directly ResetCalls(); @@ -195,6 +205,7 @@ void ResetCalls() _localGitRepo.Verify(x => x.AddRemoteIfMissingAsync(clonePath, RepoUri, It.IsAny()), Times.Never); _localGitRepo.Verify(x => x.CheckoutAsync(clonePath, Ref + "4"), Times.Once); _localGitRepo.Verify(x => x.UpdateRemoteAsync(clonePath, "new", default), Times.Never); + _localGitRepo.Verify(x => x.RunGitCommandAsync(clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Never); // Call with the second URI directly ResetCalls(); @@ -205,6 +216,7 @@ void ResetCalls() _localGitRepo.Verify(x => x.AddRemoteIfMissingAsync(clonePath, newRemote, It.IsAny()), Times.Never); _localGitRepo.Verify(x => x.CheckoutAsync(clonePath, Ref + "5"), Times.Once); _localGitRepo.Verify(x => x.UpdateRemoteAsync(clonePath, "new", default), Times.Never); + _localGitRepo.Verify(x => x.RunGitCommandAsync(clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Never); } [Test] @@ -249,6 +261,8 @@ public async Task CommitsAreFetchedGradually() .Verify(x => x.AddRemoteIfMissingAsync(clonePath, configuration["github"].RemoteUri, It.IsAny()), Times.Once); _localGitRepo .Verify(x => x.AddRemoteIfMissingAsync(clonePath, configuration["local"].RemoteUri, It.IsAny()), Times.Never); + _localGitRepo + .Verify(x => x.RunGitCommandAsync(clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.Never); } [Test] @@ -284,6 +298,9 @@ public async Task CommitIsNotFound() { _localGitRepo .Verify(x => x.AddRemoteIfMissingAsync(clonePath, pair.Value.RemoteUri, It.IsAny()), Times.Once); + + _localGitRepo + .Verify(x => x.RunGitCommandAsync(clonePath, new[] { "reset", "--hard" }, It.IsAny()), Times.AtLeastOnce); } foreach (var sha in searchedRefs)