Add BuildHost APIs for loading an in-memory project #78303

RikkiGibson · 2025-04-24T20:40:35Z

Part of dotnet/sdk#48176
Related to #78208

We need to be able to create a project (XML) in memory and run a design time build on it. Adding these APIs is a prerequisite for us doing that.

src/Workspaces/MSBuild/BuildHost/Build/ProjectBuildManager.cs

RikkiGibson · 2025-04-24T22:28:24Z

I'm wondering if this PR is not correctly handling batch builds, or whether that scenario is relevant.

src/Workspaces/MSBuild/Test/NetCoreTests.cs

src/Workspaces/MSBuild/Core/Rpc/RemoteBuildHost.cs

jasonmalinowski · 2025-04-24T23:58:47Z

src/Workspaces/MSBuild/BuildHost/MSBuild/ProjectFile/ProjectFileLoader.cs

+        }
+
+        // load project file
+        var stream = new MemoryStream(Encoding.UTF8.GetBytes(projectContent));


Do we need to tell MSBuild somewhere else that we're still encoding with UTF8? Or will we have a BOM there for it to figure out?

Do we need to tell MSBuild somewhere else that we're still encoding with UTF8?

That is not clear to me. It is also not clear to me how that would be happening in the ordinary on-disk project case.

Or will we have a BOM there for it to figure out?

It looks like the Unicode standard (ctrl+f for "neither required nor recommended") does not recommend using a BOM with UTF8. I think that GetBytes does not include a BOM.

The overload of XmlReader.Create we are using says:

The XmlReader scans the first bytes of the stream looking for a byte order mark or other sign of encoding. When encoding is determined, the encoding is used to continue reading the stream, and processing continues parsing the input as a stream of (Unicode) characters.

So should we have some way to thread the encoding through to avoid mojibake if the XmlReader doesn't infer it correctly?

Or should we have a UTF-8 BOM precisely so it can infer in this case?

The XML standard (that sure is a nice anchor name..) says:

In the absence of information provided by an external transport protocol (e.g. HTTP or MIME), it is a fatal error for an entity including an encoding declaration to be presented to the XML processor in an encoding other than that named in the declaration, or for an entity which begins with neither a Byte Order Mark nor an encoding declaration to use an encoding other than UTF-8. Note that since ASCII is a subset of UTF-8, ordinary ASCII entities do not strictly need an encoding declaration.

In other words, when the document lacks a BOM, and lacks <?xml encoding='something'?>, the standard requires UTF-8 be used to decode it.

Therefore there should be no need to add a BOM or encoding tag. I will add a comment.

jasonmalinowski · 2025-04-25T00:13:27Z

I'm wondering if this PR is not correctly handling batch builds, or whether that scenario is relevant.

It might not be -- I think we use the batch build case if we're loading an entire solution at once, and that would only happen in an OpenSolutionAsync() case. So that might not be needed right now.

But that said, at some point we'll probably need to ask how we're going to add support to MSBuildWorkspace for these projects too. And you'll need it then.

RikkiGibson · 2025-04-25T23:42:01Z

I was struggling to deal with the number of overloads with the same name here. So I tried to improve organization marginally by separating the public and private methods.

jasonmalinowski · 2025-04-26T00:07:49Z

src/Workspaces/MSBuild/Core/Rpc/RemoteBuildHost.cs

@@ -33,6 +33,16 @@ public async Task<RemoteProjectFile> LoadProjectFileAsync(string projectFilePath
        return new RemoteProjectFile(_client, remoteProjectFileTargetObject);
    }

+    /// <summary>Permits loading a project file which only exists in-memory, for example, for file-based program scenarios.</summary>
+    /// <param name="projectFilePath">A path to a project file which may or may not exist on disk. Note that an extension that is known by MSBuild, such as .csproj or .vbproj, should be used here.</param>


Excellent warning to add! 😄

...except this should go on the interface since that's what's actually seen by consumers.

I think this is a good place to document it. This is the API which the project system will call. But, I'm copying to doc to IBuildHost.

jasonmalinowski

Approved with one concern that I'm still not entirely sure how encoding is going to be handled when we convert the string to a UTF8 byte array. Having a test that ensures we round-trip that might be good to add. I'm OK though if we want to defer checking that and merging this, with the promise we file a bug and promise to fix it. 😄

jasonmalinowski · 2025-04-26T00:09:13Z

src/Workspaces/MSBuild/BuildHost/MSBuild/ProjectFile/ProjectFileLoader.cs

+        }
+
+        // load project file
+        var stream = new MemoryStream(Encoding.UTF8.GetBytes(projectContent));


So should we have some way to thread the encoding through to avoid mojibake if the XmlReader doesn't infer it correctly?

jasonmalinowski · 2025-04-26T00:10:10Z

src/Workspaces/MSBuild/BuildHost/MSBuild/ProjectFile/ProjectFileLoader.cs

+        }
+
+        // load project file
+        var stream = new MemoryStream(Encoding.UTF8.GetBytes(projectContent));


Or should we have a UTF-8 BOM precisely so it can infer in this case?

jasonmalinowski · 2025-04-26T00:10:38Z

src/Workspaces/MSBuild/BuildHost/MSBuild/ProjectFile/ProjectFileLoader.cs

@@ -27,4 +29,18 @@ public async Task<ProjectFile> LoadProjectFileAsync(string path, ProjectBuildMan

        return this.CreateProjectFile(project, buildManager, log);
    }
+
+    public ProjectFile LoadProjectFile(string path, string projectContent, ProjectBuildManager buildManager)


Suggested change

public ProjectFile LoadProjectFile(string path, string projectContent, ProjectBuildManager buildManager)

public ProjectFile LoadProject(string path, string projectContent, ProjectBuildManager buildManager)

Just to match your naming elsewhere.

jasonmalinowski · 2025-04-26T00:14:01Z

src/Workspaces/MSBuild/BuildHost/BuildHost.cs

+            _ => throw ExceptionUtilities.UnexpectedValue(languageName)
+        };
+
+        _logger.LogInformation($"Loading {projectFilePath}");


Suggested change

_logger.LogInformation($"Loading {projectFilePath}");

_logger.LogInformation($"Loading an in-memory project with the path {projectFilePath}");

jasonmalinowski · 2025-04-26T00:14:47Z

src/Workspaces/MSBuild/BuildHost/BuildHost.cs

@@ -187,6 +196,23 @@ private async Task<int> LoadProjectFileCoreAsync(string projectFilePath, string
        return _server.AddTarget(projectFile);
    }

+    [MethodImpl(MethodImplOptions.NoInlining)]


Copy the comment for the NoInlining use from the method above.

jasonmalinowski · 2025-04-26T00:17:51Z

src/Workspaces/MSBuild/BuildHost/Build/ProjectBuildManager.cs

+        var log = new DiagnosticLog();
+        try
+        {
+            var projectCollection = new MSB.Evaluation.ProjectCollection(


I admit I don't know the significance of this not using the batch project collection, or whether this creates problems around the loggers conflicting like you saw. That said, I'm not sure how to answer that question either. If we already had an existing bug there, then perhaps that can be counted as tracking the issue and this is fine.

Tracking under https://github.com/dotnet/sdk/issues/48708.

Add BuildHost APIs for loading an in-memory project

80a2d3e

ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 24, 2025

dotnet-policy-service bot added the VSCode label Apr 24, 2025

add test

514465a

RikkiGibson marked this pull request as ready for review April 24, 2025 22:18

RikkiGibson requested a review from a team as a code owner April 24, 2025 22:18

RikkiGibson commented Apr 24, 2025

View reviewed changes

src/Workspaces/MSBuild/BuildHost/Build/ProjectBuildManager.cs Outdated Show resolved Hide resolved