From 731caaf8e1bf24eb1c017d51aa74af98e3ef1d1a Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Tue, 21 Apr 2026 15:16:49 +0200 Subject: [PATCH 1/4] Migrate GetAssemblyIdentity to multithreaded execution model Absolutize item.ItemSpec via TaskEnvironment.GetAbsolutePath() before AssemblyName.GetAssemblyName. Add tests for baseline behavior, project-relative resolution, output integrity, and metadata copying. Fixes #13571 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../GetAssemblyIdentity_Tests.cs | 182 ++++++++++++++++++ src/Tasks/GetAssemblyIdentity.cs | 22 ++- 2 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs diff --git a/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs b/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs new file mode 100644 index 00000000000..153374fb940 --- /dev/null +++ b/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs @@ -0,0 +1,182 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using System.Reflection; +using Microsoft.Build.Framework; +using Microsoft.Build.Tasks; +using Microsoft.Build.Utilities; +using Shouldly; +using Xunit; + +#nullable disable + +namespace Microsoft.Build.UnitTests +{ + public sealed class GetAssemblyIdentity_Tests + { + private readonly ITestOutputHelper _output; + + public GetAssemblyIdentity_Tests(ITestOutputHelper output) + { + _output = output; + } + + private GetAssemblyIdentity CreateTaskUnderTest(MockEngine engine = null) + { + return new GetAssemblyIdentity + { + BuildEngine = engine ?? new MockEngine(_output), + }; + } + + [Fact] + public void AbsolutePathToExistingAssembly_ProducesCorrectIdentity() + { + // Use this test assembly as the input — it's guaranteed to exist. + string assemblyPath = typeof(GetAssemblyIdentity_Tests).Assembly.Location; + AssemblyName expectedName = AssemblyName.GetAssemblyName(assemblyPath); + + GetAssemblyIdentity task = CreateTaskUnderTest(); + task.AssemblyFiles = [new TaskItem(assemblyPath)]; + + task.Execute().ShouldBeTrue(); + + task.Assemblies.Length.ShouldBe(1); + task.Assemblies[0].ItemSpec.ShouldBe(expectedName.FullName); + task.Assemblies[0].GetMetadata("Name").ShouldBe(expectedName.Name); + task.Assemblies[0].GetMetadata("Version").ShouldBe(expectedName.Version.ToString()); + } + + [Fact] + public void NonExistentFile_LogsErrorAndReturnsFalse() + { + var engine = new MockEngine(_output); + GetAssemblyIdentity task = CreateTaskUnderTest(engine); + task.AssemblyFiles = [new TaskItem("does-not-exist.dll")]; + + task.Execute().ShouldBeFalse(); + + AssertCouldNotGetAssemblyName(engine); + engine.Log.ShouldContain("does-not-exist.dll"); + } + + [Fact] + public void NonAssemblyFile_LogsErrorAndReturnsFalse() + { + using TestEnvironment env = TestEnvironment.Create(_output); + TransientTestFile textFile = env.CreateFile("notanassembly.txt", "This is not an assembly."); + + var engine = new MockEngine(_output); + GetAssemblyIdentity task = CreateTaskUnderTest(engine); + task.AssemblyFiles = [new TaskItem(textFile.Path)]; + + task.Execute().ShouldBeFalse(); + + AssertCouldNotGetAssemblyName(engine); + } + + [Fact] + public void MixedBatch_GoodAndBadItems() + { + using TestEnvironment env = TestEnvironment.Create(_output); + TransientTestFile textFile = env.CreateFile("bad.txt", "not an assembly"); + + string goodAssemblyPath = typeof(GetAssemblyIdentity_Tests).Assembly.Location; + AssemblyName expectedName = AssemblyName.GetAssemblyName(goodAssemblyPath); + + var engine = new MockEngine(_output); + GetAssemblyIdentity task = CreateTaskUnderTest(engine); + task.AssemblyFiles = + [ + new TaskItem(goodAssemblyPath), + new TaskItem(textFile.Path), + ]; + + // Execute returns false because one item failed. + task.Execute().ShouldBeFalse(); + + // The good item should still appear in the output. + task.Assemblies.Length.ShouldBe(1); + task.Assemblies[0].ItemSpec.ShouldBe(expectedName.FullName); + + // The bad item should have produced an error. + engine.Errors.ShouldBe(1); + } + + [Fact] + public void EmptyItemSpec_LogsErrorAndReturnsFalse() + { + var engine = new MockEngine(_output); + GetAssemblyIdentity task = CreateTaskUnderTest(engine); + task.AssemblyFiles = [new TaskItem(string.Empty)]; + + task.Execute().ShouldBeFalse(); + + AssertCouldNotGetAssemblyName(engine); + } + + [Fact] + public void RelativePath_ResolvesAgainstProjectDirectory() + { + using TestEnvironment env = TestEnvironment.Create(_output); + TransientTestFolder projectDir = env.CreateFolder(); + + string sourceAssembly = typeof(GetAssemblyIdentity_Tests).Assembly.Location; + AssemblyName expectedName = AssemblyName.GetAssemblyName(sourceAssembly); + + string relativeFileName = "TestAssembly.dll"; + File.Copy(sourceAssembly, Path.Combine(projectDir.Path, relativeFileName)); + + var engine = new MockEngine(_output); + GetAssemblyIdentity task = CreateTaskUnderTest(engine); + task.TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path); + task.AssemblyFiles = [new TaskItem(relativeFileName)]; + + task.Execute().ShouldBeTrue(); + + task.Assemblies.Length.ShouldBe(1); + task.Assemblies[0].ItemSpec.ShouldBe(expectedName.FullName); + } + + [Fact] + public void OutputItem_DoesNotContainAbsolutizedPaths() + { + string assemblyPath = typeof(GetAssemblyIdentity_Tests).Assembly.Location; + AssemblyName expectedName = AssemblyName.GetAssemblyName(assemblyPath); + + GetAssemblyIdentity task = CreateTaskUnderTest(); + task.AssemblyFiles = [new TaskItem(assemblyPath)]; + + task.Execute().ShouldBeTrue(); + + task.Assemblies.Length.ShouldBe(1); + + // ItemSpec should be the assembly identity string, not a path. + task.Assemblies[0].ItemSpec.ShouldBe(expectedName.FullName); + } + + [Fact] + public void OutputItem_CopiesInputMetadataVerbatim() + { + string assemblyPath = typeof(GetAssemblyIdentity_Tests).Assembly.Location; + + var inputItem = new TaskItem(assemblyPath); + inputItem.SetMetadata("CustomMeta", "CustomValue"); + + GetAssemblyIdentity task = CreateTaskUnderTest(); + task.AssemblyFiles = [inputItem]; + + task.Execute().ShouldBeTrue(); + + task.Assemblies.Length.ShouldBe(1); + task.Assemblies[0].GetMetadata("CustomMeta").ShouldBe("CustomValue"); + } + + private void AssertCouldNotGetAssemblyName(MockEngine engine) + { + engine.Errors.ShouldBe(1); + engine.Log.ShouldContain("MSB3441"); + } + } +} diff --git a/src/Tasks/GetAssemblyIdentity.cs b/src/Tasks/GetAssemblyIdentity.cs index 375e0d2d430..e75e5eb4f77 100644 --- a/src/Tasks/GetAssemblyIdentity.cs +++ b/src/Tasks/GetAssemblyIdentity.cs @@ -23,8 +23,14 @@ namespace Microsoft.Build.Tasks /// Input: Assembly Include="foo.exe" /// Output: Identity Include="Foo, Version=1.0.0.0", Name="Foo, Version="1.0.0.0" /// - public class GetAssemblyIdentity : TaskExtension + [MSBuildMultiThreadableTask] + public class GetAssemblyIdentity : TaskExtension, IMultiThreadableTask { + /// + /// Gets or sets the task execution environment for thread-safe path resolution. + /// + public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; + private ITaskItem[] _assemblyFiles; [Required] @@ -65,10 +71,21 @@ public override bool Execute() var list = new List(); foreach (ITaskItem item in AssemblyFiles) { + AbsolutePath assemblyPath; + try + { + assemblyPath = TaskEnvironment.GetAbsolutePath(item.ItemSpec); + } + catch (ArgumentException e) + { + Log.LogErrorWithCodeFromResources("GetAssemblyIdentity.CouldNotGetAssemblyName", item.ItemSpec, e.Message); + continue; + } + AssemblyName an; try { - an = AssemblyName.GetAssemblyName(item.ItemSpec); + an = AssemblyName.GetAssemblyName(assemblyPath); } catch (BadImageFormatException e) { @@ -100,6 +117,7 @@ public override bool Execute() item.CopyMetadataTo(newItem); list.Add(newItem); } + Assemblies = list.ToArray(); return !Log.HasLoggedErrors; } From 3a757f87bf4e2ff927ff6d7d3ae2b125a0aeafd6 Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Thu, 23 Apr 2026 12:37:14 +0200 Subject: [PATCH 2/4] Changes behavior to throw on null args --- src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs | 13 ++++++++++--- src/Tasks/GetAssemblyIdentity.cs | 13 +++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs b/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs index 153374fb940..54961584eed 100644 --- a/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs +++ b/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.IO; using System.Reflection; using Microsoft.Build.Framework; @@ -109,11 +110,17 @@ public void EmptyItemSpec_LogsErrorAndReturnsFalse() { var engine = new MockEngine(_output); GetAssemblyIdentity task = CreateTaskUnderTest(engine); - task.AssemblyFiles = [new TaskItem(string.Empty)]; + task.AssemblyFiles = [new TaskItem("")]; - task.Execute().ShouldBeFalse(); + Assert.Throws(() => task.Execute()); + } - AssertCouldNotGetAssemblyName(engine); + [Fact] + public void NullItemSpec_LogsErrorAndReturnsFalse() + { + var engine = new MockEngine(_output); + GetAssemblyIdentity task = CreateTaskUnderTest(engine); + Assert.Throws(() => task.AssemblyFiles = [new TaskItem((ITaskItem)null)]); } [Fact] diff --git a/src/Tasks/GetAssemblyIdentity.cs b/src/Tasks/GetAssemblyIdentity.cs index e75e5eb4f77..93765f6a9e9 100644 --- a/src/Tasks/GetAssemblyIdentity.cs +++ b/src/Tasks/GetAssemblyIdentity.cs @@ -71,16 +71,9 @@ public override bool Execute() var list = new List(); foreach (ITaskItem item in AssemblyFiles) { - AbsolutePath assemblyPath; - try - { - assemblyPath = TaskEnvironment.GetAbsolutePath(item.ItemSpec); - } - catch (ArgumentException e) - { - Log.LogErrorWithCodeFromResources("GetAssemblyIdentity.CouldNotGetAssemblyName", item.ItemSpec, e.Message); - continue; - } + ArgumentNullException.ThrowIfNullOrEmpty(item.ItemSpec, nameof(item.ItemSpec)); + + AbsolutePath assemblyPath = assemblyPath = TaskEnvironment.GetAbsolutePath(item.ItemSpec); AssemblyName an; try From ffaee98a0bc8699214258a8f69d5b95513b48cee Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Thu, 23 Apr 2026 16:19:54 +0200 Subject: [PATCH 3/4] Fixes the contract to be the same as before --- src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs | 4 +++- src/Tasks/GetAssemblyIdentity.cs | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs b/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs index 54961584eed..f128675364c 100644 --- a/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs +++ b/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs @@ -112,7 +112,9 @@ public void EmptyItemSpec_LogsErrorAndReturnsFalse() GetAssemblyIdentity task = CreateTaskUnderTest(engine); task.AssemblyFiles = [new TaskItem("")]; - Assert.Throws(() => task.Execute()); + task.Execute().ShouldBeFalse(); + + AssertCouldNotGetAssemblyName(engine); } [Fact] diff --git a/src/Tasks/GetAssemblyIdentity.cs b/src/Tasks/GetAssemblyIdentity.cs index 93765f6a9e9..f886cbd15ed 100644 --- a/src/Tasks/GetAssemblyIdentity.cs +++ b/src/Tasks/GetAssemblyIdentity.cs @@ -71,9 +71,9 @@ public override bool Execute() var list = new List(); foreach (ITaskItem item in AssemblyFiles) { - ArgumentNullException.ThrowIfNullOrEmpty(item.ItemSpec, nameof(item.ItemSpec)); - - AbsolutePath assemblyPath = assemblyPath = TaskEnvironment.GetAbsolutePath(item.ItemSpec); + string assemblyPath = !string.IsNullOrEmpty(item.ItemSpec) + ? TaskEnvironment.GetAbsolutePath(item.ItemSpec) + : item.ItemSpec; AssemblyName an; try From d12b539fb1f707b22d8c180f9d3e968ade36df81 Mon Sep 17 00:00:00 2001 From: Jan Kratochvil Date: Fri, 1 May 2026 10:54:41 +0200 Subject: [PATCH 4/4] Address review feedback on PR #13588 - Replace custom on TaskEnvironment property with /// to match the convention used by Copy, MakeDir, RemoveDir, etc. - Replace tautological AssemblyName.GetAssemblyName-based test baseline with typeof(GetAssemblyIdentity_Tests).Assembly.GetName() so the test compares the task's output against a compile-time-resolved baseline rather than the same reflection API the product uses. - Collapse a verbose assertion to ShouldHaveSingleItem().GetMetadata(...) per the more compact Shouldly idiom. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs | 15 +++++++++------ src/Tasks/GetAssemblyIdentity.cs | 4 +--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs b/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs index f128675364c..c7c879c2006 100644 --- a/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs +++ b/src/Tasks.UnitTests/GetAssemblyIdentity_Tests.cs @@ -35,8 +35,11 @@ private GetAssemblyIdentity CreateTaskUnderTest(MockEngine engine = null) public void AbsolutePathToExistingAssembly_ProducesCorrectIdentity() { // Use this test assembly as the input — it's guaranteed to exist. + // Use the compile-time assembly name as the baseline rather than + // AssemblyName.GetAssemblyName(path), to avoid asserting against the same + // reflection API the production code uses internally. string assemblyPath = typeof(GetAssemblyIdentity_Tests).Assembly.Location; - AssemblyName expectedName = AssemblyName.GetAssemblyName(assemblyPath); + AssemblyName expectedName = typeof(GetAssemblyIdentity_Tests).Assembly.GetName(); GetAssemblyIdentity task = CreateTaskUnderTest(); task.AssemblyFiles = [new TaskItem(assemblyPath)]; @@ -84,7 +87,7 @@ public void MixedBatch_GoodAndBadItems() TransientTestFile textFile = env.CreateFile("bad.txt", "not an assembly"); string goodAssemblyPath = typeof(GetAssemblyIdentity_Tests).Assembly.Location; - AssemblyName expectedName = AssemblyName.GetAssemblyName(goodAssemblyPath); + AssemblyName expectedName = typeof(GetAssemblyIdentity_Tests).Assembly.GetName(); var engine = new MockEngine(_output); GetAssemblyIdentity task = CreateTaskUnderTest(engine); @@ -132,7 +135,7 @@ public void RelativePath_ResolvesAgainstProjectDirectory() TransientTestFolder projectDir = env.CreateFolder(); string sourceAssembly = typeof(GetAssemblyIdentity_Tests).Assembly.Location; - AssemblyName expectedName = AssemblyName.GetAssemblyName(sourceAssembly); + AssemblyName expectedName = typeof(GetAssemblyIdentity_Tests).Assembly.GetName(); string relativeFileName = "TestAssembly.dll"; File.Copy(sourceAssembly, Path.Combine(projectDir.Path, relativeFileName)); @@ -152,7 +155,7 @@ public void RelativePath_ResolvesAgainstProjectDirectory() public void OutputItem_DoesNotContainAbsolutizedPaths() { string assemblyPath = typeof(GetAssemblyIdentity_Tests).Assembly.Location; - AssemblyName expectedName = AssemblyName.GetAssemblyName(assemblyPath); + AssemblyName expectedName = typeof(GetAssemblyIdentity_Tests).Assembly.GetName(); GetAssemblyIdentity task = CreateTaskUnderTest(); task.AssemblyFiles = [new TaskItem(assemblyPath)]; @@ -178,8 +181,8 @@ public void OutputItem_CopiesInputMetadataVerbatim() task.Execute().ShouldBeTrue(); - task.Assemblies.Length.ShouldBe(1); - task.Assemblies[0].GetMetadata("CustomMeta").ShouldBe("CustomValue"); + task.Assemblies.ShouldHaveSingleItem() + .GetMetadata("CustomMeta").ShouldBe("CustomValue"); } private void AssertCouldNotGetAssemblyName(MockEngine engine) diff --git a/src/Tasks/GetAssemblyIdentity.cs b/src/Tasks/GetAssemblyIdentity.cs index f886cbd15ed..ab7fbed39fd 100644 --- a/src/Tasks/GetAssemblyIdentity.cs +++ b/src/Tasks/GetAssemblyIdentity.cs @@ -26,9 +26,7 @@ namespace Microsoft.Build.Tasks [MSBuildMultiThreadableTask] public class GetAssemblyIdentity : TaskExtension, IMultiThreadableTask { - /// - /// Gets or sets the task execution environment for thread-safe path resolution. - /// + /// public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; private ITaskItem[] _assemblyFiles;