Skip to content

Commit 27296fe

Browse files
authored
add multithreaded task migration agent skill (#13131)
1 parent 80d7328 commit 27296fe

1 file changed

Lines changed: 244 additions & 0 deletions

File tree

  • .github/skills/multithreaded-task-migration
Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
---
2+
name: multithreaded-task-migration
3+
description: Guide for migrating MSBuild tasks to the multithreaded mode support. Use this when asked to convert tasks to thread-safe versions, implement IMultiThreadableTask, or add TaskEnvironment support to tasks.
4+
---
5+
6+
# Migrating MSBuild Tasks to Multithreaded API
7+
8+
This skill guides you through migrating MSBuild tasks to support multithreaded execution by implementing `IMultiThreadableTask` and using `TaskEnvironment`.
9+
10+
## Overview
11+
12+
MSBuild's multithreaded execution model requires tasks to avoid global process state (working directory, environment variables). Thread-safe tasks declare this capability by annotating with `MSBuildMultiThreadableTask` and use `TaskEnvironment` provided by `IMultiThreadableTask` for safe alternatives.
13+
14+
## Migration Steps
15+
16+
### Step 1: Update Task Class Declaration
17+
18+
a. add the attribute
19+
b. AND implement the interface if it's necessary to use TaskEnvironment APIs.
20+
21+
```csharp
22+
[MSBuildMultiThreadableTask]
23+
public class MyTask : Task, IMultiThreadableTask
24+
{
25+
public TaskEnvironment TaskEnvironment { get; set; }
26+
...
27+
}
28+
```
29+
30+
### Step 2: Absolutize Paths Before File Operations
31+
32+
**Critical**: All path strings must be absolutized with `TaskEnvironment.GetAbsolutePath()` before use in file system APIs. This ensures paths resolve relative to the project directory, not the process working directory.
33+
34+
```csharp
35+
// BEFORE - File.Exists uses process working directory for relative paths (UNSAFE)
36+
if (File.Exists(inputPath))
37+
{
38+
string content = File.ReadAllText(inputPath);
39+
}
40+
41+
// AFTER - Absolutize first, then use in file operations (SAFE)
42+
AbsolutePath absolutePath = TaskEnvironment.GetAbsolutePath(inputPath);
43+
if (File.Exists(absolutePath))
44+
{
45+
string content = File.ReadAllText(absolutePath);
46+
}
47+
```
48+
49+
`GetAbsolutePath()` throws for null/empty inputs. See [Exception Handling in Batch Operations](#exception-handling-in-batch-operations) for handling strategies.
50+
51+
The [`AbsolutePath`](https://github.com/dotnet/msbuild/blob/main/src/Framework/PathHelpers/AbsolutePath.cs) struct:
52+
- Has `Value` property returning the absolute path string
53+
- Has `OriginalValue` property preserving the input path
54+
- Is implicitly convertible to `string` for File/Directory API compatibility
55+
56+
**CAUTION**: `FileInfo` can be created from relative paths - only use `FileInfo.FullName` if constructed with an absolute path.
57+
58+
#### Note:
59+
If code previously used `Path.GetFullPath()` for canonicalization (resolving `..` segments, normalizing separators), call `AbsolutePath.GetCanonicalForm()` after absolutization to preserve that behavior. Do not simply replace `Path.GetFullPath` with `GetAbsolutePath` if canonicalization was the intent. You can replace `Path.GetFullPath` behavior by combining both:
60+
61+
```csharp
62+
AbsolutePath absolutePath = TaskEnvironment.GetAbsolutePath(inputPath).GetCanonicalForm();
63+
```
64+
The goal is MAXIMUM compatibility so think about these edge cases so it behaves the same as before.
65+
66+
### Step 3: Replace Environment Variable APIs
67+
68+
```csharp
69+
// BEFORE (UNSAFE)
70+
string value = Environment.GetEnvironmentVariable("VAR");
71+
Environment.SetEnvironmentVariable("VAR", "value");
72+
73+
// AFTER (SAFE)
74+
string value = TaskEnvironment.GetEnvironmentVariable("VAR");
75+
TaskEnvironment.SetEnvironmentVariable("VAR", "value");
76+
```
77+
78+
### Step 4: Replace Process Start APIs
79+
80+
```csharp
81+
// BEFORE (UNSAFE - inherits process state)
82+
var psi = new ProcessStartInfo("tool.exe");
83+
84+
// AFTER (SAFE - uses task's isolated environment)
85+
var psi = TaskEnvironment.GetProcessStartInfo();
86+
psi.FileName = "tool.exe";
87+
```
88+
89+
## Updating Unit Tests
90+
91+
**Every test creating a task instance must set TaskEnvironment.** Use `TaskEnvironmentHelper.CreateForTest()`:
92+
93+
```csharp
94+
// BEFORE
95+
var task = new Copy
96+
{
97+
BuildEngine = new MockEngine(true),
98+
SourceFiles = sourceFiles,
99+
DestinationFolder = new TaskItem(destFolder),
100+
};
101+
102+
// AFTER
103+
var task = new Copy
104+
{
105+
TaskEnvironment = TaskEnvironmentHelper.CreateForTest(),
106+
BuildEngine = new MockEngine(true),
107+
SourceFiles = sourceFiles,
108+
DestinationFolder = new TaskItem(destFolder),
109+
};
110+
```
111+
112+
### Testing Exception Cases
113+
114+
Tasks must handle null/empty path inputs properly.
115+
116+
```csharp
117+
[Fact]
118+
public void Task_WithNullPath_Throws()
119+
{
120+
var task = CreateTask();
121+
122+
Should.Throw<ArgumentNullException>(() => task.ProcessPath(null!));
123+
}
124+
```
125+
126+
## APIs to Avoid
127+
128+
### Critical Errors (No Alternative)
129+
- `Environment.Exit()`, `Environment.FailFast()` - Return false or throw instead
130+
- `Process.GetCurrentProcess().Kill()` - Never terminate process
131+
- `ThreadPool.SetMinThreads/MaxThreads` - Process-wide settings
132+
- `CultureInfo.DefaultThreadCurrentCulture` (setter) - Affects all threads
133+
- `Console.*` - Interferes with logging
134+
135+
### Requires TaskEnvironment
136+
- `Environment.CurrentDirectory``TaskEnvironment.ProjectDirectory`
137+
- `Environment.GetEnvironmentVariable``TaskEnvironment.GetEnvironmentVariable`
138+
- `Environment.SetEnvironmentVariable``TaskEnvironment.SetEnvironmentVariable`
139+
- `Path.GetFullPath``TaskEnvironment.GetAbsolutePath`
140+
- `Process.Start`, `ProcessStartInfo``TaskEnvironment.GetProcessStartInfo`
141+
142+
### File APIs Need Absolute Paths
143+
- `File.*`, `Directory.*`, `FileInfo`, `DirectoryInfo`, `FileStream`, `StreamReader`, `StreamWriter`
144+
- All path parameters must be absolute
145+
146+
### Potential Issues (Review Required)
147+
- `Assembly.Load*`, `LoadFrom`, `LoadFile` - Version conflicts
148+
- `Activator.CreateInstance*` - Version conflicts
149+
150+
## Practical Notes
151+
152+
### CRITICAL: Trace All Path String Usage
153+
154+
**You MUST trace every path string variable through the entire codebase** to find all places where it flows into file system operations - including helper methods, utility classes, and third-party code that may internally use File APIs.
155+
156+
Steps:
157+
1. Find every path string (e.g., `item.ItemSpec`, function parameters)
158+
2. **Trace downstream**: Follow the variable through all method calls and assignments
159+
3. Absolutize BEFORE any code path that touches the file system
160+
4. Use `OriginalValue` for user-facing output (logs, errors)
161+
162+
```csharp
163+
// WRONG - LockCheck internally uses File APIs with non-absolutized path
164+
string sourceSpec = item.ItemSpec; // sourceSpec is string
165+
string lockedMsg = LockCheck.GetLockedFileMessage(sourceSpec); // BUG! Trace the call!
166+
167+
// CORRECT - absolutized path passed to helper
168+
AbsolutePath sourceFile = TaskEnvironment.GetAbsolutePath(item.ItemSpec);
169+
string lockedMsg = LockCheck.GetLockedFileMessage(sourceFile);
170+
171+
// For error messages, preserve original user input
172+
Log.LogError("...", sourceFile.OriginalValue, ...);
173+
```
174+
175+
### Exception Handling in Batch Operations
176+
177+
**Important**: `GetAbsolutePath()` throws on null/empty inputs. In batch processing scenarios (e.g., iterating over multiple files), an unhandled exception will abort the entire batch. Tasks must catch and handle these exceptions appropriately to avoid cutting short processing of valid items:
178+
179+
```csharp
180+
// WRONG - one bad path aborts entire batch
181+
foreach (ITaskItem item in SourceFiles)
182+
{
183+
AbsolutePath path = TaskEnvironment.GetAbsolutePath(item.ItemSpec); // throws, batch stops!
184+
ProcessFile(path);
185+
}
186+
187+
// CORRECT - handle exceptions, continue processing valid items
188+
bool success = true;
189+
foreach (ITaskItem item in SourceFiles)
190+
{
191+
try
192+
{
193+
AbsolutePath path = TaskEnvironment.GetAbsolutePath(item.ItemSpec);
194+
ProcessFile(path);
195+
}
196+
catch (ArgumentException ex)
197+
{
198+
Log.LogError($"Invalid path '{item.ItemSpec}': {ex.Message}");
199+
success = false;
200+
// Continue processing remaining items
201+
}
202+
}
203+
return success;
204+
```
205+
206+
Consider the task's error semantics: should one invalid path fail the entire task immediately, or should all items be processed with errors collected? Match the original task's behavior.
207+
208+
### Prefer AbsolutePath Over String
209+
210+
When working with paths, stay in the `AbsolutePath` world as much as possible rather than converting back and forth to `string`. This reduces unnecessary conversions and maintains type safety:
211+
212+
```csharp
213+
// AVOID - unnecessary conversions
214+
string path = TaskEnvironment.GetAbsolutePath(input).Value;
215+
AbsolutePath again = TaskEnvironment.GetAbsolutePath(path); // redundant!
216+
217+
// PREFER - stay in AbsolutePath
218+
AbsolutePath path = TaskEnvironment.GetAbsolutePath(input);
219+
// Use path directly - it's implicitly convertible to string where needed
220+
File.ReadAllText(path);
221+
```
222+
223+
### TaskEnvironment is Not Thread-Safe
224+
225+
If your task spawns multiple threads internally, you must synchronize access to `TaskEnvironment`. However, each task instance gets its own environment, so no synchronization with other tasks is needed.
226+
227+
## Checklist
228+
229+
- [ ] Task is annotated with `MSBuildMultiThreadableTask` attribute and implements `IMultiThreadableTask` if TaskEnvironment APIs are required
230+
- [ ] All environment variable access uses `TaskEnvironment` APIs
231+
- [ ] All process spawning uses `TaskEnvironment.GetProcessStartInfo()`
232+
- [ ] All file system APIs receive absolute paths
233+
- [ ] All helper methods receiving path strings are traced to verify they don't internally use File APIs with non-absolutized paths
234+
- [ ] No use of `Environment.CurrentDirectory`
235+
- [ ] All tests set `TaskEnvironment = TaskEnvironmentHelper.CreateForTest()`
236+
- [ ] Tests verify exception behavior for null/empty paths
237+
- [ ] No use of forbidden APIs (Environment.Exit, etc.)
238+
239+
## References
240+
241+
- [Thread-Safe Tasks Spec](https://github.com/dotnet/msbuild/blob/main/documentation/specs/multithreading/thread-safe-tasks.md) - Full specification for multithreaded task support
242+
- [`AbsolutePath`](https://github.com/dotnet/msbuild/blob/main/src/Framework/PathHelpers/AbsolutePath.cs) - Struct for representing absolute paths
243+
- [`TaskEnvironment`](https://github.com/dotnet/msbuild/blob/main/src/Framework/TaskEnvironment.cs) - Thread-safe environment APIs for tasks
244+
- [`IMultiThreadableTask`](https://github.com/dotnet/msbuild/blob/main/src/Framework/IMultiThreadableTask.cs) - Interface for multithreaded task support

0 commit comments

Comments
 (0)