Skip to content

Consolidate duplicate VSHostObject implementations#53028

Open
YuliiaKovalova wants to merge 3 commits intorelease/10.0.3xxfrom
dev/ykovalova/remove_extra_vshostobjects
Open

Consolidate duplicate VSHostObject implementations#53028
YuliiaKovalova wants to merge 3 commits intorelease/10.0.3xxfrom
dev/ykovalova/remove_extra_vshostobjects

Conversation

@YuliiaKovalova
Copy link
Member

Summary

There were two independent VSHostObject classes serving the same purpose — extracting credentials from a Visual Studio host object — one in Microsoft.NET.Build.Containers and one in Microsoft.NET.Sdk.Publish.Tasks. This PR consolidates them into a single shared implementation, following the repo's existing pattern for shared code (e.g., EnvironmentVariableNames.cs)

Details

Eliminate code duplication between the Containers and WebSdk projects.
The Containers version had a more robust implementation (JSON-based QueryAllTaskItems reflection protocol with [IEnumerable fallback), while the WebSdk version only supported the legacy IEnumerable path. The consolidated version gives both projects the improved behavior.

Copilot AI review requested due to automatic review settings February 13, 2026 10:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates two independent VSHostObject implementations that were duplicated across the Microsoft.NET.Build.Containers and Microsoft.NET.Sdk.Publish.Tasks projects. The consolidated implementation is placed in src/Common/VSHostObject.cs following the repo's established pattern for shared code, and uses the more robust JSON-based QueryAllTaskItems reflection protocol from the Containers version with IEnumerable fallback.

Changes:

  • Moved VSHostObject implementation to src/Common with namespace change to Microsoft.NET.Sdk.Common
  • Updated all usage sites to use the new constructor signature and API methods (TryGetCredentials, GetTaskItems)
  • Enhanced metadata handling to copy all metadata from JSON DTOs while gracefully handling reserved MSBuild metadata names

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Common/VSHostObject.cs Consolidated VSHostObject implementation with improved documentation, public GetTaskItems method, and robust metadata handling
src/WebSdk/Publish/Tasks/Tasks/ZipDeploy/ZipDeploy.cs Updated to use new VSHostObject constructor and TryGetCredentials API
src/WebSdk/Publish/Tasks/Tasks/OneDeploy/OneDeploy.cs Updated to use new VSHostObject constructor and TryGetCredentials API
src/WebSdk/Publish/Tasks/Tasks/MsDeploy/VsMsdeploy.cs Updated to use new VSHostObject constructor, TryGetCredentials, and GetTaskItems APIs with inline file skip filtering
src/WebSdk/Publish/Tasks/MsDeploy/VSHostObject.cs Deleted - replaced by shared implementation in src/Common
src/WebSdk/Publish/Tasks/Microsoft.NET.Sdk.Publish.Tasks.csproj Added Compile link to src/Common/VSHostObject.cs
src/Containers/Microsoft.NET.Build.Containers/Tasks/CreateNewImageToolTask.cs Added using directive for Microsoft.NET.Sdk.Common namespace
src/Containers/Microsoft.NET.Build.Containers/Tasks/CreateNewImage.cs Added using directive for Microsoft.NET.Sdk.Common namespace
src/Containers/Microsoft.NET.Build.Containers/Microsoft.NET.Build.Containers.csproj Removed old VSHostObject.cs, added Compile link to src/Common/VSHostObject.cs

Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

This LGTM but it would be great for @vijayrkn to verify

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.

2 participants