-
Notifications
You must be signed in to change notification settings - Fork 248
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
feat: automatically add ~/.fvm_flutter/bin
to the PATH var in install.sh
and introduce update.sh
#700
Conversation
@Nidal-Bakir is attempting to deploy a commit to the FlutterTools Team on Vercel. A member of the Team first needs to authorize it. |
$HOME/.local/bin/fvm
$HOME/.local/bin/fvm
in install.sh on linux
$HOME/.local/bin/fvm
in install.sh on linux$HOME/.local/bin/fvm
in install.sh for linux
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Is it good ? This is the only problem I have with FVM right now. |
@SSebigo |
@Nidal-Bakir Sorry for the delay on this, but can we maybe move the directory to ~/.fvm/bin/fvm instead of local, and make both mac and linux consistent? |
That script did work for me, fresh fedora40 install
There is no Edit: the script auto assumes that After manually creating local/bin folder and adding it to the PATH, the script worked |
The Script worked for me but still got 2 things needs to be done manually:
|
I cannot speak for the Mac because I do not have a Mac, and I do not know if Mac has a Nonetheless, we can add some extra comments to note that the |
Lets agree to disagree. You are correct. Most users should already have But it shouldn't hurt having these small checks inside the script, makes it more reliable. |
We can make the script create the |
That's trivial to add to PATH but personally I would just echo the instructions to the user add manually |
No, it's not trivial!
|
So as for now, it turns out that some distros do not follow the XDG Base Directory Specification
We need to do the following:
|
Jesus, man. when I say "echo" I mean display it to terminal to the user add it manually. But yes, its trivial to automatically add to correct shell file and still check if ts already set. |
Apologies for my mistake. I misinterpreted your comment. If you are confident about it, then please guide me in the right way. |
So we have two options I think:
|
@Nidal-Bakir When looking at similar .sh scripts to install, I found the use of Can you take a brief look at this, as it would be more straightforward to just set this? |
@Nidal-Bakir, can you confirm my message above with the example from Bun? It seems they are not using local. I would prefer this approach if it works without manual intervention. |
Hi @leoafarias sorry for the delay. I am currently only available during weekends (Friday for me) I will get this done by tomorrow. Yes, the Bun team does not use the |
feat: introduce a update script to avoid adding fvm bin path to PATH var every time the user run install.sh fix: remove the check to see if fvm is installed
I appreciate your feedback and I believe everything is in good shape. However, I wanted to note one thing: I have added an update script. Users should use this update script to update FVM. Otherwise, the install script may repeatedly add the FVM bin path to the PATH variable. While this isn't harmful, it's not ideal. There’s no way to be certain that FVM is fully added to the PATH, because we are checking the local PATH per the SHELL variable. This means that FVM might be in the PATH for zsh or fish, but not for bash. Since we are piping the script to bash, we can only see the PATH for the bash scope. This is why I have included the update.sh script. |
$HOME/.local/bin/fvm
in install.sh for linux~/.fvm_flutter/bin
to the PATH var in install.sh
and introduce update.sh
Hi. I encountered the same problem trying to install fvm on ubuntu. I can use the script provided in this PR, but is there any update about it ? Is there a reason not to merge it for now ? |
I am bringing this in, but most likely will have to do a full review across all the platforms. |
This will fix: #699