Conversation
Add a development comparer for MSBuildWorkspace and compiler-log workspaces, align SolutionReader naming and document metadata, and centralize multi-target project detection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0bfbac11-17ce-436f-b287-7443cdc00269
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #388 +/- ##
==========================================
+ Coverage 95.97% 95.98% +0.01%
==========================================
Files 54 54
Lines 6163 6182 +19
Branches 709 710 +1
==========================================
+ Hits 5915 5934 +19
Misses 130 130
Partials 118 118 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jjonescz
reviewed
Sep 2, 2026
| var data = new CompilerCallData( | ||
| compilerCall, | ||
| compilationName, | ||
| assemblyFileName, |
Contributor
There was a problem hiding this comment.
In ReadCompilationData we pass compilerCallData.AssemblyFileName (with .dll) to CreateCSharpCompilationData but args.CompilationName (without .dll) to CreateVisualBasicCompilationData, that seems inconsistent.
|
|
||
| ```bash | ||
| dotnet run --project src/Basic.CompilerLog.WorkspaceComparer -- <project-or-solution> <compiler-log> | ||
| ``` |
Contributor
There was a problem hiding this comment.
Should we reuse projects generated by CompilerLogFixture and test WorkspaceComparer.Compare on them and assert that there are no differences?
| if (compilerCall.TargetFramework is not null && | ||
| _multiTargetProjectPaths.Contains(compilerCall.ProjectFilePath)) | ||
| { | ||
| projectName = $"{projectName} ({compilerCall.TargetFramework})"; |
Contributor
There was a problem hiding this comment.
It looks like roslyn uses a slightly different style - without a space: https://github.com/dotnet/roslyn/blob/4cac4334c3ed532aea57169ebb11db0934a01ea8/src/Workspaces/MSBuild/Core/MSBuild/MSBuildProjectLoader.Worker.cs#L188-L196
|
|
||
| var fileName = sourceTextData.FilePath; | ||
| var filePath = sourceTextData.FilePath; | ||
| var fileName = Path.GetFileName(filePath); |
Contributor
There was a problem hiding this comment.
MSBuildWorkspace uses Link metadata too for linked documents.
| } | ||
|
|
||
| Console.Error.WriteLine($"{differences.Length} difference(s)"); | ||
| return 0; |
Contributor
There was a problem hiding this comment.
Should we return non-zero exit code when there are differences?
Keep the extensionless Roslyn assembly name separate from the compiler output file name so workspace output paths remain valid without changing compilation identity. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0bfbac11-17ce-436f-b287-7443cdc00269
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The PR introduces a few concrete operational/performance issues (mixed MSBuild package versions under central pinning, redundant full enumerations, and a comparer tool that always returns success) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 4
New issues introduced by this change (4)
| Severity | Finding |
|---|---|
 |
src/Basic.CompilerLog.App/CompilerLogApp.cs — reader.GetMultiTargetedProjectFilePaths(predicate) will call back into the reader and… |
|
src/Basic.CompilerLog.Util/SolutionReader.cs — reader.GetMultiTargetedProjectFilePaths() re-reads all compiler calls even though compilerCalls… |
|
src/Basic.CompilerLog.WorkspaceComparer/Program.cs — The comparer prints the number of differences but always returns exit code 0, which makes it hard… |
|
src/Directory.Packages.props — Microsoft.Build.Utilities.Core is pinned to 17.5.0 while other MSBuild packages are pinned to… |
What changed in this PR
This PR adds a new workspace comparison development tool and updates SolutionReader/readers so compiler-log reconstructed workspaces more closely match MSBuildWorkspace conventions (project naming, output paths, document/folder handling, additional files, and multi-targeting behavior), backed by new parity tests.
Changes:
- Add
Basic.CompilerLog.WorkspaceComparerto compareMSBuildWorkspacesolutions vsSolutionReadersolutions and report structural differences. - Align
SolutionReader’s project/document metadata (names, folders, output file path, reference mapping) with typical workspace expectations, including multi-targeting behavior. - Add/extend unit tests and fixtures to cover the new workspace-shape behaviors for both compiler logs and binary logs.
| File | Description |
|---|---|
| src/Directory.Packages.props | Adds MSBuild/Roslyn workspace-related package versions for the comparer tool. |
| src/Basic.CompilerLog.WorkspaceComparer/WorkspaceComparer.cs | Implements structural comparison of two Roslyn solutions/projects/documents/references/options. |
| src/Basic.CompilerLog.WorkspaceComparer/Program.cs | CLI entrypoint to load MSBuild workspace + compiler-log workspace and print diffs. |
| src/Basic.CompilerLog.WorkspaceComparer/Basic.CompilerLog.WorkspaceComparer.csproj | New net10.0 dev-tool project and dependencies. |
| src/Basic.CompilerLog.Util/SolutionReader.cs | Updates reconstructed workspace shape (names, assembly/output, folders, references) and adds multi-target awareness. |
| src/Basic.CompilerLog.Util/Impl/BasicAnalyzerHostOnDisk.cs | Removes trailing whitespace. |
| src/Basic.CompilerLog.Util/Impl/BasicAnalyzerHostInMemory.cs | Removes trailing whitespace. |
| src/Basic.CompilerLog.Util/Extensions.cs | Adds ICompilerCallReader.GetMultiTargetedProjectFilePaths extension for centralized detection. |
| src/Basic.CompilerLog.Util/CompilerLogReader.cs | Minor named-argument tweak for CompilerCallState construction. |
| src/Basic.CompilerLog.Util/CompilerCallData.cs | Adds internal OutputFileName to preserve output filename (incl. extension) separately from assembly name. |
| src/Basic.CompilerLog.Util/BinaryLogReader.cs | Fixes additional-file classification and refines output/assembly naming in CompilerCallData. |
| src/Basic.CompilerLog.UnitTests/SolutionReaderTests.cs | Adds parity tests for naming, folders, references, analyzers, metadata reference display, and output path behavior. |
| src/Basic.CompilerLog.UnitTests/CompilerLogFixture.cs | Extends fixtures for nested docs, analyzer config, additional files under folders, and a multi-target project reference scenario. |
| src/Basic.CompilerLog.UnitTests/BinaryLogReaderTests.cs | Adds test ensuring additional files are classified as AdditionalText. |
| src/Basic.CompilerLog.App/CompilerLogApp.cs | Uses centralized multi-target detection extension for naming compiler calls. |
| docs/overview.md | Documents the new workspace comparer dev tool and usage. |
| Basic.CompilerLog.slnx | Adds the new WorkspaceComparer project to the solution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
1000
to
1004
| var compilerCalls = ReadAllCompilerCalls(reader, predicate); | ||
|
|
||
| // Set used to determine if the projects are single or multi-targeted. | ||
| var isMultiTargetedMap = new Dictionary<string, bool>(PathUtil.Comparer); | ||
| foreach (var compilerCall in compilerCalls) | ||
| { | ||
| if (compilerCall.Kind == CompilerCallKind.Regular) | ||
| { | ||
| ref bool isMultiTargeted = ref CollectionsMarshal.GetValueRefOrAddDefault(isMultiTargetedMap, compilerCall.ProjectFilePath, out var exists); | ||
| isMultiTargeted = exists; | ||
| } | ||
| } | ||
| var multiTargetedProjectFilePaths = reader.GetMultiTargetedProjectFilePaths(predicate); | ||
|
|
||
| // Generate unique names for each compiler call |
Comment on lines
31
to
+33
| var map = new SortedDictionary<int, (CompilerCall, ProjectId)>(); | ||
| var compilerCalls = reader.ReadAllCompilerCalls(); | ||
| _multiTargetProjectPaths = reader.GetMultiTargetedProjectFilePaths(); |
| } | ||
|
|
||
| Console.Error.WriteLine($"{differences.Length} difference(s)"); | ||
| return 0; |
| <PackageVersion Include="Microsoft.CodeAnalysis.Workspaces.MSBuild" Version="$(_RoslynVersion)" /> | ||
| <PackageVersion Include="Microsoft.Build.Framework" Version="17.11.48" /> | ||
| <PackageVersion Include="Microsoft.Build.Locator" Version="1.11.2" /> | ||
| <PackageVersion Include="Microsoft.Build.Utilities.Core" Version="17.5.0" /> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MSBuildWorkspaceoutput with workspaces reconstructed from compiler logsICompilerCallReaderextensionTesting
dotnet build Basic.CompilerLog.slnx -warnaserror --no-restore