Skip to content
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

Shell config update #3304

Merged
merged 12 commits into from
Apr 30, 2018
Merged

Shell config update #3304

merged 12 commits into from
Apr 30, 2018

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Apr 20, 2018

This PR contains an update of shell configuration scripts to have always opam variables set (evaluation at each command). It was inspired from direnv (thanks @samoht). Available for all opam handled shells.
For bash, I kept sourcing scripts on profile config files (cf. #1447), assuming that profile files are sourced in .bashrc. Added a message for the user in this case.

@samoht
Copy link
Member

samoht commented Apr 20, 2018

I am not sure to understand when do these scripts are actually called. Is this for internal opam commands? Or is it for every user commands? In the later case maybe we should have a way to disable that?

@rjbou
Copy link
Collaborator Author

rjbou commented Apr 20, 2018

These scripts are generated at opam init. They can be found in folder ~/.opam/opam-init/. User can add (or allow opam to add) sourcing init.xx script in its shell init file.
For example, in a bash environment (opam init extract):

=-=- Required setup - please read -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

  In normal operation, opam only alters files within ~/.opam.

  However, to best integrate with your system, some environment variables
  should be set. If you allow it to, this initialisation step will update
  your bash configuration by adding the following line to ~/.profile:

    test -r /tmp/opamroot/opam-init/init.sh && . /tmp/opamroot/opam-init/init.sh > /dev/null 2> /dev/null || true

  Otherwise, every time you want to access your opam installation, you will
  need to run:

    eval $(opam env)

  You can always re-run this setup with 'opam init' later.

Do you want opam to modify ~/.profile ? [N/y/f]
(default is 'no', use 'f' to choose a different file)

This script, if it's a terminal, set evaluation of opam env for each command (not only opam ones). Otherwise, it sources the variable.xx file, containing last opam environment variables.

On current setup, only variable.xx scripts where sourced, at shell loading. For each change (switch for example), user had to run eval $(opam env). In this new setup, it's done in background, new environment variables are always well set.

@rjbou rjbou changed the title Few fixes and shell config update Shell config update Apr 20, 2018
@samoht
Copy link
Member

samoht commented Apr 20, 2018

I think it's worth thinking a bit to understand the consequences of that change (note: I think that's generally a good idea, but there are maybe some corner cases where this doesn't work very well).

Previously, the user had strong control about which switch was used (by doing opam env --switch=...). Now that opam env is called on every command the user doesn't have control anymore.
Direnv allows the user to specify .envrc files and display warnings when you go out of in of such directories, so the environment changes are quite visible. Here everything is done transparently, which might lead to confusion.

Moreover, does it mean that we are creating a new shell on every command?

Note: I am being cautious because these things have caused a lot of confusion to users in the past; changing this seems to go in the right direction but we need to be careful :-)

@AltGr
Copy link
Member

AltGr commented Apr 20, 2018

Previously, the user had strong control about which switch was used (by doing opam env --switch=...).

Indeed, you still retain such control but would now need to set OPAMSWITCH=... (and opam env --switch=, if done without --set-switch, wouldn't work anymore, which may indeed be confusing).
Actually, when this is enabled, you shouldn't ever use eval $(opam env) anymore (except with --set-switch|root, but then, you could as well set the variables manually)

Here everything is done transparently, which might lead to confusion.

It respects the intended semantics of local switches much better, and indeed I have heard of quite a few users doing similar shell tricks manually. So this should give these users a safer and supported alternative.
A message when automatically switching could be nice indeed¹, though (the current opam switch is part of my shell prompt, so I didn't notice ;) )

Let's not forget that the lack of sync between opam and the shell, and need to call eval $(opam env) is the most confusing part to newcomers, though, so while we shouldn't do too much magic, I am sure this will reduce the overall confusion, while making local switches much nicer to use.

¹ A way to do this could be to check if OPAM_SWITCH_PREFIX, which is always set, is current, and print a message (opam: selected switch XXX, environment updated)

@AltGr
Copy link
Member

AltGr commented Apr 20, 2018

I am wondering wether it would be a good idea to re-enable making --set-switch the default when necessary in this case. I don't like such adhoc cases in general, but the following interaction should really be avoided:

% cd foo
(opam: selected switch ~/foo, environment updated)
% eval $(opam env --switch 4.06.0)
(opam: selected switch ~/foo, environment updated)

See #3143 for context. In this case, there can be no point in the command if not using --set-switch.

One small glitch is that, if you issue opam switch bar in a shell, other terminals without a local switch or OPAMSWITCH set will only sync up on the next prompt. I guess there is not much we can do about that...

And no, this shouldn't spawn any subshells, it's just an eval ?

@rjbou
Copy link
Collaborator Author

rjbou commented Apr 20, 2018

Moreover, does it mean that we are creating a new shell on every command?

And no, this shouldn't spawn any subshells, it's just an eval ?

Yes, just a eval $(opam env) is done for each prompt (cf. bash example).

@rjbou
Copy link
Collaborator Author

rjbou commented Apr 24, 2018

It is now disabled by default, at opam init a message allows user to enable it. To enable/disable, it is done by re-running opam init.

src/state/opamEnv.ml Outdated Show resolved Hide resolved
src/state/opamEnv.ml Outdated Show resolved Hide resolved
src/state/opamEnv.ml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants