Skip to content

Migrate get packages to prune#54488

Open
AlesProkop wants to merge 2 commits into
dotnet:mainfrom
AlesProkop:migrate-get-packages-to-prune
Open

Migrate get packages to prune#54488
AlesProkop wants to merge 2 commits into
dotnet:mainfrom
AlesProkop:migrate-get-packages-to-prune

Conversation

@AlesProkop
Copy link
Copy Markdown
Member

@AlesProkop AlesProkop commented May 28, 2026

Fixes dotnet/msbuild#13892

Migrate GetPackagesToPrune to the MSBuild multithreaded task model

Brings GetPackagesToPrune (and its FrameworkPackages.LoadFrameworkPackagesFromPack helper) onto the multithreaded task model so the task can safely run under MSBuild multi-threaded execution mode without depending on the process working directory.

Changes

src/Tasks/Microsoft.NET.Build.Tasks/GetPackagesToPrune.cs

  • Annotated with [MSBuildMultiThreadableTask] and now implements IMultiThreadableTask with TaskEnvironment = TaskEnvironment.Fallback.
  • PrunePackageDataRoot and non-empty TargetingPackRoots are absolutized via TaskEnvironment.GetAbsolutePath(...) before any file I/O.
  • Internal helper signatures (LoadPackagesToPrune, TryLoadPackagesToPruneForVersion, LoadPackagesToPruneFromTargetingPack, LoadPackagesToPruneFromPrunePackageData) now take AbsolutePath / AbsolutePath[] so the safe-path contract is preserved end-to-end.
  • Empty entries in TargetingPackRoots are filtered out before absolutization to keep the original skip-empty-root behavior (TaskEnvironment.GetAbsolutePath("") would otherwise throw).

src/Tasks/Microsoft.NET.Build.Tasks/FrameworkPackages/FrameworkPackages.cs

  • LoadFrameworkPackagesFromPack now accepts AbsolutePath targetingPackRoot (was string). This eliminates the transitive unsafe-call diagnostics that the ThreadSafeTaskAnalyzer was reporting through this helper.
  • File I/O uses the absolute value (via the implicit AbsolutePath -> string conversion to Value), while user-facing log messages that mention the root use targetingPackRoot.OriginalValue so existing log text remains compatible.
  • Empty-root check switched from !string.IsNullOrEmpty(...) to targetingPackRoot.Value is not null to match the default(AbsolutePath) contract.

Tests

Added test/Microsoft.NET.Build.Tasks.Tests/GivenAGetPackagesToPrune.cs with three focused functional tests (no concurrency / no attribute checks):

  1. ItResolvesPrunePackageDataRootRelativeToTaskEnvironmentProjectDirectory — verifies a relative PrunePackageDataRoot is resolved against TaskEnvironment.ProjectDirectory, not the process CWD.
  2. ItResolvesTargetingPackRootsRelativeToTaskEnvironmentProjectDirectory — verifies the same for TargetingPackRoots, exercising the targeting-pack fallback path in FrameworkPackages.LoadFrameworkPackagesFromPack.
  3. ItIgnoresEmptyTargetingPackRootEntries — locks in the empty-root behavior we explicitly preserved during migration.

All three tests pass.

Validation

  • Microsoft.NET.Build.Tasks builds clean.
  • Microsoft.NET.Build.Tasks.Tests builds clean; new tests pass (3 Passed, 0 Failed).
  • ThreadSafeTaskAnalyzer (from dotnet/msbuild) no longer reports transitive unsafe-API diagnostics on GetPackagesToPrune after the helper signature change.

@AlesProkop AlesProkop marked this pull request as ready for review May 28, 2026 12:57
Copilot AI review requested due to automatic review settings May 28, 2026 12:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates GetPackagesToPrune and its targeting-pack helper path handling to MSBuild’s multi-threadable task model, resolving task path inputs through TaskEnvironment instead of process CWD.

Changes:

  • Marks GetPackagesToPrune as multi-threadable and adds TaskEnvironment path resolution.
  • Changes pruning helper methods and FrameworkPackages.LoadFrameworkPackagesFromPack to consume AbsolutePath.
  • Adds focused tests for relative prune data roots, relative targeting pack roots, and empty targeting pack root entries.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/Tasks/Microsoft.NET.Build.Tasks/GetPackagesToPrune.cs Adds multi-threadable task support and resolves pruning/targeting-pack roots through TaskEnvironment.
src/Tasks/Microsoft.NET.Build.Tasks/FrameworkPackages/FrameworkPackages.cs Updates targeting-pack loading to use AbsolutePath for file I/O while preserving original path text in logs.
test/Microsoft.NET.Build.Tasks.Tests/GivenAGetPackagesToPrune.cs Adds tests covering project-directory resolution and empty targeting-pack root behavior.

Comment on lines +146 to 148
PackagesToPrune = LoadPackagesToPrune(key, targetingPackRoots, prunePackageDataRoot, Log, AllowMissingPrunePackageData);

BuildEngine4.RegisterTaskObject(key, PackagesToPrune, RegisteredTaskObjectLifetime.Build, true);
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 GetPackagesToPrune in SDK

2 participants