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

sudo proton install to install to /usr/local/bin on Mac instead of the locked /usr/bin #651

Merged
merged 6 commits into from
Apr 10, 2024

Conversation

jovezhong
Copy link
Contributor

/usr/bin is locked on Mac. Change it to /usr/local/bin. This should also work on Linux

PR checklist:

  • Did you run ClangFormat ?
  • Did you separate headers to a different section in existing community code base ?
  • Did you surround proton: starts/ends for new code in existing community code base ?

Please write user-readable short description of the changes:

@jovezhong jovezhong requested a review from chenziliang April 9, 2024 19:29
/usr/bin is locked on Mac. Change it to /usr/local/bin. This should also work on Linux
@jovezhong jovezhong force-pushed the fix-proton-install-on-mac branch from a74ecd1 to 594ae5b Compare April 9, 2024 21:52
yokofly added 2 commits April 9, 2024 20:10
1. increase open files limit. ClickHouse/ClickHouse#41345
2. use stringview instead char*
3. maybe better for throw exception
4. try take a hardlink first before copy. ClickHouse/ClickHouse#48578
@yokofly

This comment was marked as resolved.

@yokofly
Copy link
Collaborator

yokofly commented Apr 10, 2024

I hide the previous comment, seems the problem is my homebrew or old proton-server pollution.

after clean the folder, my local is work and valid. but this is really dangerous, because the default proton search the homebrew first.
I do not take too much time to test but my locally usage is a bit ugly:

% sudo /usr/local/bin/proton start
% /usr/local/bin/proton-client -h 127.0.1
% /usr/local/bin/proton-client -h 127.0.1  -u proton --password proton@t+

@jovezhong
Copy link
Contributor Author

jovezhong commented Apr 10, 2024

Thanks for new commits, @yokofly Yes, it will mess up things if the user homebrew install first then proton install again.

I think it's fair to say

  • if the user grabs the binary via curl or download from release page, then sudo proton install will put this in /usr/local/bin with config file. The folder of initial proton binary won't be in PATH
  • if the user brew install proton, then they don't need to run sudo proton install again. Please use homebrew way to setup the files and other things. brew install proton should do similar things as proton install homebrew-timeplus#4 is created to track the enhancement in homebrew.

@yokofly
Copy link
Collaborator

yokofly commented Apr 10, 2024

do u mean make the homebrew same behavior with proton install?

edit: understand,the rest part shall be homebrew, this pr is enough for local install.

@jovezhong
Copy link
Contributor Author

Feel free to merge if you think it's ready.

Let me list 3 cases

  • if the user grabs the binary from release page, then sudo proton install will put this in /usr/local/bin with config file. The folder of initial proton binary won't be in PATH
  • if the user brew install proton, then they don't need to run sudo proton install again. Please use homebrew way to setup the files and other things. brew install proton should do similar things as proton install homebrew-timeplus#4 is created to track the enhancement in homebrew.
  • if the user use curl https://install.timeplus.com | sh, you can call homebrew to install it. Otherwise it's odd to curl https://install.timeplus.com | sh then 'sudo proton install'

This PR only addresses 1. You can keep working in https://github.com/timeplus-io/homebrew-timeplus and https://github.com/timeplus-io/install.timeplus.com

@jovezhong jovezhong changed the title sudo proton install to install to /usr/local/bin instead of the locked /usr/bin sudo proton install to install to /usr/local/bin on Mac instead of the locked /usr/bin Apr 10, 2024
@jovezhong
Copy link
Contributor Author

Since @yokofly said "this pr is enough for local install." So I will merge it and put in 1.5.5

@jovezhong jovezhong merged commit 9ef372c into develop Apr 10, 2024
21 checks passed
@yokofly yokofly deleted the fix-proton-install-on-mac branch May 15, 2024 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants