Skip to content

Enlighten CreateItem task for multithreaded mode #13568

@jankratochvilcz

Description

@jankratochvilcz

Enlighten CreateItem task for multithreaded mode

Parent: #11834

Context

CreateItem is one of the remaining tasks listed in the migration epic (#11834) under "Other (either simple or with unknown issues)". It forwards items from Include to its [Output] Include, optionally honoring Exclude and applying AdditionalMetadata. Two of its three logical paths are already cwd-independent — only one is not.

The cwd-dependence is concentrated in a single line inside TryExpandWildcards:

// src/Tasks/CreateItem.cs:181
(files, _, _, string? globFailure) =
    FileMatcher.Default.GetFiles(null /* use current directory */, i.ItemSpec);

FileMatcher.GetFiles accepts string? projectDirectoryUnescaped as its first parameter (src/Framework/Utilities/FileMatcher.cs:1923). Passing null causes wildcard expansion to be rooted at Environment.CurrentDirectory. Under multithreaded execution that is wrong — concurrent CreateItem instances may belong to different projects, and there is only one cwd per process. A relative pattern like **\*.cs could resolve against an arbitrary peer project's directory.

The other FileMatcher calls in this method are pure string analysis on the filespec and a candidate file path:

  • FileMatcher.Default.GetFileSpecInfo(i.ItemSpec, ...) — parses the filespec only.
  • FileMatcher.IsDriveEnumeratingWildcardPattern(directoryPart, wildcardPart) — pattern shape inspection.
  • FileMatcher.Default.FileMatch(i.ItemSpec, file) — regex match of the filespec against the already-returned file string; does not touch the filesystem and does not consume cwd.

The non-wildcard branch (expanded.Add(i)), CreateOutputItems, GetUniqueItems, and PropertyParser.GetTable are all path-string-pure — no migration needed.

Approach

Apply the PR #13267 (MSBuild/CallTarget) pattern: pass the project directory through at the boundary, leaving the helper untouched. The helper signature already supports it — there is nothing to refactor downstream.

  1. Mark CreateItem with [MSBuildMultiThreadableTask] and implement IMultiThreadableTask with public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback; (built-in style).
  2. In TryExpandWildcards, replace the null first argument to FileMatcher.Default.GetFiles with (string)TaskEnvironment.ProjectDirectory. TaskEnvironment.ProjectDirectory is already typed as AbsolutePath and is guaranteed canonical by the engine (PR Enlighten public versions of intrinsic tasks. #13267) — no extra GetAbsolutePath/GetCanonicalForm call is required.
  3. No other changes to CreateItem.cs. The non-wildcard path, output construction, exclude handling, and metadata application remain byte-identical.

ChangeWave consideration

For absolute and rooted wildcard patterns, GetFiles ignores the project directory entirely — output is byte-identical to today. For relative wildcard patterns the base directory changes from Environment.CurrentDirectory to TaskEnvironment.ProjectDirectory. In legacy (non-MT) execution the two are equal during normal builds, so observable behavior is unchanged. The semantic change only manifests under MT or in custom hosts that have intentionally diverged cwd from the project directory — which is the entire point of the migration.

There is one edge that could differ today: the strings in the returned string[] files are formatted relative to the projectDirectoryUnescaped argument when supplied, vs. relative to cwd when null. In the cwd == project-dir case the returned strings are character-for-character identical. If unforeseen behavioral diffs are found during review (e.g., a divergent shape on Linux with mixed separators), gate the diff behind a new ChangeWave following the precedent set in PR #13069 (which gated FindUnderPath/AssignTargetPath semantic diffs behind Wave18_5). Recommendation: ship without a wave unless a test demonstrates a real diffCreateItem's wildcard expansion is conceptually identical to evaluation-time globbing, which already passes the project directory through.

Test coverage assessment

Existing direct unit tests (src/Tasks.UnitTests/CreateItem_Tests.cs)

OneFromOneIsZero, OneFromOneMismatchIsOne, UnspecifiedFromOneIsOne, OneFromUnspecifiedIsEmpty, UnspecifiedFromUnspecifiedIsEmpty, CaseDoesntMatter, WildcardsWithRecursiveDir, RecursiveDirOutOfProc, AdditionalMetaData, AdditionalMetaDataPreserveExisting, AdditionalMetaDataOverwriteExisting, WildcardDriveEnumerationTaskItemLogsError, LogWindowsWarningUponCreateItemExecution, LogUnixWarningUponCreateItemExecution, ThrowExceptionUponItemCreationWithDriveEnumeration, LogWindowsWarningUponItemCreationWithDriveEnumeration, LogUnixWarningUponItemCreationWithDriveEnumeration.

Coverage of pre-expanded items, exclude semantics, metadata application, and drive-enumeration detection is good. Coverage of MT-specific concerns (project-relative wildcard expansion, concurrency across projects) is zero. WildcardsWithRecursiveDir and RecursiveDirOutOfProc exercise wildcard expansion but rely on the implicit Environment.CurrentDirectory == project directory assumption.

Integration tests in this repo

CreateItem is invoked across Microsoft.Common.CurrentVersion.targets and friends for dynamic item generation. End-to-end coverage in this repo is incidental via Build.UnitTests scenarios; we will not add new E2E targets-flow tests as part of this issue.

Gaps to fill in this PR

  • G1 — Concurrency test. Two CreateItem instances with different ProjectDirectory values, both given the same relative wildcard (e.g. **\*.cs), assert each returns disjoint file sets rooted at the corresponding project dir. Use TestEnvironment to scaffold two separate transient project trees with distinct contents. (SKILL red-team Phase 4.)
  • G2 — Project-relative wildcard expansion. ProjectDirectory != Environment.CurrentDirectory: relative wildcard input resolves against ProjectDirectory, not cwd. Documents the intentional semantic change. Use TaskEnvironment.CreateWithProjectDirectoryAndEnvironment("<dir>").
  • G3 — Inject TaskEnvironment into existing tests. Set t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest() in every existing test that constructs CreateItem directly so the wildcard tests (WildcardsWithRecursiveDir, RecursiveDirOutOfProc, drive-enumeration tests) keep their cwd-rooted behavior under the Fallback environment.
  • G4 — Baseline parity. Pre-expanded literal items (no wildcards), exclude lists, and AdditionalMetadata produce byte-identical [Output] Include items, including RecursiveDir metadata and item ordering. Captures that the non-wildcard branch and CreateOutputItems/GetUniqueItems remain unchanged.

Acceptance criteria

  • CreateItem decorated [MSBuildMultiThreadableTask] and implements IMultiThreadableTask with TaskEnvironment defaulted to TaskEnvironment.Fallback (built-in style).
  • TryExpandWildcards passes (string)TaskEnvironment.ProjectDirectory to FileMatcher.Default.GetFiles instead of null.
  • No signature changes to FileMatcher.GetFiles, GetFileSpecInfo, FileMatch, or IsDriveEnumeratingWildcardPattern — they already accept the project directory or are pure string analysis.
  • All existing CreateItem_Tests pass on net472 and net10.0 with no behavior changes for the non-wildcard, exclude, and metadata paths.
  • Include [Output] items have byte-identical ItemSpec and RecursiveDir metadata for relative wildcard inputs when Environment.CurrentDirectory == ProjectDirectory (the legacy invariant).
  • No AbsolutePath leak into [Output] Include ItemSpec strings or warning/error messages (SKILL Sin 1, Sin 2). Drive-enumeration warnings continue to embed EscapingUtilities.UnescapeAll(i.ItemSpec) — the user's original spec.
  • Drive-enumerating wildcard detection (WildcardResultsInDriveEnumeration) produces identical warning/error codes and message arguments.
  • Tests G1, G2, G3, G4 added and pass.
  • .\build.cmd -v quiet succeeds; Tasks.UnitTests and Build.UnitTests pass.
  • No new compiler warnings (warnings-as-errors in official build).
  • No ChangeWave required; if any test required modification beyond G3, document why in the PR and gate the diff behind the next wave per PR Enlighten FindUnderPath and AssignTargetPath tasks #13069's pattern.
  • multithreaded-task-migration SKILL sign-off checklist walked and passes.

Implementation order

  1. Apply [MSBuildMultiThreadableTask] + IMultiThreadableTask to CreateItem; route TryExpandWildcards through TaskEnvironment.ProjectDirectory.
  2. Update existing tests to inject TaskEnvironment (G3).
  3. Add G1 (concurrency), G2 (project-relative wildcard), G4 (baseline parity) tests.
  4. Run full Tasks.UnitTests and Build.UnitTests; verify clean.
  5. Run repo build with -v quiet to ensure no new warnings.

Risks / open questions

  • GetUniqueItems keys excludes by item.ItemSpec using StringComparer.OrdinalIgnoreCase. This is a path comparison performed against un-absolutized item-spec strings supplied by the user — it has the same Windows-vs-Linux case-sensitivity caveat that PR Enlighten FindUnderPath and AssignTargetPath tasks #13069 review flagged for FindUnderPath/AssignTargetPath (FileUtilities.PathComparison would be more correct on Linux). Out of scope for this issueExclude is matched by raw string today and changing it would alter observable behavior independently of the MT migration. Tracked as a follow-up.
  • The match.wildcardDirectoryPart string returned by FileMatcher.FileMatch is computed by re-parsing the filespec, not by inspecting the project directory. Verify no implicit cwd dependence in FileMatcher.FileMatch during PR review (initial reading: pure regex match, no filesystem access).
  • Custom hosts that intentionally point Environment.CurrentDirectory away from the project directory and rely on CreateItem resolving against cwd will see different output under MT mode. This is the intended semantic change.

References

Metadata

Metadata

Labels

Area: MultithreadedArea: TasksIssues impacting the tasks shipped in Microsoft.Build.Tasks.Core.dll.

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions