Migrate WriteAppConfigWithSupportedRuntime to IMultiThreadableTask#53957
Draft
SimaTian wants to merge 10 commits intodotnet:mainfrom
Draft
Migrate WriteAppConfigWithSupportedRuntime to IMultiThreadableTask#53957SimaTian wants to merge 10 commits intodotnet:mainfrom
SimaTian wants to merge 10 commits intodotnet:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates WriteAppConfigWithSupportedRuntime to be compatible with MSBuild’s multithreaded task execution model by implementing IMultiThreadableTask and routing path I/O through TaskEnvironment to avoid reliance on process-wide state (like CWD).
Changes:
- Marks the task as multi-threadable and adds
TaskEnvironmentsupport for path resolution. - Updates input (app.config) load and output write paths to resolve via
TaskEnvironment.GetAbsolutePath(). - Adds a new unit test suite covering CWD decoy routing, cross-thread isolation, parity, null inputs, and concurrent execution.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Tasks/Microsoft.NET.Build.Tasks/WriteAppConfigWithSupportedRuntime.cs | Adds IMultiThreadableTask + TaskEnvironment-based absolute path resolution for file I/O. |
| src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAWriteAppConfigWithSupportedRuntimeMultiThreading.cs | Adds behavioral tests for multithreaded/task-environment correctness and concurrency scenarios. |
| #nullable disable | ||
|
|
||
| using Microsoft.Build.Framework; | ||
| using Microsoft.Build.Utilities; |
Comment on lines
+60
to
+62
| } | ||
|
|
||
| OutputAppConfigFile.SetMetadata("FullPath", outputPath.Value); |
- Add [MSBuildMultiThreadableTask] attribute - Implement IMultiThreadableTask interface with TaskEnvironment property - Route all path operations through TaskEnvironment with null-safe fallbacks - Use AbsolutePath.OriginalValue for output metadata fidelity - Add comprehensive multithreading tests covering: - Decoy CWD path resolution - Cross-thread isolation with different environments - Multi-process parity (with and without TaskEnvironment) - Task environment resolved path verification - Null AppConfigFile handling - Concurrent execution with different framework versions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- WriteAppConfigWithSupportedRuntime.cs: use AbsolutePath.Value (not OriginalValue) when setting the reserved 'FullPath' item metadata. 'FullPath' is contractually an absolute path; setting it to OriginalValue (possibly relative) misled downstream consumers and defeated the purpose of pinning the write location. - GivenAWriteAppConfigWithSupportedRuntimeMultiThreading.cs: replace hardcoded 'obj\\Debug\\output.config' literal with Path.Combine to keep the test cross-platform (skill D9). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SimaTian
force-pushed
the
migrate-write-app-config-supported-runtime
branch
from
655d973 to
9fe6f27
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JanProvaznik
approved these changes
May 5, 2026
Member
|
if it compiles with the taskenvironment fallback lgtm |
Delegates the SDK's TaskEnvironmentDefaults and TaskEnvironmentHelper shims to MSBuild's public TaskEnvironment.Fallback / CreateWithProjectDirectoryAndEnvironment APIs (dotnet/msbuild#13462), and drops the now-unused ProcessTaskEnvironmentDriver polyfill. Pipelines are expected to fail until the consumed MSBuild API target version is bumped to a build that exposes these APIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Migrates
WriteAppConfigWithSupportedRuntimeto supportIMultiThreadableTask.Changes
[MSBuildMultiThreadableTask]attribute and implementsIMultiThreadableTask.FileStream) and input read (XDocument.Load) throughTaskEnvironment.GetAbsolutePath().AbsolutePath.Valuefor the reservedFullPathMSBuild well-known metadata key (reserved keys must be absolute).Supersedes
Part of the split of stuck merge-group PR #52936. This is the 3-of-5 split for
WriteAppConfigWithSupportedRuntime.