-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Multithreading migration: Group 10 — 4 ReadyToRun & ToolTask tasks (Pattern B) #53121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
1d956d8
Migrate 4 ReadyToRun & ToolTask tasks to IMultiThreadableTask (Group 10)
SimaTian e1a3dcc
Absolutize command-line and output paths in Group 10 tasks
SimaTian 3a373c8
Fix compilation error: pass absoluteOutputPath as parameter to Proces…
SimaTian 95dabe6
Add concurrent execution tests for Group 10 tasks
SimaTian b3d3fc8
Absolutize GenerateFullPathToTool in RunReadyToRunCompiler (Group 10)
SimaTian 276594e
Fix AbsolutePath to string conversion in RunCsWinRTGenerator
SimaTian 7b92a9a
Fix tests: use non-empty OutputPath for PrepareForReadyToRunCompilation
SimaTian fc8ef86
Add TaskEnvironment lazy-init for merge-group-10 tasks
SimaTian 2e38d21
Fix AbsolutePath combining constructor: normalize '..' segments
SimaTian 17018dc
Add UseShellExecute=false in ProcessTaskEnvironmentDriver
SimaTian bd524c7
Revert AbsolutePath: remove Path.GetFullPath wrapping
SimaTian a4faad5
Fix CWD-dependent Strings.resx loading in GivenThatWeHaveErrorCodes
SimaTian File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| // Provides a default TaskEnvironment for single-threaded MSBuild execution. | ||
| // When MSBuild supports IMultiThreadableTask, it sets TaskEnvironment directly. | ||
| // This fallback ensures tasks work with older MSBuild versions that do not set it. | ||
|
|
||
| #if NETFRAMEWORK | ||
|
|
||
| using System; | ||
|
|
||
| namespace Microsoft.Build.Framework | ||
| { | ||
| internal static class TaskEnvironmentDefaults | ||
| { | ||
| /// <summary> | ||
| /// Creates a default TaskEnvironment backed by the current process environment. | ||
| /// Uses Environment.CurrentDirectory as the project directory, which in single-threaded | ||
| /// MSBuild is set to the project directory before task execution. | ||
| /// </summary> | ||
| internal static TaskEnvironment Create() => | ||
| new TaskEnvironment(new ProcessTaskEnvironmentDriver(Environment.CurrentDirectory)); | ||
| } | ||
| } | ||
|
|
||
| #endif |
185 changes: 185 additions & 0 deletions
185
...icrosoft.NET.Build.Tasks.UnitTests/GivenAPrepareForReadyToRunCompilationMultiThreading.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections.Concurrent; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using FluentAssertions; | ||
| using Microsoft.Build.Framework; | ||
| using Microsoft.Build.Utilities; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.NET.Build.Tasks.UnitTests | ||
| { | ||
| public class GivenAPrepareForReadyToRunCompilationMultiThreading | ||
| { | ||
| [Fact] | ||
| public void NullInputs_DoNotCrash() | ||
| { | ||
| var engine = new MockBuildEngine(); | ||
| var task = new PrepareForReadyToRunCompilation | ||
| { | ||
| BuildEngine = engine, | ||
| MainAssembly = new TaskItem("nonexistent.dll"), | ||
| OutputPath = "output", | ||
| IncludeSymbolsInSingleFile = false, | ||
| Assemblies = null, | ||
| ReadyToRunUseCrossgen2 = false, | ||
| }; | ||
|
|
||
| task.TaskEnvironment = TaskEnvironmentHelper.CreateForTest(); | ||
|
|
||
| // With null Assemblies and no crossgen tool, Execute should complete without NRE | ||
| var result = task.Execute(); | ||
|
|
||
| // Task may log errors about missing crossgen tool, but must not throw NRE | ||
| result.Should().BeTrue("null Assemblies causes early return with no errors"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DiaSymReaderPath_IsResolvedRelativeToProjectDirectory() | ||
| { | ||
| var projectDir = Path.GetFullPath(Path.Combine(Path.GetTempPath(), $"r2r-prepare-mt-{Guid.NewGuid():N}")); | ||
| Directory.CreateDirectory(projectDir); | ||
| try | ||
| { | ||
| // Create a fake DiaSymReader file at a relative path | ||
| var toolsDir = Path.Combine(projectDir, "tools"); | ||
| Directory.CreateDirectory(toolsDir); | ||
| File.WriteAllText(Path.Combine(toolsDir, "diasymreader.dll"), "fake"); | ||
|
|
||
| var crossgenTool = new TaskItem("tools\\crossgen.exe"); | ||
| crossgenTool.SetMetadata("DiaSymReader", "tools\\diasymreader.dll"); | ||
| crossgenTool.SetMetadata("JitPath", "tools\\clrjit.dll"); | ||
|
|
||
| var task = new PrepareForReadyToRunCompilation | ||
| { | ||
| BuildEngine = new MockBuildEngine(), | ||
| MainAssembly = new TaskItem("test.dll"), | ||
| OutputPath = "output", | ||
| IncludeSymbolsInSingleFile = false, | ||
| Assemblies = null, | ||
| ReadyToRunUseCrossgen2 = false, | ||
| CrossgenTool = crossgenTool, | ||
| EmitSymbols = true, | ||
| TaskEnvironment = TaskEnvironmentHelper.CreateForTest(projectDir), | ||
| }; | ||
|
|
||
| var result = task.Execute(); | ||
|
|
||
| // With null Assemblies, ProcessInputFileList returns early. | ||
| // The key point is that DiaSymReader at "tools\diasymreader.dll" | ||
| // is resolved via TaskEnvironment relative to projectDir and found. | ||
| result.Should().BeTrue("null Assemblies produces no errors"); | ||
| } | ||
| finally | ||
| { | ||
| Directory.Delete(projectDir, true); | ||
| } | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DualModeParity_SameResultRegardlessOfCwd() | ||
| { | ||
| var projectDir = Path.GetFullPath(Path.Combine(Path.GetTempPath(), $"r2r-prepare-dual-{Guid.NewGuid():N}")); | ||
| var otherDir = Path.GetFullPath(Path.Combine(Path.GetTempPath(), $"r2r-prepare-other-{Guid.NewGuid():N}")); | ||
| Directory.CreateDirectory(projectDir); | ||
| Directory.CreateDirectory(otherDir); | ||
| try | ||
| { | ||
| // Create a fake DiaSymReader file under projectDir | ||
| var toolsDir = Path.Combine(projectDir, "tools"); | ||
| Directory.CreateDirectory(toolsDir); | ||
| File.WriteAllText(Path.Combine(toolsDir, "diasymreader.dll"), "fake"); | ||
|
|
||
| var taskEnv = TaskEnvironmentHelper.CreateForTest(projectDir); | ||
|
|
||
| // Run with CWD = projectDir | ||
| var savedCwd = Directory.GetCurrentDirectory(); | ||
| Directory.SetCurrentDirectory(projectDir); | ||
|
|
||
| var engine1 = new MockBuildEngine(); | ||
| var crossgen1 = new TaskItem("tools\\crossgen.exe"); | ||
| crossgen1.SetMetadata("DiaSymReader", "tools\\diasymreader.dll"); | ||
| crossgen1.SetMetadata("JitPath", "tools\\clrjit.dll"); | ||
|
|
||
| var task1 = new PrepareForReadyToRunCompilation | ||
| { | ||
| BuildEngine = engine1, | ||
| MainAssembly = new TaskItem("test.dll"), | ||
| OutputPath = "output", | ||
| IncludeSymbolsInSingleFile = false, | ||
| Assemblies = null, | ||
| ReadyToRunUseCrossgen2 = false, | ||
| CrossgenTool = crossgen1, | ||
| EmitSymbols = true, | ||
| TaskEnvironment = taskEnv, | ||
| }; | ||
| var result1 = task1.Execute(); | ||
|
|
||
| // Run with CWD = otherDir (different from projectDir) | ||
| Directory.SetCurrentDirectory(otherDir); | ||
|
|
||
| var engine2 = new MockBuildEngine(); | ||
| var crossgen2 = new TaskItem("tools\\crossgen.exe"); | ||
| crossgen2.SetMetadata("DiaSymReader", "tools\\diasymreader.dll"); | ||
| crossgen2.SetMetadata("JitPath", "tools\\clrjit.dll"); | ||
|
|
||
| var task2 = new PrepareForReadyToRunCompilation | ||
| { | ||
| BuildEngine = engine2, | ||
| MainAssembly = new TaskItem("test.dll"), | ||
| OutputPath = "output", | ||
| IncludeSymbolsInSingleFile = false, | ||
| Assemblies = null, | ||
| ReadyToRunUseCrossgen2 = false, | ||
| CrossgenTool = crossgen2, | ||
| EmitSymbols = true, | ||
| TaskEnvironment = taskEnv, | ||
| }; | ||
| var result2 = task2.Execute(); | ||
|
|
||
| Directory.SetCurrentDirectory(savedCwd); | ||
|
|
||
| // Both runs should produce the same result regardless of CWD | ||
| result1.Should().Be(result2, "TaskEnvironment resolves paths independent of CWD"); | ||
| engine1.Errors.Count.Should().Be(engine2.Errors.Count, | ||
| "same errors regardless of CWD"); | ||
| } | ||
| finally | ||
| { | ||
| Directory.Delete(projectDir, true); | ||
| Directory.Delete(otherDir, true); | ||
| } | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(4)] | ||
| [InlineData(16)] | ||
| public void PrepareForReadyToRunCompilation_ConcurrentExecution(int parallelism) | ||
| { | ||
| var errors = new ConcurrentBag<string>(); | ||
| var barrier = new Barrier(parallelism); | ||
| Parallel.For(0, parallelism, new ParallelOptions { MaxDegreeOfParallelism = parallelism }, i => | ||
| { | ||
| try | ||
| { | ||
| var task = new PrepareForReadyToRunCompilation | ||
| { | ||
| BuildEngine = new MockBuildEngine(), | ||
| TaskEnvironment = TaskEnvironmentHelper.CreateForTest(), | ||
| MainAssembly = new TaskItem("nonexistent.dll"), | ||
| OutputPath = "output", | ||
| IncludeSymbolsInSingleFile = false, | ||
| Assemblies = null, | ||
| ReadyToRunUseCrossgen2 = false, | ||
| }; | ||
| barrier.SignalAndWait(); | ||
| task.Execute(); | ||
| } | ||
| catch (Exception ex) { errors.Add($"Thread {i}: {ex.Message}"); } | ||
| }); | ||
| errors.Should().BeEmpty(); | ||
| } | ||
| } | ||
| } |
100 changes: 100 additions & 0 deletions
100
...sks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolveReadyToRunCompilersMultiThreading.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections.Concurrent; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using FluentAssertions; | ||
| using Microsoft.Build.Framework; | ||
| using Microsoft.Build.Utilities; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.NET.Build.Tasks.UnitTests | ||
| { | ||
| public class GivenAResolveReadyToRunCompilersMultiThreading | ||
| { | ||
| [Fact] | ||
| public void EmptyRuntimePacks_LogsErrorGracefully() | ||
| { | ||
| var engine = new MockBuildEngine(); | ||
| var task = new ResolveReadyToRunCompilers | ||
| { | ||
| BuildEngine = engine, | ||
| RuntimePacks = new ITaskItem[] { new TaskItem("SomePack") }, | ||
| TargetingPacks = Array.Empty<ITaskItem>(), | ||
| RuntimeGraphPath = "nonexistent.json", | ||
| NETCoreSdkRuntimeIdentifier = "win-x64", | ||
| ReadyToRunUseCrossgen2 = false, | ||
| TaskEnvironment = TaskEnvironmentHelper.CreateForTest(), | ||
| }; | ||
|
|
||
| // RuntimePacks has no matching NETCore.App pack, so it should log error gracefully | ||
| var result = task.Execute(); | ||
|
|
||
| result.Should().BeFalse("no valid NETCore.App runtime pack exists"); | ||
| engine.Errors.Should().NotBeEmpty("should log an error about missing runtime pack"); | ||
| // Verify no NullReferenceException — error is about missing pack, not a crash | ||
| engine.Errors.Select(e => e.Message).Should().NotContain( | ||
| e => e.Contains("NullReference", StringComparison.OrdinalIgnoreCase), | ||
| "should not crash with NullReferenceException"); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void TaskEnvironmentProperty_IsWirable() | ||
| { | ||
| var projectDir = Path.GetFullPath(Path.Combine(Path.GetTempPath(), $"resolve-r2r-mt-{Guid.NewGuid():N}")); | ||
| Directory.CreateDirectory(projectDir); | ||
| try | ||
| { | ||
| var task = new ResolveReadyToRunCompilers | ||
| { | ||
| BuildEngine = new MockBuildEngine(), | ||
| RuntimePacks = new ITaskItem[] { new TaskItem("SomePack") }, | ||
| TargetingPacks = Array.Empty<ITaskItem>(), | ||
| RuntimeGraphPath = "runtime.json", | ||
| NETCoreSdkRuntimeIdentifier = "win-x64", | ||
| }; | ||
|
|
||
| var teProp = task.GetType().GetProperty("TaskEnvironment"); | ||
| teProp.Should().NotBeNull("task must have a TaskEnvironment property after migration"); | ||
| teProp!.SetValue(task, TaskEnvironmentHelper.CreateForTest(projectDir)); | ||
|
|
||
| // The task should have TaskEnvironment set | ||
| task.TaskEnvironment.Should().NotBeNull(); | ||
| } | ||
| finally | ||
| { | ||
| Directory.Delete(projectDir, true); | ||
| } | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(4)] | ||
| [InlineData(16)] | ||
| public void ResolveReadyToRunCompilers_ConcurrentExecution(int parallelism) | ||
| { | ||
| var errors = new ConcurrentBag<string>(); | ||
| var barrier = new Barrier(parallelism); | ||
| Parallel.For(0, parallelism, new ParallelOptions { MaxDegreeOfParallelism = parallelism }, i => | ||
| { | ||
| try | ||
| { | ||
| var task = new ResolveReadyToRunCompilers | ||
| { | ||
| BuildEngine = new MockBuildEngine(), | ||
| TaskEnvironment = TaskEnvironmentHelper.CreateForTest(), | ||
| RuntimePacks = new ITaskItem[] { new TaskItem("SomePack") }, | ||
| TargetingPacks = Array.Empty<ITaskItem>(), | ||
| RuntimeGraphPath = "nonexistent.json", | ||
| NETCoreSdkRuntimeIdentifier = "win-x64", | ||
| ReadyToRunUseCrossgen2 = false, | ||
| }; | ||
| barrier.SignalAndWait(); | ||
| task.Execute(); | ||
| } | ||
| catch (Exception ex) { errors.Add($"Thread {i}: {ex.Message}"); } | ||
| }); | ||
| errors.Should().BeEmpty(); | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for this change?