Skip to content

Migrate GetAssembliesMetadata to multithreaded execution - #13637

Merged
jankratochvilcz merged 2 commits into
mainfrom
jankratochvilcz/multithreaded/get-assemblies-metadata
Aug 3, 2026
Merged

Migrate GetAssembliesMetadata to multithreaded execution#13637
jankratochvilcz merged 2 commits into
mainfrom
jankratochvilcz/multithreaded/get-assemblies-metadata

Conversation

@jankratochvilcz

Copy link
Copy Markdown
Contributor

Migrates GetAssembliesMetadata to support MSBuild's multithreaded execution model.

Changes

  • [MSBuildMultiThreadableTask] applied; class implements IMultiThreadableTask with TaskEnvironment = TaskEnvironment.Fallback default.
  • Each assembly file path absolutized via TaskEnvironment.GetAbsolutePath before MetadataReader-based access (AssemblyInformation opens the file via File.OpenRead / OpenScope / native metadata reads, all of which need an absolute path on a thread node).

Compatibility audit

Ran the 6-deadly-sins playbook (see .github/skills/multithreaded-task-migration/SKILL.md):

  • Sin 1 ([Output] contamination): AssembliesMetadata[i].ItemSpec is sourced from AssemblyAttributes.AssemblyFullPath, which AssemblyInformation initializes from the path passed into its constructor. To avoid leaking the absolutized path into the output items (which would change the user-observable ItemSpec), we overwrite attributes.AssemblyFullPath with absoluteAssemblyPath.OriginalValue before constructing the TaskItem.
  • Sin 2 (error message inflation): N/A — task logs no errors/warnings/messages that include the path.
  • Sin 3 (?? swallowing exceptions): N/A — no null-coalescing added.
  • Sin 4 (try-catch scope): N/A — no try/catch in Execute.
  • Sin 5 (canonicalization): N/A — no dictionary/set keyed on the path; FileSystems.Default.FileExists does not require canonical form.
  • Sin 6 (empty/null inputs): Original code passed empty/null entries to FileSystems.Default.FileExists which silently returned false and skipped them; GetAbsolutePath("") would now throw ArgumentException and abort the entire batch. Guarded with string.IsNullOrEmpty(assemblyPath) -> continue to preserve original skip-silently semantics.

Validation

  • Microsoft.Build.Tasks.csproj builds clean (0 warnings, 0 errors).
  • All GetAssembliesMetadata-filtered tests pass (2/2 on net472 — the test class is #if NETFRAMEWORK-gated, so net10.0 has no tests for this task).

Part of #11834. Closes #13570.

Adds [MSBuildMultiThreadableTask] and IMultiThreadableTask, routing
assembly file access through TaskEnvironment.GetAbsolutePath so the
task can run on a thread node without depending on process CWD.

Closes #13570.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jankratochvilcz
jankratochvilcz force-pushed the jankratochvilcz/multithreaded/get-assemblies-metadata branch from 15c2243 to c6a1b8f Compare July 7, 2026 14:42
@jankratochvilcz
jankratochvilcz marked this pull request as ready for review July 7, 2026 16:19
Copilot AI review requested due to automatic review settings July 7, 2026 16:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates GetAssembliesMetadata to be MSBuild multi-threadable and improves path handling to avoid exceptions while preserving output item behavior.

Changes:

  • Marked the task as multi-threadable and added TaskEnvironment support.
  • Added guarding for null/empty assembly path entries before path resolution.
  • Resolved assembly paths to absolute paths for file existence checks and metadata reading while attempting to preserve original output ItemSpec behavior.

Comment thread src/Tasks/GetAssembliesMetadata.cs Outdated
Comment thread src/Tasks/GetAssembliesMetadata.cs Outdated
Comment thread src/Tasks/GetAssembliesMetadata.cs Outdated
Comment thread src/Tasks/GetAssembliesMetadata.cs Outdated
…ments

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@AlesProkop AlesProkop left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a decoy test to expand test coverage.

@jankratochvilcz
jankratochvilcz enabled auto-merge (squash) July 31, 2026 12:54
auto-merge was automatically disabled July 31, 2026 12:56

Pull request was closed

@jankratochvilcz
jankratochvilcz merged commit 157590d into main Aug 3, 2026
13 checks passed
@jankratochvilcz
jankratochvilcz deleted the jankratochvilcz/multithreaded/get-assemblies-metadata branch August 3, 2026 09:47
jankratochvilcz added a commit that referenced this pull request Aug 3, 2026
The migration (#13637) is now in main. This adds the regression test that
proves a relative AssemblyPaths entry resolves against the TaskEnvironment
project directory rather than the process current directory, and that the
resulting ItemSpec keeps the caller-supplied relative path.

The test mutates the process-wide current directory, so it lives in its own
parallelization-disabled collection instead of serializing the whole
GetAssembliesMetadata_Tests class.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enlighten GetAssembliesMetadata task for multithreaded mode

4 participants