-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Write the full path of the dotnet tools directory in /etc/paths.d/dotnet-cli-tools #35411
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
base: main
Are you sure you want to change the base?
Conversation
…net-cli-tools Fixes dotnet#23165 and dotnet#9415
3aaf9ed
to
c57c8f1
Compare
This is no longer necessary when the full path is specified in /etc/paths.d/dotnet-cli-tools as per previous commit. Also rename OsxBashEnvironmentPath into OsxEnvironmentPath since it's not shell specific.
c57c8f1
to
73e039e
Compare
The |
I'm trying to understand the issue here. On the one hand, it could expose some of a user's home folder structure. However, it's generally (and by that I mean I would be surprised if you found even one counter-example) going to be the very well-documented dotnet tools path, and as such isn't likely to cause any raised eyebrows. For most users, even if there is another user's home path in that global path, there won't be any issues, as they likely won't have permissions to that other user's path and, as such, path resolution will just skip that path as "empty". There are no reasonable security concerns here. There are no performance concerns that aren't inherent with any path lookup. There are, however, very real concerns of users not being able to use global dotnet tools at all. |
This is such a rare situation that a dotnet sdk developer will have a device that other users are also using. Even if it did, the comment that @b4ux1t3 indicates -- it won't affect other users -- is true as well -- lack of perms. |
The root issue is that "global" tools are not installed globally. They are installed only for a specific user. If the global tools were installed in a folder under Or 'global' actually means user scoped and the tools path in the user directory needs to be added to the Also note that it appears that the |
again, macOS, dotnet sdk -- can't fathom a multi-user situation in reality Perfect is the enemy of good |
macOS is a multi-user operating system. I use fast user switching with family members. My son took a course that used C# and Unity. For me, although rare, it's not unfathomable to imagine this issue in reality. How many places to the left of the decimal point are needed to show a stock share price? 3? A share of Berkshire Hathaway Class A is 6 figures. Berkshire Hathaway is a very rare situation but I think a stock ticker display that doesn't handle share prices that large is simply not good. 'Perfect' is not relevant. In a similar way, the behavior of the defect is not good. This PR is, In essence, doubling down on the original defect and that is arguably worse than the current situation. If it were my decision (and to be clear it is very much not), I would reject this PR. |
@jrdodds So, you're saying if they allowed for a |
No. I'm not saying that. I don't think another 'scope' should be added. |
For On Windows the install folder for a global tool is On macOS the install folder for a global tool is But instead the "All Users"
The message about a one time user action to add the tools directory to the An improvement over the current message would be to extend the (Note that shell command completion for |
There is a marked difference between the concept of environment variables on Windows and *nix (Including, for this case, Mac). I do not allow third party tools to edit my shell configuration files, such as This is not an uncommon use of those files. They are not machine-specific configurations, they are user-specific configurations, which must maintain usability wherever that user uses them, even on other machines. Conversely, How Windows separates the User Environment from the System Environment is a Windows-specific detail, and should not be applied to other platforms. The User Environment is a configuration for that User on that machine. Yes, it can be synchronized with a federated windows login. That isn't how it is generally used. When you're working on different platforms, you have to take the conventions of the platforms into account. For many *nix developers, both on Linux and Mac, their shell configuration is their shell configuration. It isn't a dumping ground for whatever nonsense other applications want to put there. That's what |
I can imagine some ways for a reference into another user's home directory to hurt something:
|
@KalleOlaviNiemitalo You're absolutely right. Those are concerns. I just don't believe that they're reasonable concerns (I don't know if you're arguing they are, just clarifying my position). What is a reasonable concern, I think, is allowing a piece of software to change my known-good shell configuration just because Windows and *nix systems work differently. |
I very strongly agree with:
I disagree with:
When you dig into it, paths.d is not for user specific configuration. One reason is that paths.d is in There are tools that respect both that paths.d is not for user specific configuration and that dotfiles should not be edited by a third party. Usually this comes down to the tool needing an action on the user's part to update a dotfile, as There are 'replacement' versions of path_helper written by others that extend the feature to support per user paths by adding support for a user level paths.d folder, e.g. We agree that the conventions and rules of a platform have to be taken into account. It's not okay to break a rule because the rule is inconvenient. We disagree about the rules for |
@b4ux1t3, to clarify my position: I'm averse to adding per-user directories to system-wide $PATH and would not do such a thing in any software I maintain; but each scenario in which it actually matters seems to be unlikely or easily mitigated. |
@KalleOlaviNiemitalo Yeah, I wouldn't do it in most situations. In this specific situation, I'm saying that it makes sense given the oddities of the system at play. Specifically, "global" not meaning system-wide in .NET tooling combined with the convention of not messing with a user's shell configuration. There are obviously other ways to resolve this; all of them require adding a more complexity and could take a lot more time. My ideal fix would be to install global dotnet tools to a shared directory, no differently to how other tools' global packages work. Of course, that would require users to install global packages with root access. . .which is a thing I explicitly dislike about said tools. But, in the mean time, the functionality doesn't exist at all out of the box for some users. |
well, first I'd opt for to stop mucking with it's tiresome to keep telling macOS folks to change that file on every SDK update. |
@cicorias Are you telling people to change the For zsh, the following could be added to # replace ~ with $HOME
export PATH=${PATH//\~/$HOME} |
I was -- however, I'll try this -- tx. |
@jrdodds thanks so much. This works well. |
Fixes #23165 and #9415
Also remove the instructions to manually tweak the PATH when using zsh since they are no longer necessary.