Migrate 5 MSBuild tasks to IMultiThreadableTask (Group 2: Bundle, Deps, Config & Hosts)

[SimaTian]

Migrate 5 MSBuild tasks to IMultiThreadableTask (Group 2: Bundle, Deps, Config & Hosts)

Context

This PR migrates 5 MSBuild tasks to support multithreaded execution by implementing IMultiThreadableTask.

Dependency: This PR depends on #52909 (polyfills PR) which adds the IMultiThreadableTask interface, TaskEnvironment, and AbsolutePath polyfills. The diff will reduce once that PR is merged.

Migrated Tasks

Task	Changes
GenerateBundle	Absolutize OutputDir and FilesToBundle[].ItemSpec via TaskEnvironment.GetAbsolutePath()
GenerateDepsFile	Absolutize DepsFilePath before File.Create()
WriteAppConfigWithSupportedRuntime	Absolutize OutputAppConfigFile.ItemSpec and input app.config path
ResolveAppHosts	Absolutize RuntimeGraphPath and TargetingPackRoot
ShowPreviewMessage	Attribute-only (no path I/O)

Changes Per Task

Each task receives:

• [MSBuildMultiThreadableTask] attribute
• IMultiThreadableTask interface implementation
• TaskEnvironment property
• Forbidden API replacements (relative path → TaskEnvironment.GetAbsolutePath())

New Test Files

• GivenAGenerateBundleMultiThreading.cs
• GivenAGenerateDepsFileMultiThreading.cs
• GivenAWriteAppConfigWithSupportedRuntimeMultiThreading.cs
• GivenAResolveAppHostsMultiThreading.cs
• GivenAShowPreviewMessageMultiThreading.cs

Testing

• All multithreading unit tests pass
• No regressions in existing tests

[Copilot]

Pull request overview

This PR updates a set of SDK MSBuild tasks to participate in MSBuild’s multithreaded task execution model by implementing IMultiThreadableTask, wiring in TaskEnvironment, and replacing relative-path I/O with TaskEnvironment.GetAbsolutePath(...). It also adds unit-test coverage and introduces .NET Framework-gated polyfills for the new MSBuild APIs.

Changes:

• Migrate 4 tasks to IMultiThreadableTask + TaskEnvironment and absolutize relevant paths (GenerateBundle, GenerateDepsFile, WriteAppConfigWithSupportedRuntime, ResolveAppHosts).
• Mark ShowPreviewMessage as multithreadable via [MSBuildMultiThreadableTask].
• Add unit tests + a TaskEnvironmentHelper for constructing TaskEnvironment instances in tests, plus .NET Framework-gated polyfill implementations.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs	Implements IMultiThreadableTask and absolutizes OutputDir/bundle inputs via TaskEnvironment.
src/Tasks/Microsoft.NET.Build.Tasks/GenerateDepsFile.cs	Implements IMultiThreadableTask and absolutizes DepsFilePath before writing.
src/Tasks/Microsoft.NET.Build.Tasks/WriteAppConfigWithSupportedRuntime.cs	Implements IMultiThreadableTask and absolutizes input/output app.config paths.
src/Tasks/Microsoft.NET.Build.Tasks/ResolveAppHosts.cs	Implements IMultiThreadableTask and absolutizes runtime graph + targeting pack root usage.
src/Tasks/Microsoft.NET.Build.Tasks/ShowPreviewMessage.cs	Adds [MSBuildMultiThreadableTask] attribute.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAGenerateBundleMultiThreading.cs	Adds tests asserting interface/attribute and a smoke path-resolution scenario.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAGenerateDepsFileMultiThreading.cs	Adds interface/attribute/property presence tests.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAWriteAppConfigWithSupportedRuntimeMultiThreading.cs	Adds tests validating TaskEnvironment-based input/output path resolution.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolveAppHostsMultiThreading.cs	Adds test validating TargetingPackRoot resolution via TaskEnvironment.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAShowPreviewMessageMultiThreading.cs	Adds test validating [MSBuildMultiThreadableTask] presence.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/TaskEnvironmentHelper.cs	Adds helper for constructing TaskEnvironment via reflection/DispatchProxy.
src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/TaskEnvironmentHelperTests.cs	Adds smoke tests for helper behavior and path resolution.
src/Tasks/Common/IMultiThreadableTask.cs	Adds .NET Framework-gated polyfill for IMultiThreadableTask.
src/Tasks/Common/TaskEnvironment.cs	Adds .NET Framework-gated polyfill for TaskEnvironment.
src/Tasks/Common/AbsolutePath.cs	Adds .NET Framework-gated polyfill for AbsolutePath.
src/Tasks/Common/ITaskEnvironmentDriver.cs	Adds .NET Framework-gated polyfill for the internal driver interface.
src/Tasks/Common/ProcessTaskEnvironmentDriver.cs	Adds .NET Framework-gated polyfill driver implementation.

[SimaTian]

Addressed all review comments:

1. TaskEnvironment internal -> public: Fixed in the polyfills PR (#52909). \TaskEnvironment\ is now \public sealed class.

2. AbsolutePath internal -> public: Fixed in the polyfills PR (#52909). \AbsolutePath\ is now \public readonly struct.

3. GenerateBundle test assertion: Added \NotBeOfType\ assertion to verify that OutputDir is absolutized via TaskEnvironment rather than used as a relative path.

Branch rebased on updated polyfills and force-pushed.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

[SimaTian]

Deep Audit: Forbidden API Usage in Helper Functions (Group 2)

Traced all helper functions and referenced libraries called by the 5 Group 2 tasks for hidden forbidden API usage that bypasses the TaskEnvironment migration.

---

🔴 CRITICAL: FrameworkReferenceResolver.GetDefaultReferenceAssembliesPath() uses Environment.GetEnvironmentVariable

Called by: GenerateDepsFile.WriteDepsFile() → DependencyContextBuilder.WithReferenceAssembliesPath(FrameworkReferenceResolver.GetDefaultReferenceAssembliesPath())

Source: FrameworkReferenceResolver.cs

public static string GetDefaultReferenceAssembliesPath()
{
    var referenceAssembliesPath = DotNetReferenceAssembliesPathResolver.Resolve();
    // ...
    var programFiles = Environment.GetEnvironmentVariable(""ProgramFiles(x86)"");
    if (string.IsNullOrEmpty(programFiles))
    {
        programFiles = Environment.GetEnvironmentVariable(""ProgramFiles"");
    }
    // ...
}

This static method directly calls Environment.GetEnvironmentVariable — a forbidden API. Per the migration guide, this should use TaskEnvironment.GetEnvironmentVariable(). Additionally, DotNetReferenceAssembliesPathResolver.Resolve() (external NuGet library) likely also reads environment variables internally.

Impact: Medium-high. In multithreaded execution, Environment.GetEnvironmentVariable reads from process-global state, which could return stale or wrong values if another thread has modified the environment.

Fix needed: FrameworkReferenceResolver.GetDefaultReferenceAssembliesPath() needs to accept a TaskEnvironment parameter and use it for env var access. Since it's a static utility shared across tasks, this is a cross-cutting change.

---

🔴 CRITICAL: ResolvedFile constructor uses Path.GetFullPath()

Called by: GenerateDepsFile.WriteDepsFile() → new ResolvedFile(f, false) and new ResolvedFile(f, true)

Source: ResolvedFile.cs (4-arg constructor, not the ITaskItem constructor):

public ResolvedFile(string sourcePath, string destinationSubDirectory, PackageIdentity package, ...)
{
    SourcePath = Path.GetFullPath(sourcePath);  // ← FORBIDDEN API
    // ...
}

The 4-arg constructor calls Path.GetFullPath() directly. However, in GenerateDepsFile, the 2-arg constructor ResolvedFile(ITaskItem item, bool isRuntimeTarget) is used, which does not call Path.GetFullPath(). So this is a latent issue rather than an active bug for this task — but other tasks using the 4-arg constructor would be affected.

Impact for GenerateDepsFile: Low (not hit). But ResolvedFile should be migrated as part of a utility sweep.

---

🟡 WARNING: GenerateDepsFile — AssetsFilePath and RuntimeGraphPath NOT absolutized

Already flagged in PR review, confirming via deep trace:

1. AssetsFilePath is passed directly to LockFileCache.GetLockFile(AssetsFilePath) without absolutization. LockFileCache has a guard (Path.IsPathRooted check and throws), so it won't silently fail — but it will throw if a relative path is provided instead of resolving it correctly.

2. RuntimeGraphPath is passed directly to RuntimeGraphCache.GetRuntimeGraph(RuntimeGraphPath) without absolutization. Same guard exists there.

3. ProjectPath is passed to SingleProjectInfo.Create(ProjectPath, ...) — this just stores the string without file I/O, so no immediate issue.

Good news: Both LockFileCache and RuntimeGraphCache have defensive Path.IsPathRooted() guards that throw on relative paths, preventing silent CWD-dependent behavior. But the correct fix is to absolutize at the task level via TaskEnvironment.GetAbsolutePath().

---

🟡 WARNING: GenerateDepsFile — DotNetReferenceAssembliesPathResolver.Resolve()

This is from the Microsoft.Extensions.DependencyModel NuGet package. It likely reads DOTNET_REFERENCE_ASSEMBLIES_PATH environment variable internally. Since it's an external library, it can't be directly migrated, but should be documented as a known limitation.

---

✅ GenerateBundle — CLEAN

All paths absolutized correctly:

• OutputDir → TaskEnvironment.GetAbsolutePath(OutputDir)
• FilesToBundle[].ItemSpec → TaskEnvironment.GetAbsolutePath(item.ItemSpec)

The Bundler class (from Microsoft.NET.HostModel) receives already-absolutized paths. No forbidden APIs in the task's own code path.

---

✅ WriteAppConfigWithSupportedRuntime — CLEAN

Both file I/O paths absolutized:

• OutputAppConfigFile.ItemSpec → TaskEnvironment.GetAbsolutePath(...) before new FileStream(...)
• AppConfigFile.ItemSpec → TaskEnvironment.GetAbsolutePath(...) before XDocument.Load(...)

Helper methods (AddSupportedRuntimeToAppconfig, TryGetSupportRuntimeNode) are pure XML manipulation with no file I/O.

---

✅ ResolveAppHosts — CLEAN

All paths absolutized:

• RuntimeGraphPath → TaskEnvironment.GetAbsolutePath(RuntimeGraphPath) before RuntimeGraphCache.GetRuntimeGraph()
• TargetingPackRoot → TaskEnvironment.GetAbsolutePath(TargetingPackRoot) before Path.Combine + Directory.Exists

RuntimeGraphCache.GetRuntimeGraph() has a Path.IsPathRooted() guard — receives already-rooted path. Helper methods (GetHostItem, ProcessFrameworkReferences.NormalizeVersion) are pure computation.

---

✅ ShowPreviewMessage — CLEAN (Pattern A)

No file I/O, no environment access, no forbidden APIs. Uses only BuildEngine4 task object registration.

---

Summary

Task	Status	Issues
GenerateBundle	✅ Clean	None
GenerateDepsFile	🔴 Issues	FrameworkReferenceResolver uses Environment.GetEnvironmentVariable; AssetsFilePath/RuntimeGraphPath not absolutized; ResolvedFile ctor has Path.GetFullPath (not hit in this path)
WriteAppConfig	✅ Clean	None
ResolveAppHosts	✅ Clean	None
ShowPreviewMessage	✅ Clean	None

Recommended actions:

1. GenerateDepsFile: Absolutize AssetsFilePath and RuntimeGraphPath before passing to helpers
2. FrameworkReferenceResolver: Needs TaskEnvironment parameter (cross-cutting — affects multiple tasks)
3. ResolvedFile 4-arg ctor: Replace Path.GetFullPath with caller-side absolutization (latent, not active in this path)

[SimaTian]

Fixes applied (248cb91)

Resolved the forbidden API issues identified in the deep audit:

1. GenerateDepsFile — AssetsFilePath and RuntimeGraphPath now absolutized

Added TaskEnvironment.GetAbsolutePath() calls in ExecuteCore() before these paths reach LockFileCache and RuntimeGraphCache helpers:

if (AssetsFilePath != null)
{
    AssetsFilePath = TaskEnvironment.GetAbsolutePath(AssetsFilePath);
}
RuntimeGraphPath = TaskEnvironment.GetAbsolutePath(RuntimeGraphPath);

2. FrameworkReferenceResolver.GetDefaultReferenceAssembliesPath() — now accepts TaskEnvironment

Added a new overload that uses TaskEnvironment.GetEnvironmentVariable() instead of Environment.GetEnvironmentVariable():

• GetDefaultReferenceAssembliesPath() — backward-compatible, delegates to overload with null
• GetDefaultReferenceAssembliesPath(TaskEnvironment) — uses TaskEnvironment when provided

GenerateDepsFile now calls the TaskEnvironment-aware overload.

3. ResolvedFile 4-arg constructor Path.GetFullPath — not changed (documented)

This is a latent issue only. The constructor is called by AssetsFileResolver with paths from Path.Combine(absoluteLibraryPath, relativePart) — always producing absolute paths. Path.GetFullPath acts as canonicalization only (no CWD dependency). Changing it would require threading TaskEnvironment through AssetsFileResolver and all its callers — a larger refactor best handled in a separate PR.

Test results

• 231 passed, 11 failed (same pre-existing ToolsetInfo failures, unrelated)
• No regressions

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.

[SimaTian]

DotNetReferenceAssembliesPathResolver.Resolve() bypasses TaskEnvironment and reads DOTNET_REFERENCE_ASSEMBLIES_PATH directly via Environment.GetEnvironmentVariable(). This is inconsistent with the rest of this method which correctly routes env var reads through taskEnvironment.

The fix would be to replace this call with taskEnvironment.GetEnvironmentVariable("DOTNET_REFERENCE_ASSEMBLIES_PATH") and optionally inline the Mono xbuild-frameworks path probing from the library source.

This is a pre-existing issue (not introduced by this PR), but worth tracking since we are already touching this method.

[SimaTian]

Or so the copilot says. The issue is that the offending method is a part of the Runtime. Does this mean that we need to clone the method with all the issues that arise from this?
Changing the runtime method to be aware of our TaskEnvironment sounds like a really bad idea to me.
Also a grumpy note: no matter how much I poked and prodded the agents, this one I had to catch myself. They just ignored it as a safe call for some reason.

[SimaTian]

As good as I can force the copilot to make it.
There is one open question - what do we do about the Runtime dependency which accesses Environment Variable directly? Is it safe enough to risk it or do we need to clone the method?