Dotnetup: Modify PowerShell profile on Windows#54520
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Windows PowerShell profile support for dotnetup’s shell-profile environment setup, so init can configure PowerShell profiles on Windows in addition to Unix shells.
Changes:
- Adds Windows PowerShell and PowerShell Core profile path resolution using
profile.ps1. - Updates terminal profile modification flow to support Windows and system/user profile entries.
- Adds/updates tests and design documentation for PowerShell profile paths and profile entry behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Installer/dotnetup.Library/Shell/PowerShellEnvShellProvider.cs |
Resolves PowerShell profile paths for Windows and switches to all-hosts profile.ps1. |
src/Installer/dotnetup.Library/DotnetEnvironmentManager.cs |
Routes terminal profile updates through Windows/Unix-specific logic. |
src/Installer/dotnetup.Library/IDotnetEnvironmentManager.cs |
Updates the interface signature and documentation for install-type-aware profile changes. |
src/Installer/dotnetup.Library/Commands/Init/InitWorkflows.cs |
Calls profile modification with the updated API. |
test/dotnetup.Tests/ShellProfileManagerTests.cs |
Adds coverage for Windows profile paths and environment manager profile entry behavior. |
test/dotnetup.Tests/MockDotnetInstallManager.cs |
Updates test mock to match the interface signature. |
test/dotnetup.Tests/DefaultInstallCommandTests.cs |
Updates expected Unix PowerShell profile filename. |
documentation/general/dotnetup/designs/unix-environment-setup.md |
Documents Unix and Windows PowerShell profile locations. |
Windows PowerShell 5.1 reads BOM-less .ps1 files as the system ANSI code page, so a new profile containing non-ASCII paths could be mis-decoded and fail to invoke dotnetup. Add IEnvShellProvider.NewFileEncoding (default BOM-less UTF-8 for POSIX shells) and have PowerShellEnvShellProvider opt into UTF-8 with BOM on Windows. Existing files continue to have their detected encoding preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
this was helpful for me to test the functionality
nagilson
left a comment
nagilson
left a comment
There was a problem hiding this comment.
The changes look great! ⭐ I didn't know about profile.ps1, that's a cool insight to get it working in both, although it may be less clear and differs from the default $PROFILE behavior, I'm okay with this for now.
The main thing I think we should address here is that dotnetup defaultinstall and other code (example: #54543) does not actually invoke RemoveProfileEntries, nor does anything I found today, making this hard to undo if you don't know what the code is doing. We should fix this before we add the windows functionality in my opinion, since this is a more serious change.
| // If the install root matches the default dotnetup-managed path, omit it from the profile | ||
| // entry so `print-env-script` falls back to its built-in default-detection logic. This | ||
| // keeps profile entries minimal and portable across user-resolved default paths. | ||
| string? profileDotnetRoot = DotnetupUtilities.PathsEqual(dotnetRoot, GetDefaultDotnetInstallPath()) |
There was a problem hiding this comment.
nit: could extract into a reusable function for both platform implementations
| // If the install root matches the default dotnetup-managed path, omit it from the profile | ||
| // entry so `print-env-script` falls back to its built-in default-detection logic. This | ||
| // keeps profile entries minimal and portable across user-resolved default paths. |
There was a problem hiding this comment.
| // If the install root matches the default dotnetup-managed path, omit it from the profile | |
| // entry so `print-env-script` falls back to its built-in default-detection logic. This | |
| // keeps profile entries minimal and portable across user-resolved default paths. | |
| // When the install root is the default path, omit --dotnet-install-path from the | |
| // profile entry. At shell startup, `print-env-script` will call | |
| // GetDefaultDotnetInstallPath() itself, so the result is identical. | |
| // Omitting it makes the profile entry portable: if the file syncs to another | |
| // machine (e.g. via OneDrive) where the default path differs (different username), | |
| // print-env-script resolves the correct path there instead of using a stale literal. |
nit: What do you think of this wording?
| return | ||
| [ | ||
| Path.Combine(documentsFolder, WindowsPowerShellProfileFolder, ProfileFileName), | ||
| Path.Combine(documentsFolder, PowerShellCoreProfileFolder, ProfileFileName), |
There was a problem hiding this comment.
it could be a nice improvement in the future to condition this on whether powershell core is present though I understand there are tradeoffs (if you install core in the future we might not have added it to that profile file)
I've just submitted #54545 with a design proposal for replacing the |
Fix #53962
There are other improvements to the experience and the code that we'd like to make, but this gets the basic functionality working.