Improve conflict resolution#6267
Conversation
There was a problem hiding this comment.
Pull request overview
This PR targets codeflow reliability in the Virtual Mono Repo (VMR) forward-flow path by improving how git conflicts are auto-resolved—especially in scenarios involving recreated/rewound flows where conflict shapes can include delete/modify cases and conflicts outside the mapped src/<repo> folder.
Changes:
- Auto-resolve forward-flow conflicts that occur outside the mapped repo folder by preferring one side.
- Enhance
LocalGitClient.ResolveConflictto resolve delete/modify conflicts by inspectinggit status --porcelainand usinggit rm/git addwhen appropriate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/ForwardFlowConflictResolver.cs | Adds a rule to auto-resolve conflicts outside the flowed repo folder and refactors repo-source-path reuse. |
| src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs | Reworks conflict resolution to handle delete/modify by interpreting porcelain status and choosing rm/add/checkout accordingly. |
| ?? throw new Exception($"No status entry found for {file} in {repoPath} - is it actually conflicted?"); | ||
|
|
||
| if (line.Length < 2) | ||
| { | ||
| throw new Exception($"Unexpected git status output for {file} in {repoPath}: '{line}'"); |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| result.ThrowIfFailed($"Failed to stage resolved conflict in {file} in {repoPath}"); | ||
| // Map each unmerged status code to (ourBlob, theirBlob) - whether each | ||
| // side contributed a blob to the index. | ||
| var (ourBlob, theirBlob) = code switch |
There was a problem hiding this comment.
It's kinda hard to validate this and there's no tests to support this. Would it be possible to make AI generate real changes to get these combinations and verify we get the right outcomes?
Could be a follow-up if you're sure about this.
There was a problem hiding this comment.
Yeah I think this part wouldn't be hard to validate, should be pretty easy to make these conflicts in a small git repo
| var updateIndex = await _processManager.ExecuteGit( | ||
| _repoPath, | ||
| ["update-index", "--index-info"], | ||
| standardInput: indexInfo.ToString()); | ||
| updateIndex.ThrowIfFailed("git update-index --index-info failed"); |
There was a problem hiding this comment.
Interesting - this is what I was thinking of doing even in resolve-conflict where we could "materialize" a conflict of a specific kind if we know there should be one. But then we were able to let the rebase take care of this.
#6265
It looks like some git weirdness is happening in the vstest flows.
We're rewinding previous flows, and by doing so the work branch is "losing" some changes outside of the
src/repowe're flowing to, as expected. But when we try to rebase the workbranch on top of the origin branch, these files that we're lost on the workbranch conflict with the ones in the target branch.Not sure what is causing this but it's easy enough to fix.
I'm also failing to replicate in a test.
I think it might have to do something with the unsafe flow? And git is not able to figure it out.
In any case this fixes the issue