-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Migrate CreateItem task to multithreaded execution model #13590
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -444,5 +444,218 @@ public void LogUnixWarningUponItemCreationWithDriveEnumeration(string content, s | |||||||||
| Helpers.ExpectedBuildResult.SucceedWithWarning, | ||||||||||
| _testOutput); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// Relative wildcard Include resolves against TaskEnvironment.ProjectDirectory, not cwd. | ||||||||||
| /// </summary> | ||||||||||
| [Fact] | ||||||||||
| public void WildcardInclude_ResolvesAgainstProjectDirectory() | ||||||||||
| { | ||||||||||
| using TestEnvironment env = TestEnvironment.Create(_testOutput); | ||||||||||
|
|
||||||||||
| // Create a project directory with a file. | ||||||||||
| TransientTestFolder projectDir = env.CreateFolder(createFolder: true); | ||||||||||
| env.CreateFile(projectDir, "alpha.txt", "alpha"); | ||||||||||
|
|
||||||||||
| // Create a separate directory that is NOT the project directory and put a different file there. | ||||||||||
| TransientTestFolder otherDir = env.CreateFolder(createFolder: true); | ||||||||||
| env.CreateFile(otherDir, "beta.txt", "beta"); | ||||||||||
|
|
||||||||||
| // Set cwd to otherDir so that if the task used cwd, it would find beta.txt instead. | ||||||||||
| env.SetCurrentDirectory(otherDir.Path); | ||||||||||
|
|
||||||||||
| CreateItem t = new CreateItem | ||||||||||
| { | ||||||||||
| BuildEngine = new MockEngine(_testOutput), | ||||||||||
| Include = new ITaskItem[] { new TaskItem("*.txt") }, | ||||||||||
| TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path), | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| t.Execute().ShouldBeTrue(); | ||||||||||
| t.Include.Length.ShouldBe(1); | ||||||||||
| t.Include[0].ItemSpec.ShouldBe("alpha.txt"); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// Relative wildcard Exclude resolves against TaskEnvironment.ProjectDirectory, not cwd. | ||||||||||
| /// The *.log exclude should match SampleFile.log in the project directory and filter it out. | ||||||||||
| /// </summary> | ||||||||||
| [Fact] | ||||||||||
| public void WildcardExclude_ResolvesAgainstProjectDirectory() | ||||||||||
| { | ||||||||||
| using TestEnvironment env = TestEnvironment.Create(_testOutput); | ||||||||||
|
|
||||||||||
| TransientTestFolder projectDir = env.CreateFolder(createFolder: true); | ||||||||||
| env.CreateFile(projectDir, "SampleFile.log", "SampleFile"); | ||||||||||
|
|
||||||||||
| // cwd points elsewhere — Exclude must still resolve *.log against projectDir. | ||||||||||
| TransientTestFolder otherDir = env.CreateFolder(createFolder: true); | ||||||||||
| env.SetCurrentDirectory(otherDir.Path); | ||||||||||
|
|
||||||||||
| CreateItem t = new CreateItem | ||||||||||
| { | ||||||||||
| BuildEngine = new MockEngine(_testOutput), | ||||||||||
| Include = new ITaskItem[] { new TaskItem("*.log") }, | ||||||||||
| Exclude = new ITaskItem[] { new TaskItem("*.log") }, | ||||||||||
| TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path), | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| t.Execute().ShouldBeTrue(); | ||||||||||
| t.Include.ShouldBeEmpty(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// RecursiveDir metadata is correctly set when wildcard expansion uses a custom ProjectDirectory. | ||||||||||
| /// </summary> | ||||||||||
| [Fact] | ||||||||||
| public void RecursiveDir_WithCustomProjectDirectory() | ||||||||||
| { | ||||||||||
| using TestEnvironment env = TestEnvironment.Create(_testOutput); | ||||||||||
|
|
||||||||||
| TransientTestFolder projectDir = env.CreateFolder(createFolder: true); | ||||||||||
| string sampleSubdirectory = Path.Combine(projectDir.Path, "SampleSubdirectory"); | ||||||||||
| Directory.CreateDirectory(sampleSubdirectory); | ||||||||||
| File.WriteAllText(Path.Combine(sampleSubdirectory, "SampleFile.txt"), "content"); | ||||||||||
|
|
||||||||||
| // Set cwd somewhere else to prove ProjectDirectory is used. | ||||||||||
| TransientTestFolder otherDir = env.CreateFolder(createFolder: true); | ||||||||||
| env.SetCurrentDirectory(otherDir.Path); | ||||||||||
|
|
||||||||||
| CreateItem t = new CreateItem | ||||||||||
| { | ||||||||||
| BuildEngine = new MockEngine(_testOutput), | ||||||||||
| Include = new ITaskItem[] { new TaskItem($"**{Path.DirectorySeparatorChar}*.txt") }, | ||||||||||
| TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path), | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| t.Execute().ShouldBeTrue(); | ||||||||||
| t.Include.Length.ShouldBe(1); | ||||||||||
|
|
||||||||||
| string recursiveDir = t.Include[0].GetMetadata("RecursiveDir"); | ||||||||||
| recursiveDir.ShouldBe("SampleSubdirectory" + Path.DirectorySeparatorChar); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// Absolute wildcard patterns are unaffected by ProjectDirectory — the same files are returned | ||||||||||
| /// regardless of what ProjectDirectory is set to. | ||||||||||
| /// </summary> | ||||||||||
| [Fact] | ||||||||||
| public void AbsoluteWildcard_IgnoresProjectDirectory() | ||||||||||
| { | ||||||||||
| using TestEnvironment env = TestEnvironment.Create(_testOutput); | ||||||||||
|
|
||||||||||
| TransientTestFolder targetDir = env.CreateFolder(createFolder: true); | ||||||||||
| env.CreateFile(targetDir, "SampleFile.txt", "content"); | ||||||||||
|
|
||||||||||
| // Use an unrelated ProjectDirectory. | ||||||||||
| TransientTestFolder unrelatedDir = env.CreateFolder(createFolder: true); | ||||||||||
|
|
||||||||||
| string absolutePattern = Path.Combine(targetDir.Path, "*.txt"); | ||||||||||
| CreateItem t = new CreateItem | ||||||||||
| { | ||||||||||
| BuildEngine = new MockEngine(_testOutput), | ||||||||||
| Include = new ITaskItem[] { new TaskItem(absolutePattern) }, | ||||||||||
| TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(unrelatedDir.Path), | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| t.Execute().ShouldBeTrue(); | ||||||||||
| t.Include.Length.ShouldBe(1); | ||||||||||
| // Absolute pattern returns absolute path. | ||||||||||
| Path.GetFileName(t.Include[0].ItemSpec).ShouldBe("SampleFile.txt"); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// When a relative wildcard ItemSpec has a multi-segment fixed directory containing a tilde | ||||||||||
| /// (e.g. "SubDir/ALONGD~1/*.txt"), GetLongPathName inside SplitFileSpec resolves the tilde | ||||||||||
| /// segment against CWD instead of TaskEnvironment.ProjectDirectory. If CWD contains a | ||||||||||
| /// directory whose 8.3 short name matches but whose long name differs, the resolution | ||||||||||
| /// produces the wrong long name, the absolutized path doesn't exist in the project directory, | ||||||||||
| /// and GetFiles returns no results. | ||||||||||
| /// | ||||||||||
| /// This test sets CWD to a directory whose "SubDir" contains a long-named child that maps | ||||||||||
| /// to the same 8.3 short name as the project directory's child. It then verifies that | ||||||||||
| /// CreateItem still finds the file in the project directory — which currently fails. | ||||||||||
| /// </summary> | ||||||||||
| [WindowsOnlyFact(additionalMessage: "8.3 short names are Windows-only.")] | ||||||||||
| public void WildcardWithShortName_ResolvesAgainstProjectDirectory_NotCwd() | ||||||||||
| { | ||||||||||
| using TestEnvironment env = TestEnvironment.Create(_testOutput); | ||||||||||
|
|
||||||||||
| // Project directory: SubDir/ALongDirectoryNameHere/test.txt | ||||||||||
| TransientTestFolder projectDir = env.CreateFolder(createFolder: true); | ||||||||||
| string projectSubDir = Path.Combine(projectDir.Path, "SubDir"); | ||||||||||
| string projectChild = Path.Combine(projectSubDir, "ALongDirectoryNameHere"); | ||||||||||
| Directory.CreateDirectory(projectChild); | ||||||||||
| File.WriteAllText(Path.Combine(projectChild, "test.txt"), "project-content"); | ||||||||||
|
|
||||||||||
| // Determine the 8.3 short name for the child directory. | ||||||||||
| string shortChildPath = NativeMethodsShared.GetShortFilePath(projectChild); | ||||||||||
| string shortChildName = Path.GetFileName(shortChildPath); | ||||||||||
|
|
||||||||||
| // If 8.3 names are disabled on this volume, the short name won't contain '~' — skip. | ||||||||||
| if (!shortChildName.Contains("~")) | ||||||||||
| { | ||||||||||
| _testOutput.WriteLine($"Skipping: 8.3 names appear disabled (short name was '{shortChildName}')."); | ||||||||||
| return; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // CWD directory: SubDir/ALongDirectoryNameThere/decoy.txt | ||||||||||
| // "ALongDirectoryNameThere" shares the first 6 characters with "ALongDirectoryNameHere", | ||||||||||
| // so in its own parent it also gets short name ALONGD~1. | ||||||||||
| TransientTestFolder cwdDir = env.CreateFolder(createFolder: true); | ||||||||||
| string cwdSubDir = Path.Combine(cwdDir.Path, "SubDir"); | ||||||||||
| string cwdChild = Path.Combine(cwdSubDir, "ALongDirectoryNameThere"); | ||||||||||
| Directory.CreateDirectory(cwdChild); | ||||||||||
| File.WriteAllText(Path.Combine(cwdChild, "decoy.txt"), "decoy-content"); | ||||||||||
|
|
||||||||||
| // Verify the decoy also gets the same short name. | ||||||||||
| string shortDecoyPath = NativeMethodsShared.GetShortFilePath(cwdChild); | ||||||||||
| string shortDecoyName = Path.GetFileName(shortDecoyPath); | ||||||||||
| shortDecoyName.ShouldBe(shortChildName, customMessage: | ||||||||||
| "Both directories should have the same 8.3 short name since they share the first 6 characters.", | ||||||||||
| options: StringCompareShould.IgnoreCase); | ||||||||||
|
|
||||||||||
| // Set CWD to cwdDir — GetLongPathName will resolve the short name against this directory. | ||||||||||
| env.SetCurrentDirectory(cwdDir.Path); | ||||||||||
|
|
||||||||||
| // ItemSpec uses the 8.3 short name in a multi-segment relative path. | ||||||||||
| // SplitFileSpec splits into fixedDir = "SubDir/ALONGD~1/", GetLongPathName resolves | ||||||||||
| // "ALONGD~1" under "SubDir" in CWD → gets "ALongDirectoryNameThere" (WRONG). | ||||||||||
| // GetFileSearchData then absolutizes to "{projectDir}/SubDir/ALongDirectoryNameThere/" | ||||||||||
| // which doesn't exist → returns empty. | ||||||||||
| string itemSpec = Path.Combine("SubDir", shortChildName, "*.txt"); | ||||||||||
| CreateItem t = new CreateItem | ||||||||||
| { | ||||||||||
| BuildEngine = new MockEngine(_testOutput), | ||||||||||
| Include = new ITaskItem[] { new TaskItem(itemSpec) }, | ||||||||||
| TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDir.Path), | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| t.Execute().ShouldBeTrue(); | ||||||||||
| t.Include.Length.ShouldBe(1, $"Expected 1 file from project directory, but got {t.Include.Length}. " + | ||||||||||
| "GetLongPathName likely resolved the short name against CWD instead of ProjectDirectory."); | ||||||||||
| Path.GetFileName(t.Include[0].ItemSpec).ShouldBe("test.txt"); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// Pre-expanded literal items (no wildcards) produce identical output regardless of | ||||||||||
| /// ProjectDirectory, Exclude, and AdditionalMetadata settings. | ||||||||||
|
Comment on lines
+641
to
+642
|
||||||||||
| /// Pre-expanded literal items (no wildcards) produce identical output regardless of | |
| /// ProjectDirectory, Exclude, and AdditionalMetadata settings. | |
| /// Verifies that for pre-expanded literal items (no wildcards), <see cref="CreateItem"/> | |
| /// applies <c>Exclude</c> filtering and <c>AdditionalMetadata</c> to the resulting items. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,10 +14,14 @@ namespace Microsoft.Build.Tasks | |
| /// <summary> | ||
| /// Forward a list of items from input to output. This allows dynamic item lists. | ||
| /// </summary> | ||
| public class CreateItem : TaskExtension | ||
| [MSBuildMultiThreadableTask] | ||
| public class CreateItem : TaskExtension, IMultiThreadableTask | ||
| { | ||
| #region Properties | ||
|
|
||
| /// <inheritdoc /> | ||
| public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; | ||
|
|
||
| [Output] | ||
| public ITaskItem[] Include { get; set; } | ||
|
|
||
|
|
@@ -178,7 +182,7 @@ private List<ITaskItem> CreateOutputItems(Dictionary<string, string> metadataTab | |
| } | ||
| else if (isLegalFileSpec) | ||
| { | ||
| (files, _, _, string? globFailure) = FileMatcher.Default.GetFiles(null /* use current directory */, i.ItemSpec); | ||
| (files, _, _, string? globFailure) = FileMatcher.Default.GetFiles(TaskEnvironment.ProjectDirectory, i.ItemSpec); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [NIT] Performance & Allocation Awareness
string projectDir = TaskEnvironment.ProjectDirectory;
// then inside loop:
(files, _, _, string? globFailure) = FileMatcher.Default.GetFiles(projectDir, i.ItemSpec);Note: The practical impact is small since |
||
| if (globFailure != null) | ||
|
Comment on lines
+185
to
186
|
||
| { | ||
| Log.LogMessage(MessageImportance.Low, globFailure); | ||
|
|
||
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.
PR description says 7 new tests were added (including concurrency + fallback-parity cases), but this file currently adds 5 tests. Either add the missing tests or update the PR description so it matches what’s actually in the diff.