Don't log TaskAssemblyLocationMismatch for task-host-routed tasks

[JanProvaznik]

Context

-mt builds of OrchardCore emit ~1200 spurious TaskAssemblyLocationMismatch messages at normal importance that a baseline build does not:

Task assembly was loaded from 'C:\...\sdk\<version>\Microsoft.Build.dll'
  while the desired location was 'C:\...\sdk\<version>\Roslyn\Microsoft.Build.Tasks.CodeAnalysis.dll'.

They are pure noise — no build output changes — but they drown out real assembly-identity problems, inflate binlogs, and give 11 otherwise-silent targets a target header at -v:n.

Root cause

The diagnostic reads TaskInstance.GetType().Assembly.Location to detect when Assembly.LoadFrom() resolved a task assembly to a location other than the one we probed. That read is only meaningful when TaskInstance is the task. For a task routed to a TaskHost, TaskInstance is a TaskHostTask proxy declared in Microsoft.Build.dll, and the real assembly is loaded in the task host process — so the read can never say anything about where the task came from.

IsTaskAssemblyMatchFactoryType() (added in #12509) was meant to suppress the message in that case, but it compared AssemblyLoadInfo.AssemblyLocation with LoadedType.Path:

• AssemblyLoadInfo.AssemblyLocation returns the assembly name for tasks registered by AssemblyName — nearly everything in Microsoft.Common.tasks.
• For tasks registered by AssemblyFile it returns the registration-time path, which is not always byte-identical to the normalized Assembly.Location.

Either way the comparison fails and the message is logged.

The guard was also tautological in every real code path: AssemblyTaskFactory hands the same LoadedType instance to both TaskFactoryWrapper.TaskFactoryLoadedType and TaskHostTask, so it could only ever produce false positives — it could not detect a genuine mismatch.

Because virtually nothing runs in a TaskHost in a normal build, this was never exercised. -mt routes every task without [MSBuildMultiThreadableTask] to a sidecar TaskHost, making it the common path.

Changes

Skip the diagnostic when TaskInstance is TaskHostTask, before the (now wasted) Assembly.Location read. The in-proc Assembly.LoadFrom identity-redirect case the diagnostic was written for is unchanged.

This also covers the second TaskHost route — CreateTaskHostTaskForOutOfProcFactory, used for inline tasks under -mt — which a fix that merely compared AssemblyLoadInfo objects would have missed.

Removed the now-dead IsTaskAssemblyMatchFactoryType local function and TaskHostTask.LoadedTaskAssemblyInfo (internal type, no API surface change).

No ChangeWave: this only removes bogus normal-importance messages; no build behavior changes.

Testing

New TaskHostRoutedTask_DoesNotLogAssemblyLocationMismatch theory in TaskRouter_IntegrationTests, covering both TaskHost routes:

• MultiThreaded = true with the default factory (sidecar TaskHost — the reported scenario)
• explicit TaskFactory="TaskHostFactory" without -mt (proves the bug isn't -mt-specific)

The registered AssemblyFile deliberately contains a redundant . segment so it is not byte-identical to Assembly.Location — the same normalization difference real builds hit. Verified the test fails on main with exactly the reported message and passes with the fix.

Also ran TaskRouter_IntegrationTests, TaskHostFactory_Tests and TaskExecutionHost_Tests (165/165 on net11.0 and net472) plus the full Microsoft.Build.Engine.UnitTests suite; remaining failures are pre-existing/environmental and unrelated.

Notes

Reviewers may want to consider a follow-up that makes the diagnostic meaningful for out-of-proc tasks by reporting it from the task host process, where the assembly is actually loaded.

[Copilot]

Pull request overview

This PR reduces binlog and console noise by preventing TaskAssemblyLocationMismatch from being logged for TaskHost-routed tasks, where the in-proc TaskHostTask is only a proxy and its Assembly.Location is inherently unrelated to the actual task assembly (loaded in the task host process).

Changes:

• Skip the Assembly.Location mismatch diagnostic when TaskInstance is a TaskHostTask (both NET and non-NET TFMs).
• Remove the now-dead IsTaskAssemblyMatchFactoryType local function and the unused TaskHostTask.LoadedTaskAssemblyInfo property.
• Add an integration regression test ensuring TaskHost-routed tasks do not emit TaskAssemblyLocationMismatch (covers both -mt routing and explicit TaskFactory="TaskHostFactory" routing).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/Build/Instance/TaskFactories/TaskHostTask.cs	Removes unused internal LoadedTaskAssemblyInfo accessor.
src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs	Suppresses mismatch diagnostic for TaskHostTask proxies; deletes dead/incorrect guard logic.
src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs	Adds regression coverage for the “no mismatch message for TaskHost-routed tasks” behavior.

Comments suppressed due to low confidence (1)

src/Build.UnitTests/BackEnd/TaskRouter_IntegrationTests.cs:427

• Prefer collection expressions over new[] { ... } for target-typed string arrays in tests.

                new[] { "TestTarget" },

[github-actions]

Thanks for the fix — I reviewed this across the MSBuild dimensions and I don’t see any blocking issues.

The TaskInstance is not TaskHostTask guard is the right fix: for task-host-routed tasks the in-proc proxy type always comes from Microsoft.Build.dll, so the previous assembly-location comparison was inherently meaningless and produced spurious TaskAssemblyLocationMismatch messages.

A few specific checks:

• Back-compat / ChangeWave: this removes incorrect MessageImportance.Normal output only; it does not change execution semantics, warnings, or errors. I don’t think this needs a ChangeWave.
• API surface: TaskHostTask is internal, and removing LoadedTaskAssemblyInfo is not a public API break.
• Test coverage: the new regression test covers both TaskHost routing paths (MultiThreaded=true and explicit TaskHostFactory) and uses a locale-safe assertion for the message text.
• Cross-platform: the Path.Combine(..., ".", ...) non-normalized path case is valid on Unix as well, so the test looks portable.
• .NET Framework branch: the new #else guard is consistent with the bug fix there too; previously the suppression helper was ineffective for TaskHostTask, so skipping the diagnostic entirely for proxies is the correct behavior.

I also checked the removed member in TaskHostTask; it was internal-only and I found no remaining references.

Overall: no findings from my review.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Expert Code Review (on open) for #14550 · sonnet46 · 71 AIC · ⌖ 5.11 AIC · ⊞ 4.9K