diff --git a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs index 258ce6b018..c2c48a70c8 100644 --- a/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs +++ b/src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs @@ -80,7 +80,7 @@ protected async Task FlowCodeLocallyAsync( Codeflow lastFlow; try { - lastFlow = await _codeFlower.GetLastFlowAsync(mapping, productRepo, currentFlow is Backflow); + (lastFlow, _, _) = await _codeFlower.GetLastFlowsAsync(mapping, productRepo, currentFlow is Backflow); } catch (InvalidSynchronizationException) { diff --git a/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/CodeFlowResult.cs b/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/CodeFlowResult.cs index 66faf0ad7a..b12802b100 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/CodeFlowResult.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/CodeFlowResult.cs @@ -5,11 +5,12 @@ using Microsoft.DotNet.DarcLib.Helpers; using Microsoft.DotNet.DarcLib.Models.Darc; +#nullable enable namespace Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo; public record CodeFlowResult( bool HadUpdates, IReadOnlyCollection ConflictedFiles, NativePath RepoPath, - Codeflow PreviousFlow, + string? PreviouslyFlownSha, List DependencyUpdates); diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs index cda7e3b540..2cd7b21ce0 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs @@ -110,7 +110,7 @@ public async Task FlowBackAsync( headBranch, cancellationToken); - Codeflow lastFlow = await GetLastFlowAsync(mapping, targetRepo, currentIsBackflow: true); + (Codeflow lastFlow, Backflow? lastBackFlow, _) = await GetLastFlowsAsync(mapping, targetRepo, currentIsBackflow: true); return await FlowBackAsync( mapping, @@ -122,7 +122,10 @@ public async Task FlowBackAsync( headBranch, discardPatches, headBranchExisted, - cancellationToken); + cancellationToken) with + { + PreviouslyFlownSha = lastBackFlow?.SourceSha, + }; } protected async Task FlowBackAsync( @@ -168,7 +171,7 @@ protected async Task FlowBackAsync( hasChanges || mergeResult.DependencyUpdates.Count > 0, mergeResult.ConflictedFiles, targetRepo.Path, - lastFlow, + PreviouslyFlownSha: null, mergeResult.DependencyUpdates); } @@ -445,7 +448,7 @@ private async Task RecreatePreviousFlowAndApplyBuild( // checkout the previous repo sha so we can get the last last flow await targetRepo.CheckoutAsync(previousRepoSha); await targetRepo.CreateBranchAsync(headBranch, overwriteExistingBranch: true); - var lastLastFlow = await GetLastFlowAsync(mapping, targetRepo, currentIsBackflow: true); + (Codeflow lastLastFlow, _, _) = await GetLastFlowsAsync(mapping, targetRepo, currentIsBackflow: true); Build previouslyAppliedVmrBuild; if (versionDetails.Source?.BarId != null) diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs index 3b581e6ff3..31cda60e0b 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs @@ -15,7 +15,7 @@ namespace Microsoft.DotNet.DarcLib.VirtualMonoRepo; public interface IVmrCodeFlower { - Task GetLastFlowAsync( + Task<(Codeflow LastFlow, Backflow? LastBackFlow, ForwardFlow LastForwardFlow)> GetLastFlowsAsync( SourceMapping mapping, ILocalGitRepo repoClone, bool currentIsBackflow); @@ -194,7 +194,10 @@ protected abstract Task OppositeDirectionFlowAsync( /// /// Checks the last flows between a repo and a VMR and returns the most recent one. /// - public async Task GetLastFlowAsync(SourceMapping mapping, ILocalGitRepo repoClone, bool currentIsBackflow) + public async Task<(Codeflow LastFlow, Backflow? LastBackFlow, ForwardFlow LastForwardFlow)> GetLastFlowsAsync( + SourceMapping mapping, + ILocalGitRepo repoClone, + bool currentIsBackflow) { await _dependencyTracker.RefreshMetadata(); _sourceManifest.Refresh(_vmrInfo.SourceManifestPath); @@ -204,7 +207,7 @@ public async Task GetLastFlowAsync(SourceMapping mapping, ILocalGitRep if (lastBackflow is null) { - return lastForwardFlow; + return (lastForwardFlow, lastBackflow, lastForwardFlow); } string backwardSha, forwardSha; @@ -231,7 +234,12 @@ public async Task GetLastFlowAsync(SourceMapping mapping, ILocalGitRep // If the SHA's are the same, it's a commit created by inflow which was then flown out if (forwardSha == backwardSha) { - return sourceRepo == repoClone ? lastForwardFlow : lastBackflow; + return + ( + sourceRepo == repoClone ? lastForwardFlow : lastBackflow, + lastBackflow, + lastForwardFlow + ); } // Let's determine the last flow by comparing source commit of last backflow with target commit of last forward flow @@ -245,7 +253,12 @@ public async Task GetLastFlowAsync(SourceMapping mapping, ILocalGitRep throw new InvalidSynchronizationException($"Failed to determine which commit of {sourceRepo} is older ({backwardSha}, {forwardSha})"); } - return isBackwardOlder ? lastForwardFlow : lastBackflow; + return + ( + isBackwardOlder ? lastForwardFlow : lastBackflow, + lastBackflow, + lastForwardFlow + ); } /// diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs index 3c50e439e5..3cea891260 100644 --- a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs +++ b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs @@ -116,7 +116,7 @@ public async Task FlowForwardAsync( await sourceRepo.FetchAllAsync([mapping.DefaultRemote, repoInfo.RemoteUri], cancellationToken); await sourceRepo.CheckoutAsync(build.Commit); - Codeflow lastFlow = await GetLastFlowAsync(mapping, sourceRepo, currentIsBackflow: false); + (Codeflow lastFlow, _, ForwardFlow lastForwardFlow) = await GetLastFlowsAsync(mapping, sourceRepo, currentIsBackflow: false); ForwardFlow currentFlow = new(lastFlow.TargetSha, build.Commit); bool hasChanges = await FlowCodeAsync( @@ -150,9 +150,9 @@ public async Task FlowForwardAsync( return new CodeFlowResult( hasChanges, - conflictedFiles, + conflictedFiles ?? [], sourceRepo.Path, - lastFlow, + lastForwardFlow.SourceSha, DependencyUpdates: []); } @@ -537,7 +537,7 @@ private async Task RecreatePreviousFlowAndApplyBuild( { _logger.LogInformation("Failed to create PR branch because of a conflict. Re-creating the previous flow.."); - var lastLastFlow = await GetLastFlowAsync(mapping, sourceRepo, currentIsBackflow: true); + (Codeflow lastLastFlow, _, _) = await GetLastFlowsAsync(mapping, sourceRepo, currentIsBackflow: true); // Find the BarID of the last flown repo build RepositoryRecord previouslyAppliedRepositoryRecord = _sourceManifest.GetRepositoryRecord(mapping.Name); diff --git a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PcsVmrBackFlower.cs b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PcsVmrBackFlower.cs index 90f42ef20a..0879dd96f3 100644 --- a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PcsVmrBackFlower.cs +++ b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PcsVmrBackFlower.cs @@ -77,12 +77,12 @@ public async Task FlowBackAsync( targetBranch, cancellationToken); - Codeflow lastFlow = await GetLastFlowAsync(mapping, targetRepo, currentIsBackflow: true); + var lastFlows = await GetLastFlowsAsync(mapping, targetRepo, currentIsBackflow: true); var result = await FlowBackAsync( mapping, targetRepo, - lastFlow, + lastFlows.LastFlow, build, subscription.ExcludedAssets, subscription.TargetBranch, @@ -91,12 +91,11 @@ public async Task FlowBackAsync( headBranchExisted, cancellationToken); - // TODO: https://github.com/dotnet/arcade-services/issues/4763 - // We also return true when headBranchExisted so that we always flow even the tag change in an already existing PR - // This is a workaround and will be fixed later properly return result with { - HadUpdates = result.HadUpdates || headBranchExisted + // For already existing PRs, we want to always push the changes (even if only the tag changed) + HadUpdates = result.HadUpdates || headBranchExisted, + PreviouslyFlownSha = lastFlows.LastBackFlow?.SourceSha, }; } diff --git a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs index 75da4fc1f8..1717c4a25b 100644 --- a/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs +++ b/src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs @@ -57,7 +57,7 @@ string GenerateCodeFlowPRTitle( string GenerateCodeFlowPRDescription( SubscriptionUpdateWorkItem update, BuildDTO build, - string previousSourceCommit, + string? previousSourceCommit, List dependencyUpdates, string? currentDescription, bool isForwardFlow); @@ -220,7 +220,7 @@ public string GenerateCodeFlowPRTitle( public string GenerateCodeFlowPRDescription( SubscriptionUpdateWorkItem update, BuildDTO build, - string previousSourceCommit, + string? previousSourceCommit, List dependencyUpdates, string? currentDescription, bool isForwardFlow) @@ -239,7 +239,7 @@ public string GenerateCodeFlowPRDescription( private static string GenerateCodeFlowPRDescriptionInternal( SubscriptionUpdateWorkItem update, BuildDTO build, - string previousSourceCommit, + string? previousSourceCommit, List dependencyUpdates, string? currentDescription, bool isForwardFlow) @@ -278,7 +278,7 @@ This pull request brings the following source code changes private static string GenerateCodeFlowDescriptionForSubscription( Guid subscriptionId, - string previousSourceCommit, + string? previousSourceCommit, BuildDTO build, string repoUri, List dependencyUpdates) @@ -405,7 +405,7 @@ private static DependencyCategories CreateDependencyCategories(List newDependencyUpdates, bool isForwardFlow) @@ -1167,7 +1167,7 @@ private async Task UpdateCodeFlowPullRequestAsync( private async Task CreateCodeFlowPullRequestAsync( SubscriptionUpdateWorkItem update, - string previousSourceSha, + string? previousSourceSha, SubscriptionDTO subscription, string prBranch, List dependencyUpdates, diff --git a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs index 930b4d2318..1fdc927593 100644 --- a/test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs +++ b/test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs @@ -53,7 +53,7 @@ protected override void RegisterServices(IServiceCollection services) services.AddSingleton(_forwardFlower.Object); services.AddSingleton(_gitClient.Object); - CodeFlowResult codeFlowRes = new CodeFlowResult(true, [], null, new Backflow("aaa1234", "bbb2345"), []); + CodeFlowResult codeFlowRes = new CodeFlowResult(true, [], new NativePath(VmrPath), "aaa1234", []); _forwardFlower.SetReturnsDefault(Task.FromResult(codeFlowRes)); _backFlower.SetReturnsDefault(Task.FromResult(codeFlowRes)); _gitClient.SetReturnsDefault(Task.CompletedTask);