Dotnetup: Add logic to configure PATH and other default install settings when installing SDK #52453

Copilot · 2026-01-14T12:02:06Z

Summary of Changes

All review feedback has been addressed. Latest change:

Updated error message output to use Spectre APIs: Changed Console.WriteLine and Console.Error.WriteLine calls in ConfigureInstallType to use AnsiConsole.WriteLine and AnsiConsole.MarkupLine with red markup for error messages. This provides:

Consistent styling with the rest of the application
Support for markup in messages
Better testability through consistent ANSI console usage

All changes build successfully and pass tests.

Original prompt

In the dotnetup project, update SdkInstallCommand so that it uses InstallRootManager to set up the default install root.

Use InstallRootManager to check if the default install root is already set to user. If so, skip the prompt on whether it should be updated. If not, prompt whether to set it up, then after the SDK has been installed, use the InstallRootManager to apply the change.

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

…oot configuration Co-authored-by: dsplaisted <[email protected]>

Co-authored-by: dsplaisted <[email protected]>

dsplaisted · 2026-01-14T14:37:56Z

@copilot Thanks. Now remove IsOnPath and IsSetAsDotnetRoot from DotnetInstallRootConfiguration. Instead, add an IsFullyConfigured property.

Update DotnetInstallManager.GetConfiguredInstallType to be implemented in terms of InstallRootManager. It can call the InstallRootManager to get the changes necessary to switch to User or Admin mode. If either mode needs no changes it can return that type as fully configured. Otherwise, if the PATH resolves to dotnet in one of the expected paths, it can return the corresponding type with IsFullyConfigured set to false.

Then update the logic in SdkInstallCommand so that it doesn't initially directly call InstallRootManager to determine what prompts to show. If it does need to make changes, then it can use InstallRootManager at the end.

…sFullyConfigured property Co-authored-by: dsplaisted <[email protected]>

Co-authored-by: dsplaisted <[email protected]>

Copilot · 2026-01-14T14:50:06Z

@copilot Thanks. Now remove IsOnPath and IsSetAsDotnetRoot from DotnetInstallRootConfiguration. Instead, add an IsFullyConfigured property.

Update DotnetInstallManager.GetConfiguredInstallType to...

Done. Changes implemented in commits f7db809 and 9a331ed:

Removed IsOnPath and IsSetAsDotnetRoot from DotnetInstallRootConfiguration, added IsFullyConfigured property
Refactored GetConfiguredInstallType to use InstallRootManager - it now calls GetUserInstallRootChanges() and GetAdminInstallRootChanges() to determine configuration state
Updated SdkInstallCommand to use IsFullyConfigured from GetConfiguredInstallType instead of directly calling InstallRootManager during prompting
InstallRootManager is now only called at the end when applying changes

dsplaisted

See comments.

Also, update InstallRootManager and the AddPathEntry logic so that it handles the case where the specified pathToAdd is on the path, but there is another path before it that also has a dotnet in it. In that case, move the pathToAdd to the front of the path.

dsplaisted · 2026-01-14T16:33:07Z

src/Installer/dotnetup/Commands/Sdk/Install/SdkInstallCommand.cs

+                        //  If not fully configured, we leave resolvedSetDefaultInstall as null (no prompt)
+                        //  since the install path is already on PATH. The user can run "dotnetup defaultinstall"
+                        //  if they want to fully configure it.


Update this so that if the user install is not fully configured, it should display the things which need to be configured and prompt whether they should be fixed.

Done in commit dba18fc. When user install is not fully configured, the command now displays what needs to be configured (remove admin paths, add to user PATH, set DOTNET_ROOT) and prompts whether to apply these changes.

dsplaisted · 2026-01-14T16:33:59Z

src/Installer/dotnetup/Commands/Sdk/Install/SdkInstallCommand.cs

        if (resolvedSetDefaultInstall == true)
        {
-            _dotnetInstaller.ConfigureInstallType(InstallType.User, resolvedInstallPath);
+            if (OperatingSystem.IsWindows())


Refactor this so that it calls _dotnetInstaller.ConfigureInstallType on all operating systems, but the implementation on Windows uses InstallRootManager.

Done in commit dba18fc. Refactored ConfigureInstallType to work on all platforms. On Windows, it now uses InstallRootManager internally. SdkInstallCommand now calls _dotnetInstaller.ConfigureInstallType(InstallType.User, resolvedInstallPath) for all platforms.

dsplaisted · 2026-01-14T16:35:04Z

src/Installer/dotnetup/DotnetInstallManager.cs

+                string programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
+                string programFilesX86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
+                bool isAdminInstall = installDir.StartsWith(Path.Combine(programFiles, "dotnet"), StringComparison.OrdinalIgnoreCase) ||
+                                      installDir.StartsWith(Path.Combine(programFilesX86, "dotnet"), StringComparison.OrdinalIgnoreCase);


We can skip this logic here. If we don't need any changes to switch to admin mode, then we can return that the admin mode is fully configured.

Done in commit dba18fc. Removed the unnecessary logic - if GetAdminInstallRootChanges() reports no changes needed, we return Admin type as fully configured regardless of path location.

…nstallType on all platforms, improve AddPathEntry logic Co-authored-by: dsplaisted <[email protected]>

Co-authored-by: dsplaisted <[email protected]>

Copilot · 2026-01-14T16:52:11Z

See comments.

Also, update InstallRootManager and the AddPathEntry logic so that it handles the case where the specified pathToAdd is on the path, but there is another path before it that also has a ...

Done in commits dba18fc and ea5d2bb. Updated WindowsPathHelper.AddPathEntry to detect when another dotnet exists before the specified path and move it to the front. The logic now:

Checks if path is already in PATH
If yes, checks if any other path with dotnet.exe exists before it
If another dotnet is found before it, moves the path to the front to ensure correct resolution order

dsplaisted · 2026-01-14T17:04:43Z

src/Installer/dotnetup/Commands/Sdk/Install/SdkInstallCommand.cs

        }

        bool? resolvedSetDefaultInstall = _setDefaultInstall;
+        UserInstallRootChanges? userInstallRootChanges = null;


Don't declare this here. Only declare it later when it is first used.

Done in commit ef7728b. Moved the declaration to line 158 where it's first used.

dsplaisted · 2026-01-15T16:41:17Z

src/Installer/dotnetup/WindowsPathHelper.cs

    /// <param name="expandedPath">The expanded PATH string to use for detection.</param>
    /// <param name="pathToAdd">The path to add.</param>
    /// <returns>The modified unexpanded PATH string.</returns>
    public static string AddPathEntry(string unexpandedPath, string expandedPath, string pathToAdd)


Change this method so it also takes a commandName that is supposed to be resolved from the pathToAdd.

The first thing the method should do is to check whether commandName already resolves to an executable in the specified pathToAdd. It should do this using APIs from Microsoft.DotNet.Cli.Utils.EnvironmentProvider. That class will need to be updated so that there is a version of GetCommandPath where you can pass in the search paths (which in this case should come from expandedPath). The same logic that SearchPaths uses to split and clean up the PATH should also be applied.

If the resolved command is already in the pathToAdd, then just return unexpandedPath as is.

Otherwise, check to see if pathToAdd is already in the expandedPath. If it is, then move it to the front and return that. If it isn't, then add it to the front.

Done in commits ef7728b and a1389bb. Updated AddPathEntry to:

Take commandName parameter

Added GetCommandPath(string commandName, IEnumerable<string> searchPaths, ...) overload to IEnvironmentProvider and EnvironmentProvider

Use EnvironmentProvider to check if command already resolves to pathToAdd - if yes, return unchanged

If not, move existing path to front or add new path to front
The logic properly processes search paths using the same PATH splitting/cleaning logic as SearchPaths.

…on and move variable declaration closer to usage Co-authored-by: dsplaisted <[email protected]>

…ntry Co-authored-by: dsplaisted <[email protected]>

dsplaisted

In EnvironmentProvider, factor out the following logic from SearchPaths into a SplitPaths helper method:

.Split(s_pathSeparator)
.Select(p => p.Trim(s_quote))
.Where(p => !string.IsNullOrWhiteSpace(p))
.Select(p => ExpandTildeSlash(p))

Then update WindowsPathHelper.SplitPath call this helper method.

dsplaisted · 2026-01-15T17:21:59Z

src/Cli/Microsoft.DotNet.Cli.Utils/EnvironmentProvider.cs

        return commandPath;
    }

+    public string? GetCommandPath(string commandName, IEnumerable<string> searchPaths, params string[] extensions)


Never mind, you can just delete this overload. I didn't realize that the class already had a way to specify custom search paths.

Done in commit bac3323. Removed the overload since the constructor already supports custom search paths.

…undant GetCommandPath overload Co-authored-by: dsplaisted <[email protected]>

Copilot · 2026-01-15T17:35:26Z

In EnvironmentProvider, factor out the following logic from SearchPaths into a SplitPaths helper method:

Done in commit bac3323. Created SplitPaths helper method in EnvironmentProvider that encapsulates the splitting/trimming/expanding logic. Updated WindowsPathHelper.SplitPath to use this helper method for consistent PATH processing.

dsplaisted · 2026-01-16T14:00:05Z

src/Cli/Microsoft.DotNet.Cli.Utils/IEnvironmentProvider.cs


    string? GetCommandPathFromRootPath(string rootPath, string commandName, IEnumerable<string> extensions);

+    IEnumerable<string> SplitPaths(string pathString);


Don't add this method to the interface. We don't need to go through the interface in WindowsPathHelper where we use it.

Done in commit 941fe48. Removed SplitPaths from the interface since it's only used directly in WindowsPathHelper.

dsplaisted · 2026-01-16T14:04:40Z

src/Installer/dotnetup/WindowsPathHelper.cs

+        if (resolvedCommandPath != null)
+        {
+            var resolvedDir = Path.GetDirectoryName(resolvedCommandPath);
+            var normalizedPathToAdd = Path.TrimEndingDirectorySeparator(pathToAdd);


Move this line before the if statement, and then you can get rid of the normalizedPathToAddFinal variable and just use normalizedPathToAdd.

Done in commit 941fe48. Moved normalizedPathToAdd before the if statement and removed the redundant normalizedPathToAddFinal variable.

dsplaisted · 2026-01-16T14:08:01Z

src/Installer/dotnetup/WindowsPathHelper.cs

+        // Check if pathToAdd is already in the expanded PATH
+        var normalizedPathToAddFinal = Path.TrimEndingDirectorySeparator(pathToAdd);
+        int existingIndex = -1;
+        for (int i = 0; i < expandedEntries.Count; i++)


Rather than using a loop here, use the FindIndex method on List class:

int existingIndex = expandedEntries.FindIndex(expandedEntry => Path.TrimEndingDirectorySeparator(expandedEntry).Equals(normalizedPathToAddFinal, StringComparison.OrdinalIgnoreCase));

Done in commit 941fe48. Replaced the manual loop with FindIndex method.

dsplaisted · 2026-01-16T14:19:19Z

src/Installer/dotnetup/DotnetInstallManager.cs

        string installDir = Path.GetDirectoryName(foundDotnet)!;
+        var installRoot = new DotnetInstallRoot(installDir, InstallerUtilities.GetDefaultInstallArchitecture());


Change this:

Suggested change

string installDir = Path.GetDirectoryName(foundDotnet)!;

var installRoot = new DotnetInstallRoot(installDir, InstallerUtilities.GetDefaultInstallArchitecture());

var currentInstallRoot = new DotnetInstallRoot(Path.GetDirectoryName(foundDotnet)!, InstallerUtilities.GetDefaultInstallArchitecture());

Then use currentInstallRoot.Path later on instead of installDir.

Done in commit 941fe48. Created currentInstallRoot at the beginning and used currentInstallRoot.Path throughout the method instead of installDir.

…y, use currentInstallRoot consistently Co-authored-by: dsplaisted <[email protected]>

baronfel · 2026-01-16T15:59:01Z

src/Installer/dotnetup/WindowsPathHelper.cs

+        var unexpandedEntries = SplitPath(unexpandedPath);
+
+        // Check if the command already resolves to the pathToAdd using EnvironmentProvider
+        var envProvider = new Microsoft.DotNet.Cli.Utils.EnvironmentProvider(searchPathsOverride: expandedEntries);


I've seen the use of environment provider in a few places, but IMO it doesn't provide utility if we're new-ing it up every time. The point of the provider is to allow it to be injected (especially during tests) so that we can simulate different configurations without needing to actually modify process-global state. It feels like we're missing method parameters to some of the members, or they need to be instance members of types that take the provider as a constructor parameter or something.

Here I'm not trying to use it as a provider, I just want to use some of the logic that's in it without rewriting / duplicating it.

baronfel · 2026-01-16T16:00:38Z

src/Installer/dotnetup/Commands/Sdk/Install/SdkInstallCommand.cs

                    {
-                        //  No need to prompt here, the default install is already set up.
+                        //  If the current install is fully configured and matches the resolved path, skip the prompt
+                        if (currentDotnetInstallRoot.IsFullyConfigured)


this logic itself makes sense to me, but overall the flow of this execute method seems very long. this is very subjective but since installation in general is a step-by-step process I would expect to see each logic step of that process extracted out to a named method or something so that we can more easily a) verify the steps we expect are happening and b) change a specific step in isolation/narrow the blast radius.

Yes, I'd like to try to refactor this sometime. But it's tricky because I think we want to ask all of the questions first before we take any actions, and there are a lot of dependencies between the questions we ask and the current state.

baronfel · 2026-01-16T16:01:46Z

src/Installer/dotnetup/DotnetInstallManager.cs

+        // Use InstallRootManager to determine if the install is fully configured
+        if (OperatingSystem.IsWindows())
+        {
+            var installRootManager = new InstallRootManager(this);


similar question here - we do we need to new up this during the invocation of this method. if it's a dependency should/can it be provided in the constructor and/or as a method parameter to allow simulating different inputs?

There's a lot of overlap in the functionality between DotnetInstallManager and InstallRootManager, I'm not sure if they even need to be separate classes. I'm thinking of working on support for non-Windows OS's next. I suspect I will refactor this some as part of that, InstallRootManager might change to be a Windows-specific implementation of an interface, with other implementations for other OS's or shells.

baronfel · 2026-01-16T16:03:29Z

src/Installer/dotnetup/DotnetInstallManager.cs

+            string programFilesPath = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
+            string programFilesX86Path = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
+            bool isAdminPath = currentInstallRoot.Path.StartsWith(Path.Combine(programFilesPath, "dotnet"), StringComparison.OrdinalIgnoreCase) ||
+                               currentInstallRoot.Path.StartsWith(Path.Combine(programFilesX86Path, "dotnet"), StringComparison.OrdinalIgnoreCase);

-        return new(installRoot, isAdminInstall ? InstallType.Admin : InstallType.User, IsOnPath: true, isSetAsDotnetRoot);
+            return new(currentInstallRoot, isAdminPath ? InstallType.Admin : InstallType.User, IsFullyConfigured: false);
+        }
+        else
+        {
+            // For non-Windows platforms, determine based on path location
+            // TODO: This should be improved to not be windows-specific https://github.com/dotnet/sdk/issues/51601
+            string programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
+            string programFilesX86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86);
+            bool isAdminInstall = currentInstallRoot.Path.StartsWith(Path.Combine(programFiles, "dotnet"), StringComparison.OrdinalIgnoreCase) ||
+                                  currentInstallRoot.Path.StartsWith(Path.Combine(programFilesX86, "dotnet"), StringComparison.OrdinalIgnoreCase);


This logic doesn't sit right with me - we have a documented way to detect global installations for all of our primary operating systems, and we should be using it here.

baronfel · 2026-01-16T16:04:34Z

src/Installer/dotnetup/DotnetInstallManager.cs

+                        msg => Console.WriteLine(msg),
+                        msg => Console.Error.WriteLine(msg));


should this be writing to the ANSI console so we can use markup in our error messages (and so we can inject an ANSI console consistently for testing)? same questions for lines 171-172 below.

@copilot Update this and the instance below to use the Spectre APIs.

Done in commit eaab195. Updated to use AnsiConsole.WriteLine and AnsiConsole.MarkupLine with red markup for error messages.

Co-authored-by: dsplaisted <[email protected]>

Initial plan

6158d69

Copilot AI assigned Copilot and dsplaisted Jan 14, 2026

Copilot started work on behalf of dsplaisted January 14, 2026 12:02 View session

Copilot AI and others added 2 commits January 14, 2026 12:11

Update SdkInstallCommand to use InstallRootManager for user install r…

c7dec0a

…oot configuration Co-authored-by: dsplaisted <[email protected]>

Pass DotnetInstallManager to InstallRootManager for consistency

07f6f38

Co-authored-by: dsplaisted <[email protected]>

Copilot AI changed the title ~~[WIP] Update SdkInstallCommand to use InstallRootManager~~ Use InstallRootManager in SdkInstallCommand for user install root configuration Jan 14, 2026

Copilot AI requested a review from dsplaisted January 14, 2026 12:21

Copilot finished work on behalf of dsplaisted January 14, 2026 12:21

Copilot started work on behalf of dsplaisted January 14, 2026 14:38 View session

Copilot AI and others added 2 commits January 14, 2026 14:47

Refactor GetConfiguredInstallType to use InstallRootManager and add I…

f7db809

…sFullyConfigured property Co-authored-by: dsplaisted <[email protected]>

Clarify comments for partially configured install path logic

9a331ed

Co-authored-by: dsplaisted <[email protected]>

Copilot AI changed the title ~~Use InstallRootManager in SdkInstallCommand for user install root configuration~~ Refactor installation configuration to use InstallRootManager with IsFullyConfigured property Jan 14, 2026

Copilot finished work on behalf of dsplaisted January 14, 2026 14:51

dsplaisted requested changes Jan 14, 2026

View reviewed changes

Copilot started work on behalf of dsplaisted January 14, 2026 16:41 View session

Copilot AI and others added 2 commits January 14, 2026 16:50

Address review feedback: prompt for incomplete config, use ConfigureI…

dba18fc

…nstallType on all platforms, improve AddPathEntry logic Co-authored-by: dsplaisted <[email protected]>

Fix logic error in AddPathEntry when moving path to front

ea5d2bb

Co-authored-by: dsplaisted <[email protected]>

Copilot AI requested a review from dsplaisted January 14, 2026 16:52

Copilot finished work on behalf of dsplaisted January 14, 2026 16:52

build-analysis bot mentioned this pull request Jan 14, 2026

Could Not Resolve SDK - repo testfx issue #51530

Open

dsplaisted requested changes Jan 15, 2026

View reviewed changes

Copilot started work on behalf of dsplaisted January 15, 2026 16:42 View session

Copilot AI and others added 2 commits January 15, 2026 16:51

Refactor AddPathEntry to use EnvironmentProvider for command resoluti…

ef7728b

…on and move variable declaration closer to usage Co-authored-by: dsplaisted <[email protected]>

Fix redundant parameter and clarify path reordering logic in AddPathE…

a1389bb

…ntry Co-authored-by: dsplaisted <[email protected]>

Copilot AI requested a review from dsplaisted January 15, 2026 16:55

Copilot finished work on behalf of dsplaisted January 15, 2026 16:55

dsplaisted requested changes Jan 15, 2026

View reviewed changes

Copilot started work on behalf of dsplaisted January 15, 2026 17:27 View session

Factor out path splitting logic into SplitPaths helper and remove red…

bac3323

…undant GetCommandPath overload Co-authored-by: dsplaisted <[email protected]>

Copilot finished work on behalf of dsplaisted January 15, 2026 17:36

Copilot AI requested a review from dsplaisted January 15, 2026 17:36

This was referenced Jan 15, 2026

TemplateEngineToolDiscovery Network Failure: Unable to read data from the transport connection: Connection reset by peer #51154

Open

Template engine tests hitting known parallelism issue in GetVisualStudioInstances #44878

Open

dsplaisted requested changes Jan 16, 2026

View reviewed changes

Copilot started work on behalf of dsplaisted January 16, 2026 14:23 View session

Clean up code: remove SplitPaths from interface, simplify AddPathEntr…

941fe48

…y, use currentInstallRoot consistently Co-authored-by: dsplaisted <[email protected]>

Copilot AI requested a review from dsplaisted January 16, 2026 14:33

Copilot finished work on behalf of dsplaisted January 16, 2026 14:33

dsplaisted changed the title ~~Refactor installation configuration to use InstallRootManager with IsFullyConfigured property~~ Dotnetup: Add logic to configure PATH and other default install settings when installing SDK Jan 16, 2026

dsplaisted marked this pull request as ready for review January 16, 2026 15:07

baronfel reviewed Jan 16, 2026

View reviewed changes

Copilot started work on behalf of dsplaisted January 16, 2026 18:28 View session

Use Spectre AnsiConsole for error messages in ConfigureInstallType

eaab195

Co-authored-by: dsplaisted <[email protected]>

Copilot finished work on behalf of dsplaisted January 16, 2026 18:36


		string? GetCommandPathFromRootPath(string rootPath, string commandName, IEnumerable<string> extensions);

		IEnumerable<string> SplitPaths(string pathString);

		string installDir = Path.GetDirectoryName(foundDotnet)!;
		var installRoot = new DotnetInstallRoot(installDir, InstallerUtilities.GetDefaultInstallArchitecture());

	string installDir = Path.GetDirectoryName(foundDotnet)!;
	var installRoot = new DotnetInstallRoot(installDir, InstallerUtilities.GetDefaultInstallArchitecture());
	var currentInstallRoot = new DotnetInstallRoot(Path.GetDirectoryName(foundDotnet)!, InstallerUtilities.GetDefaultInstallArchitecture());

		msg => Console.WriteLine(msg),
		msg => Console.Error.WriteLine(msg));

Dotnetup: Add logic to configure PATH and other default install settings when installing SDK #52453

Are you sure you want to change the base?

Dotnetup: Add logic to configure PATH and other default install settings when installing SDK #52453

Uh oh!

Conversation

Copilot AI commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Uh oh!

dsplaisted commented Jan 14, 2026

Uh oh!

Copilot AI commented Jan 14, 2026

Uh oh!

dsplaisted left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

dsplaisted left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Copilot AI commented Jan 14, 2026 •

edited

Loading