cantino / mcfly

Fly through your shell history. Great Scott!
MIT License
6.75k stars 176 forks source link

fix: added sudo to installation via install script readme command #348

Closed ykageyama-mondo closed 6 months ago

ykageyama-mondo commented 1 year ago

Currently when trying to install via the installation script command in the README we fail with a permission denied.

This is due to the default installation location /usr/local/bin in the install.sh. Adding sudo to the command will fix this issue.

cantino commented 1 year ago

Thanks @ykageyama-mondo.

@praveenperera, opinion on this?

praveenperera commented 1 year ago

@cantino hmm this didn't use to need a sudo, maybe apple changed permissions on that folder.

Some linux users might not need sudo either. I think some people are little scared by piping into a sudo script.

Maybe a note on "if the current user doesn't have permissions to /usr/local/bin/ use sudo". Not sure I can see arguments for both sides, up to you.

ykageyama-mondo commented 1 year ago

@praveenperera Thanks for the feedback!

Good point about piping into sudo. It will definitely raise some red flags especially when executing remote scripts.

I can see benefits from adding a note for when users run into this error but it doesn't resolve the root issue and make installation seamless.

Another way I can see to solve this would be to update the installation command (or add an alternative method) to target a folder where elevated permissions are not needed. Maybe something like this: curl -LSfs https://raw.githubusercontent.com/cantino/mcfly/master/ci/install.sh | sh -s -- --git cantino/mcfly --to $HOME/.local/bin This has the tradeoff of only installing for the current user so I'm not sure if this is desirable.

What are your thoughts on this?

cantino commented 1 year ago

I think I like the suggestion of adding if the current user doesn't have permissions to '/usr/local/bin/', then use: curl -LSfs https://raw.githubusercontent.com/cantino/mcfly/master/ci/install.sh | sudo sh -s -- --git cantino/mcfly

cantino commented 6 months ago

Added to README