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

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

Open
maiste opened this issue Oct 15, 2024 · 5 comments
Open

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

maiste opened this issue Oct 15, 2024 · 5 comments
Labels
brainstorm script Related to the script provided by the website

Comments

@maiste
Copy link
Member

maiste commented Oct 15, 2024

REPOST FROM: ocaml/dune#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:

  • PR with a reproducing test:
  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

  • Version of dune (output of dune --version): 3.16.0
  • Version of ocaml (output of ocamlc --version): 5.2.0
  • Operating system (distribution and version): Fedora 40

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
Copy link
Member Author

maiste commented Oct 15, 2024

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
Copy link
Contributor

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
Copy link
Member Author

maiste commented Oct 15, 2024

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
Copy link
Contributor

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

@maiste
Copy link
Member Author

maiste commented Oct 16, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brainstorm script Related to the script provided by the website
Projects
None yet
Development

No branches or pull requests

2 participants