Skip to content

Remove RoslynForTestingButNotForVSLayer hack#19375

Open
Copilot wants to merge 3 commits intomainfrom
copilot/remove-roslyn-testing-layer
Open

Remove RoslynForTestingButNotForVSLayer hack#19375
Copilot wants to merge 3 commits intomainfrom
copilot/remove-roslyn-testing-layer

Conversation

Copy link
Contributor

Copilot AI commented Mar 2, 2026

RoslynForTestingButNotForVSLayer was a workaround that pinned Roslyn to 4.14.0 in FSharp.Test.Utilities to avoid assembly conflicts introduced by VSTest at runtime. VSTest has since been fully removed and all test projects have migrated to MTP/xunit3, making the pin obsolete.

Changes

  • eng/Versions.props: Remove RoslynForTestingButNotForVSLayer property and its explanatory comment
  • tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj: Switch Microsoft.CodeAnalysis.Workspaces.Common and Microsoft.CodeAnalysis.CSharp from the now-deleted pin to $(MicrosoftCodeAnalysisCSharpVersion) — the same maestro-managed version used by the rest of the codebase
Original prompt

Ok good. Properly summarize the reasoning, exact todo actions, and form a new PR out of this idea. Get rid of RoslynForTestingButNotForVSLayer

[Chronological Review] 1. User asked to investigate CI failures on PR #19021 (MSBuild dep update branch) in dotnet/fsharp repo 2. Fetched CI errors from AzDo build 1314759 - found 17,727 test failures, mostly net472 SCI version mismatch 3. Initial wrong fix: edited auto-managed Version.Details.xml/.props directly → user corrected me 4. Planned properly: audit which System.* packages are actually needed vs transitive 5. Removed 6 System.* entries from maestro-managed files, added hand-maintained versions in Versions.props 6. First attempt used 10.0.2 → CI still failed (build 1315831) - still SCI errors on net472 7. Deep investigation: used System.Reflection.Metadata to inspect MSBuild net472 DLL assembly references → found MSBuild references SCI assembly version 9.0.0.11, not 10.0.0.0 8. Switched to 9.0.13 (latest 9.0.x patch) → pushed 9. Investigated MSBuild's version management: manually maintained by MichalPavlik, not maestro 10. CI run with 9.0.13 showed vs_release still failing (FSharp.Editor.Tests gets SCI 10.0.2 transitively from MSBuild NuGet dep) 11. Attempted Directory.Build.targets pin with Include → caused NU1504 (duplicate) and MSB3277 (assembly conflict) errors across all platforms 12. Reverted targets change, pushed clean 9.0.13 13. User asked to plan ahead for vs_release fix 14. Discovered my earlier Python-based assembly version analysis was WRONG - SCI 10.0.2 net462 actually HAS compatible System.Memory deps (assembly version 4.0.5.0 matches System.Memory 4.6.3) 15. User suggested combining with Roslyn update PR #19229 (5.0.0 → 5.6.0) since both updates are needed 16. Merged Roslyn update, switched back to 10.0.2, bumped VS Shell packages to 18.0.2188-preview.1 17. All 6 solutions restore clean, builds pass, 42/42 tests pass 18. User asked about cleanup opportunities → investigated RoslynForTestingButNotForVSLayer hack 19. Found the hack is no longer needed since VSTest was fully removed and MTP migration completed

[Intent Mapping]

  • Original: Fix CI failures on MSBuild dep update PR [main] Update dependencies from dotnet/msbuild #19021
  • Fix the SCI version mismatch on net472
  • Clean up broken maestro runtime subscription
  • Combine MSBuild and Roslyn updates atomically
  • Investigate removable dependencies in vsintegration layer
  • Investigate whether RoslynForTestingButNotForVSLayer hack can be removed

[Technical Inventory]

  • dotnet/fsharp repo, branch: darc-main-d37c64d2-254e-47a7-9911-2f2279030766, PR [main] Update dependencies from dotnet/msbuild #19021
  • MSBuild 18.6.0-preview-26128-01 (from 18.1.0-preview-25515-01)
  • Roslyn 5.6.0-2.26151.2 (from 5.0.0-2.25480.7, merged from PR [main] Update dependencies from dotnet/roslyn #19229)
  • System.Collections.Immutable 10.0.2 (from 9.0.0)
  • System.Reflection.Metadata 10.0.2 (from 9.0.0)
  • System.Diagnostics.DiagnosticSource 10.0.2 (from 9.0.0)
  • VS Shell packages 18.0.2188-preview.1 (from 18.0.2101-preview.1)
  • AzDo CI pipeline: fsharp-ci, definition 92, org dnceng-public
  • SystemPackagesVersion shared prop pattern in Versions.props
  • net472 binding redirect mechanics, AutoGenerateBindingRedirects
  • NuGet "nearest wins" resolution rules
  • PrivateAssets="all" ExcludeAssets="runtime" pattern in vsintegration

[Code Archaeology]

  • eng/Version.Details.xml: Removed 6 System.* Dependency entries, updated Roslyn versions + SHAs
  • eng/Version.Details.props: Removed 12 lines of System.* version properties, updated Roslyn versions
  • eng/Versions.props: Added SystemPackagesVersion=10.0.2 with 3 derived properties, bumped VS Shell version, cleaned comment
  • FSharpBuild.Directory.Build.targets: Attempted pin (Include/Update) → reverted, no changes
  • Key files examined: FSharp.Build.fsproj, FSharp.Compiler.Service.fsproj, FSharp.Test.Utilities.fsproj, FSharp.Editor.fsproj, FSharp.Editor.Tests.fsproj, all test .fsproj files

[Progress Assessment]

  • MSBuild + Roslyn + System.* version update: DONE, pushed, CI running
  • Broken maestro subscription cleanup: DONE
  • vs_release fix: Addressed by combining Roslyn 5.6.0 update (aligns SCI versions)
  • RoslynForTestingButNotForVSLayer investigation: DONE, proposal ready, awaiting user confirmation for separate PR
  • Directory.Build.targets pin: REVERTED (approach was flawed)

[Context Validation]

  • Current commit: 6c35fbd pushed to branch
  • CI build 1316039 was the broken one (with targets Include), new build should be running on 6c35fbd
  • All 6 solutions (FSharp.Compiler.Service, FSharp, VisualFSharp, FSharp.Editor, LSPSolutionSlim, Microsoft.FSharp.Compiler) restore clean
  • 42/42 FSharp.Build.UnitTests pass locally

[Recent Commands Analysis]

  • Investigated RoslynForTestingButNotForVSLayer hack: checked git history, VSTest removal status, MTP migration
  • Verified zero Microsoft.NET.Test.Sdk references remain in repo
  • Checked Roslyn 4.14.0 vs 5.6.0 SCI transitive deps
  • Proposed removal plan for separate PR
  • User said "wait for my confirmation" before making changes
1....

Created from Copilot CLI via the copilot delegate command.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…isCSharpVersion

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove RoslynForTestingButNotForVSLayer Remove RoslynForTestingButNotForVSLayer hack Mar 2, 2026
Copilot finished work on behalf of T-Gro March 2, 2026 16:17
@T-Gro T-Gro marked this pull request as ready for review March 2, 2026 16:37
@T-Gro T-Gro requested a review from a team as a code owner March 2, 2026 16:37
@T-Gro T-Gro enabled auto-merge (squash) March 2, 2026 16:37
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

✅ No release notes required

@T-Gro T-Gro requested a review from abonie March 4, 2026 13:24
@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants