diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/BackFlowConflictResolver.cs b/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/BackFlowConflictResolver.cs deleted file mode 100644 index 9472afa281..0000000000 --- a/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/BackFlowConflictResolver.cs +++ /dev/null @@ -1,99 +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.Threading.Tasks; -using Microsoft.DotNet.DarcLib.Helpers; -using Microsoft.Extensions.Logging; - -#nullable enable -namespace Microsoft.DotNet.DarcLib.Conflicts; - -public interface IBackFlowConflictResolver -{ - Task TryMergingRepoBranch( - ILocalGitRepo repo, - string baseBranch, - string targetBranch); -} - -/// -/// This class is responsible for resolving well-known conflicts that can occur during a backflow operation. -/// The conflicts can happen when backward and forward flow PRs get merged out of order. -/// This can be shown on the following schema (the order of events is numbered): -/// -/// repo VMR -/// O────────────────────►O -/// │ 2. │ -/// 1.O────────────────O │ -/// │ 4. │ │ -/// │ O───────────┼────O 3. -/// │ │ │ │ -/// │ │ │ │ -/// 6.O◄───┘ └───►O 5. -/// │ 7. │ -/// │ O───────────────| -/// 8.O◄────┘ │ -/// │ │ -/// -/// The conflict arises in step 8. and is caused by the fact that: -/// - When the backflow PR branch is being opened in 7., the last sync (from the point of view of 5.) is from 1. -/// - This means that the PR branch will be based on 1. (the real PR branch will be a commit on top of 1.) -/// - This means that when 6. merged, Version.Details.xml got updated with the SHA of the 3. -/// - So the Source tag in Version.Details.xml in 6. contains the SHA of 3. -/// - The backflow PR branch contains the SHA of 5. -/// - So the Version.Details.xml file conflicts on the SHA (3. vs 5.) -/// - There's also a similar conflict in the package versions that got updated in those commits. -/// - However, if only the version files are in conflict, we can try merging 6. into 7. and resolve the conflict. -/// - This is because basically we know we want to set the version files to point at 5. -/// -public class BackFlowConflictResolver : CodeFlowConflictResolver, IBackFlowConflictResolver -{ - private readonly ILogger _logger; - - public BackFlowConflictResolver(ILogger logger) - : base(logger) - { - _logger = logger; - } - - public async Task TryMergingRepoBranch( - ILocalGitRepo repo, - string targetBranch, - string branchToMerge) - { - return await TryMergingBranch(repo, targetBranch, branchToMerge); - } - - protected override async Task TryResolvingConflicts(ILocalGitRepo repo, IEnumerable conflictedFiles) - { - foreach (var filePath in conflictedFiles) - { - // Known conflict in eng/Version.Details.xml - if (string.Equals(filePath, VersionFiles.VersionDetailsXml, StringComparison.InvariantCultureIgnoreCase)) - { - await Task.CompletedTask; - return false; - - // TODO https://github.com/dotnet/arcade-services/issues/4196: Resolve conflicts in eng/Version.Details.xml - // return true; - } - - // Known conflict in eng/Versions.props - if (string.Equals(filePath, VersionFiles.VersionProps, StringComparison.InvariantCultureIgnoreCase)) - { - await Task.CompletedTask; - return false; - - // TODO https://github.com/dotnet/arcade-services/issues/4196: Resolve conflicts in eng/Version.Details.xml - // return true; - } - - _logger.LogInformation("Unable to resolve conflicts in {file}", filePath); - return false; - } - - return true; - } -} diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/CodeFlowConflictResolver.cs b/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/CodeFlowConflictResolver.cs deleted file mode 100644 index a71854320f..0000000000 --- a/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/CodeFlowConflictResolver.cs +++ /dev/null @@ -1,76 +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.Linq; -using System.Threading.Tasks; -using Microsoft.DotNet.DarcLib.Helpers; -using Microsoft.Extensions.Logging; - -#nullable enable -namespace Microsoft.DotNet.DarcLib.Conflicts; - -/// -/// This class is responsible for resolving well-known conflicts that can occur during codeflow operations. -/// The conflicts usually happen when backward a forward flow PRs get merged out of order. -/// -public abstract class CodeFlowConflictResolver -{ - private readonly ILogger _logger; - - public CodeFlowConflictResolver(ILogger logger) - { - _logger = logger; - } - - protected async Task TryMergingBranch( - ILocalGitRepo repo, - string targetBranch, - string branchToMerge) - { - _logger.LogInformation("Trying to merge target branch {targetBranch} into {baseBranch}", branchToMerge, targetBranch); - - await repo.CheckoutAsync(targetBranch); - var result = await repo.RunGitCommandAsync(["merge", "--no-commit", "--no-ff", branchToMerge]); - if (result.Succeeded) - { - _logger.LogInformation("Successfully merged the branch {targetBranch} into {headBranch} in {repoPath}", - branchToMerge, - targetBranch, - repo.Path); - await repo.CommitAsync($"Merging {branchToMerge} into {targetBranch}", allowEmpty: true); - return true; - } - - result = await repo.RunGitCommandAsync(["diff", "--name-only", "--diff-filter=U", "--relative"]); - if (!result.Succeeded) - { - _logger.LogInformation("Failed to merge the branch {targetBranch} into {headBranch} in {repoPath}", - branchToMerge, - targetBranch, - repo.Path); - result = await repo.RunGitCommandAsync(["merge", "--abort"]); - return false; - } - - var conflictedFiles = result.StandardOutput - .Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries) - .Select(line => new UnixPath(line.Trim())); - - if (!await TryResolvingConflicts(repo, conflictedFiles)) - { - result = await repo.RunGitCommandAsync(["merge", "--abort"]); - return false; - } - - _logger.LogInformation("Successfully resolved version file conflicts between branches {targetBranch} and {headBranch} in {repoPath}", - branchToMerge, - targetBranch, - repo.Path); - await repo.CommitAsync($"Merge branch {branchToMerge} into {targetBranch}", allowEmpty: false); - return true; - } - - protected abstract Task TryResolvingConflicts(ILocalGitRepo repo, IEnumerable conflictedFiles); -} diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/ForwardFlowConflictResolver.cs b/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/ForwardFlowConflictResolver.cs deleted file mode 100644 index fa2a14194d..0000000000 --- a/src/Microsoft.DotNet.Darc/DarcLib/Conflicts/ForwardFlowConflictResolver.cs +++ /dev/null @@ -1,145 +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.Linq; -using System.Threading.Tasks; -using Microsoft.DotNet.DarcLib.Helpers; -using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; -using Microsoft.DotNet.DarcLib.VirtualMonoRepo; -using Microsoft.Extensions.Logging; - -#nullable enable -namespace Microsoft.DotNet.DarcLib.Conflicts; - -public interface IForwardFlowConflictResolver -{ - Task TryMergingBranch( - ILocalGitRepo vmr, - string mappingName, - string baseBranch, - string targetBranch); -} - -/// -/// This class is responsible for resolving well-known conflicts that can occur during a forward flow operation. -/// The conflicts can happen when backward a forward flow PRs get merged out of order. -/// This can be shown on the following schema (the order of events is numbered): -/// -/// repo VMR -/// O────────────────────►O -/// │ 2. │ 1. -/// │ O◄────────────────O- - ┐ -/// │ │ 4. │ -/// 3.O───┼────────────►O │ │ -/// │ │ │ │ -/// │ ┌─┘ │ │ │ -/// │ │ │ │ -/// 5.O◄┘ └──►O 6. │ -/// │ 7. │ O (actual branch for 7. is based on top of 1.) -/// |────────────────►O │ -/// │ └──►O 8. -/// │ │ -/// -/// The conflict arises in step 8. and is caused by the fact that: -/// - When the forward flow PR branch is being opened in 7., the last sync (from the point of view of 5.) is from 1. -/// - This means that the PR branch will be based on 1. (the real PR branch is the "actual 7.") -/// - This means that when 6. merged, VMR's source-manifest.json got updated with the SHA of the 3. -/// - So the source-manifest in 6. contains the SHA of 3. -/// - The forward flow PR branch contains the SHA of 5. -/// - So the source-manifest file conflicts on the SHA (3. vs 5.) -/// - There's also a similar conflict in the git-info files. -/// - However, if only the version files are in conflict, we can try merging 6. into 7. and resolve the conflict. -/// - This is because basically we know we want to set the version files to point at 5. -/// -public class ForwardFlowConflictResolver : CodeFlowConflictResolver, IForwardFlowConflictResolver -{ - private readonly IVmrInfo _vmrInfo; - private readonly ISourceManifest _sourceManifest; - private readonly IFileSystem _fileSystem; - private readonly ILogger _logger; - - public ForwardFlowConflictResolver( - IVmrInfo vmrInfo, - ISourceManifest sourceManifest, - IFileSystem fileSystem, - ILogger logger) - : base(logger) - { - _vmrInfo = vmrInfo; - _sourceManifest = sourceManifest; - _fileSystem = fileSystem; - _logger = logger; - } - - private string? _mappingName; - - public async Task TryMergingBranch( - ILocalGitRepo vmr, - string mappingName, - string targetBranch, - string branchToMerge) - { - _mappingName = mappingName; - return await TryMergingBranch(vmr, targetBranch, branchToMerge); - } - - protected override async Task TryResolvingConflicts(ILocalGitRepo repo, IEnumerable conflictedFiles) - { - var gitInfoFile = $"{VmrInfo.GitInfoSourcesDir}/{_mappingName}.props"; - foreach (var filePath in conflictedFiles) - { - // Known conflict in source-manifest.json - if (string.Equals(filePath, VmrInfo.DefaultRelativeSourceManifestPath, StringComparison.OrdinalIgnoreCase)) - { - await TryResolvingSourceManifestConflict(repo, _mappingName!); - continue; - } - - // Known conflict in a git-info props file - we just use our version as we expect it to be newer - // TODO https://github.com/dotnet/arcade-services/issues/3378: For batched subscriptions, we need to handle all git-info files - if (string.Equals(filePath, gitInfoFile, StringComparison.OrdinalIgnoreCase)) - { - _logger.LogInformation("Auto-resolving conflict in {file}", gitInfoFile); - await repo.RunGitCommandAsync(["checkout", "--ours", filePath]); - await repo.StageAsync([filePath]); - continue; - } - - _logger.LogInformation("Unable to resolve conflicts in {file}", filePath); - return false; - } - - return true; - } - - // TODO https://github.com/dotnet/arcade-services/issues/3378: This won't work for batched subscriptions - private async Task TryResolvingSourceManifestConflict(ILocalGitRepo vmr, string mappingName) - { - _logger.LogInformation("Auto-resolving conflict in {file}", VmrInfo.DefaultRelativeSourceManifestPath); - - // We load the source manifest from the target branch and replace the current mapping (and its submodules) with our branches' information - var result = await vmr.RunGitCommandAsync(["show", "MERGE_HEAD:" + VmrInfo.DefaultRelativeSourceManifestPath]); - - var theirSourceManifest = SourceManifest.FromJson(result.StandardOutput); - var ourSourceManifest = _sourceManifest; - var updatedMapping = ourSourceManifest.Repositories.First(r => r.Path == mappingName); - - theirSourceManifest.UpdateVersion(mappingName, updatedMapping.RemoteUri, updatedMapping.CommitSha, updatedMapping.PackageVersion, updatedMapping.BarId); - - foreach (var submodule in theirSourceManifest.Submodules.Where(s => s.Path.StartsWith(mappingName + "/"))) - { - theirSourceManifest.RemoveSubmodule(submodule); - } - - foreach (var submodule in _sourceManifest.Submodules.Where(s => s.Path.StartsWith(mappingName + "/"))) - { - theirSourceManifest.UpdateSubmodule(submodule); - } - - _fileSystem.WriteToFile(_vmrInfo.SourceManifestPath, theirSourceManifest.ToJson()); - _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); - await vmr.StageAsync([_vmrInfo.SourceManifestPath]); - } -} diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Helpers/IDependencyFileManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/Helpers/IDependencyFileManager.cs index cdf97e404b..704ebeeb83 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Helpers/IDependencyFileManager.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Helpers/IDependencyFileManager.cs @@ -54,7 +54,7 @@ Task UpdateDependencyFiles( IEnumerable itemsToUpdate, SourceDependency? sourceDependency, string repoUri, - string branch, + string? branch, IEnumerable oldDependencies, SemanticVersion? incomingDotNetSdkVersion); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Local.cs b/src/Microsoft.DotNet.Darc/DarcLib/Local.cs index 3fea72ee0e..df660238d3 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Local.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Local.cs @@ -57,7 +57,7 @@ public async Task AddDependencyAsync(DependencyDetail dependency) /// Updates existing dependencies in the dependency files /// /// Dependencies that need updates. - public async Task UpdateDependenciesAsync(List dependencies, IRemoteFactory remoteFactory, IGitRepoFactory gitRepoFactory, IBarApiClient barClient) + public async Task UpdateDependenciesAsync(List dependencies, IRemoteFactory remoteFactory, IGitRepoFactory gitRepoFactory, IBasicBarClient barClient) { // Read the current dependency files and grab their locations so that nuget.config can be updated appropriately. // Update the incoming dependencies with locations. diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs index 291ec5eec2..5e8cf88853 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs @@ -42,7 +42,7 @@ public LocalGitClient( _logger = logger; } - public async Task GetFileContentsAsync(string relativeFilePath, string repoPath, string branch) + public async Task GetFileContentsAsync(string relativeFilePath, string repoPath, string? branch) { // Load non-working-tree version if (!string.IsNullOrEmpty(branch)) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs index 165d730ddf..134f8879c3 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs @@ -5,7 +5,6 @@ using System.Threading; using System.Threading.Tasks; using LibGit2Sharp; -using Microsoft.DotNet.DarcLib.Conflicts; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; using Microsoft.DotNet.ProductConstructionService.Client.Models; @@ -61,10 +60,9 @@ public PcsVmrBackFlower( ILocalLibGit2Client libGit2Client, ICoherencyUpdateResolver coherencyUpdateResolver, IAssetLocationResolver assetLocationResolver, - IBackFlowConflictResolver conflictResolver, IFileSystem fileSystem, ILogger logger) - : base(vmrInfo, sourceManifest, dependencyTracker, dependencyFileManager, vmrCloneManager, repositoryCloneManager, localGitClient, localGitRepoFactory, versionDetailsParser, vmrPatchHandler, workBranchFactory, basicBarClient, libGit2Client, coherencyUpdateResolver, assetLocationResolver, conflictResolver, fileSystem, logger) + : base(vmrInfo, sourceManifest, dependencyTracker, dependencyFileManager, vmrCloneManager, repositoryCloneManager, localGitClient, localGitRepoFactory, versionDetailsParser, vmrPatchHandler, workBranchFactory, basicBarClient, libGit2Client, coherencyUpdateResolver, assetLocationResolver, fileSystem, logger) { _sourceManifest = sourceManifest; _dependencyTracker = dependencyTracker; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs index 738e71176a..b4351ae746 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs @@ -5,7 +5,6 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using Microsoft.DotNet.DarcLib.Conflicts; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; using Microsoft.DotNet.ProductConstructionService.Client.Models; @@ -46,20 +45,15 @@ public PcsVmrForwardFlower( IVmrUpdater vmrUpdater, IVmrDependencyTracker dependencyTracker, IVmrCloneManager vmrCloneManager, - IDependencyFileManager dependencyFileManager, IRepositoryCloneManager repositoryCloneManager, ILocalGitClient localGitClient, - ILocalLibGit2Client libGit2Client, IBasicBarClient basicBarClient, ILocalGitRepoFactory localGitRepoFactory, IVersionDetailsParser versionDetailsParser, IProcessManager processManager, - ICoherencyUpdateResolver coherencyUpdateResolver, - IAssetLocationResolver assetLocationResolver, - IForwardFlowConflictResolver conflictResolver, IFileSystem fileSystem, ILogger logger) - : base(vmrInfo, sourceManifest, vmrUpdater, dependencyTracker, vmrCloneManager, dependencyFileManager, localGitClient, libGit2Client, basicBarClient, localGitRepoFactory, versionDetailsParser, processManager, coherencyUpdateResolver, assetLocationResolver, conflictResolver, fileSystem, logger) + : base(vmrInfo, sourceManifest, vmrUpdater, dependencyTracker, vmrCloneManager, localGitClient, basicBarClient, localGitRepoFactory, versionDetailsParser, processManager, fileSystem, logger) { _sourceManifest = sourceManifest; _dependencyTracker = dependencyTracker; diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs index 3308593cc0..8d09f14e1b 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs @@ -7,11 +7,13 @@ using System.Threading; using System.Threading.Tasks; using LibGit2Sharp; -using Microsoft.DotNet.DarcLib.Conflicts; using Microsoft.DotNet.DarcLib.Helpers; +using Microsoft.DotNet.DarcLib.Models.Darc; +using Microsoft.DotNet.DarcLib.Models; using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; using Microsoft.DotNet.ProductConstructionService.Client.Models; using Microsoft.Extensions.Logging; +using NuGet.Versioning; #nullable enable namespace Microsoft.DotNet.DarcLib.VirtualMonoRepo; @@ -87,13 +89,17 @@ internal class VmrBackFlower : VmrCodeFlower, IVmrBackFlower private readonly IVmrInfo _vmrInfo; private readonly ISourceManifest _sourceManifest; private readonly IVmrDependencyTracker _dependencyTracker; + private readonly IDependencyFileManager _dependencyFileManager; private readonly IVmrCloneManager _vmrCloneManager; private readonly IRepositoryCloneManager _repositoryCloneManager; private readonly ILocalGitRepoFactory _localGitRepoFactory; + private readonly IVersionDetailsParser _versionDetailsParser; private readonly IVmrPatchHandler _vmrPatchHandler; private readonly IWorkBranchFactory _workBranchFactory; private readonly IBasicBarClient _barClient; - private readonly IBackFlowConflictResolver _conflictResolver; + private readonly ILocalLibGit2Client _libGit2Client; + private readonly ICoherencyUpdateResolver _coherencyUpdateResolver; + private readonly IAssetLocationResolver _assetLocationResolver; private readonly IFileSystem _fileSystem; private readonly ILogger _logger; @@ -113,21 +119,24 @@ public VmrBackFlower( ILocalLibGit2Client libGit2Client, ICoherencyUpdateResolver coherencyUpdateResolver, IAssetLocationResolver assetLocationResolver, - IBackFlowConflictResolver conflictResolver, IFileSystem fileSystem, ILogger logger) - : base(vmrInfo, sourceManifest, dependencyTracker, localGitClient, libGit2Client, localGitRepoFactory, versionDetailsParser, dependencyFileManager, coherencyUpdateResolver, assetLocationResolver, fileSystem, logger) + : base(vmrInfo, sourceManifest, dependencyTracker, localGitClient, localGitRepoFactory, versionDetailsParser, fileSystem, logger) { _vmrInfo = vmrInfo; _sourceManifest = sourceManifest; _dependencyTracker = dependencyTracker; + _dependencyFileManager = dependencyFileManager; _vmrCloneManager = vmrCloneManager; _repositoryCloneManager = repositoryCloneManager; _localGitRepoFactory = localGitRepoFactory; + _versionDetailsParser = versionDetailsParser; _vmrPatchHandler = vmrPatchHandler; _workBranchFactory = workBranchFactory; _barClient = basicBarClient; - _conflictResolver = conflictResolver; + _libGit2Client = libGit2Client; + _coherencyUpdateResolver = coherencyUpdateResolver; + _assetLocationResolver = assetLocationResolver; _fileSystem = fileSystem; _logger = logger; } @@ -258,7 +267,6 @@ protected async Task FlowBackAsync( targetRepo, build, excludedAssets, - sourceElementSha: build.Commit, hadPreviousChanges: hasChanges, cancellationToken); @@ -266,7 +274,14 @@ protected async Task FlowBackAsync( { // We try to merge the target branch so that we can potentially // resolve some expected conflicts in the version files - await _conflictResolver.TryMergingRepoBranch(targetRepo, targetBranch, baseBranch); + await TryMergingBranch( + mapping.Name, + targetRepo, + build, + excludedAssets, + targetBranch, + baseBranch, + cancellationToken); } return hasChanges; @@ -498,6 +513,57 @@ .. mapping.Exclude.Select(VmrPatchHandler.GetExclusionRule), return true; } + /// + /// This is a naive implementation of conflict resolution for version files. + /// It takes the versions from the target branch and updates it with current build's assets. + /// This means that any changes to the version files done in the VMR will be lost. + /// See https://github.com/dotnet/arcade-services/issues/4342 for more information + /// + protected override async Task TryResolveConflicts( + string mappingName, + ILocalGitRepo repo, + Build build, + IReadOnlyCollection? excludedAssets, + string targetBranch, + IEnumerable conflictedFiles, + CancellationToken cancellationToken) + { + var result = await repo.RunGitCommandAsync(["checkout", "--theirs", "."], cancellationToken); + result.ThrowIfFailed("Failed to check out the conflicted files"); + + await UpdateDependenciesAndToolset( + _vmrInfo.VmrPath, + repo, + build, + excludedAssets, + false, + cancellationToken); + + await repo.StageAsync(["."], cancellationToken); + + _logger.LogInformation("Auto-resolved conflicts in version files"); + return true; + } + + protected override Task TryResolvingConflict( + string mappingName, + ILocalGitRepo repo, + Build build, + string filePath, + CancellationToken cancellationToken) + => throw new NotImplementedException(); // We don't need to resolve individual files as we handle all together + + protected override IEnumerable GetAllowedConflicts(IEnumerable conflictedFiles, string mappingName) + { + var allowedVersionFiles = DependencyFileManager.DependencyFiles + .Select(f => f.ToLowerInvariant()) + .ToList(); + + return conflictedFiles + .Where(f => f.Path.ToLowerInvariant().StartsWith(Constants.CommonScriptFilesPath + '/') + || allowedVersionFiles.Contains(f.Path.ToLowerInvariant())); + } + private async Task<(bool, SourceMapping)> PrepareVmrAndRepo( string mappingName, ILocalGitRepo targetRepo, @@ -543,6 +609,126 @@ await _repositoryCloneManager.PrepareCloneAsync( return (targetBranchExisted, mapping); } - protected override NativePath GetEngCommonPath(NativePath sourceRepo) => sourceRepo / VmrInfo.SourceDirName / "arcade" / Constants.CommonScriptFilesPath; + /// + /// Updates version details, eng/common and other version files (global.json, ...) based on a build that is being flown. + /// For backflows, updates the Source element in Version.Details.xml. + /// + /// Source repository (needed when eng/common is flown too) + /// Target repository directory + /// Build with assets (dependencies) that is being flows + /// Assets to exclude from the dependency flow + /// For backflows, VMR SHA that is being flown so it can be stored in Version.Details.xml + /// Set to true when we already had a code flow commit to amend the dependency update into it + private async Task UpdateDependenciesAndToolset( + NativePath sourceRepo, + ILocalGitRepo targetRepo, + Build build, + IReadOnlyCollection? excludedAssets, + bool hadPreviousChanges, + CancellationToken cancellationToken) + { + VersionDetails versionDetails = _versionDetailsParser.ParseVersionDetailsFile(targetRepo.Path / VersionFiles.VersionDetailsXml); + await _assetLocationResolver.AddAssetLocationToDependenciesAsync(versionDetails.Dependencies); + + List updates; + bool hadUpdates = false; + + var sourceOrigin = new SourceDependency( + build.GetRepository(), + build.Commit, + build.Id); + + if (versionDetails.Source?.Sha != sourceOrigin.Sha || versionDetails.Source?.BarId != sourceOrigin.BarId) + { + hadUpdates = true; + } + + // Generate the element and get updates + if (build is not null) + { + IEnumerable assetData = build.Assets + .Where(a => excludedAssets is null || !excludedAssets.Contains(a.Name)) + .Select(a => new AssetData(a.NonShipping) + { + Name = a.Name, + Version = a.Version + }); + + updates = _coherencyUpdateResolver.GetRequiredNonCoherencyUpdates( + build.GetRepository() ?? Constants.DefaultVmrUri, + build.Commit, + assetData, + versionDetails.Dependencies); + + await _assetLocationResolver.AddAssetLocationToDependenciesAsync([.. updates.Select(u => u.To)]); + } + else + { + updates = []; + } + + // If we are updating the arcade sdk we need to update the eng/common files as well + DependencyDetail? arcadeItem = updates.GetArcadeUpdate(); + SemanticVersion? targetDotNetVersion = null; + + if (arcadeItem != null) + { + targetDotNetVersion = await _dependencyFileManager.ReadToolsDotnetVersionAsync(arcadeItem.RepoUri, arcadeItem.Commit, repoIsVmr: true); + } + + GitFileContentContainer updatedFiles = await _dependencyFileManager.UpdateDependencyFiles( + updates.Select(u => u.To), + sourceOrigin, + targetRepo.Path, + branch: null, // reads the working tree + versionDetails.Dependencies, + targetDotNetVersion); + + // This actually does not commit but stages only + await _libGit2Client.CommitFilesAsync(updatedFiles.GetFilesToCommit(), targetRepo.Path, null, null); + + // Update eng/common files + if (arcadeItem != null) + { + // Check if the VMR contains src/arcade/eng/common + var arcadeEngCommonDir = GetEngCommonPath(sourceRepo); + if (!_fileSystem.DirectoryExists(arcadeEngCommonDir)) + { + _logger.LogWarning("VMR does not contain src/arcade/eng/common, skipping eng/common update"); + return hadUpdates; + } + + var commonDir = targetRepo.Path / Constants.CommonScriptFilesPath; + if (_fileSystem.DirectoryExists(commonDir)) + { + _fileSystem.DeleteDirectory(commonDir, true); + } + + _fileSystem.CopyDirectory( + arcadeEngCommonDir, + targetRepo.Path / Constants.CommonScriptFilesPath, + true); + } + + if (!await targetRepo.HasWorkingTreeChangesAsync()) + { + return hadUpdates; + } + + await targetRepo.StageAsync(["."], cancellationToken); + + if (hadPreviousChanges) + { + await targetRepo.CommitAmendAsync(cancellationToken); + } + else + { + await targetRepo.CommitAsync("Updated dependencies", allowEmpty: true, cancellationToken: cancellationToken); + } + + return true; + } + + protected override NativePath GetEngCommonPath(NativePath sourceRepo) => sourceRepo / VmrInfo.ArcadeRepoDir / Constants.CommonScriptFilesPath; protected override bool TargetRepoIsVmr() => false; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs index 562f8cd98d..d5f749d235 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs @@ -9,11 +9,9 @@ using System.Threading.Tasks; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.Models; -using Microsoft.DotNet.DarcLib.Models.Darc; using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; using Microsoft.DotNet.ProductConstructionService.Client.Models; using Microsoft.Extensions.Logging; -using NuGet.Versioning; #nullable enable namespace Microsoft.DotNet.DarcLib.VirtualMonoRepo; @@ -28,12 +26,8 @@ internal abstract class VmrCodeFlower private readonly ISourceManifest _sourceManifest; 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; - private readonly IAssetLocationResolver _assetLocationResolver; private readonly IFileSystem _fileSystem; private readonly ILogger _logger; @@ -42,12 +36,8 @@ protected VmrCodeFlower( ISourceManifest sourceManifest, IVmrDependencyTracker dependencyTracker, ILocalGitClient localGitClient, - ILocalLibGit2Client libGit2Client, ILocalGitRepoFactory localGitRepoFactory, IVersionDetailsParser versionDetailsParser, - IDependencyFileManager dependencyFileManager, - ICoherencyUpdateResolver coherencyUpdateResolver, - IAssetLocationResolver assetLocationResolver, IFileSystem fileSystem, ILogger logger) { @@ -55,12 +45,8 @@ protected VmrCodeFlower( _sourceManifest = sourceManifest; _dependencyTracker = dependencyTracker; _localGitClient = localGitClient; - _libGit2Client = libGit2Client; _localGitRepoFactory = localGitRepoFactory; _versionDetailsParser = versionDetailsParser; - _dependencyFileManager = dependencyFileManager; - _coherencyUpdateResolver = coherencyUpdateResolver; - _assetLocationResolver = assetLocationResolver; _fileSystem = fileSystem; _logger = logger; } @@ -273,165 +259,202 @@ protected async Task GetLastFlowAsync(SourceMapping mapping, ILocalGit } /// - /// Finds the last backflow between a repo and a VMR. + /// Tries to resolve well-known conflicts that can occur during a code flow operation. + /// The conflicts can happen when backward a forward flow PRs get merged out of order. + /// This can be shown on the following schema (the order of events is numbered): + /// + /// repo VMR + /// O────────────────────►O + /// │ 2. │ 1. + /// │ O◄────────────────O- - ┐ + /// │ │ 4. │ + /// 3.O───┼────────────►O │ │ + /// │ │ │ │ + /// │ ┌─┘ │ │ │ + /// │ │ │ │ + /// 5.O◄┘ └──►O 6. │ + /// │ 7. │ O (actual branch for 7. is based on top of 1.) + /// |────────────────►O │ + /// │ └──►O 8. + /// │ │ + /// + /// The conflict arises in step 8. and is caused by the fact that: + /// - When the forward flow PR branch is being opened in 7., the last sync (from the point of view of 5.) is from 1. + /// - This means that the PR branch will be based on 1. (the real PR branch is the "actual 7.") + /// - This means that when 6. merged, VMR's source-manifest.json got updated with the SHA of the 3. + /// - So the source-manifest in 6. contains the SHA of 3. + /// - The forward flow PR branch contains the SHA of 5. + /// - So the source-manifest file conflicts on the SHA (3. vs 5.) + /// - There's also a similar conflict in the git-info files. + /// - However, if only the version files are in conflict, we can try merging 6. into 7. and resolve the conflict. + /// - This is because basically we know we want to set the version files to point at 5. /// - private async Task GetLastBackflow(NativePath repoPath) - { - // Last backflow SHA comes from Version.Details.xml in the repo - SourceDependency? source = _versionDetailsParser.ParseVersionDetailsFile(repoPath / VersionFiles.VersionDetailsXml).Source; - if (source is null) - { - return null; - } - - string lastBackflowVmrSha = source.Sha; - string lastBackflowRepoSha = await BlameLineAsync( - repoPath / VersionFiles.VersionDetailsXml, - line => line.Contains(VersionDetailsParser.SourceElementName) && line.Contains(lastBackflowVmrSha)); - - return new Backflow(lastBackflowVmrSha, lastBackflowRepoSha); - } - - /// - /// Finds the last forward flow between a repo and a VMR. - /// - private async Task GetLastForwardFlow(string mappingName) - { - ISourceComponent repoInVmr = _sourceManifest.GetRepoVersion(mappingName); - - // Last forward flow SHAs come from source-manifest.json in the VMR - string lastForwardRepoSha = repoInVmr.CommitSha; - string lastForwardVmrSha = await BlameLineAsync( - _vmrInfo.SourceManifestPath, - line => line.Contains(lastForwardRepoSha)); - - return new ForwardFlow(lastForwardRepoSha, lastForwardVmrSha); - } - - /// - /// Updates version details, eng/common and other version files (global.json, ...) based on a build that is being flown. - /// For backflows, updates the Source element in Version.Details.xml. - /// - /// Source repository (needed when eng/common is flown too) - /// Target repository directory - /// Build with assets (dependencies) that is being flows - /// Assets to exclude from the dependency flow - /// For backflows, VMR SHA that is being flown so it can be stored in Version.Details.xml - /// Set to true when we already had a code flow commit to amend the dependency update into it - protected async Task UpdateDependenciesAndToolset( - NativePath sourceRepo, - ILocalGitRepo targetRepo, + protected async Task TryMergingBranch( + string mappingName, + ILocalGitRepo repo, Build build, IReadOnlyCollection? excludedAssets, - string? sourceElementSha, - bool hadPreviousChanges, + string targetBranch, + string branchToMerge, CancellationToken cancellationToken) { - string versionDetailsXml = await targetRepo.GetFileFromGitAsync(VersionFiles.VersionDetailsXml) - ?? throw new Exception($"Failed to read {VersionFiles.VersionDetailsXml} from {targetRepo.Path} (file does not exist)"); - VersionDetails versionDetails = _versionDetailsParser.ParseVersionDetailsXml(versionDetailsXml); - await _assetLocationResolver.AddAssetLocationToDependenciesAsync(versionDetails.Dependencies); - - SourceDependency? sourceOrigin = null; - List updates; - bool hadUpdates = false; + _logger.LogInformation("Checking if target branch {targetBranch} has conflicts with {headBranch}", branchToMerge, targetBranch); - if (sourceElementSha != null) + await repo.CheckoutAsync(targetBranch); + var result = await repo.RunGitCommandAsync(["merge", "--no-commit", "--no-ff", branchToMerge], cancellationToken); + if (result.Succeeded) { - sourceOrigin = new SourceDependency( - build.GetRepository(), - sourceElementSha, - build.Id); - - if (versionDetails.Source?.Sha != sourceElementSha) + try + { + await repo.CommitAsync( + $"Merging {branchToMerge} into {targetBranch}", + allowEmpty: false, + cancellationToken: CancellationToken.None); + + _logger.LogInformation("Successfully merged the branch {targetBranch} into {headBranch} in {repoPath}", + branchToMerge, + targetBranch, + repo.Path); + } + catch (Exception e) when (e.Message.Contains("nothing to commit")) { - hadUpdates = true; + // Our branch might be fast-forward and so no commit is needed + _logger.LogInformation("Branch {targetBranch} had no updates since it was last merged into {headBranch}", + branchToMerge, + targetBranch); } + + return true; } - // Generate the element and get updates - if (build is not null) + result = await repo.RunGitCommandAsync(["diff", "--name-only", "--diff-filter=U", "--relative"], cancellationToken); + if (!result.Succeeded) { - IEnumerable assetData = build.Assets - .Where(a => excludedAssets is null || !excludedAssets.Contains(a.Name)) - .Select(a => new AssetData(a.NonShipping) - { - Name = a.Name, - Version = a.Version - }); + _logger.LogInformation("Failed to merge the branch {targetBranch} into {headBranch} in {repoPath}", + branchToMerge, + targetBranch, + repo.Path); + result = await repo.RunGitCommandAsync(["merge", "--abort"], CancellationToken.None); + return false; + } - updates = _coherencyUpdateResolver.GetRequiredNonCoherencyUpdates( - build.GetRepository() ?? Constants.DefaultVmrUri, - build.Commit, - assetData, - versionDetails.Dependencies); + var conflictedFiles = result.StandardOutput + .Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries) + .Select(line => new UnixPath(line.Trim())); - await _assetLocationResolver.AddAssetLocationToDependenciesAsync([.. updates.Select(u => u.To)]); - } - else + var unresolvableConflicts = conflictedFiles + .Except(GetAllowedConflicts(conflictedFiles, mappingName)) + .ToList(); + + if (unresolvableConflicts.Count > 0) { - updates = []; - } + _logger.LogInformation("Failed to merge the branch {targetBranch} into {headBranch} due to unresolvable conflicts: {conflicts}", + branchToMerge, + targetBranch, + string.Join(", ", unresolvableConflicts)); - // If we are updating the arcade sdk we need to update the eng/common files as well - DependencyDetail? arcadeItem = updates.GetArcadeUpdate(); - SemanticVersion? targetDotNetVersion = null; + result = await repo.RunGitCommandAsync(["merge", "--abort"], CancellationToken.None); + return false; + } - if (arcadeItem != null) + if (!await TryResolveConflicts( + mappingName, + repo, + build, + excludedAssets, + targetBranch, + conflictedFiles, + cancellationToken)) { - targetDotNetVersion = await _dependencyFileManager.ReadToolsDotnetVersionAsync(arcadeItem.RepoUri, arcadeItem.Commit, repoIsVmr: true); + return false; } - GitFileContentContainer updatedFiles = await _dependencyFileManager.UpdateDependencyFiles( - updates.Select(u => u.To), - sourceOrigin, - targetRepo.Path, - Constants.HEAD, - versionDetails.Dependencies, - targetDotNetVersion); + _logger.LogInformation("Successfully resolved file conflicts between branches {targetBranch} and {headBranch}", + branchToMerge, + targetBranch); - // TODO https://github.com/dotnet/arcade-services/issues/3251: Stop using LibGit2SharpClient for this - await _libGit2Client.CommitFilesAsync(updatedFiles.GetFilesToCommit(), targetRepo.Path, null, null); + await repo.CommitAsync( + $"Merge branch {branchToMerge} into {targetBranch}", + allowEmpty: false, + cancellationToken: CancellationToken.None); - // Update eng/common files - if (arcadeItem != null) + return true; + } + + protected virtual async Task TryResolveConflicts( + string mappingName, + ILocalGitRepo repo, + Build build, + IReadOnlyCollection? excludedAssets, + string targetBranch, + IEnumerable conflictedFiles, + CancellationToken cancellationToken) + { + foreach (var filePath in conflictedFiles) { - var commonDir = targetRepo.Path / Constants.CommonScriptFilesPath; - if (_fileSystem.DirectoryExists(commonDir)) + cancellationToken.ThrowIfCancellationRequested(); + try { - _fileSystem.DeleteDirectory(commonDir, true); + if (await TryResolvingConflict(mappingName, repo, build, filePath, cancellationToken)) + { + continue; + } } - - // Check if the VMR contains src/arcade/eng/common - var arcadeEngCommonDir = GetEngCommonPath(sourceRepo); - if (!_fileSystem.DirectoryExists(arcadeEngCommonDir)) + catch (Exception e) { - _logger.LogWarning("VMR does not contain src/arcade/eng/common, skipping eng/common update"); - return hadUpdates; + _logger.LogError(e, "Failed to resolve conflicts in {filePath}", filePath); } - _fileSystem.CopyDirectory( - arcadeEngCommonDir, - targetRepo.Path / Constants.CommonScriptFilesPath, - true); + await repo.RunGitCommandAsync(["merge", "--abort"], CancellationToken.None); + return false; } - if (!await targetRepo.HasWorkingTreeChangesAsync()) - { - return hadUpdates; - } + return true; + } - await targetRepo.StageAsync(["."], cancellationToken); + protected abstract Task TryResolvingConflict( + string mappingName, + ILocalGitRepo repo, + Build build, + string filePath, + CancellationToken cancellationToken); - if (hadPreviousChanges) - { - await targetRepo.CommitAmendAsync(cancellationToken); - } - else + protected abstract IEnumerable GetAllowedConflicts(IEnumerable conflictedFiles, string mappingName); + + /// + /// Finds the last backflow between a repo and a VMR. + /// + private async Task GetLastBackflow(NativePath repoPath) + { + // Last backflow SHA comes from Version.Details.xml in the repo + SourceDependency? source = _versionDetailsParser.ParseVersionDetailsFile(repoPath / VersionFiles.VersionDetailsXml).Source; + if (source is null) { - await targetRepo.CommitAsync("Updated dependencies", allowEmpty: true, cancellationToken: cancellationToken); + return null; } - return true; + + string lastBackflowVmrSha = source.Sha; + string lastBackflowRepoSha = await BlameLineAsync( + repoPath / VersionFiles.VersionDetailsXml, + line => line.Contains(VersionDetailsParser.SourceElementName) && line.Contains(lastBackflowVmrSha)); + + return new Backflow(lastBackflowVmrSha, lastBackflowRepoSha); + } + + /// + /// Finds the last forward flow between a repo and a VMR. + /// + private async Task GetLastForwardFlow(string mappingName) + { + ISourceComponent repoInVmr = _sourceManifest.GetRepoVersion(mappingName); + + // Last forward flow SHAs come from source-manifest.json in the VMR + string lastForwardRepoSha = repoInVmr.CommitSha; + string lastForwardVmrSha = await BlameLineAsync( + _vmrInfo.SourceManifestPath, + line => line.Contains(lastForwardRepoSha)); + + return new ForwardFlow(lastForwardRepoSha, lastForwardVmrSha); } /// diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs index e0125ad69e..b11bab371e 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs @@ -7,7 +7,6 @@ using System.Threading; using System.Threading.Tasks; using LibGit2Sharp; -using Microsoft.DotNet.DarcLib.Conflicts; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; using Microsoft.DotNet.ProductConstructionService.Client.Models; @@ -77,7 +76,7 @@ internal class VmrForwardFlower : VmrCodeFlower, IVmrForwardFlower private readonly IBasicBarClient _barClient; private readonly ILocalGitRepoFactory _localGitRepoFactory; private readonly IProcessManager _processManager; - private readonly IForwardFlowConflictResolver _conflictResolver; + private readonly IFileSystem _fileSystem; private readonly ILogger _logger; public VmrForwardFlower( @@ -86,19 +85,14 @@ public VmrForwardFlower( IVmrUpdater vmrUpdater, IVmrDependencyTracker dependencyTracker, IVmrCloneManager vmrCloneManager, - IDependencyFileManager dependencyFileManager, ILocalGitClient localGitClient, - ILocalLibGit2Client libGit2Client, IBasicBarClient basicBarClient, ILocalGitRepoFactory localGitRepoFactory, IVersionDetailsParser versionDetailsParser, IProcessManager processManager, - ICoherencyUpdateResolver coherencyUpdateResolver, - IAssetLocationResolver assetLocationResolver, - IForwardFlowConflictResolver conflictResolver, IFileSystem fileSystem, ILogger logger) - : base(vmrInfo, sourceManifest, dependencyTracker, localGitClient, libGit2Client, localGitRepoFactory, versionDetailsParser, dependencyFileManager, coherencyUpdateResolver, assetLocationResolver, fileSystem, logger) + : base(vmrInfo, sourceManifest, dependencyTracker, localGitClient, localGitRepoFactory, versionDetailsParser, fileSystem, logger) { _vmrInfo = vmrInfo; _sourceManifest = sourceManifest; @@ -108,7 +102,7 @@ public VmrForwardFlower( _barClient = basicBarClient; _localGitRepoFactory = localGitRepoFactory; _processManager = processManager; - _conflictResolver = conflictResolver; + _fileSystem = fileSystem; _logger = logger; } @@ -196,7 +190,14 @@ protected async Task FlowForwardAsync( // We try to merge the target branch so that we can potentially // resolve some expected conflicts in the version files ILocalGitRepo vmr = _localGitRepoFactory.Create(_vmrInfo.VmrPath); - await _conflictResolver.TryMergingBranch(vmr, mapping.Name, targetBranch, baseBranch); + await TryMergingBranch( + mapping.Name, + vmr, + build, + excludedAssets, + targetBranch, + baseBranch, + cancellationToken); } return hasChanges; @@ -416,6 +417,78 @@ .. submodules.Select(s => s.Path).Distinct().Select(VmrPatchHandler.GetExclusion cancellationToken: cancellationToken); } + protected override async Task TryResolvingConflict( + string mappingName, + ILocalGitRepo repo, + Build build, + string filePath, + CancellationToken cancellationToken) + { + // Known conflict in source-manifest.json + if (string.Equals(filePath, VmrInfo.DefaultRelativeSourceManifestPath, StringComparison.OrdinalIgnoreCase)) + { + await TryResolvingSourceManifestConflict(repo, mappingName!, cancellationToken); + return true; + } + + // Known conflict in a git-info props file - we just use our version as we expect it to be newer + // TODO https://github.com/dotnet/arcade-services/issues/3378: For batched subscriptions, we need to handle all git-info files + var gitInfoFile = $"{VmrInfo.GitInfoSourcesDir}/{mappingName}.props"; + if (string.Equals(filePath, gitInfoFile, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogInformation("Auto-resolving conflict in {file}", gitInfoFile); + await repo.RunGitCommandAsync(["checkout", "--ours", filePath], cancellationToken); + await repo.StageAsync([filePath], cancellationToken); + return true; + } + + _logger.LogInformation("Unable to resolve conflicts in {file}", filePath); + return false; + } + + protected override IEnumerable GetAllowedConflicts(IEnumerable conflictedFiles, string mappingName) => + [ + VmrInfo.DefaultRelativeSourceManifestPath, + new UnixPath($"{VmrInfo.GitInfoSourcesDir}/{mappingName}.props"), + ]; + + // TODO https://github.com/dotnet/arcade-services/issues/3378: This might not work for batched subscriptions + private async Task TryResolvingSourceManifestConflict(ILocalGitRepo vmr, string mappingName, CancellationToken cancellationToken) + { + _logger.LogInformation("Auto-resolving conflict in {file}", VmrInfo.DefaultRelativeSourceManifestPath); + + // We load the source manifest from the target branch and replace the + // current mapping (and its submodules) with our branches' information + var result = await vmr.RunGitCommandAsync( + ["show", "MERGE_HEAD:" + VmrInfo.DefaultRelativeSourceManifestPath], + cancellationToken); + + var theirSourceManifest = SourceManifest.FromJson(result.StandardOutput); + var ourSourceManifest = _sourceManifest; + var updatedMapping = ourSourceManifest.Repositories.First(r => r.Path == mappingName); + + theirSourceManifest.UpdateVersion( + mappingName, + updatedMapping.RemoteUri, + updatedMapping.CommitSha, + updatedMapping.PackageVersion, + updatedMapping.BarId); + + foreach (var submodule in theirSourceManifest.Submodules.Where(s => s.Path.StartsWith(mappingName + "/"))) + { + theirSourceManifest.RemoveSubmodule(submodule); + } + + foreach (var submodule in _sourceManifest.Submodules.Where(s => s.Path.StartsWith(mappingName + "/"))) + { + theirSourceManifest.UpdateSubmodule(submodule); + } + + _fileSystem.WriteToFile(_vmrInfo.SourceManifestPath, theirSourceManifest.ToJson()); + _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); + await vmr.StageAsync([_vmrInfo.SourceManifestPath], cancellationToken); + } + protected override NativePath GetEngCommonPath(NativePath sourceRepo) => sourceRepo / Constants.CommonScriptFilesPath; protected override bool TargetRepoIsVmr() => true; } diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs index 5ac58bafae..ec6f4aeecd 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrRegistrations.cs @@ -7,7 +7,6 @@ using System.Security.Cryptography.X509Certificates; using Maestro.Common; using Maestro.Common.AzureDevOpsTokens; -using Microsoft.DotNet.DarcLib.Conflicts; using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; using Microsoft.Extensions.DependencyInjection; @@ -108,8 +107,6 @@ private static IServiceCollection AddVmrManagers( services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); - services.TryAddTransient(); - services.TryAddTransient(); services.TryAddScoped(); services.TryAddScoped(); diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs index 3403141c8f..d635cac0da 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrBackflowTest.cs @@ -146,8 +146,10 @@ await File.WriteAllTextAsync(ProductRepoPath / VersionFiles.VersionProps, """); + // Create global.json in src/arcade/ in the VMR Directory.CreateDirectory(ArcadeInVmrPath); await File.WriteAllTextAsync(ArcadeInVmrPath / VersionFiles.GlobalJson, Constants.GlobalJsonTemplate); + await GitOperations.CommitAll(VmrPath, "Creating global.json in src/arcade"); // Level the repo and the VMR await GitOperations.CommitAll(ProductRepoPath, "Changing version files"); diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrCodeFlowTests.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrCodeFlowTests.cs index 0851caafaa..91ed1b1791 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrCodeFlowTests.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrCodeFlowTests.cs @@ -143,6 +143,23 @@ protected async Task ChangeVmrFileAndFlowIt(string newContent, string bran return hadUpdates; } + protected async Task VerifyDependenciesInRepo(NativePath repo, List expectedDependencies) + { + var dependencies = await GetLocal(repo) + .GetDependenciesAsync(); + + dependencies + .Where(d => d.Type == DependencyType.Product) + .Should().BeEquivalentTo(expectedDependencies); + + var versionProps = await File.ReadAllTextAsync(repo / VersionFiles.VersionProps); + foreach (var dependency in expectedDependencies) + { + var tagName = VersionFiles.GetVersionPropsPackageVersionElementName(dependency.Name); + versionProps.Should().Contain($"<{tagName}>{dependency.Version}"); + } + } + protected override async Task CopyReposForCurrentTest() { CopyDirectory(VmrTestsOneTimeSetUp.TestsDirectory / Constants.SecondRepoName, SecondRepoPath); diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs index a232746498..01af5bdfd8 100644 --- a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs +++ b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.IO; +using System.Linq; using System.Threading.Tasks; using FluentAssertions; using Microsoft.DotNet.DarcLib; @@ -216,8 +217,10 @@ repo VMR │ │ │ │ 6.O◄───┘ └───►O 5. │ 7. │ - │ O───────────────| - 8.O◄────┘ │ + │ O◄──────────────| + 8.O │ │ + │ 10.O◄──────────────O 9. + 11.O◄────┘ │ │ │ */ [Test] @@ -248,13 +251,15 @@ public async Task BackwardFlowConflictResolutionTest() await GitOperations.CommitAll(VmrPath, "3a.txt"); // 4. Open a backflow PR - hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, backBranchName); + var build = await CreateNewBuild(VmrPath, [(FakePackageName, "1.0.1")]); + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, backBranchName, build); 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 / "3b.txt", "three again"); await GitOperations.CommitAll(VmrPath, "3b.txt"); - hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, backBranchName); + build = await CreateNewBuild(VmrPath, [(FakePackageName, "1.0.2")]); + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, backBranchName, build); hadUpdates.ShouldHaveUpdates(); // 5. Merge the forward flow PR @@ -265,25 +270,90 @@ public async Task BackwardFlowConflictResolutionTest() // 7. Flow back again so the VMR version of the file will flow back to the repo await GitOperations.Checkout(ProductRepoPath, "main"); + + // We add a new dependency in the repo to see if it survives the conflict + var productRepo = GetLocal(ProductRepoPath); + await productRepo.AddDependencyAsync( + new DependencyDetail + { + Name = "Package.A1", + Version = "1.0.1", + RepoUri = "https://github.com/dotnet/repo1", + Commit = "abc", + Type = DependencyType.Product, + }); + await GitOperations.CommitAll(ProductRepoPath, "Adding a new dependency"); + await GitOperations.Checkout(VmrPath, "main"); - hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, branch: backBranchName); + build = await CreateNewBuild(VmrPath, [(FakePackageName, "1.0.3")]); + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, backBranchName, build); hadUpdates.ShouldHaveUpdates(); - // 8. Merge the forward flow PR - any conflicts in version files are dealt with automatically + // Verify the version files got merged properly + List expectedDependencies = + [ + new() + { + Name = "Package.A1", + Version = "1.0.1", + RepoUri = "https://github.com/dotnet/repo1", + Commit = "abc", + Type = DependencyType.Product, + }, + new() + { + Name = FakePackageName, + Version = "1.0.3", + RepoUri = build.GitHubRepository, + Commit = build.Commit, + Type = DependencyType.Product, + } + ]; + + await VerifyDependenciesInRepo(ProductRepoPath, expectedDependencies); + + // 8. We make another change on the target branch in the repo + await GitOperations.Checkout(ProductRepoPath, "main"); + await File.WriteAllTextAsync(ProductRepoPath / "1a.txt", "one one"); + expectedDependencies.Add(new DependencyDetail + { + Name = "Package.B2", + Version = "5.0.4", + RepoUri = "https://github.com/dotnet/repo2", + Commit = "def", + Type = DependencyType.Product, + }); + await productRepo.AddDependencyAsync(expectedDependencies.Last()); + await GitOperations.CommitAll(ProductRepoPath, "Change in main in the meantime"); + + // 9. We make a new commit in the VMR while the PR from 7. is still open + await File.WriteAllTextAsync(_productRepoVmrPath / "3c.txt", "three again again"); + await GitOperations.CommitAll(VmrPath, "3c.txt"); + + // 10. We flow it into the open PR from 7. + build = await CreateNewBuild(VmrPath, [(FakePackageName, "1.0.4")]); + hadUpdates = await CallDarcBackflow(Constants.ProductRepoName, ProductRepoPath, backBranchName, build); + hadUpdates.ShouldHaveUpdates(); + expectedDependencies[1].Version = "1.0.4"; + expectedDependencies[1].Commit = build.Commit; + + // 11. Merge the forward flow PR - any conflicts in version files are dealt with automatically // The conflict is described in the BackwardFlowConflictResolver class - // TODO https://github.com/dotnet/arcade-services/issues/4196: The conflict should get resolved automatically - // await GitOperations.MergePrBranch(ProductRepoPath, backBranchName); - await GitOperations.VerifyMergeConflict(ProductRepoPath, backBranchName, "eng/Version.Details.xml", mergeTheirs: true); + await GitOperations.MergePrBranch(ProductRepoPath, backBranchName); // 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"), + ("1a.txt", "one one"), ("1b.txt", "one again"), ("3a.txt", "three"), ("3b.txt", "three again"), + ("3c.txt", "three again again"), ]; + // Level the repos so that we can verify the contents + await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branch: forwardBranchName); + foreach (var (file, content) in expectedFiles) { CheckFileContents(_productRepoVmrPath / file, content); @@ -292,6 +362,7 @@ public async Task BackwardFlowConflictResolutionTest() await GitOperations.CheckAllIsCommitted(VmrPath); await GitOperations.CheckAllIsCommitted(ProductRepoPath); + await VerifyDependenciesInRepo(ProductRepoPath, expectedDependencies); } // This one simulates what would happen if PR both ways are open and the one that was open later merges first. @@ -556,15 +627,12 @@ await GetLocal(ProductRepoPath).UpdateDependenciesAsync( }, ]; - var dependencies = await GetLocal(ProductRepoPath) - .GetDependenciesAsync(); + await VerifyDependenciesInRepo(ProductRepoPath, expectedDependencies); - var vmrDependencies = new VersionDetailsParser() + new VersionDetailsParser() .ParseVersionDetailsFile(_productRepoVmrPath / VersionFiles.VersionDetailsXml) - .Dependencies; - - dependencies.Should().BeEquivalentTo(expectedDependencies); - vmrDependencies.Should().BeEquivalentTo(expectedDependencies); + .Dependencies + .Should().BeEquivalentTo(expectedDependencies); vmrVersionProps = await File.ReadAllTextAsync(_productRepoVmrPath / VersionFiles.VersionProps); CheckFileContents(ProductRepoPath / VersionFiles.VersionProps, expected: vmrVersionProps);