-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Dotnetup: add env command to update PathPreference mode
#54545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 19 commits
1cbba05
88140c0
76aceb7
443fd6d
80492b4
28920f2
a9523f5
4028290
879393b
fd43c72
f0d95ea
2ce0a39
018cb94
be903cb
b2d1492
2f4b463
6450069
34d6b6e
eddece0
4bed5c3
ff37b1e
a5c5866
a0b2b9a
b1d2a47
1bb89b5
1ddcecc
ac7de84
12b75c5
1edabc0
aff4911
a2590ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.CommandLine; | ||
| using Microsoft.DotNet.Tools.Bootstrapper.Shell; | ||
|
|
||
| namespace Microsoft.DotNet.Tools.Bootstrapper.Commands.Env; | ||
|
|
||
| /// <summary> | ||
| /// Fully removes everything dotnetup wrote into the environment: equivalent to | ||
| /// <c>env set none --dotnetup-on-path false</c>. Removes the managed profile block and the | ||
| /// dotnetup PATH entry, leaving no dotnet access and no dotnetup-on-PATH. The closest thing | ||
| /// to an "env uninstall" since dotnetup has no uninstall command. | ||
| /// </summary> | ||
| internal class EnvClearCommand : CommandBase | ||
| { | ||
| private readonly IDotnetEnvironmentManager _dotnetEnvironment; | ||
| private readonly IEnvironmentStateInspector _inspector; | ||
| private readonly IEnvShellProvider? _shellProvider; | ||
|
|
||
| public EnvClearCommand(ParseResult result, IDotnetEnvironmentManager? dotnetEnvironment = null, IEnvironmentStateInspector? inspector = null) : base(result) | ||
| { | ||
| _dotnetEnvironment = dotnetEnvironment ?? new DotnetEnvironmentManager(); | ||
| _inspector = inspector ?? new EnvironmentStateInspector(_dotnetEnvironment); | ||
| _shellProvider = result.GetValue(CommonOptions.ShellOption); | ||
| } | ||
|
|
||
| protected override string GetCommandName() => "env clear"; | ||
|
|
||
| protected override void ExecuteCore() | ||
| { | ||
| EnvSettingsWriter.ApplyAndPersist( | ||
| DotnetAccessMode.None, | ||
| targetDotnetupOnPath: false, | ||
| _dotnetEnvironment, | ||
| _shellProvider, | ||
| _inspector); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.CommandLine; | ||
|
|
||
| namespace Microsoft.DotNet.Tools.Bootstrapper.Commands.Env; | ||
|
|
||
| internal static class EnvClearCommandParser | ||
| { | ||
| public static Command ConstructCommand() | ||
| { | ||
| Command command = new("clear", "Remove all dotnetup environment wiring (equivalent to 'env set none --dotnetup-on-path false')."); | ||
| command.Options.Add(CommonOptions.ShellOption); | ||
| command.SetAction(parseResult => new EnvClearCommand(parseResult).Execute()); | ||
| return command; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.CommandLine; | ||
|
|
||
| namespace Microsoft.DotNet.Tools.Bootstrapper.Commands.Env; | ||
|
|
||
| internal static class EnvCommandParser | ||
| { | ||
| private static readonly Command s_envCommand = ConstructCommand(); | ||
|
|
||
| public static Command GetCommand() => s_envCommand; | ||
|
|
||
| private static Command ConstructCommand() | ||
| { | ||
| Command command = new("env", "Manage the dotnetup environment configuration (PATH and DOTNET_ROOT wiring)."); | ||
|
|
||
| command.Subcommands.Add(EnvSetCommandParser.ConstructCommand()); | ||
| command.Subcommands.Add(EnvClearCommandParser.ConstructCommand()); | ||
| command.Subcommands.Add(EnvShowCommandParser.ConstructCommand()); | ||
| command.Subcommands.Add(EnvScriptCommandParser.ConstructCommand(name: "script")); | ||
|
|
||
| return command; | ||
| } | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Globalization; | ||
|
|
||
| namespace Microsoft.DotNet.Tools.Bootstrapper.Commands.Env; | ||
|
|
||
| /// <summary> | ||
| /// Pure comparison of the configured env settings against an observed environment snapshot, | ||
|
dsplaisted marked this conversation as resolved.
Outdated
|
||
| /// producing human-readable drift descriptions. Kept free of any environment reads (the | ||
| /// <see cref="EnvironmentStateInspector"/> does those) so it can be unit-tested without touching | ||
| /// the registry or shell profiles. Used by <see cref="EnvShowCommand"/>. | ||
| /// </summary> | ||
| internal static class EnvDriftAnalyzer | ||
| { | ||
| public static IReadOnlyList<string> Compare(DotnetupConfigData config, ObservedEnvironmentState observed) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(config); | ||
|
dsplaisted marked this conversation as resolved.
Outdated
|
||
| ArgumentNullException.ThrowIfNull(observed); | ||
|
|
||
| var drift = new List<string>(); | ||
|
|
||
| bool expectsProfileDotnet = config.AccessMode is DotnetAccessMode.Shell or DotnetAccessMode.All; | ||
| bool expectsProfileBlock = expectsProfileDotnet || config.DotnetupOnPath; | ||
| bool expectsDotnetEnvVars = config.AccessMode == DotnetAccessMode.All; | ||
|
|
||
| // Profile-block presence: only assert when the profile state is known. | ||
| if (observed.ProfileBlockPresent is bool profileBlockPresent) | ||
|
dsplaisted marked this conversation as resolved.
Outdated
|
||
| { | ||
| if (expectsProfileBlock && !profileBlockPresent) | ||
| { | ||
| drift.Add("Shell profile is missing the dotnetup managed block."); | ||
|
dsplaisted marked this conversation as resolved.
Outdated
|
||
| } | ||
| else if (!expectsProfileBlock && profileBlockPresent) | ||
| { | ||
| drift.Add("Shell profile contains a dotnetup managed block but neither dotnet access nor dotnetup-on-PATH is configured."); | ||
| } | ||
| } | ||
|
|
||
| if (OperatingSystem.IsWindows()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I worry that this adds another maintenance point about only supporting 'full' path on windows - which I think will change shortly since we need to support mac gui apps and linux gui apps not launched from the shell. I would suggest changing this pattern.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code could be made more future-proof, but I'm pretty sure it works as is and changing it would add some risk of breaking it. Plus, I don't know what "everywhere" mode on non-Windows will look like, and we might still end up with OS-specific code in the end. So I'd prefer to leave it as is for now.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Totally understandable, let's keep this as is. |
||
| { | ||
| if (expectsDotnetEnvVars && !observed.DotnetUserEnvVarsComplete) | ||
| { | ||
| drift.Add("Windows user PATH / DOTNET_ROOT / system PATH do not match 'all' mode expectations."); | ||
| } | ||
| else if (!expectsDotnetEnvVars && observed.DotnetUserEnvVarsPresent) | ||
| { | ||
| drift.Add(string.Format( | ||
| CultureInfo.InvariantCulture, | ||
| "Windows user PATH / DOTNET_ROOT still has 'all'-mode wiring (expected dotnet access: '{0}').", | ||
| config.AccessMode.ToString().ToLowerInvariant())); | ||
| } | ||
|
|
||
| // The user-scope PATH is authoritative for dotnetup-on-PATH on Windows (the profile | ||
|
dsplaisted marked this conversation as resolved.
Outdated
|
||
| // block copy is just a convenience). | ||
| if (config.DotnetupOnPath && !observed.DotnetupOnUserPath) | ||
|
dsplaisted marked this conversation as resolved.
|
||
| { | ||
| drift.Add("dotnetup is configured to be on PATH but is missing from the user PATH."); | ||
| } | ||
| else if (!config.DotnetupOnPath && observed.DotnetupOnUserPath) | ||
| { | ||
| drift.Add("dotnetup is on the user PATH but is configured to be off."); | ||
| } | ||
| } | ||
|
|
||
| return drift; | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.