Move NodeMode to Framework, add TryParse, unify with DebugUtils#13205
Move NodeMode to Framework, add TryParse, unify with DebugUtils#13205baronfel merged 4 commits intonicer-parsing-for-nodemodefrom
Conversation
|
Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo. |
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
|
@copilot make DebugUtils use the new nodemode instead of having its own processmode enum |
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Done in commit 6cab0f1. DebugUtils now uses |
### Context The NodeMode enum was in Shared namespace, duplicated logic existed in DebugUtils, and parsing used string.Format placeholders instead of direct construction. ### Changes Made - **Moved NodeMode to Framework**: Relocated from `Microsoft.Build.Shared` to `Microsoft.Build.Framework` for better accessibility - **Added NodeModeHelper.TryParse()**: Safe parsing supporting both integers (1, 2, 3, 8) and enum names (case-insensitive) with `Enum.IsDefined` validation - **Refactored XMake.ParseNodeMode()**: Now uses TryParse pattern, made internal for testability, preserves original error messages - **Unified DebugUtils with NodeMode**: Removed the private `ProcessMode` enum from DebugUtils and replaced it with `NodeMode?` from Framework. The nullable type allows `null` to represent the central/main MSBuild process (not running as a node), while actual node modes use the shared enum. Updated parsing to use `NodeModeHelper.TryParse()` for consistency across the codebase. - **Cleaned up command line construction**: Replaced string.Format placeholders in NodeProviderOutOfProcTaskHost with direct string interpolation matching NodeProviderOutOfProc pattern ### Testing Added 6 unit tests covering: - Valid integers: 1, 2, 3, 8 - Valid enum names: case-sensitive and case-insensitive - Error cases: undefined integers, negative numbers, invalid strings, empty strings ### Notes - The `Enum.IsDefined` check after `Enum.TryParse` is required because TryParse succeeds for string representations of undefined integer values (e.g., "5" parses successfully even though only 1, 2, 3, and 8 are defined). - DebugUtils now uses `NodeMode?` (nullable) where `null` represents the central/main process, eliminating the need for a separate `ProcessMode` enum and ensuring a single source of truth for node mode values. <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Context
The NodeMode enum was in Shared namespace, duplicated logic existed in DebugUtils, and parsing used string.Format placeholders instead of direct construction.
Changes Made
Microsoft.Build.SharedtoMicrosoft.Build.Frameworkfor better accessibilityEnum.IsDefinedvalidationProcessModeenum from DebugUtils and replaced it withNodeMode?from Framework. The nullable type allowsnullto represent the central/main MSBuild process (not running as a node), while actual node modes use the shared enum. Updated parsing to useNodeModeHelper.TryParse()for consistency across the codebase.Testing
Added 6 unit tests covering:
Notes
Enum.IsDefinedcheck afterEnum.TryParseis required because TryParse succeeds for string representations of undefined integer values (e.g., "5" parses successfully even though only 1, 2, 3, and 8 are defined).NodeMode?(nullable) wherenullrepresents the central/main process, eliminating the need for a separateProcessModeenum and ensuring a single source of truth for node mode values.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.