ocaml / opam

opam is a source-based package manager. It supports multiple simultaneous compiler installations, flexible package constraints, and a Git-friendly development workflow.
https://opam.ocaml.org
Other
1.25k stars 365 forks source link

opam init advises misconfiguring shell startup scripts #4201

Open Blaisorblade opened 4 years ago

Blaisorblade commented 4 years ago

opam outputs

[NOTE] Make sure that ~/.profile is well sourced in your ~/.bashrc.

But that's backwards. As also mentioned in this old opam issue, a typical setup is that:

The rationale is that environment variables are set by login shells loading ~/.profile and inherited by other shells — while other settings that are not inherited belong in ~/.bashrc, which ends up loaded by all shells.

The recommended setup would load ~/.profile multiple times, which is unexpected for lots of initialization code.

This bug is described in https://github.com/ocaml/opam/pull/3304 — the message comes from https://github.com/ocaml/opam/pull/3304/files#diff-234457b80298a00240e5fcd7591e5432R603.

Here's my proposed fix:

MisterDA commented 4 years ago

I was just about to report this issue!

Login shells load the first file (in that order) of .bash_profile, .bash_login, .profile. Opam could search for the files and suggest to load the correct one.

Bash documentation states that:

§ Invoked as an interactive non-login shell

So, typically, your ~/.bash_profile contains the line

if [ -f ~/.bashrc ]; then . ~/.bashrc; fi

after (or before) any login-specific initializations.

AltGr commented 4 years ago

Thanks for reporting. Bash init is a mess ;)

The solutions often seems simple, but the practice always shows different... E.g. initially we had been using .bash_profile, but just creating that file is enough to change bash's init process and preventing it from loading .profile, hence potentially breaking some setups.

IIRC, the choice of .profile was made for non-bash cases, because it is much more general. On most Linux systems, the "login" part is loaded when your graphic session starts up, then terminal emulators you start are only set to interactive. Now, when we start a shell we want up-to-date variables, so the opam init should be loaded for interactive shells (although if shell-hooks are set up, it's not a problem anymore) ; but it's also best if other apps, e.g. editors, start with the opam environment by default (e.g. opam-user-setup will set emacs to load it on startup, but we can't rely on that being used).

On the specific message Make sure that ~/.profile is well sourced, it indeed seems quite weird on second reading. That was added by request from OSX users, whose shells by default are started as login shells, without a link between .profile and .bashrc in either way ; it seems sourcing .profile from .bashrc is thus how they generally do it. I'd be happy if one could confirm.

  • the script should load ~/.opam/opam-init/variables.sh in ~/.profile
  • the script should load ~/.opam/opam-init/env_hook.sh in ~/.bashrc

We really want to keep the automatic modifications to the strict minimum, i.e. one line in one file. We could check in the scripts whether they were run as login or not, but due to the above, we probably don't even want to.

Blaisorblade commented 4 years ago

The solutions often seems simple, but the practice always shows different...

Luckily, I have no illusion things are simple. And I'm afraid some requirements will have to go.

  1. Whatever opam does, it's .profile that must load .bashrc, and not viceversa, as recommended by the bash manual (they mention .bash_profile, but it seems all Linux distros nowadays ship .profile instead). https://www.gnu.org/software/bash/manual/html_node/Bash-Startup-Files.html

    That's also the correct setup on Mac, even if Mac is indeed misconfigured out-of-the-box.

    Linux systems follow bash's advice. So if you also loaded .profile from .bashrc, you'd get an infinite loop for interactive login shells, which you can certainly get via ssh or by logging in on Ctrl-Alt-F1.

  2. The rest depends (in part) on the following question: when starting a Linux graphical session, I'm not sure if the login shell involved is marked as interactive or not. Do you know?

  3. Loading everything in .bashrc gives you fresh values, but won't affect non-interactive login shells, despite .profile loading .bashrc, because .bashrc usually exits immediately for non-interactive shells. Also, variables.sh would be invoked multiple times in nested shells and risk extending the PATH multiple times, maybe with different directories, so it should first remove anything it added.

  4. Loading everything in .profile wouldn't give you up-to-date settings after you change switch — which honestly seems acceptable. However, it also won't set PROMPT_COMMAND in nested shells (unless that's exported), which I can confirm, and it certainly won't affect bash completion (which is not exported), if that's what /Users/pgiarrusso/.opam/opam-init/complete.sh is about.

EDIT: so I'd drop at least "only change one file".

MisterDA commented 4 years ago
  1. (they mention .bash_profile, but it seems all Linux distros nowadays ship .profile instead).

    Arch Linux ships .bash_profile: https://git.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/bash.

  2. The rest depends (in part) on the following question: when starting a Linux graphical session, I'm not sure if the login shell involved is marked as interactive or not. Do you know?

    There’s no reason why it would be interactive.

    Note that there are efforts to ditch login shells completely for graphical sessions in the systemd/Wayland world.

    https://wiki.archlinux.org/index.php/Environment_variables https://wiki.archlinux.org/index.php/Systemd/User#Environment_variables https://www.freedesktop.org/software/systemd/man/environment.d.html https://wiki.gnome.org/Initiatives/Wayland/SessionStart https://lwn.net/Articles/709769/

  3. would be invoked multiple times in nested shells and risk extending the PATH multiple times

    What I do in zsh is that I define all my environment variables in .zshenv that is always sourced, and I protect nested shell by placing all my definitions under if [[ $SHLVL == 1 ]]; then XXX fi.

For the Bash case, if following the flowchart here I’d suggest patching the first file of ~/.bash_profile, ~/.bash_login, or ~/.profile.

Blaisorblade commented 4 years ago

But many of those settings are not exported (because they involve functions or special settings), so they don't belong to any profile files, but into .bashrc. I expect trying to export PROMPT_COMMAND might break other .profile scripts, such as iterm2 integration, which call non-exported functions. And exporting functions seems a bash-only extension... (Edit: there seems be other ways in other shells, they don't seem portable - see https://stackoverflow.com/a/1886397/53974).