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

starship doesn't work if not in PATH (starship: command not found) #220

Closed
bbigras opened this issue Aug 21, 2019 · 10 comments · Fixed by #224
Closed

starship doesn't work if not in PATH (starship: command not found) #220

bbigras opened this issue Aug 21, 2019 · 10 comments · Fixed by #224
Labels
🐛 bug Something isn't working as expected.

Comments

@bbigras
Copy link
Contributor

bbigras commented Aug 21, 2019

Bug Report

Current Behavior

$ which starship
which: no starship in [...]
$ eval "$(devstarship/target/debug/starship init bash)"
starship: command not found

Expected Behavior

It should work.

Additional context/Screenshots

Environment

  • Starship version: 88d65f1
  • Shell type: bash
  • Shell version: 4.4.23(1)-release
  • Terminal emulator: alacritty
  • Operating system: NixOS

Possible Solution

The problem is that starship init and starship prompt only use starship and not its full path.

I have a PR for this.

@bbigras bbigras added the 🐛 bug Something isn't working as expected. label Aug 21, 2019
@matchai
Copy link
Member

matchai commented Aug 21, 2019

I don't know if this really is a starship problem.
I would assume that correct installation of any of the installation methods would result in it being in $PATH. How is this usually addressed for other applications?

@bbigras
Copy link
Contributor Author

bbigras commented Aug 21, 2019

I use direnv and pazi in a similar way. direnv use its full path but not pazi.

$ direnv hook bash

_direnv_hook() {
  local previous_exit_status=$?;
  eval "$("/nix/store/da2qgg70p6dp88vvxbi7xm8zaynipbil-direnv-2.20.1-bin/bin/direnv" export bash)";
  return $previous_exit_status;
};
if ! [[ "${PROMPT_COMMAND:-}" =~ _direnv_hook ]]; then
  PROMPT_COMMAND="_direnv_hook${PROMPT_COMMAND:+;$PROMPT_COMMAND}"
fi

I would assume that correct installation of any of the installation methods would result in it being in $PATH.

I use home-manager with Nix and I can use starship like this:

  programs.bash = {
    enable = true;
    initExtra = ''
eval "$(${pkgs.starship}/bin/starship init bash)"
'';

This way, when I run home-manager switch, starship would get installed automatically and added to my .bashrc wihtout being in my path. I don't think I need starship to be in my path.

I think fixing this issue would also be easier to use starship as a prompt inside a pure nix-shell where the PATH is not defined the same way. I can test that.

@bbigras
Copy link
Contributor Author

bbigras commented Aug 21, 2019

I think fixing this issue would also be easier to use starship as a prompt inside a pure nix-shell where the PATH is not defined the same way. I can test that.

Yeah if I have eval "$(/home/bbigras/.cargo/bin/starship init bash)" in my .bashrc and with /home/bbigras/.cargo/bin is in my path, starship wont work in a pure nix-shell because PATH will not contain /home/bbigras/.cargo/bin anymore.

@matchai
Copy link
Member

matchai commented Aug 21, 2019

Sorry, I misunderstood the problem.
This is caused by the two-layer init system, which itself calls starship. Ideally, we would be using the same binary used when initially calling starship, including the path if one was provided. 🤔

Going to add @chipbuster to the conversation 👈

@bbigras
Copy link
Contributor Author

bbigras commented Aug 21, 2019

Ideally, we would be using the same binary used when initially calling starship, including the path if one was provided.

I have a PR almost ready for that. Using std::env::current_exe which has a warning about security (see https://doc.rust-lang.org/std/env/fn.current_exe.html#security). Not sure if it applies.

It's also tricky in case there's spaces in the path (which I solved) and doublesquotes " (not done yet).

@matchai
Copy link
Member

matchai commented Aug 21, 2019

Thank you for tackling it! 😄

@chipbuster
Copy link
Contributor

@bbigras I think the security warning there might actually apply to us, if someone decides to use Starship as their prompt for a superuser. Will need to think about it carefully, since the linked security vulnerability involves a race condition (and those are hard to get right).

Good to have a patch, but we may need to do some modifications before merging.

@chipbuster
Copy link
Contributor

chipbuster commented Aug 21, 2019

@bbigras I just thought of an alternate solution:

The only reason we do this two-phase at the moment is because I wanted to retain compatibility with eval $(starship init {blah}). Were it not for that requirement, I would have just made everyone switch over to source <(starship init {blah}).

Would it be possible for you to instead replace eval "$(${pkgs.starship}/bin/starship init bash)" with source <(${pkgs.starship}/bin/starship init bash --print-full-init)?

@bbigras
Copy link
Contributor Author

bbigras commented Aug 21, 2019

starship init bash --print-full-init also has starship without the path:

❯ starship init bash --print-full-init | rg "starship prompt"
        PS1="$(starship prompt --status=$STATUS --jobs="$(jobs -p | wc -l)" --cmd-duration=$STARSHIP_DURATION)"
        PS1="$(starship prompt --status=$STATUS --jobs="$(jobs -p | wc -l)")"

@chipbuster
Copy link
Contributor

@bbigras Dang :(

It seems like the old one-phase init wouldn't have worked here either then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working as expected.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants