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

Valet v4 doesn't handle a previously-installed version of ngrok correctly (if that version of ngrok wasn't installed by Brew) #1373

Closed
mattstauffer opened this issue Feb 22, 2023 · 4 comments
Labels

Comments

@mattstauffer
Copy link
Collaborator

@laytan reported this bug testing Valet v4:

I have installed ngrok previously using a method other than homebrew, valet has no idea and is trying to run a homebrew version of ngrok which I don't have installed:

$ which ngrok
/usr/local/bin/ngrok
$ valet share
sudo: /opt/homebrew/bin/ngrok: command not found

I'm pretty sure the best solution will be to update the "is ngrok installed" check to make sure the Brew-installed version of ngrok is installed, but it's possible the solution is instead to allow it to call /usr/local/bin/ngrok instead.

If we allow it to call the alternate version of ngrok, we run into problems if it's not the right version, and have to take responsibility for checking its version, so I'm leaning more toward requiring a Brew-installed version.

@mattstauffer
Copy link
Collaborator Author

@laytan I just realized this isn't an issue of Valet not correctly checking for your installed Valet, but instead simply that you expected it to work with your existing ngrok install but it only works with Homebrew.

I plan for it to only work with the Homebrew-installed version, unless there's a really strong case against it.

But maybe I should make this more clear in the instructions; maybe when you set the share-tool to ngrok and it asks if you want to install it, it should clarify that it'll use the Homebrew-installed version.

@mattstauffer
Copy link
Collaborator Author

mattstauffer commented Feb 23, 2023

image

I think I'll add this note and call this good for now.

@laytan
Copy link

laytan commented Feb 23, 2023

This is fine I think.

Just curious, is there anything different about the homebrew version which sparks the homebrew requirement?

Anyway, not a big deal to reinstall with brew.

@mattstauffer
Copy link
Collaborator Author

The Homebrew version just makes it a lot easier for Valet to consistently install it, manage it, and know where it's supposed to live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants