Properly reset PCS branches during codeflow

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs

@@ -56,7 +56,8 @@ protected async Task<ILocalGitRepo> PrepareCloneInternalAsync(
         IReadOnlyCollection<string> remoteUris,
         IReadOnlyCollection<string> requestedRefs,
         string checkoutRef,
-        CancellationToken cancellationToken)
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default)
     {
         if (remoteUris.Count == 0)
         {
@@ -104,6 +105,19 @@ protected async Task<ILocalGitRepo> PrepareCloneInternalAsync(
 
         var repo = _localGitRepoFactory.Create(path);
         await repo.CheckoutAsync(checkoutRef);
+
+        if (resetToRemote)
+        {
+            // get the upstream branch for the currently checked out branch
+            var result = await _localGitRepo.RunGitCommandAsync(path, ["for-each-ref", "--format=%(upstream:short)", $"refs/heads/{checkoutRef}"]);
+            result.ThrowIfFailed("Couldn't get upstream branch for the current branch");
+            var upstream = result.StandardOutput.Trim();
+
+            // reset the branch to the remote one
+            result = await _localGitRepo.RunGitCommandAsync(path, ["reset", "--hard", upstream]);
+            result.ThrowIfFailed($"Couldn't reset to remote ref {upstream}");
+        }
+
         return repo;
     }
 

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/IVmrUpdater.cs

@@ -42,5 +42,6 @@ Task<bool> UpdateRepository(
         bool reapplyVmrPatches,
         bool lookUpBuilds,
         CancellationToken cancellationToken,
-        bool amendReapplyCommit = false);
+        bool amendReapplyCommit = false,
+        bool resetToRemoteWhenCloningRepo = false);
 }

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrBackFlower.cs

@@ -112,6 +112,7 @@ await _vmrCloneManager.PrepareVmrAsync(
             [build.GetRepository()],
             [build.Commit],
             build.Commit,
+            ShouldResetVmr,
             cancellationToken);
 
         // Prepare repo
@@ -132,6 +133,7 @@ await _vmrCloneManager.PrepareVmrAsync(
                 remotes,
                 [subscription.TargetBranch, targetBranch],
                 targetBranch,
+                ShouldResetClones,
                 cancellationToken);
             targetBranchExisted = true;
         }
@@ -142,11 +144,15 @@ await _vmrCloneManager.PrepareVmrAsync(
                 mapping,
                 remotes,
                 subscription.TargetBranch,
+                ShouldResetClones,
                 cancellationToken);
             await targetRepo.CreateBranchAsync(targetBranch);
             targetBranchExisted = false;
         }
 
         return (targetBranchExisted, mapping, targetRepo);
     }
+
+    // During backflow, we're targeting a specific repo branch, so we should make sure we reset local branch to the remote one
+    protected bool ShouldResetClones => true;
 }

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/PcsVmrForwardFlower.cs

@@ -91,6 +91,7 @@ public async Task<bool> FlowForwardAsync(
             mapping,
             remotes,
             build.Commit,
+            ShouldResetClones,
             cancellationToken);
 
         return await FlowForwardAsync(
@@ -104,4 +105,7 @@ public async Task<bool> FlowForwardAsync(
             discardPatches: true,
             cancellationToken);
     }
+
+    // During forward flow, we're targeting a specific remote VMR branch, so we should make sure our local branch is reset to it
+    protected override bool ShouldResetVmr => true;
 }

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/RepositoryCloneManager.cs

@@ -23,11 +23,13 @@ public interface IRepositoryCloneManager
     /// </summary>
     /// <param name="repoUri">Remote to fetch from</param>
     /// <param name="checkoutRef">Ref to check out at the end</param>
+    /// <param name="resetToRemote">Whether to reset the branch to the remote one</param>
     /// <returns>Path to the clone</returns>
     Task<ILocalGitRepo> PrepareCloneAsync(
         string repoUri,
         string checkoutRef,
-        CancellationToken cancellationToken);
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default);
 
     /// <summary>
     /// Clones a repo in a target directory, fetches from given remotes and checks out a given ref.
@@ -37,12 +39,14 @@ Task<ILocalGitRepo> PrepareCloneAsync(
     /// <param name="mapping">VMR repo mapping to associate the clone with</param>
     /// <param name="remotes">Remotes to fetch from</param>
     /// <param name="checkoutRef">Ref to check out at the end</param>
+    /// <param name="resetToRemote">Whether to reset the branch to the remote one</param>
     /// <returns>Path to the clone</returns>
     Task<ILocalGitRepo> PrepareCloneAsync(
         SourceMapping mapping,
         IReadOnlyCollection<string> remotes,
         string checkoutRef,
-        CancellationToken cancellationToken);
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default);
 
     /// <summary>
     /// Prepares a clone of a repository by fetching from given remotes one-by-one until all requested commits are available.
@@ -52,13 +56,15 @@ Task<ILocalGitRepo> PrepareCloneAsync(
     /// <param name="remoteUris">Remotes to fetch one by one</param>
     /// <param name="requestedRefs">List of refs that need to be available</param>
     /// <param name="checkoutRef">Ref to check out at the end</param>
+    /// <param name="resetToRemote">Whether to reset the branch to the remote one</param>
     /// <returns>Path to the clone</returns>
     Task<ILocalGitRepo> PrepareCloneAsync(
         SourceMapping mapping,
         IReadOnlyCollection<string> remoteUris,
         IReadOnlyCollection<string> requestedRefs,
         string checkoutRef,
-        CancellationToken cancellationToken);
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default);
 }
 
 /// <summary>
@@ -88,46 +94,34 @@ public async Task<ILocalGitRepo> PrepareCloneAsync(
         SourceMapping mapping,
         IReadOnlyCollection<string> remoteUris,
         string checkoutRef,
-        CancellationToken cancellationToken)
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default)
     {
         if (remoteUris.Count == 0)
         {
             throw new ArgumentException("No remote URIs provided to clone");
         }
 
-        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, cleanup, cancellationToken);
-            cleanup = false;
-        }
-
-        var repo = _localGitRepoFactory.Create(path);
-        await repo.CheckoutAsync(checkoutRef);
-        return repo;
+        return await PrepareCloneInternalAsync(mapping.Name, remoteUris, [checkoutRef], checkoutRef, resetToRemote, cancellationToken);
     }
 
     public async Task<ILocalGitRepo> PrepareCloneAsync(
         string repoUri,
         string checkoutRef,
-        CancellationToken cancellationToken)
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default)
     {
         // We store clones in directories named as a hash of the repo URI
         var cloneDir = StringUtils.GetXxHash64(repoUri);
-        var path = await PrepareCloneInternal(repoUri, cloneDir, performCleanup: true, cancellationToken);
-        var repo = _localGitRepoFactory.Create(path);
-        await repo.CheckoutAsync(checkoutRef);
-        return repo;
+        return await PrepareCloneInternalAsync(cloneDir, [repoUri], [checkoutRef], checkoutRef, resetToRemote, cancellationToken);
     }
 
     public Task<ILocalGitRepo> PrepareCloneAsync(
         SourceMapping mapping,
         IReadOnlyCollection<string> remoteUris,
         IReadOnlyCollection<string> requestedRefs,
         string checkoutRef,
-        CancellationToken cancellationToken)
-        => PrepareCloneInternalAsync(mapping.Name, remoteUris, requestedRefs, checkoutRef, cancellationToken);
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default)
+        => PrepareCloneInternalAsync(mapping.Name, remoteUris, requestedRefs, checkoutRef, resetToRemote, cancellationToken);
 }

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs

@@ -506,7 +506,7 @@ .. mapping.Exclude.Select(VmrPatchHandler.GetExclusionRule),
         string targetBranch,
         CancellationToken cancellationToken)
     {
-        await _vmrCloneManager.PrepareVmrAsync([build.GetRepository()], [build.Commit], build.Commit, cancellationToken);
+        await _vmrCloneManager.PrepareVmrAsync([build.GetRepository()], [build.Commit], build.Commit, ShouldResetVmr, cancellationToken);
 
         SourceMapping mapping = _dependencyTracker.GetMapping(mappingName);
         ISourceComponent repoInfo = _sourceManifest.GetRepoVersion(mappingName);
@@ -529,6 +529,7 @@ await _repositoryCloneManager.PrepareCloneAsync(
                 remotes,
                 [baseBranch, targetBranch],
                 targetBranch,
+                ShouldResetVmr,
                 cancellationToken);
             targetBranchExisted = true;
         }
@@ -545,4 +546,6 @@ await _repositoryCloneManager.PrepareCloneAsync(
 
     protected override NativePath GetEngCommonPath(NativePath sourceRepo) => sourceRepo / VmrInfo.SourceDirName / "arcade" / Constants.CommonScriptFilesPath;
     protected override bool TargetRepoIsVmr() => false;
+    // During backflow, we're flowing a specific VMR commit that the build was built from, so we should just check it out
+    protected virtual bool ShouldResetVmr => false;
 }

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCloneManager.cs

@@ -21,12 +21,14 @@ public interface IVmrCloneManager
     /// <param name="remoteUris">Remotes to fetch from one by one</param>
     /// <param name="requestedRefs">List of refs that need to be available</param>
     /// <param name="checkoutRef">Ref to check out at the end</param>
+    /// <param name="resetToRemote">Whether to reset to the remote ref after fetching</param>
     /// <returns>Path to the clone</returns>
     Task<ILocalGitRepo> PrepareVmrAsync(
         IReadOnlyCollection<string> remoteUris,
         IReadOnlyCollection<string> requestedRefs,
         string checkoutRef,
-        CancellationToken cancellationToken);
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default);
 }
 
 public class VmrCloneManager : CloneManager, IVmrCloneManager
@@ -54,7 +56,8 @@ public async Task<ILocalGitRepo> PrepareVmrAsync(
         IReadOnlyCollection<string> remoteUris,
         IReadOnlyCollection<string> requestedRefs,
         string checkoutRef,
-        CancellationToken cancellationToken)
+        bool resetToRemote = false,
+        CancellationToken cancellationToken = default)
     {
         // This makes sure we keep different VMRs separate
         // We expect to have up to 3:
@@ -69,6 +72,7 @@ public async Task<ILocalGitRepo> PrepareVmrAsync(
             remoteUris,
             requestedRefs,
             checkoutRef,
+            resetToRemote,
             cancellationToken);
 
         _vmrInfo.VmrPath = vmr.Path;

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs

@@ -216,6 +216,7 @@ await _vmrCloneManager.PrepareVmrAsync(
                 [vmrUri],
                 [baseBranch, targetBranch],
                 targetBranch,
+                ShouldResetVmr,
                 cancellationToken);
             branchExisted = true;
         }
@@ -227,6 +228,7 @@ await _vmrCloneManager.PrepareVmrAsync(
                 [vmrUri],
                 [baseBranch],
                 baseBranch,
+                ShouldResetVmr,
                 cancellationToken);
 
             await vmr.CheckoutAsync(baseBranch);
@@ -281,6 +283,7 @@ protected override async Task<bool> SameDirectionFlowAsync(
                 reapplyVmrPatches: true,
                 lookUpBuilds: true,
                 amendReapplyCommit: true,
+                resetToRemoteWhenCloningRepo: ShouldResetClones,
                 cancellationToken: cancellationToken);
         }
         catch (PatchApplicationFailedException e)
@@ -301,7 +304,12 @@ protected override async Task<bool> SameDirectionFlowAsync(
                 _vmrInfo.SourceManifestPath,
                 line => line.Contains(lastFlow.SourceSha),
                 lastFlow.TargetSha);
-            var vmr = await _vmrCloneManager.PrepareVmrAsync([_vmrInfo.VmrUri], [previousFlowTargetSha], previousFlowTargetSha, cancellationToken);
+            var vmr = await _vmrCloneManager.PrepareVmrAsync(
+                [_vmrInfo.VmrUri],
+                [previousFlowTargetSha],
+                previousFlowTargetSha,
+                ShouldResetVmr,
+                cancellationToken);
             await vmr.CreateBranchAsync(targetBranch, overwriteExistingBranch: true);
 
             // Reconstruct the previous flow's branch
@@ -337,6 +345,7 @@ await FlowCodeAsync(
                 reapplyVmrPatches: true,
                 lookUpBuilds: true,
                 amendReapplyCommit: true,
+                resetToRemoteWhenCloningRepo: ShouldResetClones,
                 cancellationToken: cancellationToken);
         }
 
@@ -413,9 +422,14 @@ .. submodules.Select(s => s.Path).Distinct().Select(VmrPatchHandler.GetExclusion
             reapplyVmrPatches: true,
             lookUpBuilds: true,
             amendReapplyCommit: true,
+            resetToRemoteWhenCloningRepo: ShouldResetClones,
             cancellationToken: cancellationToken);
     }
 
     protected override NativePath GetEngCommonPath(NativePath sourceRepo) => sourceRepo / Constants.CommonScriptFilesPath;
     protected override bool TargetRepoIsVmr() => true;
+    // When flowing local repos, we should never reset branches to the remote ones, we might lose some changes devs wanted
+    protected virtual bool ShouldResetVmr => false;
+    // In forward flow, we're flowing a specific commit, so we should just check it out, no need to sync local branch to remote
+    protected virtual bool ShouldResetClones => false;
 }

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInitializer.cs

@@ -200,6 +200,7 @@ private async Task InitializeRepository(
             remotes,
             new[] { update.TargetRevision },
             update.TargetRevision,
+            resetToRemote: false,
             cancellationToken);
 
         cancellationToken.ThrowIfCancellationRequested();

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrPatchHandler.cs

@@ -561,7 +561,7 @@ private async Task<List<VmrIngestionPatch>> GetPatchesForSubmoduleChange(
         CancellationToken cancellationToken)
     {
         var checkoutCommit = change.Before == Constants.EmptyGitObject ? change.After : change.Before;
-        var clonePath = await _cloneManager.PrepareCloneAsync(change.Url, checkoutCommit, cancellationToken);   
+        var clonePath = await _cloneManager.PrepareCloneAsync(change.Url, checkoutCommit, resetToRemote: false, cancellationToken);   
 
         // We are only interested in filters specific to submodule's path
         ImmutableArray<string> GetSubmoduleFilters(IReadOnlyCollection<string> filters)

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrUpdater.cs

@@ -108,7 +108,8 @@ public async Task<bool> UpdateRepository(
         bool reapplyVmrPatches,
         bool lookUpBuilds,
         CancellationToken cancellationToken,
-        bool amendReapplyPatchesCommit = false)
+        bool amendReapplyPatchesCommit = false,
+        bool resetToRemoteWhenCloningRepo = false)
     {
         await _dependencyTracker.RefreshMetadata();
 
@@ -165,6 +166,7 @@ public async Task<bool> UpdateRepository(
                     generateCodeowners,
                     generateCredScanSuppressions,
                     discardPatches,
+                    resetToRemoteWhenCloningRepo,
                     cancellationToken);
 
                 if (reapplyVmrPatches)
@@ -190,7 +192,8 @@ private async Task<IReadOnlyCollection<VmrIngestionPatch>> UpdateRepositoryInter
         bool generateCodeowners,
         bool generateCredScanSuppressions,
         bool discardPatches,
-        CancellationToken cancellationToken)
+        bool resetToRemoteWhenCloningRepo = false,
+        CancellationToken cancellationToken = default)
     {
         VmrDependencyVersion currentVersion = _dependencyTracker.GetDependencyVersion(update.Mapping)
             ?? throw new Exception($"Failed to find current version for {update.Mapping.Name}");
@@ -242,6 +245,7 @@ await UpdateTargetVersionOnly(
             remotes,
             requestedRefs: new[] { currentVersion.Sha, update.TargetRevision },
             checkoutRef: update.TargetRevision,
+            resetToRemoteWhenCloningRepo,
             cancellationToken);
 
         update = update with
@@ -370,6 +374,7 @@ private async Task<bool> UpdateRepositoryRecursively(
                     generateCodeowners,
                     generateCredScanSuppressions,
                     discardPatches,
+                    resetToRemoteWhenCloningRepo: false,
                     cancellationToken);
             }
             catch (EmptySyncException e) when (e.Message.Contains("is already at"))

src/ProductConstructionService/ProductConstructionService.Api/InitializationBackgroundService.cs

@@ -28,7 +28,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
             var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(stoppingToken, new CancellationTokenSource(TimeSpan.FromMinutes(20)).Token);
 
             IVmrCloneManager vmrCloneManager = scope.ServiceProvider.GetRequiredService<IVmrCloneManager>();
-            await vmrCloneManager.PrepareVmrAsync([options.VmrUri], ["main"], "main", linkedTokenSource.Token);
+            await vmrCloneManager.PrepareVmrAsync([options.VmrUri], ["main"], "main", resetToRemote: true, linkedTokenSource.Token);
             linkedTokenSource.Token.ThrowIfCancellationRequested();
 
             telemetryScope.SetSuccess();

test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/GitOperationsHelper.cs

@@ -135,7 +135,7 @@ public async Task MergePrBranch(NativePath repo, string branch, string targetBra
         await DeleteBranch(repo, branch);
     }
 
-    private async Task ConfigureGit(NativePath repo)
+    public async Task ConfigureGit(NativePath repo)
     {
         await _processManager.ExecuteGit(repo, "config", "user.email", DarcLib.Constants.DarcBotEmail);
         await _processManager.ExecuteGit(repo, "config", "user.name", DarcLib.Constants.DarcBotName);