Installer modifies ~/.profile which is ignored for graphical terminals

Problem
When running installer using curl https://sh.rustup.rs -sSf | sh in Debian 10 "Buster", installer adds a line to modify PATH in ~/.profile. That file is not used at all in graphical terminals, so this is wrong. I would expect modified PATH to be active in both login shells and graphical terminals.

Steps

run curl https://sh.rustup.rs -sSf | sh accepting defaults
restart computer
start graphical terminal, see that PATH has not been modified

Possible Solution(s)
The line to modify PATH needs to be added to ~/.bashrc instead of ~/.profile.

Notes

Output of rustup --version: none, as rustup is not in PATH
Output of rustup show: none, as rustup is not in PATH

I agree that this isn't ideal. We actually modify any/all of:

.profile
.bash_profile (if it exists)
.zprofile (if zsh)

I suppose in theory we should actually edit .bashrc and .zprofile instead. Hmm.

If someone wants to work on this, I'd suggest we ought to UNDO the change to .profile at the same time as adding to .bashrc so that might need some clever upgrade processing in src/cli/self_update.rs

Come along to the Discord and/or discuss here for further input.

Am I correct in understanding that you are suggesting the installer actively removes the export line that adds the cargo bin directory to PATH in .profile if present and the user also has a .bashrc file?

I guess if we're going to add it to .bashrc then either it shouldn't exist in .profile or else we need to have a cleverer line to add which only adds $CARGO_HOME/bin if it's not already in the PATH.

If I remember correctly, terminal of mac os would be using .bash_profile. It is usually a non-login shell.

Hi, this past winter I got sniped by the "incorrectly modifying the wrong .bashrc/.profile/whatever file" issue. It took a surprising amount of executive function to figure out what was up and fix it for myself, in spite of being "easy", so I'd like to work on fixing this for everyone... and for myself, in case I ever forget this.

My understanding is currently:
If we're handling a Unixy environment, we can kiiinda set aside worrying about Windows compat being weird for this concern, hopefully, as long as we don't do anything too exciting, as hypothetically if we're worrying about .bashrc on Windows we'll be on some repackaging of Linux or BSD tools on a Windows platform.

There are three main Unix environments of concern:
Darwin (MacOS),
Linux,
(Free|Net|Open)BSD.
Of these, the "login shell" logic is almost universal for Linux, and I believe that the BSD clan shares the same logic. So we just need to see
if we're on Darwin
then
we can edit .bash_profile and the like instead. Except Mac now uses zsh by default, so that might be different... modifying all the kinds of .profile-flavored files we find is still a safe bet, unfortunately.
else
we can sniff the environment further, but we're probably on Linux, and similar logic will probably work fine on FreeBSD as well. If not, they know where we live, for better or worse. 😅 So we modify the .shrc-flavored files. There's still an off chance, however, that the graphical terminal will load a .profile-flavored file instead, but hopefully in those cases the .bashrc will also be loaded. I'm seeing some indications this was an issue before, though?
fi

as a one time thing for updating: we apply a similar logic, sniff around, and maybe ask the user if they'd like us to revert our previous ~~mistake~~ helpful command insertion? My main concern is doing that in a truly "one-shot" or easily configurable manner, it would be best otherwise to simply avoid updating incorrectly.

I'll be on the Discord to ask further questions in a more sync manner.

I think choosing the file to modify based on the platform is not the right approach. For example, in #2311 I was using CentOS, not a BSD, there's no way to tell that from the operating system alone. Instead I think rustup should read the SHELL environment variable which should work in all Unix like environments.

I guess if we're going to add it to .bashrc then either it shouldn't exist in .profile or else we need to have a cleverer line to add which only adds $CARGO_HOME/bin if it's not already in the PATH.

I don't think existence of a .bashrc should be taken as an indication that the default shell is bash. In many cases this file is copied from /etc/skel when the user was first created, and just never deleted if the user changed their default shell. Reading SHELL is much simpler and more accurate.

Note also that putting it in .bashrc alone will stop it from being seen by other shells such as dash, which is usually the non-interactive shell on Debian-based systems. Unfortunately it seems like the only way to have both is to check in both .profile and the rcfile (usually .bashrc but could be .zshrc or something exotic like .cshrc) if cargo is in PATH and not adding it if so. See https://github.com/jyn514/dotfiles/blob/master/config/profile#L3 for an example of what this could look like. Alternatively you could just have two copies of the directory in PATH which doesn't hurt anything.

Aha. Reading the SHELL env var as the main point of guidance works for me, but still have a bit of nervousness about that solution though due to Apple choosing to act Very Weird however and only read profile.

Actually it looks like some apps will read .bashrc and some will read .bash_profile 🤦 https://scriptingosx.com/2017/04/about-bash_profile-and-bashrc-on-macos/

In that case there doesn't seem to be a good way to choose only a single file, since it's not even dependent on the shell but on the terminal emulator. IMO (as someone who isn't part of rustup and doesn't have to deal with angry users :P) this seems like too much of an edge case to support.

Note also that this is a bit of an unbounded problem in that anyone can write a shell and ask you to support it, for example fish has a different syntax from bash has a different syntax from tcsh.

Yes, that is why I formulated an intention of detecting *_apple_darwin environment and doing something different there.

And I am not a part of wg-rustup either! Yet I am working on fixing it, because no one else has, as wg-rustup effort has mostly been tied up in other tasks. Almost like it's some sort of community-project driven forward mostly by the effort of volunteers! 💦

I'd suggest that the best thing we could do would be to make .cargo/env be idempotent (i.e. if you source it more than once, it doesn't add itself to the path over and over) and then simply add instructions to source it from as many places as possible (.bashrc .bash_proifile .profile .zshenv etc)

OK, so apparently MacOS does something different now as of Catalina. I asked a MacOS Catalina user to go over their stuff and for their Terminal.app, their .zshrc was consistently sourced, both by their default and brew installed zsh, and so was their .bashrc (when loading bash). This was universal across all of their terminal apps, actually. As a result I am inclined to think that prioritizing rcfile manipulation (over *profile manipulation) and merely adding an additional caution to Mac users is sufficient, rather than special-casing their behavior overmuch.

I was examining an offered example, another project (starship) handles a similar problem by having an init binary which is eval'd and returns appropriate shell commands. This is slightly spooky to me, but on the other hand it would allow .cargo/env to not become split into several different shell scripts, and to have inspecting the path variable to see if it needs the path added be handled by a Rust program (so it can just use std::env to check the shell's paths, and then decide to not emit shell commands), so it could be a maintenance boon.

Here's the results of my investigation of {rc,pro}file usage over the suite of shells that commonly come with Rust supported platforms. I will be including a further expansion of my reasoning in my actual commits, but I thought I would leave the notes here in case some other poor benighted soul trawls this issue.

# POSIX, Non-Specified
/etc/profile
~/.profile

# Bash, Login
/etc/profile
~/.bash_profile || ~/.bash_login || ~/.profile
# Bash, Interactive Non-Login
~/.bashrc

# zsh, Always
/etc/zshenv
($ZDOTDIR | $HOME)/.zshenv
# zsh, On Interactive
($ZDOTDIR | $HOME)/.zshrc
# zsh appears to source .zshrc in interactive shells, even login shells.
# So .zprofile is irrelevant.

# tcsh, Always
/etc/csh.cshrc
~/.tcshrc || ~/.cshrc

Navigation Menu

Repository navigation

Installer modifies ~/.profile which is ignored for graphical terminals #2106

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Installer modifies ~/.profile which is ignored for graphical terminals #2106

Description

Activity

kinnison commented on Nov 1, 2019

kinnison commented on Nov 1, 2019

c3st7n commented on Nov 1, 2019

kinnison commented on Nov 3, 2019

xiongmao86 commented on Mar 20, 2020

workingjubilee commented on Apr 27, 2020

jyn514 commented on Apr 28, 2020

jyn514 commented on Apr 28, 2020

jyn514 commented on Apr 28, 2020

workingjubilee commented on Apr 28, 2020

jyn514 commented on Apr 28, 2020

workingjubilee commented on Apr 28, 2020

kinnison commented on Apr 28, 2020

workingjubilee commented on Apr 28, 2020

workingjubilee commented on Apr 29, 2020

rbtcollins commented on Apr 29, 2020

rbtcollins commented on Apr 29, 2020

workingjubilee commented on May 1, 2020

workingjubilee commented on May 21, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions