Repository navigation
Migrate GenerateDepsFile to IMultiThreadableTask - #53950
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates GenerateDepsFile to support MSBuild multithreaded execution by implementing IMultiThreadableTask, using TaskEnvironment for path resolution, and adding unit tests intended to validate correctness under concurrent execution / non-default working directories.
Changes:
- Add
[MSBuildMultiThreadableTask]and implementIMultiThreadableTaskonGenerateDepsFile, includingTaskEnvironmenthandling. - Absolutize
ProjectPath/DepsFilePathbefore writing and moveAssetsFilePath/RuntimeGraphPathabsolutization to point-of-use. - Add a new multithreading-focused unit test suite for
GenerateDepsFile.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Tasks/Microsoft.NET.Build.Tasks/GenerateDepsFile.cs | Adds IMultiThreadableTask support and adjusts path handling for multithread-safe execution and output fidelity. |
| src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAGenerateDepsFileMultiThreading.cs | Adds behavioral/concurrency tests for GenerateDepsFile in multithreaded / CWD-sensitive scenarios. |
f281378 to
f6620e9
Compare
Implements IMultiThreadableTask to enable safe multi-threaded execution: - Added [MSBuildMultiThreadableTask] attribute - Added TaskEnvironment property with NETFRAMEWORK polyfill - Absolutize ProjectPath at task level before passing to DependencyContextBuilder - Moved AssetsFilePath and RuntimeGraphPath absolutization into WriteDepsFile - Use AbsolutePath.OriginalValue for FilesWritten output to preserve relativity Addresses all 3 AR-May findings from PR dotnet#52936: 1. ProjectPath absolutization: Now absolutized at ExecuteCore level via TaskEnvironment.GetAbsolutePath, passes .Value (absolute) to DependencyContextBuilder 2. Cleanliness: AssetsFilePath and RuntimeGraphPath absolutization moved into WriteDepsFile where consumed 3. Output fidelity: FilesWritten uses AbsolutePath.OriginalValue to preserve user-supplied relative paths Added comprehensive behavioral tests in GivenAGenerateDepsFileMultiThreading.cs: - OutputRelativityIsPreserved: Verifies FilesWritten preserves relative paths - ProjectPathIsAbsolutized: Ensures ProjectPath is absolutized before use - DecoyCwdDoesNotAffectPathResolution: Validates CWD independence - CrossThreadEnvVarIsolation: Tests environment variable isolation - MultiProcessParity: Confirms identical output with/without TaskEnvironment - ConcurrentExecutionProducesCorrectResults: Parallel execution test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fa3642d to
5dc9a2b
Compare
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
jankratochvilcz
left a comment
There was a problem hiding this comment.
Few minor changes, but no significant issues that I see ✨
jankratochvilcz
left a comment
There was a problem hiding this comment.
Looks good, no more concerns from me!
OvesN
left a comment
There was a problem hiding this comment.
Please recheck the flow of all paths that we receive as inputs. They can touch forbidden APIs. Mb there is more than I commented. Can analyzer help here?
SummaryEliminate analyzer-flagged unsafe-API calls on the
|
Fixes dotnet/msbuild#13774
Migrates \GenerateDepsFile\ to support \IMultiThreadableTask.
Changes
Addresses AR-May findings from #52936
Supersedes
Part of the split of stuck merge-group PR #52936. This is the 2-of-5 split for \GenerateDepsFile.