Skip to content

Commit 41e492b

Browse files
JanProvaznikCopilot
andcommitted
Fix compatibility issues in manifest task migration
Issue 1 & 3: Absolutize _manifest.SourcePath in GetOutputPath(). GenerateManifestBase.BuildManifest sets _manifest.SourcePath = GetOutputPath(), which returned the raw OutputManifest.ItemSpec (potentially relative). The parameterless Manifest.ResolveFiles() derives search paths from SourcePath and falls back to Directory.GetCurrentDirectory() for non-rooted paths, breaking multithreaded isolation. Fix: absolutize via TaskEnvironment.GetAbsolutePath(). Issue 2: Guard Path.GetDirectoryName null result in CreateManifestResourceName. Path.GetDirectoryName returns null for root paths (e.g. 'C:\') and empty strings on .NET Core. Passing null to Path.Combine throws ArgumentNullException. Add ?? string.Empty to match pre-migration behavior where the exception was caught by IsIoRelatedException but could be avoided entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 86f7785 commit 41e492b

File tree

3 files changed

+90
-3
lines changed

3 files changed

+90
-3
lines changed

src/Tasks.UnitTests/ManifestTaskEnvironmentTests.cs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.IO;
6+
using System.Runtime.Versioning;
67
using Microsoft.Build.Framework;
78
using Microsoft.Build.Tasks;
89
using Microsoft.Build.UnitTests;
@@ -232,5 +233,86 @@ public void CreateManifestResourceName_PathWithSpaces_ShouldWork()
232233
bool result = task.Execute();
233234
result.ShouldBeTrue();
234235
}
236+
// Regression test for Issue 1 & 3: GetOutputPath must return absolutized path.
237+
// GenerateManifestBase.BuildManifest sets _manifest.SourcePath = GetOutputPath().
238+
// If GetOutputPath returns a relative path, the parameterless Manifest.ResolveFiles()
239+
// (used e.g. by ResolveNativeReference) falls back to Directory.GetCurrentDirectory()
240+
// instead of the project directory, breaking multithreaded isolation.
241+
// The fix: absolutize the path in GetOutputPath.
242+
[WindowsOnlyFact]
243+
[SupportedOSPlatform("windows")]
244+
public void GenerateManifestBase_RelativeOutputManifest_WritesToProjectDir()
245+
{
246+
using var env = TestEnvironment.Create();
247+
var folder = env.CreateFolder();
248+
249+
// Create a dummy entry point file
250+
string entryPointPath = Path.Combine(folder.Path, "TestApp.exe");
251+
File.WriteAllText(entryPointPath, "dummy");
252+
253+
// Use a relative OutputManifest path
254+
string relativeManifest = "myapp.exe.manifest";
255+
256+
var engine = new MockEngine(true);
257+
var task = new GenerateApplicationManifest
258+
{
259+
TaskEnvironment = TaskEnvironmentHelper.CreateForTest(),
260+
BuildEngine = engine,
261+
AssemblyName = "TestApp",
262+
AssemblyVersion = "1.0.0.0",
263+
ManifestType = "Native",
264+
OutputManifest = new TaskItem(relativeManifest),
265+
};
266+
267+
bool result = task.Execute();
268+
269+
// Task should succeed writing a native manifest (no entry point required)
270+
result.ShouldBeTrue("Task should succeed. Log: " + engine.Log);
271+
272+
// The manifest should exist at the absolutized path
273+
string expectedPath = Path.Combine(Directory.GetCurrentDirectory(), relativeManifest);
274+
File.Exists(expectedPath).ShouldBeTrue($"Manifest should be written at: {expectedPath}");
275+
276+
// Cleanup
277+
File.Delete(expectedPath);
278+
279+
// Output property should preserve the original relative path (no contamination)
280+
task.OutputManifest.ItemSpec.ShouldBe(relativeManifest);
281+
}
282+
283+
// Regression test for Issue 2: bare filename resource files should not cause
284+
// different exception behavior vs pre-migration code.
285+
// Path.GetDirectoryName("file.resx") returns "" and Path.Combine("", "file.cs")
286+
// returns "file.cs" — GetAbsolutePath("file.cs") should absolutize without error.
287+
[Fact]
288+
public void CreateManifestResourceName_BareFilename_WithConvention_ShouldNotThrow()
289+
{
290+
using var env = TestEnvironment.Create();
291+
var folder = env.CreateFolder();
292+
293+
// Create files in the folder
294+
var resxPath = Path.Combine(folder.Path, "MyForm.resx");
295+
File.WriteAllText(resxPath, "<root></root>");
296+
var csPath = Path.Combine(folder.Path, "MyForm.cs");
297+
File.WriteAllText(csPath, "namespace Test { class MyForm { } }");
298+
299+
// Use bare filename (no directory component) — this is the edge case
300+
var task = new CreateCSharpManifestResourceName
301+
{
302+
TaskEnvironment = TaskEnvironmentHelper.CreateForTest(),
303+
BuildEngine = new MockEngine(true),
304+
ResourceFiles = new ITaskItem[] { new TaskItem("MyForm.resx") },
305+
RootNamespace = "Test",
306+
UseDependentUponConvention = true,
307+
};
308+
309+
// Should not throw — the convention check calls
310+
// GetAbsolutePath(Path.Combine(Path.GetDirectoryName("MyForm.resx"), "MyForm.cs"))
311+
// = GetAbsolutePath(Path.Combine("", "MyForm.cs"))
312+
// = GetAbsolutePath("MyForm.cs") — should absolutize against project directory
313+
bool result = task.Execute();
314+
// The task may return true or false depending on whether the file is found
315+
// relative to project dir, but it should NOT throw an unhandled exception
316+
}
235317
}
236318
}

src/Tasks/CreateManifestResourceName.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ internal bool Execute(
194194
}
195195
}
196196

197-
AbsolutePath dependentPath = TaskEnvironment.GetAbsolutePath(Path.Combine(Path.GetDirectoryName(fileName), conventionDependentUpon));
197+
string fileDir = Path.GetDirectoryName(fileName) ?? string.Empty;
198+
AbsolutePath dependentPath = TaskEnvironment.GetAbsolutePath(Path.Combine(fileDir, conventionDependentUpon));
198199
if (FileSystems.Default.FileExists(dependentPath))
199200
{
200201
dependentUpon = conventionDependentUpon;
@@ -219,7 +220,7 @@ internal bool Execute(
219220

220221
if (isDependentOnSourceFile)
221222
{
222-
AbsolutePath pathToDependent = TaskEnvironment.GetAbsolutePath(Path.Combine(Path.GetDirectoryName(fileName), dependentUpon));
223+
AbsolutePath pathToDependent = TaskEnvironment.GetAbsolutePath(Path.Combine(Path.GetDirectoryName(fileName) ?? string.Empty, dependentUpon));
223224
binaryStream = createFileStream(pathToDependent, FileMode.Open, FileAccess.Read);
224225
}
225226

src/Tasks/GenerateManifestBase.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,11 @@ private string GetOutputPath()
418418
{
419419
if (OutputManifest != null)
420420
{
421-
return OutputManifest.ItemSpec;
421+
// Absolutize so that Manifest.SourcePath (set from this value) is always
422+
// rooted. The parameterless Manifest.ResolveFiles() derives search paths
423+
// from SourcePath and falls back to Directory.GetCurrentDirectory() for
424+
// relative paths, which breaks multithreaded isolation.
425+
return TaskEnvironment.GetAbsolutePath(OutputManifest.ItemSpec);
422426
}
423427
return GetDefaultFileName();
424428
}

0 commit comments

Comments
 (0)