Skip to content

Migrate Create(CSharp|VisualBasic)ManifestResourceName to multithreaded execution#13638

Draft
jankratochvilcz wants to merge 1 commit intomainfrom
jankratochvilcz/multithreaded/manifest-resource-name
Draft

Migrate Create(CSharp|VisualBasic)ManifestResourceName to multithreaded execution#13638
jankratochvilcz wants to merge 1 commit intomainfrom
jankratochvilcz/multithreaded/manifest-resource-name

Conversation

@jankratochvilcz
Copy link
Copy Markdown
Contributor

Migrates the C# and VB ManifestResourceName tasks to support MSBuild's multithreaded execution model.

Changes

  • [MSBuildMultiThreadableTask] applied to both concrete subclasses (CreateCSharpManifestResourceName and CreateVisualBasicManifestResourceName). The attribute has Inherited=false, so it must be on each concrete class.
  • Shared CreateManifestResourceName base implements IMultiThreadableTask with TaskEnvironment = TaskEnvironment.Fallback default, so both subclasses inherit the property.
  • File system access in the base is routed through TaskEnvironment.GetAbsolutePath(...): the existence probe for the convention-based DependentUpon source file (FileSystems.Default.FileExists), and the FileStream opened to parse the dependent source file for class-name extraction.

Compatibility audit

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

  • Sin 1 ([Output] contamination): N/A. ManifestResourceNames and ResourceFilesWithManifestResourceNames are constructed from resourceFile and manifestName; neither is derived from AbsolutePath. The original fileName (resourceFile.ItemSpec) is what subclasses see in CreateManifestName, so the produced manifest string keeps its existing form.
  • Sin 2 (error message inflation): N/A. The single catch uses resourceFile.ItemSpec (original) for LogErrorWithCodeFromResources; absolutized paths are only used for the FileExists and FileStream calls and never logged.
  • Sin 3 (?? swallowing exceptions): N/A; no new ?? operators added. Path.GetDirectoryName(fileName) returning null still flows into Path.Combine, preserving the original throw-and-catch semantics under ExceptionHandling.IsIoRelatedException.
  • Sin 4 (try-catch scope): N/A. The GetAbsolutePath calls live inside the same try block as the file ops; the catch only references the original resourceFile.ItemSpec and e.Message, so no absolutized value is needed in the catch.
  • Sin 5 (canonicalization): N/A; no path-keyed dictionaries. itemSpecToTaskitem keys on the original resourceFile.ItemSpec (unchanged), and the existing code never used Path.GetFullPath for keys/comparisons.
  • Sin 6 (empty/null inputs): The pre-existing logic already lets Path.GetDirectoryName/Path.Combine throw on bad inputs, with the resulting exception caught by ExceptionHandling.IsIoRelatedException (which includes ArgumentException). GetAbsolutePath throwing ArgumentException for null/empty is funneled through the same catch, so Execute returns false exactly as before.

Validation

  • Microsoft.Build.Tasks.csproj builds clean (0 warnings, 0 errors).
  • ManifestResourceName-filtered tests pass on both targets:
    • net10.0: total 35, succeeded 33, skipped 2, failed 0
    • net472: total 59, succeeded 59, failed 0

Part of #11834. Closes part of #13172.

…tResourceName to multithreaded execution

Adds [MSBuildMultiThreadableTask] to both concrete classes and routes file
I/O through TaskEnvironment.GetAbsolutePath in the shared
CreateManifestResourceName base. Subscribes the base class to
IMultiThreadableTask with a TaskEnvironment.Fallback default.

Closes part of #13172.

Co-authored-by: Copilot <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.

1 participant