add enum for nodemode and support using it with ints and readable strings#13148
Merged
add enum for nodemode and support using it with ints and readable strings#13148
Conversation
baronfel
commented
Jan 29, 2026
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs
Outdated
Show resolved
Hide resolved
61cf314 to
e0100f3
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request introduces a strongly-typed NodeMode enum to replace the use of magic numbers throughout the MSBuild codebase for identifying different types of out-of-process nodes. The change improves code clarity and maintainability by giving meaningful names to the previously unnamed node mode values (1, 2, 3, and 8).
Changes:
- Added a new
NodeModeenum insrc/Shared/NodeMode.cswith four values: OutOfProcNode (1), OutOfProcTaskHostNode (2), OutOfProcRarNode (3), and OutOfProcServerNode (8) - Enhanced
XMake.csto parse node mode values from both integers and enum names (case-insensitive) for backward compatibility and improved developer experience - Updated all node launching code to use the enum values via
NodeModeHelper.ToCommandLineArgument()instead of hardcoded integers
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Shared/NodeMode.cs | New file defining the NodeMode enum and NodeModeHelper utility class for converting enum values to command-line arguments |
| src/MSBuild/XMake.cs | Added ParseNodeMode method to support parsing both integers and enum names; refactored StartLocalNode to use switch expression on enum values; added (unused) public NodeModeToCommandLineArgument wrapper method |
| src/Build/Microsoft.Build.csproj | Added reference to the new NodeMode.cs shared file |
| src/Build/BackEnd/Components/Communications/RarNodeLauncher.cs | Replaced hardcoded "/nodemode:3" with NodeModeHelper.ToCommandLineArgument(NodeMode.OutOfProcRarNode) |
| src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcTaskHost.cs | Replaced hardcoded "/nodemode:2" with NodeModeHelper.ToCommandLineArgument(NodeMode.OutOfProcTaskHostNode) |
| src/Build/BackEnd/Components/Communications/NodeProviderOutOfProc.cs | Replaced hardcoded "/nodemode:1" with NodeModeHelper.ToCommandLineArgument(NodeMode.OutOfProcNode) |
| src/Build/BackEnd/Client/MSBuildClient.cs | Replaced hardcoded "/nodemode:8" with NodeModeHelper.ToCommandLineArgument(NodeMode.OutOfProcServerNode) |
Comments suppressed due to low confidence (1)
src/MSBuild/XMake.cs:123
- This public method appears to be unused throughout the codebase. All callers use NodeModeHelper.ToCommandLineArgument directly instead. Additionally, the cast from NodeMode to Shared.NodeMode is redundant since NodeMode (imported via "using Microsoft.Build.Shared;") already refers to Shared.NodeMode. Consider removing this method or clarifying its intended purpose if it's meant to be a public API for external consumers.
/// <summary>
/// The object used to synchronize access to shared build state
/// </summary>
private static readonly LockType s_buildLock = new LockType();
JanProvaznik
requested changes
Feb 5, 2026
Member
Author
|
@copilot implement @JanProvaznik's feedback:
|
Contributor
Member
Author
|
@JanProvaznik mind taking another look now that some of your feedback has been integrated? |
JanProvaznik
approved these changes
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>
7ec5f59 to
2bfb167
Compare
This was referenced Feb 10, 2026
Copilot AI
added a commit
that referenced
this pull request
Feb 17, 2026
…ings (#13148) ### Context The team was discussing nodemodes in sync this morning and we were all getting confused. This PR gives names to the existing modes and has us use them consistently across the codebase for clarity. --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com> Co-authored-by: Jan Provazník <janprovaznik@microsoft.com>
JanProvaznik
added a commit
to JanProvaznik/msbuild
that referenced
this pull request
Feb 25, 2026
…ings (dotnet#13148) ### Context The team was discussing nodemodes in sync this morning and we were all getting confused. This PR gives names to the existing modes and has us use them consistently across the codebase for clarity. --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com> Co-authored-by: Jan Provazník <janprovaznik@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The team was discussing nodemodes in sync this morning and we were all getting confused. This PR gives names to the existing modes and has us use them consistently across the codebase for clarity.