Skip to content

Migrate GenerateDepsFile to IMultiThreadableTask - #53950

Merged
AlesProkop merged 11 commits into
dotnet:mainfrom
SimaTian:migrate-generate-deps-file
May 29, 2026
Merged

AlesProkop merged 11 commits into
dotnet:mainfrom
SimaTian:migrate-generate-deps-file

Conversation

@SimaTian

@SimaTian SimaTian commented Apr 17, 2026 •

Copy link
Copy Markdown
Contributor

Fixes dotnet/msbuild#13774

Migrates \GenerateDepsFile\ to support \IMultiThreadableTask.

Changes

  • Adds [MSBuildMultiThreadableTask]\ attribute and implements \IMultiThreadableTask.
  • Absolutizes \ProjectPath\ and \DepsFilePath\ in \ExecuteCore; scopes \AssetsFilePath\ / \RuntimeGraphPath\ absolutization inside \WriteDepsFile\ (point-of-use).
  • \DepsFilePath\ is backed by \AbsolutePath\ throughout: \File.Create\ uses .Value; \FilesWritten\ output uses .OriginalValue\ to preserve user-supplied relativity.
  • Adds 6 behavioral tests: output-relativity preservation, ProjectPath absolutization, decoy-CWD resolution, cross-thread env-var isolation, multi-process parity, concurrent execution.

Addresses AR-May findings from #52936

  • ✅ ProjectPath absolutized at task level before passing to \DependencyContextBuilder.
  • ✅ Absolutization moved into \WriteDepsFile\ helper (point-of-use).
  • ✅ \FilesWritten\ uses \AbsolutePath.OriginalValue\ (output fidelity).

Supersedes

Part of the split of stuck merge-group PR #52936. This is the 2-of-5 split for \GenerateDepsFile.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 implement IMultiThreadableTask on GenerateDepsFile, including TaskEnvironment handling.
  • Absolutize ProjectPath/DepsFilePath before writing and move AssetsFilePath/RuntimeGraphPath absolutization 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.

SimaTian and others added 2 commits May 13, 2026 12:16
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>
@AlesProkop
AlesProkop marked this pull request as ready for review May 19, 2026 14:32
@AlesProkop

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@jankratochvilcz jankratochvilcz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few minor changes, but no significant issues that I see ✨

@jankratochvilcz jankratochvilcz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, no more concerns from me!

@OvesN OvesN left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@AlesProkop

Copy link
Copy Markdown
Member

Summary

Eliminate analyzer-flagged unsafe-API calls on the GenerateDepsFile → WriteDepsFile graph; route all path/env access through TaskEnvironment.

  • FrameworkReferenceResolver — dropped parameterless ctor; GenerateDepsFile now passes TaskEnvironment.GetEnvironmentVariable.
  • GenerateDepsFile — File.Create(depsFilePath) uses the AbsolutePath overload; AssetsFilePath / RuntimeGraphPath go through string.IsNullOrEmpty(...) ? null : TaskEnvironment.GetAbsolutePath(...) to preserve prior null/empty semantics; both DependencyContextBuilder ctors get TaskEnvironment.GetAbsolutePath injected.
  • DependencyContextBuilder — new _getAbsolutePath field + GetAbsolutePath helper; CreateRuntimeFile now takes AbsolutePath fullPath; GetRuntimeLibrary and GetRuntimePackLibraries wrap raw Path strings through GetAbsolutePath before File.Exists / CreateRuntimeFile.
  • DependencyContextBuilder.GetProjectInfo — keeps Path.GetFullPath(Path.Combine(...)) for the _referenceProjectInfos lookup key (canonical MSBuildSourceProjectFile); AbsolutePath would preserve .. and miss the lookup. Pure string key, no file I/O, so MSBuildTask0003 doesn't apply.
  • FileUtilities — added AbsolutePath overloads for GetFileVersion / TryGetAssemblyVersion, plus GetAssemblyVersion(AbsolutePath) in FileUtilities.MetadataReader.cs. Legacy string overloads retained for unmigrated callers (ResolvePackageFileConflicts, GenerateRegFreeComManifest); their MSBuildTask0005 transitive hits are out of scope.

Comment thread src/Tasks/Common/FileUtilities.cs Resolved
Comment thread src/Tasks/Common/FileUtilities.cs Resolved
@AlesProkop
AlesProkop enabled auto-merge (squash) May 29, 2026 13:41
@AlesProkop
AlesProkop disabled auto-merge May 29, 2026 13:48
@AlesProkop
AlesProkop enabled auto-merge (squash) May 29, 2026 14:18
@AlesProkop
AlesProkop merged commit 1554065 into dotnet:main May 29, 2026
20 of 25 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview6 milestone May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Multithreaded] Migrate GenerateDepsFile in SDK

6 participants