Skip to content

macOS: Include global tools in zsh $PATH #48893

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: release/9.0.1xx
Choose a base branch
from
Open

macOS: Include global tools in zsh $PATH #48893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
faf7058
Fix path for tools on macOS
edvilme May 9, 2025
37fa3bd
Update zsh detector
edvilme May 9, 2025
fb6754a
Remove special cases for zsh
edvilme May 9, 2025
d9a32bc
Show correct instructions for bash/zsh
edvilme May 9, 2025
a8b1e66
Rename MacOSEnvironmentPath
edvilme May 9, 2025
1874e42
Remove OsxZshEnvironmentPathInstruction
edvilme May 9, 2025
29cb349
Update tests
edvilme May 9, 2025
1698a22
Update remaining tests
edvilme May 10, 2025
ff66f7e
Update tests
edvilme May 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions src/Cli/dotnet/ShellShim/EnvironmentPathFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static IEnvironmentPath CreateEnvironmentPath(
}
else if (OperatingSystem.IsMacOS() && isDotnetBeingInvokedFromNativeInstaller)
{
environmentPath = new OsxBashEnvironmentPath(
environmentPath = new MacOSEnvironmentPath(
executablePath: CliFolderPathCalculator.ToolsShimPathInUnix,
reporter: Reporter.Output,
environmentProvider: environmentProvider,
Expand All @@ -64,14 +64,6 @@ public static IEnvironmentPathInstruction CreateEnvironmentPathInstruction(
environmentProvider = new EnvironmentProvider();
}

if (OperatingSystem.IsMacOS() && ZshDetector.IsZshTheUsersShell(environmentProvider))
{
return new OsxZshEnvironmentPathInstruction(
executablePath: CliFolderPathCalculator.ToolsShimPathInUnix,
reporter: Reporter.Output,
environmentProvider: environmentProvider);
}

if (OperatingSystem.IsWindows())
{
return new WindowsEnvironmentPath(
Expand Down
18 changes: 11 additions & 7 deletions ...otnet/ShellShim/OsxBashEnvironmentPath.cs → .../dotnet/ShellShim/MacOSEnvironmentPath.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

namespace Microsoft.DotNet.ShellShim
{
internal class OsxBashEnvironmentPath : IEnvironmentPath
internal class MacOSEnvironmentPath : IEnvironmentPath
{
private const string PathName = "PATH";
private readonly BashPathUnderHomeDirectory _packageExecutablePath;
Expand All @@ -20,7 +20,7 @@ internal static readonly string DotnetCliToolsPathsDPath
= Environment.GetEnvironmentVariable("DOTNET_CLI_TEST_OSX_PATHSD_PATH")
?? @"/etc/paths.d/dotnet-cli-tools";

public OsxBashEnvironmentPath(
public MacOSEnvironmentPath(
BashPathUnderHomeDirectory executablePath,
IReporter reporter,
IEnvironmentProvider environmentProvider,
Expand All @@ -42,7 +42,7 @@ public void AddPackageExecutablePathToUserPath()
return;
}

_fileSystem.WriteAllText(DotnetCliToolsPathsDPath, _packageExecutablePath.PathWithTilde);
_fileSystem.WriteAllText(DotnetCliToolsPathsDPath, _packageExecutablePath.Path);
}

private bool PackageExecutablePathExists()
Expand All @@ -55,7 +55,7 @@ private bool PackageExecutablePathExists()

return value
.Split(':')
.Any(p => p == _packageExecutablePath.Path || p == _packageExecutablePath.PathWithTilde);
.Any(p => p.Equals(_packageExecutablePath.Path, StringComparison.OrdinalIgnoreCase));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why OrdinalIgnoreCase on mac? I thought it was case-sensitive normally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh suggested by Copilot, didn't pay much attention. You're right :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macOS can be set to case insensitive

}

public void PrintAddPathInstructionIfPathDoesNotExist()
Expand All @@ -71,9 +71,13 @@ public void PrintAddPathInstructionIfPathDoesNotExist()
{
// similar to https://code.visualstudio.com/docs/setup/mac
_reporter.WriteLine(
string.Format(
CommonLocalizableStrings.EnvironmentPathOSXBashManualInstructions,
_packageExecutablePath.Path));
ZshDetector.IsZshTheUsersShell(_environmentProvider)
? string.Format(
CommonLocalizableStrings.EnvironmentPathOSXZshManualInstructions,
_packageExecutablePath.Path)
: string.Format(
CommonLocalizableStrings.EnvironmentPathOSXBashManualInstructions,
_packageExecutablePath.Path));
}
}
}
Expand Down
56 changes: 0 additions & 56 deletions src/Cli/dotnet/ShellShim/OsxZshEnvironmentPathInstruction.cs
View file
Open in desktop

This file was deleted.

11 changes: 1 addition & 10 deletions src/Cli/dotnet/ShellShim/ZshDetector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,8 @@ internal static class ZshDetector
public static bool IsZshTheUsersShell(IEnvironmentProvider environmentProvider)
{
string environmentVariable = environmentProvider.GetEnvironmentVariable("SHELL");
if (string.IsNullOrWhiteSpace(environmentVariable))
{
return false;
}

if (Path.GetFileName(environmentVariable).Equals(ZshFileName, StringComparison.OrdinalIgnoreCase))
{
return true;
}

return false;
return Path.GetFileName(environmentVariable)?.Equals(ZshFileName, StringComparison.OrdinalIgnoreCase) ?? false;
}
}
}
24 changes: 6 additions & 18 deletions test/Microsoft.DotNet.ShellShim.Tests/EnvironmentPathFactoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,20 @@ namespace Microsoft.DotNet.ShellShim.Tests
{
public class EnvironmentPathFactoryTests
{
[MacOsOnlyFact]
public void GivenFollowingEnvironmentVariableValueItCanReturnOsxZshEnvironmentPathInstruction()
[MacOSOnlyTheory]
[InlineData("/bin/bash")]
[InlineData("/bin/zsh")]
public void GivenFollowingEnvironmentVariableValueItShouldReturnOsxBashEnvironmentPath(string shell)
{
Mock<IEnvironmentProvider> provider = new(MockBehavior.Strict);

provider
.Setup(p => p.GetEnvironmentVariable("SHELL"))
.Returns("/bin/zsh");
.Returns(shell);

IEnvironmentPathInstruction result =
EnvironmentPathFactory.CreateEnvironmentPathInstruction(provider.Object);
(result is OsxZshEnvironmentPathInstruction).Should().BeTrue();
}

[MacOsOnlyFact]
public void GivenFollowingEnvironmentVariableValueItShouldReturnOsxBashEnvironmentPath()
{
Mock<IEnvironmentProvider> provider = new(MockBehavior.Strict);

provider
.Setup(p => p.GetEnvironmentVariable("SHELL"))
.Returns("/bin/bash");

IEnvironmentPathInstruction result =
EnvironmentPathFactory.CreateEnvironmentPathInstruction(provider.Object);
(result is OsxBashEnvironmentPath).Should().BeTrue();
(result is MacOSEnvironmentPath).Should().BeTrue();
}

[WindowsOnlyFact]
Expand Down
20 changes: 20 additions & 0 deletions test/Microsoft.DotNet.ShellShim.Tests/LinuxEnvironmentPathTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ public void GivenPathNotSetItPrintsManualInstructions()
.Setup(p => p.GetEnvironmentVariable("PATH"))
.Returns(pathValue);

provider
.Setup(p => p.GetEnvironmentVariable("SHELL"))
.Returns("/bin/bash");

var environmentPath = new LinuxEnvironmentPath(
toolsPath,
reporter,
Expand All @@ -50,6 +54,10 @@ public void GivenPathNotSetAndProfileExistsItPrintsLogoutMessage()
.Setup(p => p.GetEnvironmentVariable("PATH"))
.Returns(pathValue);

provider
.Setup(p => p.GetEnvironmentVariable("SHELL"))
.Returns("/bin/bash");

var environmentPath = new LinuxEnvironmentPath(
toolsPath,
reporter,
Expand Down Expand Up @@ -78,6 +86,10 @@ public void GivenPathSetItPrintsNothing(string toolsDirectoryOnPath)
.Setup(p => p.GetEnvironmentVariable("PATH"))
.Returns(pathValue + ":" + toolsDirectoryOnPath);

provider
.Setup(p => p.GetEnvironmentVariable("SHELL"))
.Returns("/bin/bash");

var environmentPath = new LinuxEnvironmentPath(
toolsPath,
reporter,
Expand All @@ -102,6 +114,10 @@ public void GivenPathSetItDoesNotAddPathToEnvironment()
.Setup(p => p.GetEnvironmentVariable("PATH"))
.Returns(pathValue + ":" + toolsPath.Path);

provider
.Setup(p => p.GetEnvironmentVariable("SHELL"))
.Returns("/bin/bash");

var environmentPath = new LinuxEnvironmentPath(
toolsPath,
reporter,
Expand Down Expand Up @@ -132,6 +148,10 @@ public void GivenPathNotSetItAddsToEnvironment()
.Setup(p => p.GetEnvironmentVariable("PATH"))
.Returns(pathValue);

provider
.Setup(p => p.GetEnvironmentVariable("SHELL"))
.Returns("/bin/bash");

var environmentPath = new LinuxEnvironmentPath(
toolsPath,
reporter,
Expand Down
Loading
Loading