Skip to content

Fix MSBuildTask0002 analyzer warnings in already-migrated tasks#13466

Merged
JanProvaznik merged 1 commit into
dotnet:mainfrom
JanProvaznik:fix/mt-task-warnings-pr1
Apr 7, 2026
Merged

Fix MSBuildTask0002 analyzer warnings in already-migrated tasks#13466
JanProvaznik merged 1 commit into
dotnet:mainfrom
JanProvaznik:fix/mt-task-warnings-pr1

Conversation

@JanProvaznik

Copy link
Copy Markdown
Member

Replace direct Environment.GetEnvironmentVariable and Path.GetFullPath calls in tasks that already implement IMultiThreadableTask, addressing analyzer warnings in the 'Linux Core Multithreaded Mode' CI leg.

Changes:

  • Copy.cs: Convert static env var reads to instance fields initialized from TaskEnvironment.GetEnvironmentVariable() at the start of Execute(). This is MT-safe since each task instance gets its own TaskEnvironment. Constants remain on Copy for test access. RefreshInternalEnvironmentValues() is now a no-op (env vars are read fresh per Execute call).
  • GetFileHash.cs: Change ComputeHash parameter from string to AbsolutePath to preserve type safety through to File.OpenRead call.
  • Unzip.cs: Replace Path.GetFullPath with TaskEnvironment.GetAbsolutePath().GetCanonicalForm() using AbsolutePath type. GetCanonicalForm resolves '..' for zip slip protection.
  • FindUnderPath.cs: Pragma-suppress MSBuildTask0002 in pre-ChangeWave branches where Path.GetFullPath is called on already-absolute paths from TaskEnvironment.GetAbsolutePath (only canonicalizes, CWD not consulted).
  • AssignTargetPath.cs: Same pragma-suppress pattern as FindUnderPath.

Copilot AI review requested due to automatic review settings March 31, 2026 11:50

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

This PR addresses MSBuildTask0002 analyzer warnings by switching already-IMultiThreadableTask tasks to use TaskEnvironment-based APIs (and AbsolutePath) instead of direct Environment.GetEnvironmentVariable / Path.GetFullPath, improving correctness in multithreaded execution modes.

Changes:

  • Update path canonicalization and path-type plumbing to use TaskEnvironment.GetAbsolutePath(...).GetCanonicalForm() and AbsolutePath where appropriate.
  • Refactor Copy to read relevant environment variables via TaskEnvironment per-task-execution (instead of static reads) and adjust related unit tests.
  • Add targeted #pragma suppressions in pre-ChangeWave branches where Path.GetFullPath is still required for behavior compatibility.

Reviewed changes

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

Show a summary per file
File Description
src/Tasks/Unzip.cs Use TaskEnvironment + AbsolutePath canonicalization for zip extraction destination validation.
src/Tasks/NativeMethods.cs Change MoveFileEx wrapper to use AbsolutePath parameters.
src/Tasks/ListOperators/FindUnderPath.cs Add MSBuildTask0002 suppression in pre-ChangeWave branch around Path.GetFullPath.
src/Tasks/FileIO/GetFileHash.cs Strengthen ComputeHash signature to accept AbsolutePath.
src/Tasks/Copy.cs Replace static environment-variable reads with per-Execute() TaskEnvironment reads; adjust symlink retry flags accordingly.
src/Tasks/AssignTargetPath.cs Add MSBuildTask0002 suppression in pre-ChangeWave branch around Path.GetFullPath.
src/Tasks.UnitTests/Copy_Tests.cs Remove reliance on Copy.RefreshInternalEnvironmentValues() after env-var handling refactor.

Comment thread src/Tasks/Copy.cs Outdated
Replace direct Environment.GetEnvironmentVariable and Path.GetFullPath calls
in tasks that already implement IMultiThreadableTask, addressing analyzer
warnings in the 'Linux Core Multithreaded Mode' CI leg.

Changes:
- Copy.cs: Convert static env var reads to instance fields initialized from
  TaskEnvironment.GetEnvironmentVariable() at the start of Execute(). This is
  MT-safe since each task instance gets its own TaskEnvironment. Constants
  remain on Copy for test access. RefreshInternalEnvironmentValues() is now
  a no-op (env vars are read fresh per Execute call).
- GetFileHash.cs: Change ComputeHash parameter from string to AbsolutePath to
  preserve type safety through to File.OpenRead call.
- Unzip.cs: Replace Path.GetFullPath with TaskEnvironment.GetAbsolutePath().GetCanonicalForm()
  using AbsolutePath type. GetCanonicalForm resolves '..' for zip slip protection.
- FindUnderPath.cs: Pragma-suppress MSBuildTask0002 in pre-ChangeWave branches
  where Path.GetFullPath is called on already-absolute paths from
  TaskEnvironment.GetAbsolutePath (only canonicalizes, CWD not consulted).
- AssignTargetPath.cs: Same pragma-suppress pattern as FindUnderPath.

Remaining MSBuildTask0005 (transitive) warnings in Copy/Move should be addressed
by improving the ThreadSafeTaskAnalyzer's AbsolutePath recognition in transitive
call chains.

Co-authored-by: Copilot <[email protected]>
@JanProvaznik JanProvaznik force-pushed the fix/mt-task-warnings-pr1 branch from b1e1236 to aabceed Compare March 31, 2026 12:15

@AR-May AR-May left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall looks good. The only thing I’m unsure about is whether the Copy change should be placed behind a changewave, given the slight behavior change. I don’t have a strong opinion either way.

Comment thread src/Tasks/Copy.cs
@JanProvaznik JanProvaznik merged commit 264c606 into dotnet:main Apr 7, 2026
10 checks passed
@JanProvaznik JanProvaznik self-assigned this Apr 7, 2026
dfederm pushed a commit to dfederm/msbuild that referenced this pull request Apr 9, 2026
…et#13466)

Replace direct Environment.GetEnvironmentVariable and Path.GetFullPath
calls in tasks that already implement IMultiThreadableTask, addressing
analyzer warnings in the 'Linux Core Multithreaded Mode' CI leg.

Changes:
- Copy.cs: Convert static env var reads to instance fields initialized
from TaskEnvironment.GetEnvironmentVariable() at the start of Execute().
This is MT-safe since each task instance gets its own TaskEnvironment.
Constants remain on Copy for test access.
RefreshInternalEnvironmentValues() is now a no-op (env vars are read
fresh per Execute call).
- GetFileHash.cs: Change ComputeHash parameter from string to
AbsolutePath to preserve type safety through to File.OpenRead call.
- Unzip.cs: Replace Path.GetFullPath with
TaskEnvironment.GetAbsolutePath().GetCanonicalForm() using AbsolutePath
type. GetCanonicalForm resolves '..' for zip slip protection.
- FindUnderPath.cs: Pragma-suppress MSBuildTask0002 in pre-ChangeWave
branches where Path.GetFullPath is called on already-absolute paths from
TaskEnvironment.GetAbsolutePath (only canonicalizes, CWD not consulted).
- AssignTargetPath.cs: Same pragma-suppress pattern as FindUnderPath.

Co-authored-by: Copilot <[email protected]>
This was referenced Jun 10, 2026
github-actions Bot pushed a commit to IntelliTect/Multitool that referenced this pull request Jun 29, 2026
Updated [Microsoft.Build](https://github.com/dotnet/msbuild) from 18.6.3
to 18.7.1.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.Build's
releases](https://github.com/dotnet/msbuild/releases)._

## 18.7.1

## What's Changed
* Fix TraceEngine file contention deadlock in multithreaded mode by
@​JanProvaznik in dotnet/msbuild#13446
* Remove duplicate test cases in MultithreadableTaskAnalyzer by
@​Youssef1313 in dotnet/msbuild#13483
* Ensure ThreadSafeTaskAnalyzer.Tests is considered as a unit test
project by @​Youssef1313 in dotnet/msbuild#13481
* Fix MSBuildTask0002 analyzer warnings in already-migrated tasks by
@​JanProvaznik in dotnet/msbuild#13466
* Fix race conditions in task host path resolution by @​AR-May in
dotnet/msbuild#13485
* Migrate ToolTask and Al task to TaskEnvironment API by @​OvesN in
dotnet/msbuild#13423
* Bump main to 18.7, add vs18.6 to merge flow by @​MichalPavlik in
dotnet/msbuild#13472
* Avoid allocations in GetHashCode implementations by @​DustinCampbell
in dotnet/msbuild#13475
* Add PATs rotation to agentic workflow(s) by @​JanKrivanek in
dotnet/msbuild#13496
* Fix ASP.NET WebSite projects to copy netstandard.dll facade when
required by @​JanProvaznik in
dotnet/msbuild#13058
* Migrate AspNetCompiler to TaskEnvironment API by @​OvesN in
dotnet/msbuild#13424
* Add review workflow by @​JanKrivanek in
dotnet/msbuild#13503
* Strengthen reviewer skill: add step-back analysis dimensions by
@​JanProvaznik in dotnet/msbuild#13504
* Add 'Request Speedometer Perf Run' to VS experimental insertion build
policies by @​Copilot in dotnet/msbuild#13505
* Remove duplicate @ prefix from issueAuthor in GitOps by @​akoeplinger
in dotnet/msbuild#13492
* Improve review aw by @​JanKrivanek in
dotnet/msbuild#13510
* Migrates unit tests to use RoslynCodeTaskFactory to enable running
tests under .NET Core by @​jankratochvilcz in
dotnet/msbuild#13500
* Fix cross-AppDomain TaskItem modifier cache regression by
@​DustinCampbell in dotnet/msbuild#13493
* Discourage review agent from approving PRs by @​JanKrivanek in
dotnet/msbuild#13512
* Stop trying to deploy ValueTuple by @​rainersigwald in
dotnet/msbuild#13507
* Ad-hoc re-sign bootstrap dotnet on macOS to prevent SIGKILL by
@​jankratochvilcz in dotnet/msbuild#13513
* RoslynCodeTaskFactory: Log MSB3753 when task class does not implement
ITask by @​jankratochvilcz in
dotnet/msbuild#13517
* Update gh-aw (upon mcp policy changes) by @​JanKrivanek in
dotnet/msbuild#13526
* Eliminate XmlChildNodes allocations in GetXmlNodeInnerContents by
@​nareshjo in dotnet/msbuild#13509
* Fix telemetry allocation regression: per-engine collector ownership by
@​JanProvaznik in dotnet/msbuild#13516
* Migrate to xunit.v3 by @​Youssef1313 in
dotnet/msbuild#13482
* Fix stray brace in HandleBuildCancel trace string causing MSB1025 by
@​Copilot in dotnet/msbuild#13535
* Bumping to 10.0.4 runtime packages by @​MichalPavlik in
dotnet/msbuild#13533
* Remove early return in GetCanonicalForm, always call System.IO.Path by
@​OvesN in dotnet/msbuild#13532
* Do not overwrite GetCopyToOutputDirectoryItemsDependsOn, just add new…
by @​snechaev in dotnet/msbuild#13474
* Migrate GetReferenceAssemblyPaths task to TaskEnvironment API by
@​OvesN in dotnet/msbuild#13495
* Stabilize ToolTaskThatTimeoutAndRetry test by @​rainersigwald in
dotnet/msbuild#13489
* [automated] Merge branch 'vs18.6' => 'main' by @​github-actions[bot]
in dotnet/msbuild#13506
* Add extra test assertions around tests by @​Youssef1313 in
dotnet/msbuild#13536
* Add static eval for repo skills/agents via skill-validator by
@​JanKrivanek in dotnet/msbuild#13537
* Migrate SGen task to Task environment API by @​OvesN in
dotnet/msbuild#13457
* Fix TerminalLogger assert failure for metaproj files and cached
project eval ID by @​OvesN in
dotnet/msbuild#13480
* Filter out approving review from pr-reviewer agent by @​JanKrivanek in
dotnet/msbuild#13553
* Use a unique task name per invocation to tabilize
RoslynCodeTaskFactory_ReuseCompilation test by @​huulinhnguyen-dev in
dotnet/msbuild#13551
* Brief doc on feedback/logging/data systems by @​rainersigwald in
dotnet/msbuild#13554
* Localized file check-in by OneLocBuild Task: Build definition ID 9434:
Build ID 13881982 by @​dotnet-bot in
dotnet/msbuild#13437
* Stage 3: Forward BuildProjectFile* callbacks from OOP TaskHost to
worker node by @​JanProvaznik in
dotnet/msbuild#13350
* Enable TaskHost Callbacks by default by @​JanProvaznik in
dotnet/msbuild#13579
* Remove unactionable info from reviewer agent by @​JanKrivanek in
dotnet/msbuild#13578
* Enlighten RequiresFramework35SP1Assembly task for multithreaded mode
by @​jankratochvilcz in dotnet/msbuild#13575
* Make SdkResolver-provided environment variables take precedence over
ambient environment by @​Copilot in
dotnet/msbuild#12655
* Add dotnet/skills marketplace and enable plugins by @​Evangelink in
dotnet/msbuild#13582
* The skills/agents check filters-in only touched files by @​JanKrivanek
in dotnet/msbuild#13586
* Fix skill-validation workflow failing when agents directory is deleted
by @​JeremyKuhne in dotnet/msbuild#13592
 ... (truncated)

Commits viewable in [compare
view](dotnet/msbuild@v18.6.3...v18.7.1).
</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This was referenced Jun 29, 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.

4 participants