Skip to content

Move NodeMode to Framework, add TryParse, unify with DebugUtils#13205

Merged
baronfel merged 4 commits intonicer-parsing-for-nodemodefrom
copilot/sub-pr-13148
Feb 5, 2026
Merged

Move NodeMode to Framework, add TryParse, unify with DebugUtils#13205
baronfel merged 4 commits intonicer-parsing-for-nodemodefrom
copilot/sub-pr-13148

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 5, 2026

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.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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.

Copilot AI and others added 2 commits February 5, 2026 15:50
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Copilot AI changed the title [WIP] Add enum for nodemode and support using it Move NodeMode to Framework, add TryParse, unify with DebugUtils Feb 5, 2026
Copilot AI requested a review from baronfel February 5, 2026 15:56
@baronfel
Copy link
Copy Markdown
Member

baronfel commented Feb 5, 2026

@copilot make DebugUtils use the new nodemode instead of having its own processmode enum

Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 5, 2026

@copilot make DebugUtils use the new nodemode instead of having its own processmode enum

Done in commit 6cab0f1. DebugUtils now uses NodeMode? from Framework instead of its own ProcessMode enum. The nullable type allows null to represent the central/main MSBuild process (not running as a node), while actual node modes use the shared NodeMode enum values. Parsing now uses NodeModeHelper.TryParse() for consistency across the codebase.

@baronfel baronfel marked this pull request as ready for review February 5, 2026 16:15
@baronfel baronfel merged commit 1810121 into nicer-parsing-for-nodemode Feb 5, 2026
2 of 8 checks passed
@baronfel baronfel deleted the copilot/sub-pr-13148 branch February 5, 2026 16:15
baronfel added a commit that referenced this pull request Feb 5, 2026
### 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>
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