ocaml-dune / binary-distribution

A web page for dune binary distribution
https://preview.dune.build/
ISC License
2 stars 6 forks source link

Installing developer preview didn't set up PATH correctly on zsh #85

Open maiste opened 6 days ago

maiste commented 6 days ago

REPOST FROM: https://github.com/ocaml/dune/issues/10963 by @edwintorok

Expected Behavior

$ dune --version
"Dune Developer Preview: build 2024-09-28T01:30:13+00:00, git revision
17071ec30d10390badcb6cb1f6a43984b1be54a6"
$ echo $PATH
/var/home/edwin/.dune/bin:/var/home/edwin/.opam/5.2.0+fp/bin:/var/home/edwin/.cargo/bin:/var/home/edwin/.local/bin:/var/home/edwin/.dune/bin:/var/home/edwin/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/var/home/edwin/.dotnet/tools

Actual Behavior

$ dune --version
3.16.0
$ zsh -l
$ echo $PATH
/var/home/edwin/.opam/5.2.0+fp/bin:/var/home/edwin/.cargo/bin:/var/home/edwin/.local/bin:/var/home/edwin/.nix-profile/bin:/var/home/edwin/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/var/home/edwin/.dotnet/tools
$ zsh
$ echo $PATH
/var/home/edwin/.opam/5.2.0+fp/bin:/var/home/edwin/.cargo/bin:/var/home/edwin/.local/bin:/var/home/edwin/.nix-profile/bin:/var/home/edwin/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/var/home/edwin/.dotnet/tools

Reproduction

You can find my full ZSH config here: https://gitlab.com/edwintorok/dotfiles/-/tree/master/zsh?ref_type=heads

However for the purposes of this bug you can repro it with:

  1. opam install dune
  2. echo 'return 0' >.zshrc
  3. curl https://dune.ci.dev/install | bash 1.zsh -l
  4. dune --version
  5. echo $PATH

Specifications

Additional information

Dune added this to my .zshrc, but that has no effect

# dune
export PATH="/var/home/edwin/.dune/bin:$PATH"

If I instead I add it to .zshenv it is better, but still in the wrong place (opam takes precedence):

/var/home/edwin/.opam/5.2.0+fp/bin:/var/home/edwin/.dune/bin:/var/home/edwin/.cargo/bin:/var/home/edwin/.local/bin:/var/home/edwin/.nix-profile/bin:/var/home/edwin/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/var/home/edwin/.dotnet/tools

The reason it doesn't work in .zshrc because mine ends like this:

# Setup highlighting {{{
# Must be last according to its docs
ZSH_HIGHLIGHT_HIGHLIGHTERS=(main brackets)
#SYNTAX=/usr/share/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh
SYNTAX=~/dotfiles/zsh/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh
[[ -s ${SYNTAX} ]] && . ${SYNTAX}
return 0

So I need to move the export PATH before the 'return 0', so this works:

export PATH="/var/home/edwin/.dune/bin:$PATH"

# Setup highlighting {{{
# Must be last according to its docs
ZSH_HIGHLIGHT_HIGHLIGHTERS=(main brackets)
#SYNTAX=/usr/share/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh
SYNTAX=~/dotfiles/zsh/zsh-syntax-highlighting/zsh-syntax-highlighting.zsh
[[ -s ${SYNTAX} ]] && . ${SYNTAX}
return 0
maiste commented 6 days ago

Quote @edwintorok:

It might be difficult to automatically modify the user's PATH (there could be a lot of other things than return 0 that could prevent the last line from being executed), but one improvement the dune installer could do is to verify whether the PATH modification worked, and if it didn't ask the user to check that the script didn't exit early.

One way to test that is: zsh -i -c 'echo $PATH'

And then check that the newly installed dune is really the first one found in that PATH (and e.g. that opam or something else didn't override it)

(note the -i for interactive shell, if you just do zsh -l -c 'echo $PATH' or zsh -c 'echo $PATH', then it wouldn't work without also modifying .zshenv).

Leonidas-from-XIV commented 5 days ago

There is also another problem: the way OPAM modifies the PATH (via the eval $(opam env) thing which is recommended) it will always put the switch bin before the users' configured PATH, thus dune from OPAM will override the developer preview.

One way this could be fixed would be symlinking our developer preview as dune-preview (or similar) thus people who call that binary will always get the preview.

maiste commented 5 days ago

I agree. Another ugly solution is to create an empty switch global switch, but we can't ask people to do so in the name of "enjoying the full experience". I agree the dune-preview might be a solution to prevent this.

About setting the PATH, I would be curious to see how others are doing (cargo, nvm, deno, etc).

Leonidas-from-XIV commented 5 days ago

The empty global switch will not help, if someone cd's into a project that uses a local switch with dune, does it?

maiste commented 4 days ago

You are right. My bias here is that, in my mind, if people move to a directory with the local _opam I wouldn't expect them to use the dune binary from developer preview.