Conversation
src/Workspaces/MSBuild/BuildHost/MSBuild/ProjectFile/ProjectInstanceReader.cs
Show resolved
Hide resolved
src/Workspaces/MSBuild/BuildHost/MSBuild/ProjectFile/ProjectInstanceReader.cs
Show resolved
Hide resolved
jasonmalinowski
left a comment
There was a problem hiding this comment.
Leaving comments so far; no real concerns so far.
src/Features/ExternalAccess/HotReload/Api/HotReloadMSBuildWorkspace.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
| ImmutableArray<DocumentInfo> MapDocuments(ProjectId mappedProjectId, IReadOnlyList<DocumentInfo> documents) | ||
| => documents.Select(docInfo => | ||
| { | ||
| // TODO: can there be multiple documents of the same path in the project? |
There was a problem hiding this comment.
Just to document it somewhere, yes that can happen if a file is added twice. Or is added as both a source file and an additional file. None of these are "real" scenarios, but can happen in broken project scenarios. The only expectation is we don't crash. There might be a bug here since maybe if a source file is being added as an additional file we might try to reuse the same ID which would be bad in this case. Maybe file a bug for tracking, if we refactor this later this code might go away.
jasonmalinowski
left a comment
There was a problem hiding this comment.
Fantastic job cleaning this up. Although I imagine there's further refactorings here that will delete some of the code that migrated, I'm much happier to see all the "complexity" of the loading be in Roslyn rather than being in the SDK. That means we can clean it up without cross-cutting changes.
src/Workspaces/MSBuild/BuildHost/MSBuild/ProjectFile/ProjectInstanceReader.cs
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Refactors BuildHost and MSBuildProjectLoader to allow dotnet-watch to populate workspace in-proc based on already evaluated ProjectInstances (loaded via ProjectGraph).
ProjectFileinto separate types:XyzProjectCommandLineProvider.ProjectFileInfobased onProjectInstanceinto a separate type:ProjectInstanceReader. The reader takes project instance and optionally MSBuildProjectand extracts the necessary info from its properties and items. TheProjectis optional - it is only needed to populateFileGlobs.HotReloadMSBuildWorkspace- this is a replacement for dotnet-watch's IncrementalMSBuildWorkspaceIProjectFileInfoProviderthat abstract away retrieval ofProjectFileInfos.MSBuildWorkspaceuses an implementation that dispatches to OOP whileHotReloadMSBuildWorkspaceuses impl that calculates the information directly (in-proc) from project instances provided by dotnet-watch.dotnet-watch usage: dotnet/sdk#52163