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

fix(updater): run cleanup before exit on Windows #1070

Merged
merged 8 commits into from
Mar 13, 2024

Conversation

Legend-Master
Copy link
Contributor

@Legend-Master Legend-Master commented Mar 12, 2024

Fix #1014

@Legend-Master Legend-Master marked this pull request as ready for review March 13, 2024 02:30
@Legend-Master Legend-Master requested a review from a team as a code owner March 13, 2024 02:30
@Legend-Master
Copy link
Contributor Author

The downside about this is that we will have to assume ShellExecute will run successfully, since we need to clean up before the installer gets executed (installer will kill all running instances)

@Legend-Master
Copy link
Contributor Author

And, I'm not familiar with how updaters shutdown the app on other platforms, is this just a Windows problem or we need to do something for other platforms as well?

@amrbashir
Copy link
Member

And, I'm not familiar with how updaters shutdown the app on other platforms, is this just a Windows problem or we need to do something for other platforms as well?

correct, other platforms update without restarting and wait for users to relaunch.

@Legend-Master Legend-Master changed the title fix(updater): doesn't shutdown app gracefully fix(updater): doesn't shutdown app gracefully on Windows Mar 13, 2024
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite like the additional argument, I think a better way to handle this would be to take a generic callback to be executed on before restart, here is some psuedo-code of what I mean:

let builder = UpdaterBuilder::new()
  .with_before_exit(|| {
    app.cleanup_before_exist();
  })
  .build();

and we can provide this callback automatically in the UpdaterExt trait implementation.

@Legend-Master
Copy link
Contributor Author

Legend-Master commented Mar 13, 2024

A bit off topic for this PR, do we want to expose download and install directly, so that the app can download the update in background and prompt the user once it's done (like the way vscode did it)?

@amrbashir
Copy link
Member

@Legend-Master
Copy link
Contributor Author

I think a better way to handle this would be to take a generic callback to be executed on before restart

I'll take a look

they are exposed https://docs.rs/tauri-plugin-updater/latest/tauri_plugin_updater/struct.Update.html#method.download

I mean the JavaScript side

@amrbashir
Copy link
Member

I mean the JavaScript side

yeah sure, was hoping to do it at some point

amrbashir
amrbashir previously approved these changes Mar 13, 2024
Copy link
Member

@amrbashir amrbashir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

amrbashir
amrbashir previously approved these changes Mar 13, 2024
amrbashir
amrbashir previously approved these changes Mar 13, 2024
@amrbashir amrbashir changed the title fix(updater): doesn't shutdown app gracefully on Windows fix(updater): run cleanup before exit on Windows Mar 13, 2024
@amrbashir amrbashir merged commit 1d7dc86 into tauri-apps:v2 Mar 13, 2024
15 of 16 checks passed
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.

2 participants