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
Add cli update command #740
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@blackheaven thanks for putting the effort into this PR!
I believe there are couple additional features we would need to make this feature complete and usable:
- We would want to check that there is already an installation of Wasp on the disk, and that it was done by Wasp installer script. To ensure we are not e.g. installing Wasp while there is some other kind of installation already there, e.g. via package manager or smth. We could edit installer script to have
--dry-run
option that just says where it would write the files, and then we can check if those paths exist. That would confirm previous installation.
[Ok, this is a bit advanced actually and we can even leave it out of this PR]. - We should let them know which version are they going to upgrade to, and warn them if the version is a breaking change compared to their current version. How to do this? Well, ideally we would modify installer script so we can do
--dry-run
on it that just tells us the version to which it would update wasp. Keep in mind, installer script is in separate repo: https://github.com/wasp-lang/get-wasp/blob/master/installer.sh . Then, we could use that information to obtain the new version, and we already can obtain the current version from Haskell code, and then we can use the SemVer module in our codebase to do the comparison of the versions and produce appropriate messages / asks / dialogue options.
[This feature is really important for usability].
Easy way to get the latest version of Wasp:
curl -LIs -o /dev/null -w %{url_effective} https://github.com/wasp-lang/wasp/releases/latest | awk -F/ '{print $NF}'
update :: Command () | ||
update = do | ||
cliSendMessageC $ Msg.Start "Updating wasp..." | ||
status <- liftIO $ system "curl -sSL https://get.wasp-lang.dev/installer.sh | sh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I don't think this will work, since wasp installer will refuse to write over existing files, and if we are doing updating, it will almost certainly write over existing files.
It will then let you know you need to run it with:
curl -sSL https://get.wasp-lang.dev/installer.sh | sh -s -- --force
So we can just go immediately with that, to make sure it actually performs the update?
update :: Command () | ||
update = do | ||
cliSendMessageC $ Msg.Start "Updating wasp..." | ||
status <- liftIO $ system "curl -sSL https://get.wasp-lang.dev/installer.sh | sh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call to system
, will it forward STDIO/STDERR to the STDIO/STDERR of our haskell program? It would be cool if it would! That way they can see progress, and possibly any errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it does :)
Actually I thought doing it the way round:
My aim is to avoid logic duplication since users are able to call the script anyway, while they are only able to call I may not be clear, let me know what you think.
I agree |
Interesting proposal that we move logic to the installer script! So I would certainly go with |
I've made a new iteration integration update logic, tell me if we're going in the right direction and/or if there are things to improve. |
@blackheaven sorry for a bit late response , we have been on company retreat so it took me a bit to get to this! Installer script has a bit more knowledge about where it installs binary, and any additional steps it does, for example right now it also creates On the other hand, if we do the updating via Haskell CLI as you did, then we can be a bit more "surgical", as you were, by updating only a specific thing, not going through the whole installation process. Ha, I am not sure what is better! My intuition is still pushing me a bit toward calling In that case you would figure out the version you want to install from Haskell, you would ask user if they are ok with that, and then, once all is confirmed and ready, you would call installer script. I described that above in the previous comments. |
I'm not sure to understand the difference between my first attempt (calling the installer script, which I would modify latter) and your proposal of creating a dedicated script for update. IIUC, we'll have to create a dedicated script (I'm not sure where to put it), which will be called to do the task. It feels quite confusing to me as, as you mentioned, bash scripts are harder to maintain, plus it will looks like a lot the installer script. Maybe I'm wrong but extending current installer script to precise/refine the |
Well that is exactly what I am recommending! I suggested that from Haskell code, we call installer script (which already exists) to do the basic work for us. We would need to modify installer script a bit to possibly support that dry run we mentioned, or smth else, not 100% sure yet. Let me know if there is anything else confusing! |
Clearer, thanks. I have modified the code here according to your comments, and modified the install script accordingly: wasp-lang/get-wasp#2 |
I commented on the other PR on the details of how installer script could work. Here, one thing that is missing is to check if update is a breaking change and warn them, and ideally offer to update to a non-breaking version instead. That is somewhat more work though so we can possibly postpone that for the future PR. |
@Martinsos to move forward on it, should I add parameters to pass-through get-wasp? |
@blackheaven pls check my comment on the PR, and also this comment on relevant issue: #616 (comment) . |
1 similar comment
@blackheaven pls check my comment on the PR, and also this comment on relevant issue: #616 (comment) . |
@blackheaven could you assign me this issue |
@weedertree I cannot, I am not a maintainer |
@weedertree if you want to take this one on, I can assign you! |
Description
The aim is to smooth user experience as it comes to updating.
Fixes #616
Type of change
Please select the option(s) that is more relevant.